Skip to content

gcc: deduplicate version expressions#249707

Merged
21 commits merged intomasterfrom
unknown repository
Aug 21, 2023
Merged

gcc: deduplicate version expressions#249707
21 commits merged intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Aug 17, 2023 •
edited by ghost
Loading

Description of changes

This PR combines our 10 copy-pasted-but-slightly-different gcc expressions into a single Nix expression which uses conditionals where necessary. It reduces the size of our GCC expression by 64% -- from over 5,000 lines of Nix code to under 2,000.

Prior to this PR, making any kind of change uniformly across all versions of GCC was either impossible or extremely error-prone.

This (mostly) completes the work begun in #243607

This PR causes no change in eval -- all gcc packages have the same drvPath after it as they did before:

$ nix-instantiate . -A gcc48 -A gcc49 -A gcc6 -A gcc7 -A gcc8 -A gcc9 -A gcc10 -A gcc11 -A gcc12 -A gcc13
/nix/store/1y7jfw7siqlkz6lnjgl20y54q1bm4v9j-gcc-wrapper-4.8.5.drv
/nix/store/sz8pywyyxxdl42l3pd8kj3wyzfwpr1fm-gcc-wrapper-4.9.4.drv
/nix/store/3q7k9c694c8bcdpdiq4q8z64cr7xjv3d-gcc-wrapper-6.5.0.drv
/nix/store/dbdn4fy29bb86wpm7xsbjsvy27fi7byz-gcc-wrapper-7.5.0.drv
/nix/store/qcih34x7kvs3w0njfmc8k7f9i0hl3fnz-gcc-wrapper-8.5.0.drv
/nix/store/w98c0lzlc774sny2f9ff477mwa3zk4f4-gcc-wrapper-9.5.0.drv
/nix/store/cvq5ghz01az3kl5i7bb2nmiia23ayy16-gcc-wrapper-10.5.0.drv
/nix/store/4b3w1l9fx1398w8m1nwq5p89ri519yqw-gcc-wrapper-11.4.0.drv
/nix/store/bcgvf0frnxj0qxhi04f0f13dkd9naxk7-gcc-wrapper-12.3.0.drv
/nix/store/n3k4pmvv9079j91w9b5slr3nicc6fdlw-gcc-wrapper-13.2.0.drv

The patches/default.nix expression is more complicated than necessary because it preserves the order of the patches exactly. A follow-up PR will greatly simplify that file, but that follow-up PR won't preserve drvPaths like this one does. Likewise, the fixing the GLIBC_DYNAMIC_LINKER... part of postPatch will be made massively simpler by not worrying about keeping all the whitespace exactly the same.

Things done

@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Aug 17, 2023
@ghost ghost marked this pull request as ready for review August 17, 2023 12:03
@ghost ghost requested a review from matthewbauer as a code owner August 17, 2023 12:03
@ghost ghost added the 6.topic: hygiene Cleaning up and removing cruft label Aug 17, 2023
@ghost ghost mentioned this pull request Aug 17, 2023
Closed
25 tasks
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 21, 2023
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 21, 2023

Rebased.

The last merge of staging into master caused a really nasty merge conflict here.

Since this is strictly cleanup (no change after eval), in order to prevent that from happening again I am going to merge this to both master and staging once ofborg goes green. It's much easier to rebase other patches past this one than it is to rebase this one past other patches.

@ghost ghost mentioned this pull request Aug 21, 2023
Merged
12 tasks
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 21, 2023
@ghost ghost mentioned this pull request Aug 21, 2023
Merged
12 tasks
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 21, 2023

The patches/default.nix expression is more complicated than necessary because it preserves the order of the patches exactly. A follow-up PR will greatly simplify that file, but that follow-up PR won't preserve drvPaths like this one does.

Found here:

@ghost ghost mentioned this pull request Aug 21, 2023
Merged
12 tasks
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 21, 2023

Likewise, the fixing the GLIBC_DYNAMIC_LINKER... part of postPatch will be made massively simpler by not worrying about keeping all the whitespace exactly the same.

Found here:

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 21, 2023

The last merge of staging into master caused a really nasty merge conflict here.

Since this is strictly cleanup (no change after eval), in order to prevent that from happening again I am merging this to both master and staging. It's much easier to rebase other patches past this one than it is to rebase this one past other patches.

@ghost ghost merged commit e61c8da into NixOS:master Aug 21, 2023
@ghost ghost deleted the pr/gcc/deduplicate branch August 21, 2023 09:17
@ghost ghost linked an issue Aug 21, 2023 that may be closed by this pull request
our gcc-unwrapped expression is illegible #242336
Closed
AndersonTorres pushed a commit that referenced this pull request Sep 11, 2023
@AndersonTorres
gcc: patches: reorganize and simplify
d43e772 
This PR sorts the huge pile of patches and deduplicates it.  Unlike
its predecessors #249707 and
#250489 this commit *does*
affect eval.

Patches below are organized into three general categories:

1. Patches relevant to gcc>=12 on every platform
2. Patches relevant to gcc>=12 on specific platforms
3. Patches relevant only to gcc<12, sorted by the newest version to
   which they might be relevant
@ghost ghost mentioned this pull request Oct 26, 2023
Closed
@ghost ghost mentioned this pull request Nov 2, 2023
Draft
12 tasks
@Artturin Artturin mentioned this pull request Jan 29, 2024
Open
ConnorBaker pushed a commit to nixos-cuda/cuda-legacy that referenced this pull request Apr 14, 2025
gcc: patches: reorganize and simplify
87ea658 
This PR sorts the huge pile of patches and deduplicates it.  Unlike
its predecessors NixOS/nixpkgs#249707 and
NixOS/nixpkgs#250489 this commit *does*
affect eval.

Patches below are organized into three general categories:

1. Patches relevant to gcc>=12 on every platform
2. Patches relevant to gcc>=12 on specific platforms
3. Patches relevant only to gcc<12, sorted by the newest version to
   which they might be relevant
ConnorBaker pushed a commit to nixos-cuda/cuda-legacy that referenced this pull request Apr 14, 2025
gcc: simplify expression by applying workarounds more broadly
3635bb7 
This commit simplifies our gcc expression by applying various
workarounds unconditionally, rather than only to exactly the
versions which applied them prior to the deduplication cleanup.

Unlike its predecessors NixOS/nixpkgs#249707
and NixOS/nixpkgs#250489 this commit *does*
affect eval.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees

No one assigned

Labels

6.topic: hygiene Cleaning up and removing cruft 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

our gcc-unwrapped expression is illegible

0 participants