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

stdenv: better names for cc and bintools #39458

Merged
merged 3 commits into from Apr 26, 2018

Conversation

oxij
Copy link
Member

@oxij oxij commented Apr 25, 2018

Motivation for this change

Mostly beautification, but with a long crossSystem.config I managed to break nix-build on XFS without this. See commit messages.

Things done
  • Writing this from a system with these patches applied.
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/ivrakxyb6l8k7nz5ds21x1vinayngpas-gcc-wrapper-7.3.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/6sv9c7ajsv8kald53ckkr16gcnb1gby2-gcc-wrapper-7.3.0

@Ericson2314
Copy link
Member

Thanks! It was quite ugly before. Just one thing:

Multi-target compilers like clang need single-target wrappers, so the wrappers need to atleast add the target if one isn't already there. (Make sure not to make targetPrefix itself change but rather just the prefixing optional as that attribute is used downstream.)

Probably also good to change GCC so it also has a targetPrefix attribute for consistency.

@oxij
Copy link
Member Author

oxij commented Apr 26, 2018

Fixed.

Probably also good to change GCC so it also has a targetPrefix attribute for consistency.

Too lazy :)

... binutils and gcc add it already anyway.

Without this it's easy to get cross-toolchain paths longer than 256
chars and nix-daemon will then fail to commit them to /nix/store on XFS.
@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/18r2ixnwxgh04pvm4izg5zwxvj0jz9hj-gcc-wrapper-7.3.0

@Ericson2314
Copy link
Member

Too lazy :)

Acceptable :)

@Ericson2314 Ericson2314 merged commit 591d8c7 into NixOS:staging Apr 26, 2018
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/7az5qwcia4w18f452925lzxxr53mr0r5-gcc-wrapper-7.3.0

@oxij oxij deleted the stdenv/beautifications branch September 8, 2018 22:19
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.

None yet

3 participants