gitMinimal: Fix wrong shell for cross compiled package by ck3d · Pull Request #141959 · NixOS/nixpkgs

ck3d · 2021-10-17T09:15:47Z

The current package depends on the build hosts shell and therefor a lot of scripts can not be used on the target:

❯ nix-build -A pkgsCross.aarch64-multiplatform.gitMinimal
/nix/store/52lfla8dfp22520axc9jalz8bnp8l6xx-git-aarch64-unknown-linux-gnu-2.33.0

❯ nix-store -q --references ./result/
/nix/store/nrwmkyqhqczn61zrb3z05k31nnkqjqyb-glibc-aarch64-unknown-linux-gnu-2.33-50
/nix/store/hwzhnqhjgry727m5vkqy8zf9ppngbp3p-zlib-1.2.11-aarch64-unknown-linux-gnu
/nix/store/kkll2mjnj1dsdlhyvp78v9fjisvby86z-openssl-aarch64-unknown-linux-gnu-1.1.1l
/nix/store/2kjjiic8x1adyps4wn7p1w1ah404kdga-curl-aarch64-unknown-linux-gnu-7.76.1
/nix/store/7i3q6zdhwqfahyfm6a4lb7m4gh0m0ra4-aarch64-unknown-linux-gnu-stage-final-gcc-debug-9.3.0-lib
/nix/store/gixyqr6hc6d296ymgicklqnixv4dbfxd-expat-aarch64-unknown-linux-gnu-2.4.1
/nix/store/jwy7714ddm0yqzbii5hbj3ycn1xb7sdk-coreutils-aarch64-unknown-linux-gnu-8.32
/nix/store/k4ai6vqsqgixrbrjq14w5pg8z8pn9mfl-gettext-aarch64-unknown-linux-gnu-0.21
/nix/store/r7ag3jn3vgrr2s0jb5k9s98x7768dfgf-openssh-aarch64-unknown-linux-gnu-8.7p1
/nix/store/rgm0p4yrq6gl2475v779z25l355c9bdv-gnused-aarch64-unknown-linux-gnu-4.8
/nix/store/ss074bgk5fwz85mif0is1hwz6ig1mwaj-gnugrep-aarch64-unknown-linux-gnu-3.6
/nix/store/wadmyilr414n7bimxysbny876i2vlm5r-bash-5.1-p8
/nix/store/52lfla8dfp22520axc9jalz8bnp8l6xx-git-aarch64-unknown-linux-gnu-2.33.0

This patch fixed the dependency to the build hosts shell:

❯ nix-build -A pkgsCross.aarch64-multiplatform.gitMinimal
/nix/store/i08892l479l8yvbrlz64hh071kwky14r-git-aarch64-unknown-linux-gnu-2.33.0

❯ nix-store -q --references ./result/
/nix/store/nrwmkyqhqczn61zrb3z05k31nnkqjqyb-glibc-aarch64-unknown-linux-gnu-2.33-50
/nix/store/hwzhnqhjgry727m5vkqy8zf9ppngbp3p-zlib-1.2.11-aarch64-unknown-linux-gnu
/nix/store/kkll2mjnj1dsdlhyvp78v9fjisvby86z-openssl-aarch64-unknown-linux-gnu-1.1.1l
/nix/store/2kjjiic8x1adyps4wn7p1w1ah404kdga-curl-aarch64-unknown-linux-gnu-7.76.1
/nix/store/7i3q6zdhwqfahyfm6a4lb7m4gh0m0ra4-aarch64-unknown-linux-gnu-stage-final-gcc-debug-9.3.0-lib
/nix/store/gixyqr6hc6d296ymgicklqnixv4dbfxd-expat-aarch64-unknown-linux-gnu-2.4.1
/nix/store/jwy7714ddm0yqzbii5hbj3ycn1xb7sdk-coreutils-aarch64-unknown-linux-gnu-8.32
/nix/store/k4ai6vqsqgixrbrjq14w5pg8z8pn9mfl-gettext-aarch64-unknown-linux-gnu-0.21
/nix/store/r7ag3jn3vgrr2s0jb5k9s98x7768dfgf-openssh-aarch64-unknown-linux-gnu-8.7p1
/nix/store/rgm0p4yrq6gl2475v779z25l355c9bdv-gnused-aarch64-unknown-linux-gnu-4.8
/nix/store/ss074bgk5fwz85mif0is1hwz6ig1mwaj-gnugrep-aarch64-unknown-linux-gnu-3.6
/nix/store/i08892l479l8yvbrlz64hh071kwky14r-git-aarch64-unknown-linux-gnu-2.33.0

