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

tomb: remove gpg symlink #57946

Merged
merged 1 commit into from Apr 8, 2019
Merged

tomb: remove gpg symlink #57946

merged 1 commit into from Apr 8, 2019

Conversation

bricewge
Copy link
Contributor

Motivation for this change

This symlink has been added after the release of gnupg2 to use the newer version instead of the legacy one. Later, when gnupg changed the gpg binary to be the v2 the symlink has been modified but not removed. This commit fix that.

I have tested the use of tomb and it work correctly.

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.

@peterhoeg

@bricewge bricewge mentioned this pull request Mar 20, 2019
Closed
@bricewge bricewge force-pushed the tomb-2.5-no-gpg-symlink branch 3 times, most recently from fbc3e6b to 3a1732e Compare March 23, 2019 15:31
@bricewge
Copy link
Contributor Author

bricewge commented Apr 2, 2019

The PR has been updated with the missing dependencies and a test of the actual command.

@peterhoeg peterhoeg merged commit f68549c into NixOS:master Apr 8, 2019
@peterhoeg
Copy link
Member

Cool, thanks for your patience @bricewge !

@ArdaXi
Copy link
Contributor

ArdaXi commented Apr 12, 2019

This PR broke evaluation of the tomb package on my Hydra and locally. Removing the doInstallCheck fixed it, and restored full functionality.

I have no idea how you managed to fix #57986 for yourself, but the issue seems to persist.

@bricewge bricewge mentioned this pull request Apr 12, 2019
Merged
10 tasks
@bricewge
Copy link
Contributor Author

@ArdaXi It is fixed in #59361, sudo needed to be available in the PATH.

#57986 isn't fixed, I closed the issue because it seems an edge case to me: nix-shell --pure -p bash zsh tomb pinentry --command "bash -c 'tomb -h'" still fail but not when you replace bash by zsh.

@ArdaXi
Copy link
Contributor

ArdaXi commented Apr 12, 2019

Under #59361 I can't reproduce #57986 either. I would be very surprised if the two failures aren't related.

The process in which the check runs is very similar to the one you create with your nix-shell. With the right buildInputs, the dependencies are propagated and tomb has access to everything it needs, even if you just run it in nix-shell --pure -p tomb. If nix-shell --pure -p bash tomb --command "bash -c 'tomb -h'" doesn't work, that's a fair reason to assume tomb wasn't packaged properly.

@bricewge
Copy link
Contributor Author

To me, it seems that the core issue with tomb is it's wrapping, to set the PATH. tomb is written in zsh while the wrapper is in bash.

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

@Mic92 Mic92 Mic92 left review comments

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bricewge @peterhoeg @ArdaXi @Mic92 @GrahamcOfBorg