build-support/cc-wrapper: revert "pass in non-existent --sysroot= to … #213185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
ofborg
bot
added
10.rebuild-darwin-stdenv
trofi
force-pushed
the
revert-sysroot-hack
branch
from
7473168
to
b3ef181
Compare
…untangle from libc" This reverts commit 8c80bd0 ("build-support/cc-wrapper: pass in non-existent --sysroot= to untangle from libc"). This change was good in spirit: we caught a few genuine problems with `scons` based packages (`godot`, `fluxus`) and unexpected `-idirafter` includes in various boot loadres (`ipxe`, wimboot`): NixOS#210004 (comment) Unfortunately `--sysroot=` also has a negative impact on libary search order for DT_NEEDED libraries and RUNPATHs of linked libraries. This unexpectedly broke `dmd`, `d-seams`, `llvmPackages_rocm.compiler-rt`). An interesting case of unexpected breakage is `usbmuxd2` where the bug exposed incomplete library move on `libstdc++fs` in `gcc`. The library breakage is very non-intuitive (on top of already unusual layout of `cc-wrapper` driver). Let's revert this change for now. Once it lands we can undo `--sysroot=/` workarounds merged for `staging-next`.
trofi
force-pushed
the
revert-sysroot-hack
branch
from
b3ef181
to
3f9fe5c
Compare
|
Validated that |
4 tasks
|
Did anybody review this PR before it was merged? |
|
|
13 tasks
ghost
mentioned this pull request
13 tasks
|
Do you also need to revert ? If not, the comment it added needs to be updated. It is no longer correct. |
|
nixpkgs/pkgs/development/compilers/gcc/builder.sh Lines 207 to 208 in 585af06 |
|
I would say the change does not hurt if we are to allow any other value or |
|
Proposed full revert as #213929 |
This was referenced Feb 5, 2023
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/libssl-so-3-not-found-when-compiling-rust/25213/2 |
This was referenced Feb 7, 2023
This was
linked to
issues
Feb 11, 2023
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/so-dependencies-of-gd-library-have-undefined-reference-errors/25800/4 |
13 tasks
pwaller
added a commit
to pwaller/nixpkgs
that referenced
this pull request
Nov 15, 2024
Remove a patch to decrease the divergence from upstream. Motivation is a continuation of trying to reduce the number of clang builds floating around. Wrapper builds are cheap, compiler builds are not. It's expected that this patch can be removed at some point and replaced with some other cc-wrapper logic. This will be a little bit easier with the logic already in the cc-wrapper. Ongoing discussion in NixOS#191152. It would be nice to drop this but we think it's probably needed at least on systems where /usr/include may be available to prevent the possibility of picking up unwanted includes. One potential method for eliminating nostdlibinc in the future is by providing a non-existent -sysroot but that was reverted in NixOS#213185, and remains a can of worms. Signed-off-by: Peter Waller <[email protected]>
pwaller
added a commit
to pwaller/nixpkgs
that referenced
this pull request
Nov 15, 2024
Remove a patch to decrease the divergence from upstream. Motivation is a continuation of trying to reduce the number of clang builds floating around. Wrapper builds are cheap, compiler builds are not. It's expected that this patch can be removed at some point and replaced with some other cc-wrapper logic. This will be a little bit easier with the logic already in the cc-wrapper. Ongoing discussion in NixOS#191152. It would be nice to drop this but we think it's probably needed at least on systems where /usr/include may be available to prevent the possibility of picking up unwanted includes. One potential method for eliminating nostdlibinc in the future is by providing a non-existent -sysroot but that was reverted in NixOS#213185, and remains a can of worms. Signed-off-by: Peter Waller <[email protected]>
pwaller
added a commit
to pwaller/nixpkgs
that referenced
this pull request
Nov 15, 2024
Remove a patch to decrease the divergence from upstream. Motivation is a continuation of trying to reduce the number of clang builds floating around. Wrapper builds are cheap, compiler builds are not. It's expected that this patch can be removed at some point and replaced with some other cc-wrapper logic. This will be a little bit easier with the logic already in the cc-wrapper. Ongoing discussion in NixOS#191152. It would be nice to drop this but we think it's probably needed at least on systems where /usr/include may be available to prevent the possibility of picking up unwanted includes. One potential method for eliminating nostdlibinc in the future is by providing a non-existent -sysroot but that was reverted in NixOS#213185, and remains a can of worms. Signed-off-by: Peter Waller <[email protected]>
pwaller
added a commit
to pwaller/nixpkgs
that referenced
this pull request
Nov 15, 2024
Remove a patch to decrease the divergence from upstream. Motivation is a continuation of trying to reduce the number of clang builds floating around. Wrapper builds are cheap, compiler builds are not. It's expected that this patch can be removed at some point and replaced with some other cc-wrapper logic. This will be a little bit easier with the logic already in the cc-wrapper. Ongoing discussion in NixOS#191152. It would be nice to drop this but we think it's probably needed at least on systems where /usr/include may be available to prevent the possibility of picking up unwanted includes. One potential method for eliminating nostdlibinc in the future is by providing a non-existent -sysroot but that was reverted in NixOS#213185, and remains a can of worms. Signed-off-by: Peter Waller <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…untangle from libc"
This reverts commit 8c80bd0 ("build-support/cc-wrapper: pass in non-existent --sysroot= to untangle from libc").
This change was good in spirit: we caught a few genuine problems with
sconsbased packages (godot,fluxus) and unexpected-idirafterincludes in various boot loadres (ipxe, wimboot`):Unfortunately
--sysroot=also has a negative impact on libary search order for DT_NEEDED libraries and RUNPATHs of linked libraries. This unexpectedly brokedmd,d-seams,llvmPackages_rocm.compiler-rt).An interesting case of unexpected breakage is
usbmuxd2where the bug exposed incomplete library move onlibstdc++fsingcc.The library breakage is very non-intuitive (on top of already unusual layout of
cc-wrapperdriver). Let's revert this change for now.Once it lands we can undo
--sysroot=/workarounds merged forstaging-next.Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes