[RFC] A large batch of cross-compilation fixes by bgamari · Pull Request #30882 · NixOS/nixpkgs

bgamari · 2017-10-28T00:06:15Z

Note: The prefix of commits authored by @Ericson2314 can be ignored for the purposes of this pull request. These are prerequisites for this work but I'll leave upstreaming these to @Ericson2314.

Motivation for this change

Allow Nix to cross-compile a basic system environment.

Things done

The only testing I have done thusfar is to build systemd and its dependencies with the following overlay:

self: super:                                                                                                                                                                                                                                                                                                                 
                                                                                                                                                                                                                                                                                                                             
let                                                                                                                                                                                                                                                                                                                          
  pkgs = self.pkgs;                                                                                                                                                                                                                                                                                                          
  callPackage = self.pkgs.callPackage;                                                                                                                                                                                                                                                                                       
in rec {                                                                                                                                                                                                                                                                                                                     
  # guile 2.2 doesn't cross compile. See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28920                                                                                                                                                                                                                                 
  guile = super.guile_2_0;                                                                                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                                                                             
  # We really don't need libapparmor and it is a pain to cross compile due to                                                                                                                                                                                                                                                
  # its perl bindings                                                                                                                                                                                                                                                                                                        
  systemd = super.systemd.override { libapparmor = null; };                                                                                                                                                                                                                                                                  
}

That being said, this was no small feat. I would like to start getting pieces of this work upstream but I'm a bit uncertain as to how to go about this as there are a few open questions. Namely,

Are we okay with continuing to pepper expressions with doCheck = stdenv.buildPlatform == stdenv.hostPlatform?
Why have I needed to add so many buildPackages. to nativeBuildInputs? Shouldn't this be redundant? Is there perhaps a bug, @Ericson2314?
How do we guarantee that things like fetchurl always use build packages? Is the approach I took here acceptable? It seems something similar will need to be done with unpackCmd.
Is the Always search DT_RPATH hack acceptable?
Some packages' configure scripts have tests that want to run host programs on the build machine. This obviously isn't possible so I provide these test results manually (see, for instance, the krb5 expression). These are of course guesses, but I believe they should apply in most sensible environments. Is this acceptable?

Whatever reviews you can offer are greatly appreciated but I really just wanted to raise a flag so people know this work exists.

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

bgamari · 2017-10-28T00:38:22Z

I have also opened #30883 which isolates the (hopefully reasonably uncontroversial) doCheck changes.

zimbatm · 2017-11-05T21:05:18Z

Wow, this is impressive. Is there any way to cut the PR in smaller manageable chunks?

Also, how do I test this? The nixpkgs has some notes on cross-compilation but it's not really clear how to trigger it.

Ericson2314 · 2017-11-05T21:55:51Z

Once the other PRs contained within here are merged, the changes are fairly self-similar so I don't think breaking up is too important.
I was told at NixCon to provide some easy instantiatations for that.

dtzWill · 2017-11-05T22:03:51Z

pkgs/os-specific/linux/iptables/default.nix

  preConfigure = ''
-    export NIX_LDFLAGS="$NIX_LDFLAGS -lmnl -lnftnl"
+    export NIX_LDFLAGS="$NIX_LDFLAGS -lmnl -lnftnl -ldl"
  '';


commit message typo, libld -> libdl

Thanks! Fixed in my local branch.

Ericson2314 · 2017-11-05T22:05:36Z

@zimbatm buried within section 5.3 is

nix-build <nixpkgs> --arg crossSystem '(import <nixpkgs/lib>).systems.examples.fooBarBaz

A concrete example would be

nix-instantiate --arg crossSystem '(import ./lib).systems.examples.aarch64-multiplatform' -A systemd

until I do that above that should test this.

bgamari · 2017-11-05T23:20:53Z

Wow, this is impressive. Is there any way to cut the PR in smaller manageable chunks?

As @Ericson2314 has said, many of the changes are quite similar. That being said, some are not and deserve closer consideration. I can certainly break these out.

zimbatm · 2017-11-06T12:38:42Z

Is there any way to make doCheck = stdenv.buildPlatform == stdenv.hostPlatform automatic? I guess it would require to change the builder.sh. I think that it's too much to ask for normal packagers to remember cross-compilation, we are going to get regressions all the time otherwise.

periklis · 2017-11-06T13:18:14Z

pkgs/build-support/cc-wrapper/default.nix

+        ''
+
+        + ''
+      done


Is this not a good candidate for a dedicated PR on top of #29396 ?

Yes, I think so.

