Add option to include submodules from vendoring #137583
Open
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
|
rustbot has assigned @albertlarsan68. Use |
rustbot
added
S-waiting-on-review
|
This PR modifies If appropriate, please update This PR modifies If appropriate, please update |
erickt
force-pushed
the
exclude-submodules-from-vendoring
branch
from
b507517 to
3064c02
Compare
bjorn3
reviewed
Feb 25, 2025
erickt
force-pushed
the
exclude-submodules-from-vendoring
branch
from
3064c02 to
c53d067
Compare
|
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
erickt
force-pushed
the
exclude-submodules-from-vendoring
branch
from
c53d067 to
21f240e
Compare
|
Confirmed the latest works for us. |
Kobzol
reviewed
Feb 28, 2025
| ("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"][..]), |
There was a problem hiding this comment.
Suggested change
| ("src/tools/cargo/Cargo.toml", &["src/tools/cargo"][..]), | |
| ("src/tools/cargo/Cargo.toml", &["src/tools/cargo"]), |
Just a small nit.
| pub fn default_paths_to_vendor(builder: &Builder<'_>) -> Vec<(PathBuf, Vec<&'static str>)> { | ||
| pub fn default_paths_to_vendor( | ||
| builder: &Builder<'_>, | ||
| excluded_submodules: &BTreeSet<String>, |
There was a problem hiding this comment.
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
Mar 4, 2025
… 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: 1. rust-lang/rust#137583 has landed 2. 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
Shourya742
reviewed
Mar 14, 2025
| } = 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); |
There was a problem hiding this comment.
nit:
Suggested change
| exclude_submodules_from_vendoring.unwrap_or_else(BTreeSet::new); | |
| exclude_submodules_from_vendoring.unwrap_or_default(); |
jieyouxu
added
S-waiting-on-author
|
Ping @erickt, do you still want to move forward with this? :) |
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.
This adds a config option
exclude-submodules-from-vendoringthat explicitly filters out listed submodules from vendoring. This allows us to exclude submodules likerustc-perf, which vendors a number of libraries for benchmarking, and has a complicated licensing story.Closes #137272
cc @Kobzol @pietroalbini @jieyouxu