Motivation for this change

Want to use cross compiled gitMinimal package.

Things done

bjornfor · 2021-10-21T18:20:02Z

pkgs/applications/version-management/git-and-tools/git/default.nix

Don't we have a way to get the target shell at compile time? (runtimeShell?) Will git use that shell at build time as well as runtime?

Yes, Git will use SHELL_PATH during build as well on the target.

The target shell can be access via targetPacages.bash.

pkgs/applications/version-management/git-and-tools/git/default.nix

Artturin · 2021-10-23T21:51:30Z

setting the SHELL_PATH is a variable that has been there since at least 2007 and is not needed anymore.

80e42ed (#142602) fixes the build shell reference by adding bash to buildInputs

ck3d · 2021-10-23T22:10:56Z

Removing SHELL_PATH is a regression for the native built package, since the fixup phase patches not all /bin/sh occurrences.

Artturin · 2021-10-23T22:54:21Z

alright, should be fine now 69780f2 (#142602)

ck3d · 2021-10-24T09:44:08Z

Your version do not build, since SHELL_PATH is executed during build:

nix-build -A pkgsCross.aarch64-multiplatform.gitMinimal
...
    CC hashmap.o
    GEN command-list.h
/nix/store/wadmyilr414n7bimxysbny876i2vlm5r-bash-5.1-p8/bin/bash: line 1: /nix/store/z1mid8asz0cnbpdd71h34k1slk9sypnm-bash-5.1-p8-aarch64-unknown-linux-gnu/bin/bash: cannot execute binary file: Exec format error
make: *** [Makefile:2256: command-list.h] Error 126

The Git package do not allow to specify build and target shell differently. On non NixOS systems, this doesn't matter, since the absolute paths to a shell is the same between build and target system.

ck3d · 2021-10-24T15:51:58Z

I think you configured your system to emulate aarch64 via NixOS option boot.binfmt.emulatedSystems. Please disable this feature and re-build the same package.
From my point of view, this is a big impurity issue of Nix.

Artturin · 2021-10-24T17:23:23Z

Thanks, that was indeed the case :/

mohe2015 · 2021-10-24T17:32:11Z

I think you configured your system to emulate aarch64 via NixOS option boot.binfmt.emulatedSystems. Please disable this feature and re-build the same package.
From my point of view, this is a big impurity issue of Nix.

I agree with you maybe open an issue for that?

ck3d · 2021-10-24T20:14:46Z

I created #142778 to face the impurity problem.

So we can come back to gitMinimal, this PR is still valid and feedback is welcome. I hope we can get the cross compiled gitMinimal fixed, since it is a mandatory dependency for Nix flakes.

r-burns

Thank you for this, I was not aware of this issue but have now verified locally that cross-compiled git uses incorrect shebangs because of SHELL_PATH.

To mitigate future regressions, how about adding something like this:

disallowedReferences = optionals (buildPlatform != hostPlatform) [
  stdenv.shell
]

Then the package will fail the build, instead of silently using bad shebangs

r-burns · 2021-10-24T20:38:50Z

pkgs/applications/version-management/git-and-tools/git/default.nix

This is already an improvement, but I'd still like the cross-compiled git to refer exactly to its buildInputs in the store when possible. Can we add a postInstall patchShebangs --host step when cross-compiling?

There are references which aren't shebangs

Sure, would be good to patch those too if possible. If not, /bin/sh is at least better than a shell that can't be executed at all.

The path /bin/sh is embedded in binaries and I did not find an easy way to replace the path without breaking the executable.

ck3d · 2021-10-25T06:21:06Z

Thanks, great idea, but when I set disallowedReferences then Nix complains:

checking for references to /build/ in /nix/store/fxcin2ismi5z8wpbvbnc65hvc5ldqjax-git-aarch64-unknown-linux-gnu-2.33.0...
error: derivation contains an illegal reference specifier '/nix/store/wadmyilr414n7bimxysbny876i2vlm5r-bash-5.1-p8/bin/bash'

That's weird, since nix-store doesn't show the dependency.
Has anyone an idea why Nix complains?

Artturin · 2021-11-03T22:37:23Z

had to force push to fix the checks

Synthetica9 · 2021-11-30T17:16:58Z

I think this broke the NixOS option hardware.pulseaudio.support32Bit. At least, that's what my bisect shows.

Artturin · 2021-11-30T17:19:15Z

in what way did it break?

Artturin · 2021-11-30T17:20:20Z

this shouldn't have affected that at all?

Synthetica9 · 2021-11-30T17:30:16Z

I was getting error: a 'i686-linux' with features {} is required to build '/nix/store/xds3y616a8apx3y36s2dzsxx5qzdpdyf-builder.pl.drv', but I am a 'x86_64-linux' with features {benchmark, big-parallel, kvm, nixos-test}

But it looks like git bisect borked? I assumed it was right because this is also a cross-compiling thing, but the commit before this also fails... I'll investigate further

Artturin · 2021-11-30T17:38:07Z

lets continue in https://matrix.to/#/!kjdutkOsheZdjqYmqp:nixos.org/$tniRbDvMRTh4peCoBCkqdvvTOZjV9iVfM8PMFKEexmA?via=nixos.org&via=matrix.org&via=nixos.dev

ck3d added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Oct 17, 2021

ofborg bot requested review from globin, primeos and wmertens October 17, 2021 09:28

ck3d force-pushed the fix-cross-git branch from 918233d to e7662c0 Compare October 18, 2021 16:42

ck3d changed the title ~~git: Fix wrong shell for cross compiled package~~ gitMnimal: Fix wrong shell for cross compiled package Oct 18, 2021

ck3d changed the title ~~gitMnimal: Fix wrong shell for cross compiled package~~ gitMinimal: Fix wrong shell for cross compiled package Oct 18, 2021

bjornfor reviewed Oct 21, 2021

View reviewed changes

SuperSandro2000 reviewed Oct 21, 2021

View reviewed changes

pkgs/applications/version-management/git-and-tools/git/default.nix Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

r-burns reviewed Oct 24, 2021

View reviewed changes

ofborg bot added 8.has: package (new) This PR adds a new package and removed 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels Nov 1, 2021

SuperSandro2000 removed request for Ericson2314, FRidh, LnL7, Mic92, andir, globin, jonringer, matthewbauer, primeos, stigtsp, ttuegel, wmertens, zakame and zowoq November 1, 2021 12:54

gitMinimal: Fix wrong shell for cross compiled package

7bfd3a8

Artturin force-pushed the fix-cross-git branch from 7d44fb7 to 7bfd3a8 Compare November 3, 2021 22:36

ofborg bot requested review from globin, primeos and wmertens November 3, 2021 22:48

Artturin merged commit 219339e into NixOS:staging Nov 5, 2021

Artturin mentioned this pull request Nov 19, 2021

Cross-compiled git uses wrong bash #107639

Closed

Uh oh!

Conversation

ck3d commented Oct 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

bjornfor Oct 21, 2021

Choose a reason for hiding this comment

Uh oh!

ck3d Oct 21, 2021

Choose a reason for hiding this comment

Uh oh!

ck3d Oct 21, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Artturin commented Oct 23, 2021

Uh oh!

ck3d commented Oct 23, 2021

Uh oh!

Artturin commented Oct 23, 2021

Uh oh!

ck3d commented Oct 24, 2021

Uh oh!

This comment has been minimized.

ck3d commented Oct 24, 2021

Uh oh!

Artturin commented Oct 24, 2021

Uh oh!

mohe2015 commented Oct 24, 2021

Uh oh!

ck3d commented Oct 24, 2021

Uh oh!

r-burns left a comment

Choose a reason for hiding this comment

Uh oh!

r-burns Oct 24, 2021

Choose a reason for hiding this comment

Uh oh!

Artturin Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

r-burns Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

ck3d Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

ck3d commented Oct 25, 2021

Uh oh!

Artturin commented Nov 3, 2021

Uh oh!

Synthetica9 commented Nov 30, 2021

Uh oh!

Artturin commented Nov 30, 2021

Uh oh!

Artturin commented Nov 30, 2021

Uh oh!

Synthetica9 commented Nov 30, 2021

Uh oh!

Artturin commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ck3d commented Oct 17, 2021 •

edited

Loading

Artturin commented Nov 30, 2021 •

edited

Loading