Skip to content

gcc: link $lib/lib -> $lib/$targetConfig correctly and consistently#296219

Merged
ConnorBaker merged 1 commit intoNixOS:stagingfrom
ConnorBaker:fix/gcc-cross-nested-libs
Mar 16, 2024
Merged

gcc: link $lib/lib -> $lib/$targetConfig correctly and consistently#296219
ConnorBaker merged 1 commit intoNixOS:stagingfrom
ConnorBaker:fix/gcc-cross-nested-libs

Conversation

@ConnorBaker
Copy link
Contributor

@ConnorBaker ConnorBaker commented Mar 15, 2024

Note

This PR is a re-upload of #282236 (part of #284796)

When native-compiling, gcc will install libraries into:

/nix/store/...-$targetConfig-gcc-$version-lib/lib

When cross-compiling, gcc will install libraries into:

/nix/store/...-$targetConfig-gcc-$version-lib/$targetConfig

When cross-compiling, we intended to create a link from $lib/lib to $lib/$targetConfig, so that downstream users can always safely assume that "${lib.getLib stdenv.cc.cc}/lib" is where the gcc libraries are, regardless of whether stdenv.cc.cc is a cross compiler or a native compiler.

Unfortunately, there were two problems with how we were trying to create these links:

  1. The link would be created only when enableLibGccOutput==true

  2. The link was being created from the incorrect source $lib/lib/lib instead of $lib/lib.

Both of these mistakes are my fault. This commit corrects them by creating the link using ln -Ts (which is more predictable) and by creating the link from gcc/common/builder.nix rather than from gcc/common/libgcc.nix.

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

When native-compiling, gcc will install libraries into:

  /nix/store/...-$targetConfig-gcc-$version-lib/lib

When cross-compiling, gcc will install libraries into:

  /nix/store/...-$targetConfig-gcc-$version-lib/$targetConfig

When cross-compiling, we intended to create a link from $lib/lib to
$lib/$targetConfig, so that downstream users can always safely
assume that "${lib.getLib stdenv.cc.cc}/lib" is where the gcc
libraries are, regardless of whether `stdenv.cc.cc` is a cross
compiler or a native compiler.

Unfortunately, there were two problems with how we were trying to
create these links:

1. The link would be created only when `enableLibGccOutput==true`

2. The link was being created from the incorrect source
   `$lib/lib/lib` instead of `$lib/lib`.

Both of these mistakes are my fault.  This commit corrects them by
creating the link using `ln -Ts` (which is more predictable) and by
creating the link from `gcc/common/builder.nix` rather than from
`gcc/common/libgcc.nix`.
@ConnorBaker ConnorBaker self-assigned this Mar 15, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. labels Mar 15, 2024
Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Symlink is correct this time!

Comment on lines +202 to +205
# symlink), this ensures that in every case we can assume that
# $lib/lib contains the .so files
lib.optionalString (with stdenv; targetPlatform.config != hostPlatform.config) ''
ln -Ts "''${!outputLib}/''${targetConfig}/lib" $lib/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to come up with any adverse side-effects...

I imagine that without this PR we can use some kind of buildEnv/symlinkJoin to generate FHS prefixes with cross-compilation tools that would somehow approximate what you get with ubuntu-like distributions?..

All in all, I don't see any reason not to merge this, but I lack familiarity with wrapper

@ConnorBaker ConnorBaker merged commit c7293f7 into NixOS:staging Mar 16, 2024
@ConnorBaker ConnorBaker deleted the fix/gcc-cross-nested-libs branch March 16, 2024 15:06
@SomeoneSerge
Copy link
Contributor

I think darwin tests hadn't finished?

@ConnorBaker
Copy link
Contributor Author

@SomeoneSerge any idea if there's an easy way to see the actions that were triggered by a PR (meaning, can I see if the action is still running or if it has failed for this PR)?

Failing that when I get home I can try doing a nixpkgs-review on my laptop for aarch64-darwin and x86_64-darwin, though that looks like it'll be about 8000 packages :l

@SomeoneSerge
Copy link
Contributor

Maybe https://github.com/NixOS/nixpkgs/pull/296219/checks?check_run_id=22722654479 and the adjacent ones

@ConnorBaker
Copy link
Contributor Author

I don't know how I missed the tab labeled Checks...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants