Various small crossDrv fixes #24871

elitak · 2017-04-13T09:06:55Z

Motivation for this change

Fixed and tested for armv5 cross builds.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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.

mention-bot · 2017-04-13T09:06:58Z

@elitak, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @vcunat and @DavidEGrayson to be potential reviewers.

elitak · 2017-04-13T09:14:26Z

I have more mass-rebuild triggering cross-build fixes to glibc, gcc, etc that I've yet to stage. Should I make an effort to include them all here at once?

dezgeg · 2017-04-13T10:49:12Z

As far as I can see, none of these actually cause mass rebuilds (for non-cross packages). So better to get them separately.

DavidEGrayson · 2017-04-13T16:35:11Z

In the coreutils package, I'm pretty sure you can simplify stdenv.ccCross.libc or {} ? libiconv to stdenv.ccCross ? libc.libiconv.

For kmod, I assume the reason you had to add the native xz to the front of the path was so you could hide the cross-built xz. I am kind of surprised that cross-built inputs have their binaries added to the PATH. Seems like you would almost never want that, and fixing the root of that problem in another pull request would be cool.

Overall, the pull request seems fine to me.

I'm not the best person to review this though, @Ericson2314 would be better.

elitak · 2017-04-13T18:14:37Z

I made the coreutils change.

Stdenv does need some rework to fix the general issue of having its PATH clobbered by crossDrv buildInputs. I've already opened an issue for this, but didn't get any input. I've just now added some proposals for fixing the issue: #21191

Ericson2314

Right idea, but in 2 places there's a more idiomatic way to acomplish the same thing --- sorry these idioms aren't very obvious in the future it should be better.

Ericson2314 · 2017-04-13T21:19:11Z

pkgs/build-support/buildenv/default.nix

@@ -66,6 +66,6 @@ runCommand name
    passAsFile = if builtins.stringLength pkgs >= 128*1024 then [ "pkgs" ] else null;
  }
  ''
-    ${perl}/bin/perl -w ${./builder.pl}
+    ${perl.nativeDrv or perl}/bin/perl -w ${./builder.pl}


Use buildPackages.perl.

Ericson2314 · 2017-04-13T21:20:01Z

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

@@ -14,13 +14,17 @@ in stdenv.mkDerivation rec {
  };

  nativeBuildInputs = [ autoreconfHook pkgconfig libxslt ];
-  buildInputs = [ xz /* zlib */ ];
+  buildInputs = [ xz ];


Use nativeBuildInputs = [ xz ] and no crossAttrs.

This derivation requires liblzma.so from xz.crossDrv.out, so I don't think that will work.

Ericson2314 · 2017-04-13T21:20:48Z

pkgs/tools/misc/coreutils/default.nix

@@ -53,7 +53,7 @@ stdenv.mkDerivation rec {
      ++ optional aclSupport acl.crossDrv
      ++ optional attrSupport attr.crossDrv
      ++ optionals selinuxSupport [ libselinux.crossDrv libsepol.crossDrv ]
-      ++ optional (stdenv.ccCross.libc ? libiconv)
+      ++ optional (stdenv.ccCross or {} ? libc.libiconv)


Nice trick :)

@DavidEGrayson's is nice too. I have no idea whether it works. These sort of "utility attrs" aren't very consistent in nixpkgs, sadly.

The or {} part is not needed. If you think ccCross might not exist, you can do:

stdenv ? ccCross.libc.libiconv

I think that is just part of Nix syntax.

I can't really remember, but I think the issue I originally ran into was that some part of that expression had a null instead of just being absent, so that's what I came up with. I don't think I had considered putting more than 0 dots on the rhs of the ?, though. I will just go with your last suggestion, if it works.

I think the Nix or operator doesn't help you when an attribute is set to null, unlike the "or" operations in a lot of other languages. The or is just used to specify a backup value in case the attribute path you specified to the left cannot be evaluated because one of the attributes is missing. Anyway, I'm sure you are able to play around with it and find a simple expression that works.

Ericson2314 · 2017-04-13T21:28:09Z

@DavidEGrayson

I am kind of surprised that cross-built inputs have their binaries added to the PATH. Seems like you would almost never want that, and fixing the root of that problem in another pull request would be cool.

Right this is aweful and should indeed be fixed, but would cause a mass rebuild.

@elitak
In general I have a huge number of native-hash-preserving changes to merge for cross, and then some mass rebuild ones too. It would be great to coordinate on this :). See https://github.com/obsidiansystems/nixpkgs/tree/ghc-android for the backlog of native-hash-preserving changes---good things most of those near-100 commits subsume previous ones!

elitak · 2017-04-13T21:46:33Z

