Skip to content

atuin: only generate completions if program can be executed#316749

Merged
SuperSandro2000 merged 1 commit intoNixOS:masterfrom
SFrijters:atuin-conditional-completions
Jun 5, 2024
Merged

atuin: only generate completions if program can be executed#316749
SuperSandro2000 merged 1 commit intoNixOS:masterfrom
SFrijters:atuin-conditional-completions

Conversation

@SFrijters
Copy link
Member

Description of changes

I expect #289517 will break cross otherwise, as we end up with 0 byte files (the actual error is ignored):

$ nix build .#pkgsCross.aarch64-multiplatform.atuin -L
[...]
atuin-aarch64-unknown-linux-gnu> /nix/store/l30525lnvzaiwqihqzspgrljmvjfx9j6-stdenv-linux/setup: line 114: /nix/store/xrv6dznmsr7kc2645q3c40mgp0wn11h5-atuin-aarch64-unknown-linux-gnu-18.2.0/bin/atuin: cannot execute binary file: Exec format error
atuin-aarch64-unknown-linux-gnu> /nix/store/l30525lnvzaiwqihqzspgrljmvjfx9j6-stdenv-linux/setup: line 114: /nix/store/xrv6dznmsr7kc2645q3c40mgp0wn11h5-atuin-aarch64-unknown-linux-gnu-18.2.0/bin/atuin: cannot execute binary file: Exec format error
atuin-aarch64-unknown-linux-gnu> /nix/store/l30525lnvzaiwqihqzspgrljmvjfx9j6-stdenv-linux/setup: line 114: /nix/store/xrv6dznmsr7kc2645q3c40mgp0wn11h5-atuin-aarch64-unknown-linux-gnu-18.2.0/bin/atuin: cannot execute binary file: Exec format error

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@SFrijters SFrijters requested a review from SuperSandro2000 June 2, 2024 22:27
@SFrijters
Copy link
Member Author

Hm, since this kind of error will probably pop up on more packages, I would like to know if this is the best fix.

I think checking stdenv.buildPlatform.canExecute stdenv.hostPlatform is strictly better (more permissive while still avoiding errors) than stdenv.buildPlatform == stdenv.hostPlatform, but I also see stdenv.hostPlatform.emulatorAvailable buildPackages e.g. in https://github.com/NixOS/nixpkgs/blob/df2577c6a4938e291283b0d8fab175bf72a86b73/pkgs/tools/misc/broot/default.nix where the actual generation command then turns into something like ${stdenv.hostPlatform.emulator buildPackages} $out/bin/broot --print-shell-function bash > br.bash.

Does the emulator fall back to a no-op/passthrough command if stdenv.buildPlatform == stdenv.hostPlatform? Is this overkill, or better than the canExecute variant?

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jun 2, 2024
@NickCao
Copy link
Member

NickCao commented Jun 3, 2024

Does the emulator fall back to a no-op/passthrough command if stdenv.buildPlatform == stdenv.hostPlatform? Is this overkill, or better than the canExecute variant?

Yes. I personally don't like bringing the emulator (qemu) into the build closure of packages, shell completions are not critical to the functioning of packages thus not worth it.

@SuperSandro2000
Copy link
Member

The best solution would be, if upstream could just commit those files.

qemu is kinda slow, building the package twice, also.

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

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants