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

Jump to bottom

gitAndTools.git-annex: wrap binary on not-Linux to use Nixpkgs' coreutils #53480

Merged
merged 1 commit into from Feb 17, 2019
Merged

gitAndTools.git-annex: wrap binary on not-Linux to use Nixpkgs' coreutils #53480

merged 1 commit into from Feb 17, 2019

Conversation

andrew-d
Copy link
Contributor

@andrew-d andrew-d commented Jan 6, 2019 •
edited

Motivation for this change

Fixes #53423

I do have a remaining question of whether to wrap unconditionally, or just on Darwin? Defaulting to just Darwin for now.

Note: I put this in configuration-nix.nix instead of the existing block in configuration-common.nix because this is a Nix-specific fix; happy to combine the two if people would prefer.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @kirelagin

@kirelagin
Copy link
Member

👍

to wrap unconditionally, or just on Darwin?

Maybe, not isLinux?

By the way, cc has an isGNU flag, we actually need something like this here :).

@andrew-d
Copy link
Contributor Author

andrew-d commented Jan 6, 2019

Maybe, not isLinux?

Good idea; done!

@andrew-d andrew-d changed the title gitAndTools.git-annex: wrap binary on Darwin to use Nixpkgs' coreutils gitAndTools.git-annex: wrap binary on not-Linux to use Nixpkgs' coreutils Jan 6, 2019
@andrew-d
Copy link
Contributor Author

andrew-d commented Feb 4, 2019

@basvandijk and/or @peti - Any chance I could get a review on this, when you get a moment?

@peti
Copy link
Member

peti commented Feb 4, 2019

Wouldn't it be a good idea in general to have coreutils in $PATH before the default utilities? I reckon there's more than one program in Nix that would be confused by seeing one toolset at configuration-time and another toolset at runtime.

peti
peti approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

👍

@andrew-d
Copy link
Contributor Author

@peti - Apologies for the ping, but I saw you approved the PR; is there anything else I need to do to help get this merged?

@ryantm ryantm merged commit 9a32caa into NixOS:master Feb 17, 2019
@andrew-d andrew-d deleted the andrew/git-annex-darwin branch February 17, 2019 23:16
@andrew-d
Copy link
Contributor Author

@ryantm - Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peti peti peti approved these changes

@basvandijk basvandijk Awaiting requested review from basvandijk

@ryantm ryantm Awaiting requested review from ryantm

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: haskell 10.rebuild-darwin: 1-10 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

git-annex has a wrong idea about cp on darwin
5 participants
@andrew-d @kirelagin @peti @ryantm @GrahamcOfBorg