periklis · 2017-11-06T13:25:20Z

pkgs/development/libraries/libgcrypt/default.nix

  '';
+  configureFlags = [
+    ''--with-libgpg-error-prefix=${libgpgerror.dev.outPath}''
+  ];


I am not sure this works at all. libgcrypt/libgpgerror produce some asm code during configure by producing binaries according to the machine architecture. I know these issues from my experiments with webassembly, which is similar to cross-compilation.

I had a look at the git repo of libgcrypt and i meant libgcrypt/mpi in my previous comment.

periklis · 2017-11-06T13:33:19Z

pkgs/development/libraries/gnu-efi/default.nix

-    "AR=ar"
-    "RANLIB=ranlib"
-    "OBJCOPY=objcopy"
+    "CC=${stdenv.cc.prefix}gcc"


Shouldn't this be cc instead of gcc to be more clang-friendly?

Indeed it should.

periklis · 2017-11-06T13:34:20Z

pkgs/build-support/binutils-wrapper/ld-wrapper.sh

+                    # However, some packages (libssh2) build non-installed executables
+                    # in their build trees which link against shared libraries
+                    # in their tree. Linking these breaks if we are so restrictive.
+                    */*.so | */*.so.*)


Same here, shouldn't this be a separate PR by its own on top of #29396 ?

periklis · 2017-11-06T13:41:42Z

pkgs/development/tools/misc/binutils/default.nix

+    # shared objects when cross-compiling. Consequently, we are forced to
+    # override this behavior, forcing ld to search DT_RPATH even when
+    # cross-compiling.
+    ./always-search-rpath.patch


Same here, shouldn't this be a separate PR on top of #29396 ?

periklis · 2017-11-06T13:42:37Z

pkgs/stdenv/adapters.nix

-          propagatedBuildInputs = propagatedBuildInputs ++ buildInputs;
+          propagatedBuildInputs =
+            propagatedBuildInputs ++ buildInputs ++ __depsBuildTargetPropagated ++ __depsHostHostPropagated ++ __depsTargetTarget ++
+            depsBuildBuild ++ depsBuildTarget ++ __depsHostHost ++ __depsBuildBuildPropagated ++ __depsTargetTargetPropagated;


Same here, shouldn't this be a separate PR on top of #29396 ?

periklis · 2017-11-06T13:44:02Z

pkgs/os-specific/linux/apparmor/default.nix


    buildInputs = [
-      perl
+      #perl


why not drop this comment here? Imho the --without-perl below is quite self-explanatory.

periklis · 2017-11-06T14:25:21Z

First of all many many kudos to @bgamari for this batch of insightful work.

@Ericson2314 & @bgamari
I think this PR hits the surface in terms of cross compilation. These changes make visible what work has already been done and what cc patterns for maintainers emerge. We will need a docs/education/migration step for packages to use the cross-compilation pipeline after #29396 and #26805 get merged beyond just explaining all the derivation inputs variables.

I noticed the following patterns for maintainers, please beat me if i misunderstood:

Package maintainers have to adapt their doCheck to assert host==build or blacklist tests not cross-compiling
Package maintainers have to split their nativeBuildInputs and nativePropagatedBuildInputs because of the minimum dependency for buildPackages.stdenv.cc in the first variable
Package maintainers of Autoconf-based packages need to be aware of the autoreconfHook
Package maintainers of packages with assembler-generating configure code should be aware of how to manage their build process for cc, e.g. libgcrypt/mpi
Package maintainers need to be aware when to use configurePlatforms, e.g. guile
Educate maintainers not to hardcode usage of tools from the GNU Toolchain, eg. ar

Also the following hacks are not obvious from a maintenance perspective, but work:

A pattern of manual configureFlags/makeFlags wiring seems to emerge in some packages, e.g. gnu-efi: makeFlags and lzip: configureFlags, which seems a brittle approach by not using our wrapper?!?
Manual -rpath-link usage, e.g. libssh2 or curl

Finally, the following list of packages mentioned in this PR-changeset, should be picked in a separate PR in order to provide a minimum proper cross-compiling base set for package maintainers, in case this PR takes to long to hit master/staging:

P.S. Yes i went through each commit :)

zimbatm · 2017-11-06T17:01:14Z

nice review @periklis!

Would it make sense to start with a simple package like hello before touching so many files?

periklis · 2017-11-06T17:03:49Z

@zimbatm afaik nixpkgs.hello is not representative enough to hit so many important missing parts as @bgamari did with systemd. Imho we need repsentative closures for people that cross-compile by purpose, e.g.

