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

cc-wrapper: Use stdenvNoCC to build #29617

Merged
merged 1 commit into from Sep 26, 2017

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Sep 20, 2017

Motivation for this change

cc-wrapper may wrap a C compiler, but it doesn't need one to build itself. (c.f. expand-response-params is a separate derivation.) This helps avoid cycles on the cross stuff, in addition to removing a useless dependency edge.

I could have been super careful with overrides in the stdenv to avoid the mass rebuild, but I don't think it's worth it. [If all goes well, we'll merge this with the other PRs in https://github.com/NixOS/nixpkgs/projects/8, so they'll be a mas-rebuild anyways.]

Things done

Building stdenv as part of other things. PR has more commits than it should because it depends on other PRs.

  • 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 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.

cc-wrapper may wrap a cc-compiler, but it doesn't need one to build
itself. (c.f. expand-response-params is a separate derivation.) This
helps avoid cycles on the cross stuff, in addition to removing a
useless dependency edge.

I could have been super careful with overrides in the stdenv to avoid
the mass rebuild, but I don't think it's worth it.
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

Looks OK to me, both in the idea and implementation.

@@ -1,15 +1,13 @@
{ lib
, crossSystem, config
, crossSystem, config, overlays
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this was missing since 92edcb7 and noone has encountered/reported the error, I guess, as we currently only use this file for "x86_64-solaris".

@vcunat vcunat merged commit d349f9a into NixOS:staging Sep 26, 2017
vcunat added a commit that referenced this pull request Sep 26, 2017
@Ericson2314 Ericson2314 deleted the cc-wrapper-stdenvNoCC branch September 26, 2017 20:45
vcunat added a commit that referenced this pull request Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Cross compilation
Needed for binutils-wrapper
Development

Successfully merging this pull request may close these issues.

None yet

5 participants