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

openmpi: use pkgsHostTarget for gfortran #88984

Merged

Conversation

matthewbauer
Copy link
Member

This is very confusing. “stdenv” is created from the parent stage so
pkgsTargetTarget.stdenv.cc is a compiler that runs /on/ host platform
and creates binaries for target platform. gfortran on the other hand
is not special cased like stdenv, so the equivalent to
pkgsTargetTarget.stdenv.cc is pkgsHostTarget.gfortran.

I’ve rewritten this to be a little less confusing, “pkgsHostTarget” is
equivalent to “pkgs” so it is unneeded. All that is left is
“pkgsTargetTarget.stdenv” which is equivalent to
“targetPackages.stdenv”.

Fixes #88951

/cc @markuskowa @Ericson2314

Motivation for this change
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.

This is very confusing. “stdenv” is created from the parent stage so
pkgsTargetTarget.stdenv.cc is a compiler that runs /on/ host platform
and creates binaries for target platform. gfortran on the other hand
is not special cased like stdenv, so the equivalent to
pkgsTargetTarget.stdenv.cc is pkgsHostTarget.gfortran.

I’ve rewritten this to be a little less confusing, “pkgsHostTarget” is
equivalent to “pkgs” so it is unneeded. All that is left is
“pkgsTargetTarget.stdenv” which is equivalent to
“targetPackages.stdenv”.

Fixes NixOS#88951

/cc @markuskowa @Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Yeah this is good. Sometimes I like to do things like:

pkgsBuildBuild.targetPackages.stdenv.cc
pkgsBuildHost.targetPackages.stdenv.cc
pkgsBuildTarget.targetPackages.stdenv.cc

where the targetPackages and stdenv effectively "cancel out", and then the pkgsXxxYyy says the host and target platform of the tool, but that doesn't really apply here.

Copy link
Member

@markuskowa markuskowa left a comment

Choose a reason for hiding this comment

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

Looks good to me. I am no expert in cross compilation. Thanks for fixing it.

@matthewbauer matthewbauer merged commit 673827f into NixOS:master May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openmpi fails to cross-compile
3 participants