the ARM-stuffs needs most parts of a running linux system
the Darwin->linux stuffs for docker images needs mostly some libc and build inputs of the final package.

bgamari · 2017-11-06T18:50:49Z

Thanks for the review @periklis! I'll split this up a bit more in the next few days.

Ericson2314 · 2017-11-06T20:20:11Z

I sort of mentioned this to @bgamari on IRC, but let me reiterate here.

Package maintainers have to split their nativeBuildInputs and nativePropagatedBuildInputs because of the minimum dependency for buildPackages.stdenv.cc in the first variable

The propagated variable is actually propagatedNativeBuildInputs.
buildPackages.stdenv.cc should always be a depsBuildBuild (that wouldn't be propagated).
Currently, there are very few uses of buildPackages.stdenv.cc, just what I and others recently added. They are all nativeBuildInputs as a stop-gap, it is fairly easy to grep and make them depsBuildBuild as I did.

periklis · 2017-11-07T07:43:39Z

@Ericson2314 Thx for re-iterating the information on IRC. What do you think about the rest of the patterns, i identified above? I would be glad to know your opinion on that. Maybe these are too minor to bother.

Ericson2314 · 2017-11-09T00:41:15Z

@periklis sure, I was thinking of going through all of them anywas

Package maintainers have to adapt their doCheck to assert host==build or blacklist tests not cross-compiling

#30437 is for this

Package maintainers of Autoconf-based packages need to be aware of the autoreconfHook

That hook already exists, to be clear. I suppose its just being added if sufficient support for the host platform we're building for postdates the version of autoconf originally used with the package. In theory this applies to native compilation on that same platform, too?

Package maintainers of packages with assembler-generating configure code should be aware of how to manage their build process for cc, e.g. libgcrypt/mpi

I'm not sure what's going on with this, but for code stuff built on the fly in general:

{
  depsBuildBuild = [ buildPackages.stdenv.cc ];
  nativeBuildInputs = [/* libraries needed by code gen tools */];
  buildInputs = [ /* libraries needed at run time */ ];
}

Package maintainers need to be aware when to use configurePlatforms, e.g. guile

This should only be compilers, which are rare, hopefully.

Educate maintainers not to hardcode usage of tools from the GNU Toolchain, eg. ar

Indeed! haha. And have them don't do LD ?= $(CD)!

A pattern of manual configureFlags/makeFlags wiring seems to emerge in some packages, e.g. gnu-efi: makeFlags and lzip: configureFlags, which seems a brittle approach by not using our wrapper?!?

Yeah. Thankfully, many non-autoconf packages also support these env vars, but yes not all do. I hope that if we always prefix the binaries however, a lack of this stuff will break on cross and native alike, making this impossible to slip through the cracks.

Manual -rpath-link usage

I hope those changes are no longer needed with @bgamari's clever patch of ld, partway through here.

It crashes with gcc7.

Otherwise building this fails with -Werror warnings when bootstrapping from gcc7.

Otherwise gems tries to delete files for the build ruby.

This provides fw_printenv and fw_setenv, which are used to manipulate the bootloader configuration on some platforms.

The ipsec utility provided by strongswan is a shell script. Consequently we need to patch it to use the target's shell.

The strongswan-swanctl systemd service starts charon-systemd. This implements a IKE daemon very similar to charon, but it's specifically designed for use with systemd. It uses the systemd libraries for a native integration. Instead of using starter and an ipsec.conf based configuration, the daemon is directly managed by systemd and configured with the swanctl configuration backend. See: https://wiki.strongswan.org/projects/strongswan/wiki/Charon-systemd Note that the strongswan.conf and swantctl.conf configuration files are automatically generated based on NixOS options under services.strongswan-swanctl.strongswan and services.strongswan-swanctl.swanctl respectively.

I determined which options got changed by executing the following commands in the strongswan repository: git diff -U20 5.6.0..5.6.1 src/swanctl/swanctl.opt git diff -U20 5.6.0..5.6.1 conf

This reduces the number of options from 1152 to 756.

dtzWill · 2018-05-18T06:47:38Z

Just curious, are there still pieces of this that need to be "upstreamed"?

bgamari · 2018-05-20T22:37:53Z

There are a few more patches, but they really aren't upstreamable in their current form. Perl still remains the largest issue (#36675). I have reverted the commit cited in that ticket on my branch.

Unfortunately I have been unable to build my rebased branch recently due to an inexplicable stack overflow in the nix interpreter. I'll need to sit down and try to debug this some day.

FRidh · 2019-01-06T12:22:44Z

A lot of cross-related changes have been merged since this PR. Because this can't be merged in this state anyway I am closing this now. I think it's better to pick the things that are mergeable and open smaller PR's for them.

bgamari requested review from FRidh, edolstra and peti as code owners October 28, 2017 00:06

bgamari mentioned this pull request Oct 28, 2017

Don't run tests when cross-compiling #30883

Merged

8 tasks

bgamari force-pushed the ben-cross branch from 619d2cd to ee2de10 Compare November 5, 2017 19:22

bgamari requested a review from zimbatm as a code owner November 5, 2017 19:22

dtzWill reviewed Nov 5, 2017

View reviewed changes

Ericson2314 added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Nov 5, 2017

periklis reviewed Nov 6, 2017

View reviewed changes

This was referenced Nov 8, 2017

MinGW-W64 cross-compilation bug roundup #24984

Open

Derivation platform checking compares against wrong machine #30902

Closed

bgamari and others added 22 commits February 27, 2018 12:41

fixup! XXX: iputils: Use from git

71f373a

[HACK] libgpg-error: Fix armv7l build

6ddce89

perl-cross: Build with gcc6

93b812b

It crashes with gcc7.

gcc6: Mark fallthrus explicitly

981bed0

Otherwise building this fails with -Werror warnings when bootstrapping from gcc7.

ruby: XXX: Don't use buildRuby

a314b29

Otherwise gems tries to delete files for the build ruby.

fixup! gcc6: Mark fallthrus explicitly

53c5d9d

ldns: Enable cross-compilation

9d8e303

systemd: m4 is a nativeBuildInput

47b5b4f

perl-cross: Update versions

6a17313

fixup! perl-cross: Build with gcc6

c6a6521

ubootTools: Build envtools as well

df8eab1

This provides fw_printenv and fw_setenv, which are used to manipulate the bootloader configuration on some platforms.

strongswan: Enable cross-compilation

997142f

The ipsec utility provided by strongswan is a shell script. Consequently we need to patch it to use the target's shell.

fixup! w3m: Fix cross-compilation

17599f3

rng-tools: Enable cross-compilation

163a835

strongswan-swanctl: support strongswan-5.6.1 configuration options

bb4f9d0

I determined which options got changed by executing the following commands in the strongswan repository: git diff -U20 5.6.0..5.6.1 src/swanctl/swanctl.opt git diff -U20 5.6.0..5.6.1 conf

strongswan-swanctl: don't generate options for charon

b347de0

This reduces the number of options from 1152 to 756.

fixup! libnfsidmap: Fix setting of configureFlags

e9b8ed6

fixup! ti-rpc: Enable cross-compilation

e57de88

fixup! fuse: Fix cross-compilation

30cec12

fixup! bind: Enable cross-compilation

8cd439d

fixup! linux/kernel: Use stdenvNoCC for configuration

caea96e

bgamari force-pushed the ben-cross branch from 9d7d462 to caea96e Compare February 27, 2018 23:05

GrahamcOfBorg added the 8.has: documentation This PR adds or changes documentation label Feb 27, 2018

bgamari mentioned this pull request Apr 24, 2018

A selection of cross-compiling fixes #39420

Merged

8 tasks

Ericson2314 added this to the 19.03 milestone Aug 26, 2018

FRidh closed this Jan 6, 2019

+                      ''
+                      + ''
+                    done

Uh oh!

Conversation

bgamari commented Oct 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

bgamari commented Oct 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zimbatm commented Nov 5, 2017

Uh oh!

Ericson2314 commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ericson2314 commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bgamari commented Nov 5, 2017

Uh oh!

zimbatm commented Nov 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

periklis Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

periklis commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zimbatm commented Nov 6, 2017

Uh oh!

periklis commented Nov 6, 2017

Uh oh!

bgamari commented Nov 6, 2017

Uh oh!

Ericson2314 commented Nov 6, 2017

Uh oh!

periklis commented Nov 7, 2017

Uh oh!

Ericson2314 commented Nov 9, 2017

Uh oh!

dtzWill commented May 18, 2018

Uh oh!

bgamari commented May 20, 2018

Uh oh!

FRidh commented Jan 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

bgamari commented Oct 28, 2017 •

edited

Loading

bgamari commented Oct 28, 2017 •

edited

Loading

Ericson2314 commented Nov 5, 2017 •

edited

Loading

Ericson2314 commented Nov 5, 2017 •

edited

Loading

periklis Nov 6, 2017 •

edited

Loading

periklis commented Nov 6, 2017 •

edited

Loading