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

ripgrep: fix shell completions when cross compiling #271072

Merged
merged 1 commit into from
Dec 31, 2023

Conversation

uninsane
Copy link
Contributor

Description of changes

#270521 fixed cross compilation but the result was missing shell completions. this patch fixes shell completions too.

$ nix build '.#ripgrep'
$ tree result
result
├── bin
│   └── rg
└── share
    ├── bash-completion
    │   └── completions
    │       └── rg.bash
    ├── fish
    │   └── vendor_completions.d
    │       └── rg.fish
    ├── man
    │   └── man1
    │       └── rg.1.gz
    └── zsh
        └── site-functions
            └── _rg

11 directories, 5 files

$ nix build '.#pkgsCross.aarch64-multiplatform.ripgrep'
$ tree result
result
├── bin
│   └── rg
└── share
    ├── bash-completion
    │   └── completions
    │       └── rg.bash
    ├── fish
    │   └── vendor_completions.d
    │       └── rg.fish
    ├── man
    │   └── man1
    │       └── rg.1.gz
    └── zsh
        └── site-functions
            └── _rg

11 directories, 5 files

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.05 Release Notes (or backporting 23.05 and 23.11 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.

Priorities

Add a 👍 reaction to pull requests you find important.

@uninsane
Copy link
Contributor Author

combed through the output of nix log ./result this time to make sure there weren't any non-fatal errors.

@uninsane uninsane mentioned this pull request Nov 30, 2023
13 tasks
@uninsane uninsane requested a review from Ma27 November 30, 2023 00:45
@ofborg ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Nov 30, 2023
@ofborg ofborg bot requested review from zowoq and globin November 30, 2023 05:34
pkgs/tools/text/ripgrep/default.nix Show resolved Hide resolved
pkgs/tools/text/ripgrep/default.nix Outdated Show resolved Hide resolved
@uninsane uninsane force-pushed the pr-ripgrep-270521-feedback branch from 8631ddf to d8dcdda Compare December 1, 2023 00:02
@ofborg ofborg bot requested a review from Ma27 December 1, 2023 07:41
@Ma27
Copy link
Member

Ma27 commented Dec 9, 2023

The change in here looks good, however I spotted another issue: the installCheckPhase will never be run on cross builds (that's hard-wired in mkDerivation as I learned now and I'll open an issue for that).

I'm not keen on skipping a basic functionality test just because of some shortcomings in the current stdenv. A possible workaround which forces the face to be executed is

diff --git a/pkgs/tools/text/ripgrep/default.nix b/pkgs/tools/text/ripgrep/default.nix
index 8aa91640fd1f..b16b350757e4 100644
--- a/pkgs/tools/text/ripgrep/default.nix
+++ b/pkgs/tools/text/ripgrep/default.nix
@@ -42,7 +42,10 @@ in rustPlatform.buildRustPackage rec {
       --zsh <(${rg} --generate complete-zsh)
   '';
 
-  doInstallCheck = canRunRg;
+  postInstall = ''
+    export doInstallCheck=${toString canRunRg}
+  '';
+
   installCheckPhase = ''
     file="$(mktemp)"
     echo "abc\nbcd\ncde" > "$file"

Opinions?
cc @zowoq @globin @SuperSandro2000

@SuperSandro2000
Copy link
Member

I don't really care if we run the tests on cross.

pkgs/tools/text/ripgrep/default.nix Outdated Show resolved Hide resolved
@Ma27 Ma27 merged commit f152e6c into NixOS:master Dec 31, 2023
22 of 23 checks passed
@Ma27
Copy link
Member

Ma27 commented Dec 31, 2023

Thanks!

And sorry for the long wait!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different platform than they will be used on 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants