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.

Sign up for GitHub

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

Jump to bottom

vmTools: fix cross compilation #109636

Merged
merged 1 commit into from Sep 21, 2021
Merged

vmTools: fix cross compilation #109636

merged 1 commit into from Sep 21, 2021

Conversation

mroi
Copy link
Contributor

@mroi mroi commented Jan 17, 2021

I am creating a Linux initrd on Darwin via cross compilation. This is one of the needed fixes:

initrdUtils runs the nukeReferences tool at build-time, but a host-machine dependency is added instead of a build-machine dependency.

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.

@mroi
Copy link
Contributor Author

mroi commented Jan 24, 2021

The reason why I labeled this a draft is that other attributes in vmTools might need the same treatment. I can investigate, if completeness is desired.

Mic92
Mic92 reviewed Jan 24, 2021
View reviewed changes
pkgs/build-support/vm/default.nix
@@ -23,7 +23,7 @@ rec {
hd = "vda"; # either "sda" or "vda"

initrdUtils = runCommand "initrd-utils"
{ buildInputs = [ nukeReferences ];
{ nativeBuildInputs = [ buildPackages.nukeReferences ];
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 likely needed because nukeReferences does not come from callPackage context where our cross-compile overlay can override it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. The alternative would be to pass nukeReferences as an argument fulfilled one level up by callPackage. I think then the blanket with pkgs; should go away as it would overlay these names. Do you think this is the better solution?

In addition to nukeReferences, I believe the dependencies perl, perlPackages.XMLSimple, and dpkg further down would need the same treatment.

@mroi mroi force-pushed the patch-vmtools branch 2 times, most recently from 5c03555 to 915b7e6 Compare March 23, 2021 12:02
@mroi mroi changed the title initrdUtils: fix cross compilation vmTools: fix cross compilation Mar 23, 2021
@mroi
vmTools: fix cross compilation
e56bddf 
executables used at build-time should be taken from buildPackages
@mroi mroi marked this pull request as ready for review March 23, 2021 12:25
@mroi
Copy link
Contributor Author

mroi commented Mar 23, 2021 •
edited

I think I found all tools that need to be prefixed with buildPackages, because they need to run at build time.

The following works on Darwin with this patch:

  • I can successfully cross compile a Linux initrd on Darwin. (It also runs correctly inside QEMU on Darwin.)
  • I can successfully cross-evaluate the derivations for the Linux distribution’s disk images (both RPM and dpkg). I cannot build these derivations yet, because of other roadblocks.

Because buildPackages should be equal to pkgs on Linux, I would not expect any impact there. Please comment.

@mroi mroi requested a review from Mic92 March 23, 2021 12:32
@stale
Copy link

stale bot commented Sep 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 21, 2021
@Mic92 Mic92 merged commit 57e69a6 into NixOS:master Sep 21, 2021
@mroi mroi deleted the patch-vmtools branch October 18, 2021 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Awaiting requested review from Mic92

Assignees
No one assigned
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mroi @Mic92