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.

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

stdenv: always pass --build, --host to configure #87909

Draft
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

Gaelan
Copy link
Contributor

@Gaelan Gaelan commented May 15, 2020

Motivation for this change

See #21471 for why this is a good idea. The consensus there seemed to be "go for it," so here's me going for it. This is too big to reasonably test on my laptop, but I imagine this will break some stuff, so it might want to be its own hydra job? I'm not sure how one makes that happen. @edolstra, maybe?

This PR sets configurePlatforms to always default to [ "build" "host" ] in mkDerivation, which was probably the single biggest source of special-casing for cross compiling. However, many individual packages still have special cases; once the dust settles, we should clean those up as well.

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.

@ofborg ofborg bot added the 6.topic: stdenv Standard environment label May 15, 2020
@Ericson2314
Copy link
Member

Check out https://github.com/NixOS/nixpkgs/pull/44583/files, which was deemed going too far.

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux-stdenv This PR causes stdenv to rebuild labels May 15, 2020
@ofborg ofborg bot requested a review from edolstra May 15, 2020 23:13
@Gaelan Gaelan changed the title stdenv: always pass --build, --host to configure WIP: stdenv: always pass --build, --host to configure May 16, 2020
@Gaelan
Copy link
Contributor Author

Gaelan commented May 16, 2020

adding WIP to minimize OfBorg abuse

@Gaelan
Copy link
Contributor Author

Gaelan commented May 16, 2020

With the perl change, this successfully builds stdenv on macOS.

@FRidh FRidh marked this pull request as draft June 13, 2020 09:08
@xaverdh
Copy link
Contributor

xaverdh commented Nov 10, 2020

friendly ping

@Gaelan
Copy link
Contributor Author

Gaelan commented Nov 10, 2020

Not sure what next steps are here - I'll see if I can still build some basic things with this PR and latest staging, but any serious testing of this requires computing infrastructure I don't have.

@Gaelan
Copy link
Contributor Author

Gaelan commented Nov 11, 2020

Ok, rebased against latest staging, successfully build hello (and therefore stdenv) on Darwin; I'm trying the same thing on Linux (in a Docker container) now.

@Ericson2314
Copy link
Member

@Gaelan if you can build both stdenvs / hellos, and you have time for the next 2-3 weeks to monitor staging on hydra (something I was never good enough about with my original burst of cross work!!), I think we can merge this. :)

@Gaelan
Copy link
Contributor Author

Gaelan commented Nov 11, 2020

Linux hello builds (and runs) successfully as well. I'd be happy to watch Hydra as well, though I'm a little confused about how all that works; there seems to be a hydra job for staging at https://hydra.nixos.org/jobset/nixpkgs/staging, but it looks like it only gets run every few months?

@Ericson2314
Copy link
Member

@Gaelan I'm not up to date myself, but I think the idea is that things will be merged into staging-next which is CI'd more.

@Gaelan Gaelan marked this pull request as ready for review November 15, 2020 11:50
@Gaelan Gaelan requested a review from stigtsp as a code owner November 15, 2020 11:50
@ryantm ryantm marked this pull request as draft November 17, 2020 04:41
@Ericson2314
Copy link
Member

/rebase staging

@github-actions
Copy link
Contributor

Failed to rebase

@Ericson2314
Copy link
Member

In other news, #119625 is working well and merged. That makes me feel like we should finally do this.

I big question is whether passing the same --build and --host makes autoconf think cross or native. If native, if falls short of #21471 but is especially harmless.

trofi added a commit to trofi/nixpkgs that referenced this pull request Jun 21, 2022
Useful to enable tree-wide occasionally to have incremental progress
towards NixOS#87909 resolution.
@trofi
Copy link
Contributor

trofi commented Jun 23, 2022

To get a bit of incremental progress i'll collect packages known to fail here and possible links to fix them incrementally:

trofi added a commit to trofi/nixpkgs that referenced this pull request Jul 16, 2022
In NixOS#87909 ("stdenv: always
pass --build, --host to configure") we noticed that stdenv.cc
exposes `c++` as a wrapped bianry and `x86_64-unknown-linux-gnu-c++`
as unsrapped binary.

As a result `x86_64-unknown-linux-gnu-c++` was not able to create
basic bianries. This makes seepingly no-op `--host=x86_64-unknown-linux-gnu`
option for `./configure` a breaking change.

This change adds wrappers to prefixed binaries for cross and
native cases where such an inconsistency exists. New validation
change will catch new possible inconsistencies. It already caught
existing inconsistencies in: `gcc`, `gfortran`, `gcj`, `gccgo`, `gdc`,
`clang`.

At least `radare2` build is fixed by consistent wrappers. It used to fail
before as:

    radare2> ERROR: x86_64-unknown-linux-gnu-gcc cannot create executables

Issue: NixOS#178802
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Feb 20, 2023

Is anyone still working on this?

@trofi
Copy link
Contributor

trofi commented Feb 20, 2023

I have not seen PRs recently. I think it's still worth doing.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Jun 9, 2023

So, I finally have some free time: what can I do to help get this done?

@trofi
Copy link
Contributor

trofi commented Jun 9, 2023

I would suggest trying configurePlatformsByDefault = true; for your system and try to fix whatever is failing.

radare2 is a good example that shows $tool-gcc (unwrapped) vs gcc (wrapped) mismatch. They have to be the same to not break ./configure. There are a few more issues like that.

@tobim
Copy link
Contributor

tobim commented Jun 9, 2023

I tried to build some stuff on this branch, here is what I got in failures so far:

clisp.log
ctx.log
ekho.log
elf-dissector-unstable.log
etesync-dav.log
grub4dos.log
infamousPlugins.log
keybinder.log
koboredux.log
libftdi.log
netris.log
nokogiri.log
poppler-qt5.log
python310Packages.mlflow-server.log
python310Packages.rfc3986-validator.log
redshift.log
runit.log
shocco.log
shogun.log
tremor.log
tudu.log
yash.log

I planned to start opening PRs over the weekend.
I can share logs is you want them.

@tobim
Copy link
Contributor

tobim commented Jun 9, 2023

I would look for derivations with custom configureScripts as candidates.
Unfortunately we don't know what packages use hand written Makefiles from nixpkgs directly. Those are likely candidates to fail too.

@uri-canva
Copy link
Contributor

@tobim are you trying on this branch, or on master with the commits in this branch rebased on top of it?

@tobim
Copy link
Contributor

tobim commented Jun 26, 2023

I rebased the branch on top of 1460498 (April 5 2023) when I started. I didn't update after that to save time.

@uri-canva
Copy link
Contributor

To prevent regressions would it be helpful to override this at a package level? So we could start building packages in hydra with this set?

@uri-canva
Copy link
Contributor

This fix might be relevant if you're testing changes in cross compiling with this PR: #240435.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: qt/kde 6.topic: stdenv Standard environment 8.has: clean-up 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.