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

make dist vendoring configurable #130110

Merged
merged 3 commits into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -942,3 +942,6 @@
# Copy the linker, DLLs, and various libraries from MinGW into the Rust toolchain.
# Only applies when the host or target is pc-windows-gnu.
#include-mingw-linker = true

# Whether to vendor dependencies for the dist tarball.
#vendor = if "is a tarball source" || "is a git repository" { true } else { false }
Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be necessary to vendor when building from a source tarball. We can just reuse the existing vendor directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, but then decided to leave the task to cargo instead of handling it manually. I couldn't get rid of the paranoia about users potentially cluttering the vendor folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

It shouldn't be necessary to vendor when building from a source tarball. We can just reuse the existing vendor directory.

Exactly!

I'm building using a source release tarball (when packaging for pkgsrc), and expect the tarball to be self-contained. Ref. issue #130708.

6 changes: 1 addition & 5 deletions src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,11 +1011,7 @@ impl Step for PlainSourceTarball {
write_git_info(builder.rust_info().info(), plain_dst_src);
write_git_info(builder.cargo_info.info(), &plain_dst_src.join("./src/tools/cargo"));

// If we're building from git or tarball sources, we need to vendor
// a complete distribution.
if builder.rust_info().is_managed_git_subrepository()
|| builder.rust_info().is_from_tarball()
{
if builder.config.dist_vendor {
builder.require_and_update_all_submodules();

// Vendor all Cargo dependencies
Expand Down
10 changes: 9 additions & 1 deletion src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ pub struct Config {
pub dist_compression_formats: Option<Vec<String>>,
pub dist_compression_profile: String,
pub dist_include_mingw_linker: bool,
pub dist_vendor: bool,

// libstd features
pub backtrace: bool, // support for RUST_BACKTRACE
Expand Down Expand Up @@ -933,6 +934,7 @@ define_config! {
compression_formats: Option<Vec<String>> = "compression-formats",
compression_profile: Option<String> = "compression-profile",
include_mingw_linker: Option<bool> = "include-mingw-linker",
vendor: Option<bool> = "vendor",
}
}

Expand Down Expand Up @@ -2028,13 +2030,19 @@ impl Config {
compression_formats,
compression_profile,
include_mingw_linker,
vendor,
} = dist;
config.dist_sign_folder = sign_folder.map(PathBuf::from);
config.dist_upload_addr = upload_addr;
config.dist_compression_formats = compression_formats;
set(&mut config.dist_compression_profile, compression_profile);
set(&mut config.rust_dist_src, src_tarball);
set(&mut config.dist_include_mingw_linker, include_mingw_linker)
set(&mut config.dist_include_mingw_linker, include_mingw_linker);
config.dist_vendor = vendor.unwrap_or_else(|| {
// If we're building from git or tarball sources, enable it by default.
Copy link
Member

Choose a reason for hiding this comment

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

What's the alternative option to these two? Someone clones git and then deletes .git?

Copy link
Member Author

Choose a reason for hiding this comment

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

Someone clones git and then deletes .git?

That might be a bit awkward, but it’s still one possibility.

Another case that fits this scenario is using this download button:

Ekran görüntüsü 2024-09-14 182025

Copy link
Member

Choose a reason for hiding this comment

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

Another case that fits this scenario is using this download button:

That produces a broken source archive as all git submodules missing. You can't even build the standard library from that archive as both stdarch (core::arch) and backtrace (std::backtrace) are submodules.

config.rust_info.is_managed_git_subrepository()
|| config.rust_info.is_from_tarball()
});
}

if let Some(r) = rustfmt {
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Info,
summary: "'tools' and 'library' profiles switched `download-ci-llvm` option from `if-unchanged` to `true`.",
},
ChangeInfo {
change_id: 130110,
severity: ChangeSeverity::Info,
summary: "New option `dist.vendor` added to control whether bootstrap should vendor dependencies for dist tarball.",
},
];
Loading