@Ericson2314, your branch's commits subsume commits from what branch? Do you mean you want to coordinate the native-mass-rebuild-triggering changes? I've got some small (but crucial to crossDrv) changes to glibc, I think, as well as some fixing the very broken mingw32 target platforms. I'll try to extricate these from my backlog and make it my next PR.

Also take a look at the proposals on issue #21191 and tell me which you feel is best and I may PR a fix.

DavidEGrayson · 2017-04-13T21:53:10Z

Ooh, feel free to mention me with an @-sign when you make pull requests about cross-compiling for MinGW. I've done a lot of work in that area on a separate project.

elitak · 2017-04-15T01:31:28Z

Okay, I think everything has been addressed and tested now.

Ericson2314 · 2017-04-15T05:44:39Z

@elitak I mean most of those commits canned squashed as they are themselves broken and fixed by later commits. That makes the overall doff not so bad.

And yeah, I want to first merge my "platform normalization PR", second rebase that non-breaking "ghc-android" branch, and third do the mass rebuild. If you have anything to stage with any of those three, I'm all ears! :)

The first should be very interesting for you because windows target triples are subtle and I do not yet handle that complexity.

elitak force-pushed the cross-staging branch from 4e8f345 to 99976b2 Compare April 13, 2017 09:08

Mic92 added the 1.severity: mass-rebuild label Apr 13, 2017

Mic92 removed the 1.severity: mass-rebuild label Apr 13, 2017

elitak force-pushed the cross-staging branch from 99976b2 to ee9f7aa Compare April 13, 2017 18:16

Ericson2314 requested changes Apr 13, 2017

View reviewed changes

elitak added 3 commits April 14, 2017 01:59

netcat-openbsd: pkgconfig is a nativeBuildInput

6b73c0f

buildEnv: use buildPackages.perl so crossDrv works

bd45ec6

coreutils: fixing ccCross.libc lookup

980ced5

elitak force-pushed the cross-staging branch 2 times, most recently from 27d52ea to 6d4ad0c Compare April 15, 2017 00:45

kmod: crossDrv should use native xz

f89504d

elitak force-pushed the cross-staging branch from 6d4ad0c to f89504d Compare April 15, 2017 01:04

Ericson2314 merged commit 3bff114 into NixOS:master Apr 15, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various small crossDrv fixes #24871

Various small crossDrv fixes #24871

elitak commented Apr 13, 2017

mention-bot commented Apr 13, 2017

elitak commented Apr 13, 2017

dezgeg commented Apr 13, 2017

DavidEGrayson commented Apr 13, 2017 •

edited

elitak commented Apr 13, 2017

Ericson2314 left a comment

Ericson2314 Apr 13, 2017

Ericson2314 Apr 13, 2017

elitak Apr 13, 2017

Ericson2314 Apr 13, 2017

Ericson2314 Apr 13, 2017

DavidEGrayson Apr 13, 2017

elitak Apr 13, 2017

DavidEGrayson Apr 13, 2017 •

edited

Ericson2314 commented Apr 13, 2017 •

edited

elitak commented Apr 13, 2017 •

edited

DavidEGrayson commented Apr 13, 2017 •

edited

elitak commented Apr 15, 2017

Ericson2314 commented Apr 15, 2017

Various small crossDrv fixes #24871

Various small crossDrv fixes #24871

Conversation

elitak commented Apr 13, 2017

Motivation for this change

Things done

mention-bot commented Apr 13, 2017

elitak commented Apr 13, 2017

dezgeg commented Apr 13, 2017

DavidEGrayson commented Apr 13, 2017 • edited

elitak commented Apr 13, 2017

Ericson2314 left a comment

Choose a reason for hiding this comment

Ericson2314 Apr 13, 2017

Choose a reason for hiding this comment

Ericson2314 Apr 13, 2017

Choose a reason for hiding this comment

elitak Apr 13, 2017

Choose a reason for hiding this comment

Ericson2314 Apr 13, 2017

Choose a reason for hiding this comment

Ericson2314 Apr 13, 2017

Choose a reason for hiding this comment

DavidEGrayson Apr 13, 2017

Choose a reason for hiding this comment

elitak Apr 13, 2017

Choose a reason for hiding this comment

DavidEGrayson Apr 13, 2017 • edited

Choose a reason for hiding this comment

Ericson2314 commented Apr 13, 2017 • edited

elitak commented Apr 13, 2017 • edited

DavidEGrayson commented Apr 13, 2017 • edited

elitak commented Apr 15, 2017

Ericson2314 commented Apr 15, 2017

DavidEGrayson commented Apr 13, 2017 •

edited

DavidEGrayson Apr 13, 2017 •

edited

Ericson2314 commented Apr 13, 2017 •

edited

elitak commented Apr 13, 2017 •

edited

DavidEGrayson commented Apr 13, 2017 •

edited