Add option to include submodules from vendoring by erickt · Pull Request #137583 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
This adds a config option exclude-submodules-from-vendoring that explicitly filters out listed submodules from vendoring. This allows us to exclude submodules like rustc-perf, which vendors a number of libraries for benchmarking, and has a complicated licensing story.
Closes #137272
cc @Kobzol @pietroalbini @jieyouxu
rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Status: Awaiting review from the assignee but also interested parties.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
labels
This PR modifies config.example.toml.
If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.
This PR modifies src/bootstrap/src/core/config.
If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.
Actually this patch doesn't work, I'm working on a revision that should though.
This adds a config option exclude-submodules-from-vendoring that
explicitly filters out listed submodules from vendoring. This allows us
to exclude submodules like rustc-perf, which vendors a number of
libraries for benchmarking, and has a complicated licensing story.
Closes rust-lang#137272
Confirmed the latest works for us.
| ("src/tools/rustc-perf/Cargo.toml", vec!["src/tools/rustc-perf"]), |
|---|
| ("src/tools/opt-dist/Cargo.toml", vec![]), |
| ("src/doc/book/packages/trpl/Cargo.toml", vec![]), |
| ("src/tools/cargo/Cargo.toml", &["src/tools/cargo"][..]), |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ("src/tools/cargo/Cargo.toml", &["src/tools/cargo"][..]), |
|---|
| ("src/tools/cargo/Cargo.toml", &["src/tools/cargo"]), |
Just a small nit.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! r=me with my and Kobzol's nits fixed.
| pub fn default_paths_to_vendor(builder: &Builder<'_>) -> Vec<(PathBuf, Vec<&'static str>)> { |
|---|
| pub fn default_paths_to_vendor( |
| builder: &Builder<'_>, |
| excluded_submodules: &BTreeSet, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that all callers of exclude_submodules pass the same argument to it. Can we remove the argument and access builder.config.dist_exclude_submodules_from_vendoring directly inside of the function?
bherrera pushed a commit to misttech/integration that referenced this pull request
… vendoring
Building rust broke it now requires all submodules to be present when vendoring, but we don't check it out because it has a complicated licensing story. This patch excludes it from the check to allow rust to work. We should be able to remove once:
- rust-lang/rust#137583 has landed
- our stage0 has advanced passed this checkout version.
Original-Bug: b/399417231 Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/recipes/+/1215144 Original-Revision: a3421adf94a9221a2038108da4566da6089aae02 GitOrigin-RevId: bb529ac503d02b941202e3be6bfcd542c8a90d60 Change-Id: Ie0f8d5aa5c31e2b22e7c7b3253095fb7acb216cb
| } = dist; |
|---|
| config.dist_sign_folder = sign_folder.map(PathBuf::from); |
| config.dist_upload_addr = upload_addr; |
| config.dist_compression_formats = compression_formats; |
| config.dist_exclude_submodules_from_vendoring = |
| exclude_submodules_from_vendoring.unwrap_or_else(BTreeSet::new); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
| exclude_submodules_from_vendoring.unwrap_or_else(BTreeSet::new); |
|---|
| exclude_submodules_from_vendoring.unwrap_or_default(); |
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
Ping @erickt, do you still want to move forward with this? :)
Labels
Status: This is awaiting some action (such as code changes or more information) from the author.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)