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

treewide: makeWrapper buildInputs to nativeBuildInputs, makeWrapper: use proper shell when cross-compiling #112276

Merged
merged 2 commits into from
Feb 19, 2021

Conversation

siraben
Copy link
Member

@siraben siraben commented Feb 7, 2021

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@siraben siraben marked this pull request as draft February 7, 2021 09:29
@siraben
Copy link
Member Author

siraben commented Feb 7, 2021

Even though I notice potentially many other incorrect placements of dependencies in buildInputs, I will restrict myself to moving makeWrapper only.

@siraben
Copy link
Member Author

siraben commented Feb 7, 2021

This is being done manually. Many of the packages that I moved makeWrapper into nativeBuildInputs use it in the installPhase. So it should not actually affect the end result.

@samueldr
Copy link
Member

samueldr commented Mar 3, 2021

.../nixpkgs $ env -i nix-build -A pkgsCross.aarch64-multiplatform.python3Packages.libevdev
error: attribute 'runtimeShell' missing, at .../nixpkgs/pkgs/top-level/all-packages.nix:517:79
(use '--show-trace' to show detailed location information)

Reverting e899331 fixes eval.

It seems to affect anything using buildPythonPackage.

This happens at the merge commit of this PR (c34b032), on the current tip of nixos-unstable (0aeba64), and at the tip of staging (d39fafe). Note that #114902 changes nothing to this issue.

Ericson2314 added a commit to obsidiansystems/nixpkgs that referenced this pull request Mar 3, 2021
Fix NixOS#114952

Before it was using the wrong `makeWrapper`, the one that expected to
run on the target platform and produce packages for the "super-target".

The "super target" (sliding off the end of the platforms list) is
defined to be the same as the target, to cap it with a loop so there is
no sliding off. However, `targetPackages.targetPackages` is *not*
similarly defined. Per the bottom of `pkgs/stdenv/booter.nix` it is an
almost-empty package set that just exists as a hack to help some things
with exotic `depsTargetTarget`.

In particular, that rump "super `targetPackages`" does not contain a
`runtimeShell`, and that's the source of our eval-time error.
Ericson2314 added a commit that referenced this pull request Mar 4, 2021
buildInputs = [ makeWrapper ];
nativeBuildInputs = [ makeWrapper ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a buildEnv (which doesn't support nativeBuildInputs). I can submit a PR to revert the change in this file, but I wonder if other packages were broken this way as well, and whether buildEnv should perhaps support nativeBuildInputs after all?

r-burns pushed a commit to r-burns/nixpkgs that referenced this pull request Jun 8, 2021
Since NixOS#112276, we should always put `makeWrapper` in
`nativeBuildInputs`. But `buildEnv` was saying put it in `buildInputs`.
That's wrong!

Fix the instructions, and make the right thing possible.

(cherry picked from commit 4f6ec19)
r-burns pushed a commit to r-burns/nixpkgs that referenced this pull request Jun 8, 2021
Now that `buildEnv` is ready, always put `makeWrapper` in
`nativeBuildInputs`, rather than `buildInputs` or (worse) mucking around
with setup hooks by hand.

(C.f. NixOS#112276, which didn't catch these because the manual setup hook
sourcing is such a hack to being with!)

Fixes NixOS#114687

(cherry picked from commit 07ecf87)
Yarny0 added a commit to Yarny0/nixpkgs that referenced this pull request Jan 17, 2022
Although I'm not sure if `tsm-client` will ever be
subject to cross-compiling, referencing makeWrapper
from native BuildInputs is The Right Thing.

This is a kind of follow-up of
NixOS#112276
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.