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

[bootstrap] Add support for building gcc and libgccjit #125419

Merged
merged 21 commits into from
Sep 14, 2024

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented May 22, 2024

As @eholk summarized below:

From my understanding, this change would add libgccjit as an optional component to the Rust distribution. This library is licensed under GPLv2 and currently we do not have any other components under that license so it would be a new license, and one that is generally more restrictive than the other licenses we use.

It'll greatly improve the experience for anyone wanting to work on the GCC backend from the compiler.

Should help with #124172.
Will unblock #124353.

r? @Kobzol

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2024

⚠️ Warning ⚠️

  • These commits modify submodules.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 22, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 22, 2024

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label May 23, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the A-meta Area: Issues & PRs about the rust-lang/rust repository itself label May 23, 2024
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

From what I can see, it's not possible to tell reuse to allow deprecated licenses. So in this case, I have no clue how we can go around this issue. If someone has an idea?

@Kobzol
Copy link
Contributor

Kobzol commented May 23, 2024

@davidtwco Any suggestions on what to do about the GPL-2.0 license of GCC?

@davidtwco
Copy link
Member

@davidtwco Any suggestions on what to do about the GPL-2.0 license of GCC?

Will get back to you about this

@davidtwco
Copy link
Member

@wesleywiser and I discussed this and we're not sure about how to get REUSE to play ball (@pietroalbini might know), but we've forwarded this on to t-compiler's council rep to double-check that adding gcc like this is okay, license-wise.

@pietroalbini
Copy link
Member

From what I can see, it's not possible to tell reuse to allow deprecated licenses. So in this case, I have no clue how we can go around this issue. If someone has an idea?

Note that REUSE doesn't mark the GPL 2.0 license as deprecated. What is deprecated is referring to it as GPL-2.0. You should refer to the license as either GPL-2.0-only or GPL-2.0-or-later, depending on what is appropriate.

Also, the warning doesn't come from REUSE itself, but from the SPDX License List (you'll find GPL-2.0 as deprecated in there).

@eholk
Copy link
Contributor

eholk commented Jun 7, 2024

We discussed this at the Council meeting today and we don't have any concerns with this change. However, we would like to confirm with the Foundation. @abibroom - do you know of any concerns from the Foundation side (feel free to ping me on Zulip or reach out via email if you'd rather not discuss this publicly.

From my understanding, this change would add libgccjit as an optional component to the Rust distribution. This library is licensed under GPLv2 and currently we do not have any other components under that license so it would be a new license, and one that is generally more restrictive than the other licenses we use.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 9, 2024

@bors try

Just to see what happens on Linux dist.

