gcc: use standard builder instead of custom builder.sh #237856
Merged
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
ghost
changed the title
ghost
marked this pull request as ready for review
trofi
previously approved these changes
Jun 16, 2023
|
Having said that |
figsoda
added
the
12.approvals: 1
ghost
marked this pull request as draft
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
ofborg
bot
added
the
2.status: merge conflict
added 3 commits
July 7, 2023 05:55
This commit is left unsquashed to make review of the subsequent commits easier.
This commit performs two search-and-replace operations:
- replace all `${` with `''${`
- replace `''` with `""` in shell scripts
This commit is left unsquashed to make review of the subsequent
commits easier.
This commit adds arguments to `builder.nix`, making it a callable function, and splits up the single massive shell script into separate attributes for each phase. It also drops the first two lines and the last line because these are part of the default builder. All script lines which were not part of a phase function have been moved into `preUnpack` since this is the first phase that runs. Subsequent commits will move parts of this to more sensible locations.
This comment was marked as outdated.
This comment was marked as outdated.
ofborg
bot
removed
the
2.status: merge conflict
This comment was marked as outdated.
This comment was marked as outdated.
12 tasks
|
Okay, I found the problem with Since last review I have:
So I think e02be1883fb2f1eacde8b193400c2168fce20737 is the only commit that needs additional review. @ofborg build wineWowPackages.waylandFull.stdenv.cc.cc |
|
Building:
|
ghost
mentioned this pull request
|
wineWowPackages.waylandFull.stdenv.cc.cc on x86_64-linux — Success |
ghost
mentioned this pull request
|
Looks like #241980 was lost (and maybe more?) |
Fixed, thanks for noticing. |
|
@ofborg build perlPackages.XMLLibXML |
|
Rebased to fix merge conflict. |
Artturin
approved these changes
Jul 21, 2023
Artturin
reviewed
Jul 21, 2023
Comment on lines
+122
to
+124
| # It seems there is a bug in GCC 5 | ||
| #"CFLAGS=$EXTRA_FLAGS $EXTRA_LDFLAGS" | ||
| #"CXXFLAGS=$EXTRA_FLAGS $EXTRA_LDFLAGS" |
There was a problem hiding this comment.
This has been commented since it's introduction in 2017 d4595b3#diff-7e60bf21d59f0d31c6d1a2c867692b9bcbad6dc8d8611e06e2e7bb91a9e07877R116-R118
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.
Please review the commits in this PR separately, ignoring whitespace.
If you read them that way, most of them are trivial. You can skip the first six commits, which are from this PR (merged to
masterbut has not yet reachedstaging):Description of changes
This PR causes our gcc expressions (all 10 of them) to use the standard builder instead of their own custom
builder.sh.This PR deliberately does the minimum amount possible to achieve that, because it is likely to merge-conflict with almost any other PR which touches any version of GCC. Most of the commits in this PR are simple, repeatable operations (like search and replace) to try to make it easier to rebase them past any conflicts.
Most of the benefits of this PR will be realized in future PRs (see the last few commits of #238069)
Future Benefits
gcc/builder.shcauses a mass rebuild of every package on every platform. This is crazy.staging.builder.sh; see the gymnastics with "extra fixup phases" I had to add tolibgcc.nix. Those will go away.And, ultimately the big long-term benefit:
gccexpression should look like ourboostandnixexpressions, usinglib.versionswhen it is necessary to set minimum versions for things like patch applicability or configure flags.Things done
gcc48,gcc49,gcc6,gcc7,gcc8,gcc9,gcc10,gcc11,gcc12,gcc13x86_64-linux): in progresssandbox = 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/)