-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
installShellCompletion: add sanity check #289517
installShellCompletion: add sanity check #289517
Conversation
98cd7f1
to
6fe9762
Compare
6fe9762
to
2678ea4
Compare
2678ea4
to
6b6073f
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3976 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this with atuin and it works as expected.
There will be probably a handful of regressions but I think we can just fix them as they happen.
NixOS#289517 will break cross otherwise.
It was running unpatched binary which was failing and thus generating empty output. After NixOS#289517 installShellCompletion errors out because of this, which lead to broken build. Move installShellCompletion call to after autoPatchelfHook in fixupPhase.
Description of changes
When implementing #288279 I first made a typo (i.e. calling
<($out/bin/espflash completion bash)
instead of<($out/bin/espflash completions bash)
) and I didn't notice it until I manually checked the generated completion files - any errors when using the<(...)
syntax are apparently silently ignored.I tried to add some error handing to it but it looked a bit tricky to fix without unindended breakage. But maybe having this simple sanity check would be nice.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.