@@ -2365,6 +2366,10 @@ impl Step for RustDev {
// just broadly useful to be able to link against the bundled LLVM.
tarball.add_dir(builder.llvm_out(target).join("include"), "include");

tarball.add_dir(builder.gcc_out(target).join("install/lib/libgccjit.so"), "libgccjit.so");
Copy link
Contributor

Choose a reason for hiding this comment

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

What will this do on Windows? 🤔 Does the gcc build also work on non-Linux OSes, in general?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it doesn't, good catch!

@bors
Copy link
Contributor

bors commented Jun 9, 2024

⌛ Trying commit f302cfd with merge 9e6f6bf...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 9, 2024
[bootstrap] Add gcc to dist generation

As `@eholk` summarized below:

> From my understanding, this change would add libgccjit as an optional component to the Rust distribution. This library is licensed under GPLv2 and currently we do not have any other components under that license so it would be a new license, and one that is generally more restrictive than the other licenses we use.

It'll greatly improve the experience for anyone wanting to work on the GCC backend from the compiler.

Should help with rust-lang#124172.
Will unblock rust-lang#124353.

r? `@Kobzol`
@nikic
Copy link
Contributor

nikic commented Jun 9, 2024

Can this be done without adding gcc as a submodule?

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 9, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 9, 2024
@abibroom
Copy link

We discussed this at the Council meeting today and we don't have any concerns with this change. However, we would like to confirm with the Foundation. @abibroom - do you know of any concerns from the Foundation side (feel free to ping me on Zulip or reach out via email if you'd rather not discuss this publicly.

I'd like to be able to say this is all fine, but want to run it past Foundation legal counsel first just to make sure. Please bear with me whilst that happens!

@abibroom
Copy link

We discussed this at the Council meeting today and we don't have any concerns with this change. However, we would like to confirm with the Foundation. @abibroom - do you know of any concerns from the Foundation side (feel free to ping me on Zulip or reach out via email if you'd rather not discuss this publicly.

I'd like to be able to say this is all fine, but want to run it past Foundation legal counsel first just to make sure. Please bear with me whilst that happens!

We should be good to go with this. Legal raised two points, neither of which I believe to be blocking concerns:

  1. Compliance with the GPLv2 license, specifically, the requirement to provide source code as well as binary distributions. This has hopefully already been thoroughly considered before deciding to add the component.

  2. Enabling downstream compliance: "When it comes to downstream distribution, anyone providing a binary distribution of Rust incorporating this component will need to provide source code for the whole distribution. This is ultimately their problem, but if you're aware of anyone who's doing this currently, it's probably worth talking through those cases and figuring out how to provide notice to them and other potential distributors of their additional obligations with respect to this component."

@eholk
Copy link
Contributor

eholk commented Jun 14, 2024

Enabling downstream compliance: "When it comes to downstream distribution, anyone providing a binary distribution of Rust incorporating this component will need to provide source code for the whole distribution. This is ultimately their problem, but if you're aware of anyone who's doing this currently, it's probably worth talking through those cases and figuring out how to provide notice to them and other potential distributors of their additional obligations with respect to this component."

@abibroom - just to make sure, if someone wanted to make a downstream binary distribution without releasing source code (which strikes me as rather unlikely), they could still do so if they chose not to include the libgccjit component?

I want to make sure this change doesn't effectively mean we have to relicense Rust as GPL2.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 6, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Sep 6, 2024

Ah, crap. On Linux CI, we're not allowed to write to the source directory. Hmm, then I would probably only do the "download dependencies" step if we're not on CI.

@Kobzol
Copy link
Contributor

Kobzol commented Sep 6, 2024

That being said, why is this step executed on CI..? 😕 That shouldn't happen.

@@ -2273,6 +2273,9 @@ impl Step for RustDev {
tarball.permit_symlinks(true);

builder.ensure(crate::core::build_steps::llvm::Llvm { target });
if target.contains("linux") && target.contains("x86_64") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this was kept in, we forgot to remove it. Please revert the changes to dist to make this PR only affect local builds.

@GuillaumeGomez
Copy link
Member Author

Removed changes to dist.rs.

@Kobzol
Copy link
Contributor

Kobzol commented Sep 13, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2024

📌 Commit e7fa03b has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2024
@bors
Copy link
Contributor

bors commented Sep 13, 2024

⌛ Testing commit e7fa03b with merge e0ebd86...

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2024
…bzol

[bootstrap] Add support for building gcc and libgccjit

As `@eholk` summarized below:

> From my understanding, this change would add libgccjit as an optional component to the Rust distribution. This library is licensed under GPLv2 and currently we do not have any other components under that license so it would be a new license, and one that is generally more restrictive than the other licenses we use.

It'll greatly improve the experience for anyone wanting to work on the GCC backend from the compiler.

Should help with rust-lang#124172.
Will unblock rust-lang#124353.

r? `@Kobzol`
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Sep 13, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 13, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Sep 13, 2024

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2024
@bors
Copy link
Contributor

bors commented Sep 14, 2024

⌛ Testing commit e7fa03b with merge 23b04c0...

@bors
Copy link
Contributor

bors commented Sep 14, 2024

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 23b04c0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 14, 2024
@bors bors merged commit 23b04c0 into rust-lang:master Sep 14, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 14, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (23b04c0): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.6% [-0.6%, -0.6%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -2.9%, secondary -1.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
-1.8% [-1.8%, -1.8%] 1
All ❌✅ (primary) -2.9% [-2.9%, -2.9%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 756.859s -> 756.265s (-0.08%)
Artifact size: 341.11 MiB -> 341.19 MiB (0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.