misc pkgs: Clean up cross #29028
misc pkgs: Clean up cross #29028
Conversation
Ericson2314
added
the
6.topic: cross-compilation
Ericson2314
added
8.has: port to stable
Ericson2314
added this to Needed by the big PR---nice to move pick off pieces of it and move here, rebasing the big PR on top
in Cross compilation
| @@ -11,9 +11,6 @@ stdenv.mkDerivation rec { | |||
| buildInputs = [ pkgconfig ncurses boehmgc ]; | |||
| nativeBuildInputs = [ help2man perl ]; | |||
|
|
|||
| # `help2man' wants to run Zile, which fails when cross-compiling. | |||
| crossAttrs.nativeBuildInputs = []; | |||
There was a problem hiding this comment.
Did you actually test that? IIRC coreutils suffers from the same problem, that to build a manpage for foo, it needs to execute foo --help piped to help2man which isn't obviously going to work when cross-compiling (and omitting help2man "helps" in the way that manpages aren't built and installed at all).
There was a problem hiding this comment.
I'm sorry @dezgeg. Indeed I had seen that same thing with coreutils. I forgot and thought the comment meant native help2man failed to build, which isn't true (and I checked that) but perhaps could have been when the nativeDrvs were weird.
I'll revert this part of it.
There was a problem hiding this comment.
Cross unfortunately fairly broken until #26805 or similar lands. My intent--which I failed at here--was to clean these things up without changing their behavior so I can more easily fix them later.
| enableGuile ? false, guile ? null | ||
| , enablePython ? false, python ? null | ||
| , enablePerl ? (stdenv.hostPlatform == stdenv.buildPlatform), perl ? null | ||
| , enableSpidermonkey ? (stdenv.hostPlatform == stdenv.buildPlatform), spidermonkey_1_8_5 ? null |
There was a problem hiding this comment.
Ehh. Why was there a need to add these conditionals. If the cross-compilation for this package has been broken for years without anyone noticing, then better to drop the crossAttrs instead of adding conditionals like this (Unless you actually personally want/need this package of course...).
There was a problem hiding this comment.
I've done this a few times earlier. On one hand while dependencies may also be broken I can't test the package so I just don't want to rip out functionality. On the other, missing deps was likely to be the reason they were disabled in the first place. When written like this, it's very easy to later make things unconditional if they prove unnecessary.
| @@ -100,8 +102,10 @@ stdenv.mkDerivation rec { | |||
| rm -rf ffmpeg | |||
| ''; | |||
|
|
|||
| depsBuildBuild = [ buildPackages.stdenv.cc ]; | |||
There was a problem hiding this comment.
Since when is this an API? No results in git grep...
There was a problem hiding this comment.
That is my mistake. This is an API I intended to add, but hadn't yet. This change was not supposed to make this PR. It didn't affect the native build so I didn't notice.
|
Why was there the need to pick any of this to 17.09? |
My goal is to land #26805 for 17.09. I've been merging many PRs that page its way but are themselves far more minor changes. See https://github.com/NixOS/nixpkgs/projects/8 for my full road-map recently. That said, @dezgeg, I'd like to apologize for this PR. While we've disagreed on various strategy for improving cross compilation, you just caught a bunch of changes that were plain mistakes in both our eyes. |
|
I've pushed two commits (master, and cherry-picked to 17.09 for 4 total) fixing those mistakes. |
samueldr
removed
the
9.needs: port to stable
Motivation for this change
Clean up a few of these packages ahead of #26805 fixing cross-compilation.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)