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 vendoring requires rustc-perf submodule (unconditionally?) #137272

Open
jieyouxu opened this issue Feb 19, 2025 · 10 comments
Open

Bootstrap vendoring requires rustc-perf submodule (unconditionally?) #137272

jieyouxu opened this issue Feb 19, 2025 · 10 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-fuchsia Operating system: Fuchsia T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Feb 19, 2025

[...] it seems this patch has broken our Rust mirror for Fuchsia. We've had to exclude rustc-perf from our local mirror since it has a pretty complicated licensing story. Rust previously built fine for us since we weren't running the performance tests, but with this patch we're now getting this error:

submodule src/tools/rustc-perf does not appear to be checked out, but it is required for
this step.

Consider setting `build.submodules = true` or manually initializing the submodules.

Is there any way we could work around this, or add a flag where we could skip this check?

Originally posted by @erickt in #137020 (comment)

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 19, 2025
@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Feb 19, 2025
@jieyouxu
Copy link
Member Author

jieyouxu commented Feb 19, 2025

cc @pietroalbini @Kobzol: I feel like it's "more correct" (at least safer license wise) for copyright generation to collect all licenses transitively (including by explicitly checking out submodules) as done in #137020. Unless we modify the copyright generation logic to somehow keep track of what tools that some dist configuration may never use (at the risk of this diverging and oops license violation)?

@Kobzol
Copy link
Contributor

Kobzol commented Feb 19, 2025

Yeah, I think that we should collect everything, just to make sure we don't miss stuff.

@jieyouxu
Copy link
Member Author

@erickt can Fuchsia checkout the src/tools/rustc-perf submodule beforehand?

@jieyouxu jieyouxu added the O-fuchsia Operating system: Fuchsia label Feb 19, 2025
@Sandlot19
Copy link

Hi - I'm working with Erick on the Fuchsia side to get this resolved. Our open source licensing approval process prevents us from checking out src/tools/rustc-perf since it has no top level LICENSE file (Erick please correct me if I'm wrong). Without that top level file, we cannot checkout the submodule without a more involved approval process and doing some additional work on our mirror (which doesn't exist yet).

@Kobzol
Copy link
Contributor

Kobzol commented Feb 19, 2025

Hi, rustc-perf uses REUSE (https://github.com/rust-lang/rustc-perf/tree/master/LICENSES, https://github.com/rust-lang/rustc-perf/blob/master/REUSE.toml), which should be even more comprehensive than just a single top-level license file. The Rust compiler now also uses REUSE, btw.

@erickt
Copy link
Contributor

erickt commented Feb 20, 2025

Hi folks, I talked with our internal opensource team, and it sounds like our internal tooling is blocked on licensee/licensee#737 / fsfe/reuse-tool#901 and so it can't interpret the REUSE.toml at the moment, and it'd take some some time to get that implemented. Our other supported option would be to instead use our internal mechanism that'd fork rustc-perf (and rust) to construct the top-level LICENSE file for us derived off teh REUSE.toml-based files. The biggest downside there is that'd change our git hashes, complicate how we create our stage0 builds, and make it more challenging for us to coordinate with upstream rust on bugs.

I know this is a situation caused by my company and not Rust, but if it'd really help us out in the meantime if we could re-land the equivalent to rust-lang/rustc-perf#1881 to add back the top-level LICENSE.

@Kobzol
Copy link
Contributor

Kobzol commented Feb 21, 2025

Well, the problem is that the previous top level license was outright wrong, and did not accurately describe what was in the repository.. :) So it would be a technical solution for your tooling to work, but it would also be inaccurate from the licensing standpoint.

@Mark-Simulacrum Thoughts?

@erickt
Copy link
Contributor

erickt commented Feb 21, 2025

How does this work for the top level rust repo, which also has the top-level LICENSE-APACHE and LICENSE-MIT, as well as LICENSES/ that contains a number of other licenses? Maybe something like this file COPYRIGHT would be sufficient to explain the situation? I can check with my internal team if that’d cover it.

For a longer term solution, we are indirectly using GitHub’s license reporting, so I put together a PR licensee/licensee#820 that I hope will teach GitHub how to handle the REUSE LICENSES/ directory. Not sure how long it’ll take to land and get used by GitHub though.

@Kobzol
Copy link
Contributor

Kobzol commented Feb 21, 2025

Well, it works in a similar way as it did before for rustc-perf :) It just mentions MIT and Apache, without considering the actual complexity of the Rust compiler licensing story 🤷

@pietroalbini
Copy link
Member

I think if there is a strong need to avoid cloning rustc-perf I'd rather add a new config option to exclude specific submodules from vendoring. The licensing situation of rustc-perf is complex, and we shouldn't paper over it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. O-fuchsia Operating system: Fuchsia T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

6 participants