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.

Sign up for GitHub

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

Jump to bottom

argp-standalone: init at 1.3 #48738

Merged
merged 2 commits into from Nov 12, 2018
Merged

argp-standalone: init at 1.3 #48738

merged 2 commits into from Nov 12, 2018

Conversation

Amar1729
Copy link
Contributor

@Amar1729 Amar1729 commented Oct 20, 2018 •
edited

Motivation for this change

This library ships the argp.h header for the argparse lib. Lack of it can result in build issues with certain packages on Mac (notably, the goxel nixpkg fails on macOS builds partly because of lack of argp.h).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.
  • can't currently run nix path-info -S on macOS - returns error on master.
  • this library is also available for linux (which is why I wrote the conditional fetchpatch), but I can't test it there. I suspect that these patches will be required to get it compiling on linux.
  • this PR includes a commit I made adding myself to the maintainers list, hope that's ok.

@Amar1729 Amar1729 changed the title Init/argp standalone argp-standalone: init Oct 20, 2018
@Amar1729 Amar1729 changed the title argp-standalone: init argp-standalone: init at 1.3 Oct 20, 2018
@dtzWill
Copy link
Member

dtzWill commented Oct 21, 2018

Useful for musl as well!

@Amar1729
Copy link
Contributor Author

Anything else I can do to get this merged? Not sure who to @

@samueldr
Copy link
Member

Let's see how well it builds (as a formality)

@GrahamcOfBorg build argp-standalone

@GrahamcOfBorg
Copy link

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: argp-standalone

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: argp-standalone

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: argp-standalone

Partial log (click to expand)

make[2]: Entering directory '/private/tmp/nix-build-argp-standalone-1.3.drv-0/argp-standalone-1.3/testsuite'
make[2]: Nothing to be done for 'install-exec-am'.
make[2]: Nothing to be done for 'install-data-am'.
make[2]: Leaving directory '/private/tmp/nix-build-argp-standalone-1.3.drv-0/argp-standalone-1.3/testsuite'
make[1]: Leaving directory '/private/tmp/nix-build-argp-standalone-1.3.drv-0/argp-standalone-1.3/testsuite'
post-installation fixup
strip is /nix/store/g5r4apl0za012ffs6ladinwa5w0m1l3k-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/nmdkpkn051zarc54vzsc88cbwxh3f2gx-argp-standalone-1.3/lib
patching script interpreter paths in /nix/store/nmdkpkn051zarc54vzsc88cbwxh3f2gx-argp-standalone-1.3
/nix/store/nmdkpkn051zarc54vzsc88cbwxh3f2gx-argp-standalone-1.3

@samueldr
Copy link
Member

@LnL7 any thoughts? (Seems to be a darwin-exclusive.)

@Amar1729
Copy link
Contributor Author

Amar1729 commented Nov 12, 2018 •
edited

@samueldr i admit the expression as written is only for darwin, but argp itself works on linux too. I just don't have access to a linux box yet to test the compilation there.
I could try updating the PR to include the linux patches/functionality if that might help.

@LnL7
Copy link
Member

LnL7 commented Nov 12, 2018

I didn't try to build something with it but it looks good to me.

samueldr
samueldr approved these changes Nov 12, 2018
View reviewed changes
Copy link
Member

@samueldr samueldr left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't build as-is on Linux, so don't add platforms.linux, but assuming it's mostly unneeded on Linux, as it is seems fine.

I don't have real experience with Darwin stuff, preferred to defer to someone in the know :).

@samueldr samueldr merged commit 9c86aca into NixOS:master Nov 12, 2018
@dtzWill
Copy link
Member

dtzWill commented Nov 13, 2018 via email

@Amar1729 Amar1729 deleted the init/argp-standalone branch November 19, 2018 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samueldr samueldr samueldr approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Amar1729 @dtzWill @samueldr @GrahamcOfBorg @LnL7 @Janik-Haag