cc-wrapper: clean-up post stdenvAdapters.useLibsFrom: init#281750
cc-wrapper: clean-up post stdenvAdapters.useLibsFrom: init#281750SomeoneSerge wants to merge 1 commit intoNixOS:masterfrom
stdenvAdapters.useLibsFrom: init#281750Conversation
...from "cudaPackages.backendStdenv: useGccForLibs", 210ce38
There was a problem hiding this comment.
This:
nixpkgs/pkgs/build-support/cc-wrapper/default.nix
Lines 113 to 114 in ff1232c
used to be this:
Afaiu this was exactly the same logic as
nixpkgs/pkgs/build-support/cc-wrapper/default.nix
Lines 439 to 441 in 1a5bd69
...is that correct?
There was a problem hiding this comment.
As the description mentions, the prime motivation of the cc-wrapper change was to be able to inherit the nixpkgs' default stdenv's libraries (primarily, libstdc++ on {x86_64,aarch64}-linux) in compat stdenvs, e.g. when using an older gcc.
I still find it questionable e.g. to
- pass
wrappCCWiththe wrapped (g)cc instead of of the unwrappedliboutput, - pass the stdenv instead of the
libinstdenvAdapters.useLibsFrom.
stdenvAdapters.useLibsFrom: init
There was a problem hiding this comment.
In relation to #281371: it isn't enough to propagate the pkgs.stdenv's libraries to the adapted stdenv, it's also necessary to take the other stdenv's (e.g. pkgs.gcc11Stdenv's) native platform's tools, e.g. coreutils:
nixpkgs/pkgs/build-support/cc-wrapper/default.nix
Lines 90 to 91 in 09f9f1f
Currently these aren't exposed in passthru or anyhow (except for env, which barely can be considered a part of the public interface)
There was a problem hiding this comment.
The question regarding
nixpkgs/pkgs/stdenv/adapters.nix
Lines 244 to 246 in 2589d8b
useLibsFrom pkgs.stdenv pkgsStatic.gcc11Stdenv is going to discard all of the flags related to static linkage I'm pretty sure
...from "cudaPackages.backendStdenv: useGccForLibs", 210ce38
Description of changes
#275947 (based on @rrbutani's suggestions and PoC) introduced (without approval) passthru attributes in the cc-wrapper, the purpose of which is to allow propagating the standard c++ libraries used by another wrapper. These attributes are now (ab)used to fix the libstdc++ compatibility issues in
cudaSupport = truepackages and inpython3Packages.tensorflow, which use older gcc than the rest of nixpkgs.As #275947 (comment) points out, this had introduced more junk in the wrapper, lacks longer-term vision, etc.
This draft is a placeholder just removing the not-so-useful comments introduced by the preceding PR, but I'm hoping we can also have a conversation about whether the passthru solution hits a local minima, about what the other cc-wrapper bits touched by the PR were actually intended for, etc.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.