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

rustc-1.83 tarball contains a GCC checkout #135606

Closed
maurer opened this issue Jan 17, 2025 · 13 comments · Fixed by #135658 or #136650
Closed

rustc-1.83 tarball contains a GCC checkout #135606

maurer opened this issue Jan 17, 2025 · 13 comments · Fixed by #135658 or #136650
Labels
A-gcc Things relevant to the [future] GCC backend C-discussion Category: Discussion or questions that doesn't represent real issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue.

Comments

@maurer
Copy link
Contributor

maurer commented Jan 17, 2025

While I understand the motivation for putting a GCC in dev environments / CI (it's good to keep the GCC backend tested), I think it might be overkill to ship it in the source tarball. This has a number of downsides:

  1. It puts GPL source code into the rustc release tarball.
  2. It's a significant increase in the size of the tarball (part of the reason we noticed was that one of the ingestion systems was unhappy with the file count).
  3. To the best of my knowledge, the GCC backend is not stable - providing the ability to build it out of the stable tarball without an additional library doesn't seem necessary.
  4. I suspect that most people who want the GCC backend (e.g. distros, specialized environments) will want to provide their own GCC.
@maurer maurer added the C-bug Category: This is a bug. label Jan 17, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 17, 2025
@jieyouxu
Copy link
Member

cc @rust-lang/wg-gcc-backend

@jieyouxu jieyouxu added T-release Relevant to the release subteam, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. A-gcc Things relevant to the [future] GCC backend and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 17, 2025
@GuillaumeGomez
Copy link
Member

As far as I know, this is not supposed to happen.

cc @Kobzol

@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

Well, it's true that right now it's probably not necessary. But my assumption was that once we integrate the GCC backend into bootstrap more, we will in fact also include GCC's source code in this tarball, same as we do for LLVM. The source tarball includes pretty much all the source code from our repo, including submodules and vendored crates.

@GuillaumeGomez
Copy link
Member

Ah I thought we were planning to do it only once GCC support in bootstrap was done. My bad.

@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

I mean, I cannot say that the inclusion was fully intentional, I'm not sure where/how it started happening (probably just by us adding GCC as a submodule or something?). I just wanted to say that eventually, it would happen anyway.

@GuillaumeGomez
Copy link
Member

This is fate, no trying to fight against GCC future. 😛

@maurer
Copy link
Contributor Author

maurer commented Jan 17, 2025

LLVM is offered under an Apache license like the Rust compiler - GCC is not. I think it is a potential accident waiting to happen for people to download the source for something that is supposedly Apache-2.0/MIT licensed and accidentally download something GPL'd.

I would think that #63232 would lead to more caution when introducing a component with a license like this that is potentially dangerous to users.

If this is the long term intention of upstream, we're going to have to build a filter on our end to find and delete any potentially GPL'd files during tarball import, and I suspect we won't be the only ones required to do this.

@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

We don't currently actually use the GPL code in our binary artifacts, we just put GPL source code into the tarball archive (same as we have it in our git repository). But IANAL, and this situation is indeed quite complicated and unclear.

CC @ehuss - Do you know if this is something that has been cleared by the Foundation's lawyers already?

@ehuss
Copy link
Contributor

ehuss commented Jan 17, 2025

I don't have any specific legal insight beyond what was already discussed in #125419. My understanding is that it should be fine (from a legal standpoint) for us to include the GPL parts in the source tarball. We already include code from a very wide range of licenses in there. With #133461 we now have a slightly better communication of those licenses. (Unfortunately we don't have good communication of the relationship between those licenses versus when they are used.)

But it does seem like a reasonable concern of being considerate to the people who use the source tarball to consider excluding it and making it separate. Looking at the diff from 1.82 to 1.83, the source tarball increased from 210M to 337M (+60%?) which is massive.

I'm not familiar enough with the requirements for exactly which version of gcc is needed, or if it would be possible to not include that in the source tarball (and have bootstrap fetch it from somewhere, or have the user provide their own copy). However, I think it would be nice to consider doing that.

@bjorn3
Copy link
Member

bjorn3 commented Jan 17, 2025

I'm not familiar enough with the requirements for exactly which version of gcc is needed

A rather recent one I believe + a bunch of patches: gcc-mirror/gcc@master...rust-lang:gcc:master

@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

I don't have any specific legal insight beyond what was already discussed in #125419. My understanding is that it should be fine (from a legal standpoint) for us to include the GPL parts in the source tarball. We already include code from a very wide range of licenses in there. With #133461 we now have a slightly better communication of those licenses. (Unfortunately we don't have good communication of the relationship between those licenses versus when they are used.)

But it does seem like a reasonable concern of being considerate to the people who use the source tarball to consider excluding it and making it separate. Looking at the diff from 1.82 to 1.83, the source tarball increased from 210M to 337M (+60%?) which is massive.

I'm not familiar enough with the requirements for exactly which version of gcc is needed, or if it would be possible to not include that in the source tarball (and have bootstrap fetch it from somewhere, or have the user provide their own copy). However, I think it would be nice to consider doing that.

Fair enough. Sent #135658 to remove GCC sources from the tarball.

@bors bors closed this as completed in a41d652 Jan 20, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 20, 2025
Rollup merge of rust-lang#135658 - Kobzol:src-tarball-remove-gcc, r=jieyouxu

Do not include GCC source code in source tarballs

The licensing story is unclear, it makes the archive much larger, and we should not need it for building anything in the tarballs (yet).

```
Before:
121s building the archive
1.3 GiB gzipped size
5.7 GiB extracted size
402519 extracted files

After:
64s building the archive
961 MiB gzipped size
4.5 GiB extracted size
257719 extracfed files
```

Fixes: rust-lang#135606

r? `@ehuss`
@jieyouxu jieyouxu added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jan 21, 2025
@jieyouxu
Copy link
Member

Reopening to track potential stable/beta backports, nominated in #135658 (comment).

@jieyouxu jieyouxu reopened this Jan 21, 2025
@cuviper cuviper linked a pull request Feb 6, 2025 that will close this issue
@cuviper
Copy link
Member

cuviper commented Feb 7, 2025

#136650 backported the removal to beta.

@cuviper cuviper closed this as completed Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-gcc Things relevant to the [future] GCC backend C-discussion Category: Discussion or questions that doesn't represent real issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants