Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mg building on darwin by changing to new upstream #85627

Merged
merged 4 commits into from May 11, 2020

Conversation

@druimalban
Copy link
Contributor

druimalban commented Apr 20, 2020

For context, this is for the issue #71136.

This PR now sets the upstream to https://github.com/ibara/mg. For context I will leave the original comment (which patched the original upstream using patches from https://github.com/ibara/mg which apply missing header files on darwin) below.

Original comment:

I am not happy with this at all. It avoids using libbsd at all on darwin because there are conflicting header files, particularly the header string.h.

Ideally that would be the thing to fix upstream. But in the meantime, this patch should do the trick. This is taken from https://github.com/ibara/mg and the license of the patch is the same as mg itself. Homebrew and macports carry https://github.com/ibara/mg. Fink does not package it. Debian and many Linux distributions carry https://github.com/hboetes/mg. NetBSD pkgsrc carries the hboetes version albeit from 2011 (!). Something also relevant, is that MacOS as of MacOS Catalina removed their very old (pre-GPLv3) Emacs from the base operating system, and actually replaced it with a reasonably recent version of mg.

The patch declares a header file apple.h, which files include if target is darwin (checks for presence of __APPLE__). This header file also references the three source files futimens.c, reallocarray.c and strtonum.c. The former and latter are from https://github.com/ibara/mg. Also of note is that futimens was added to macOS as of 10.13, but 10.12 does not have it so it still is necessary to patch it for now. There might be a better way.

From the OpenBSD source tree is _null.h and tree.h. There is a check so that if __APPLE__ is present then it will include them from this source tree.

Now, the makefile GNUMakefile will not link against libbsd on darwin, by checking the output of uname in much the same way as it did already for FreeBSD.

I would really appreciate testing on NixOS and other Linux distributions!

This applies to the same version in 19.09 and nixpkgs-unstable.

Motivation for this change

This package is not marked as broken, but it is on macOS. It is a generally useful small emacs-like editor. The package in nixpkgs is also quite old, so it makes sense to update it to the most recent release from the upstream source repository.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
I am not happy with this at all. It avoids using libbsd at all on
darwin because there are conflicting header files, particularly the
header string.h.

Ideally that would be the thing to fix. But in the meantime, this patch
should do the trick.

It declares a header file apple.h, which files include if target is
darwin (checks for presence of __APPLE__). This header file also
references the three source files futimens.c, reallocarray.c and
strtonum.c. The former and  latter are from https://github.com/ibara/mg
which is another portable fork, but building against more platforms.

From the OpenBSD source tree is _null.h and tree.h. There is a check so
that if __APPLE__ is present then it will include them from this source
tree.

Now,the makefile GNUMakefile will not link against libbsd on darwin, by
checking the output of uname in much the same way as it did already for
FreeBSD.
The actual most recent version is 20200215, so update the package version to this.
Furthermore, update the patch accordingly so it still builds. This adds back the
reallocarray.c file.
@druimalban druimalban changed the title Fix mg building on darwin and update to most recent release (20180927) Fix mg building on darwin and update to most recent release (20200215) Apr 20, 2020
…ot to remove temporary files
@Mic92
Copy link
Contributor

Mic92 commented May 7, 2020

Would it make sense to just use ibara/mg to not having to patch it ourself?

@druimalban
Copy link
Contributor Author

druimalban commented May 7, 2020

Would it make sense to just use ibara/mg to not having to patch it ourself?

I think so? It removes the dependency on libbsd.

I have just tested it and it works just as fine. Should I push to this PR?

@Mic92
Copy link
Contributor

Mic92 commented May 7, 2020

Yes.

As per reasonable suggestion, instead of extensively patching the
hboetes version of mg to work on MacOS using bits from ibara's mg,
just set upstream to ibara's mg.

This also removes the dependency on libbsd, although I'm not sure
if this is a good or bad thing.
@druimalban druimalban changed the title Fix mg building on darwin and update to most recent release (20200215) Fix mg building on darwin by changing to new upstream May 7, 2020
@Mic92
Copy link
Contributor

Mic92 commented May 9, 2020

@GrahamcOfBorg build mg

@Mic92 Mic92 merged commit e985344 into NixOS:master May 11, 2020
17 checks passed
17 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="c54d375"; rev="c54d37598a27941621228c7b4bb1eebb2dfaafdd"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
mg on aarch64-linux Success
Details
mg on x86_64-darwin Success
Details
mg on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.