Low-impact cross fixes, batch 2 by dtzWill · Pull Request #33480 · NixOS/nixpkgs

dtzWill · 2018-01-05T19:51:08Z

Motivation for this change

Next batch in spirit of #33474 (picked from #30882).

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
- 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 · 2018-01-05T23:37:26Z

Thank you for doing this!

Ericson2314

Just need to check with @bgamari on that one thing bit then we are good.

Ericson2314 · 2018-01-07T06:40:12Z

nixos/modules/security/wrappers/default.nix

    installPhase = ''
      mkdir -p $out/bin
-      gcc -Wall -O2 -DWRAPPER_DIR=\"${parentWrapperDir}\" \
+      ${pkgs.stdenv.cc.targetPrefix}cc -Wall -O2 -DWRAPPER_DIR=\"${parentWrapperDir}\" \


Can use $CC I'd assume? @bgamari

Ahh, fair point.

dezgeg · 2018-01-07T15:32:56Z

pkgs/development/libraries/pixman/default.nix

  configureFlags = stdenv.lib.optional stdenv.isArm "--disable-arm-iwmmxt";

-  doCheck = true;
+  doCheck = stdenv.hostPlatform == stdenv.buildPlatform;


Again stdenv should be changed to cope with this.

Should it? It's not clear to me that stdenv should dictate this.

dezgeg · 2018-01-07T15:33:47Z

pkgs/tools/security/sudo/default.nix

    "--with-logpath=/var/log/sudo.log"
    "--with-iologdir=/var/log/sudo-io"
    "--with-sendmail=${sendmailPath}"
+    "--enable-tmpfiles.d=no"


What's the reason for this?

Indeed it could use a comment. I'll have to revert it to recall why this was necessary.

Dropping-- presumably this is forcing the expect value so as to avoid performing a cross-unfriendly check. Perl is on the path to sudo and so perhaps this should be revisited once perl works.

dezgeg · 2018-01-07T15:34:19Z

pkgs/development/perl-modules/generic/default.nix

  {
    outputs = [ "out" "devdoc" ];

-    doCheck = true;


Again for stdenv to handle.

Ericson2314

Few more catches.

Ericson2314 · 2018-01-07T17:35:37Z

pkgs/applications/networking/browsers/w3m/default.nix

+  configureFlags =
+    [ "--with-ssl=${openssl.dev}" "--with-gc=${boehmgc.dev}" ]
+    ++ optionals (stdenv.buildPlatform != stdenv.hostPlatform) [
+      "ac_cv_func_setpgrp_void=yes"


While we're checking things. I found I needed a lot less of these configure test overrides than @bgamari, so maybe pull this out initially?

Ericson2314 · 2018-01-07T17:35:55Z

pkgs/development/libraries/libomxil-bellagio/default.nix

-
+
+  configureFlags =
+    stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [ "ac_cv_func_malloc_0_nonnull=yes" ];


Same might not be needed

Ericson2314 · 2018-01-07T17:36:36Z

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

  src = kernel.src;

+  nativeBuildInputs = [ gettext ];
  buildInputs = [ coreutils pciutils gettext ];


Gettext need not be build input, I'd think?

Ericson2314 · 2018-01-07T17:38:49Z

@dtzWill do you have hydra access? It would make testing whether some of those changes are needed far easier

dtzWill · 2018-01-07T18:07:54Z

@Ericson2314 nope, no Hydra access.

Testing is difficult--many of these packages can't be built even after these changes since they depend on packages that required mass-rebuilds to fix. Hmm.

Do you know if you two were testing on the same set of cross configurations?

Ericson2314 · 2018-01-07T18:33:35Z

@dtzWill mmm. then let's just land without any configure test overrides, and assume will need to more later. The idea is these PRs will land the changes we're sure we need, and then later we will land anything else we end up needing.

Sorry landing these things is so complicated!

dtzWill · 2018-01-07T18:48:29Z

No worries, thanks for working with me on this. And the reviewers 👍.

I'll think how to best proceed, probably nuking both of these PR's and pulling out things that are less "controversial".

I think the current guidelines are something like:

Doesn't induce a mass-rebuild for non-cross (these should be sent to staging, separately)

2a) On the build-frontier for cross-compilation as currently shown by Hydra
or
2b) trivial/minor/"obvious" such as fixing incorrect use of nativeBuildInputs

... sound good?

dtzWill · 2018-01-07T18:49:38Z

For example, the pkg-config fix satisfies 2a but probably not 2b and as a result is possible for reviewers to try out and inspect.

Ericson2314 · 2018-01-08T14:24:09Z

@dtzWill Well if it's on the build frontier (I think by which you mean we can build all deps just not it, good term :)) then it's fine if it's a bit weirder, as we can actually confirm the weirdness is needed.

GrahamcOfBorg added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 5, 2018

joachifm requested a review from Ericson2314 January 6, 2018 16:11

Ericson2314 approved these changes Jan 7, 2018

View reviewed changes

dezgeg reviewed Jan 7, 2018

View reviewed changes

pkgs/development/perl-modules/generic/default.nix

{

outputs = [ "out" "devdoc" ];

doCheck = true;

Copy link

Contributor

dezgeg Jan 7, 2018

Choose a reason for hiding this comment

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

Again for stdenv to handle.

Ericson2314 reviewed Jan 7, 2018

View reviewed changes

bgamari added 13 commits January 7, 2018 11:52

perl-modules: Don't run tests when cross-compiling

9f064cc

w3m: Enable cross-compilation

a37a8d9

makeSquashfs: Inputs are nativeBuildInputs

519cfff

make-ext4-fs: Dependencies are nativeBuildInputs

492bf45

cpupower: Enable cross-compilation

f8e1635

ddrescue: Enable cross-compilation

cbfaf2e

pixman: Don't run tests when cross-compiling

df7d697

parted: Don't run tests when cross-compiling

006febd

uthash: Don't run testsuite when cross-compiling

0214771

glibc: Fix cross-compilation of locales

5e3b605

mdadm: Fix cross-compilation

46e58bd

libomxil-bellagio: Enable cross-compilation

2bdaa07

ms-sys: Fix cross-compilation

47bd982

dtzWill force-pushed the fix/cross-batch-2 branch from 37e5757 to 93aa130 Compare January 7, 2018 17:52

nixos/security-wrapper: Fix cross-compilation

f866587

dtzWill force-pushed the fix/cross-batch-2 branch from 93aa130 to f866587 Compare January 7, 2018 17:57

dtzWill closed this Jan 7, 2018



		configureFlags =
		stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [ "ac_cv_func_malloc_0_nonnull=yes" ];

Uh oh!

Conversation

dtzWill commented Jan 5, 2018

Motivation for this change

Things done

Uh oh!

bgamari commented Jan 5, 2018

Uh oh!

Ericson2314 left a comment

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!

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!

Ericson2314 left a comment

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!

Ericson2314 commented Jan 7, 2018

Uh oh!

dtzWill commented Jan 7, 2018

Uh oh!

Ericson2314 commented Jan 7, 2018

Uh oh!

dtzWill commented Jan 7, 2018

Uh oh!

dtzWill commented Jan 7, 2018

Uh oh!

Ericson2314 commented Jan 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants