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: vendor crates required by opt-dist to collect profiles #125465

Merged
merged 3 commits into from
Jun 9, 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
15 changes: 15 additions & 0 deletions src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,21 @@ impl Step for PlainSourceTarball {
.env("RUSTC_BOOTSTRAP", "1")
.current_dir(plain_dst_src);

// Vendor packages that are required by opt-dist to collect PGO profiles.
let pkgs_for_pgo_training = build_helper::LLVM_PGO_CRATES
.iter()
.chain(build_helper::RUSTC_PGO_CRATES)
.map(|pkg| {
let mut manifest_path =
builder.src.join("./src/tools/rustc-perf/collector/compile-benchmarks");
manifest_path.push(pkg);
manifest_path.push("Cargo.toml");
manifest_path
});
for manifest_path in pkgs_for_pgo_training {
cmd.arg("--sync").arg(manifest_path);
}

let config = if !builder.config.dry_run() {
t!(String::from_utf8(t!(cmd.output()).stdout))
} else {
Expand Down
1 change: 1 addition & 0 deletions src/tools/build_helper/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Types and functions shared across tools in this workspace.
25 changes: 25 additions & 0 deletions src/tools/build_helper/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,30 @@
//! Types and functions shared across tools in this workspace.

pub mod ci;
pub mod git;
pub mod metrics;
pub mod stage0_parser;
pub mod util;

/// The default set of crates for opt-dist to collect LLVM profiles.
pub const LLVM_PGO_CRATES: &[&str] = &[
"syn-1.0.89",
"cargo-0.60.0",
"serde-1.0.136",
"ripgrep-13.0.0",
"regex-1.5.5",
"clap-3.1.6",
"hyper-0.14.18",
];

/// The default set of crates for opt-dist to collect rustc profiles.
pub const RUSTC_PGO_CRATES: &[&str] = &[
"externs",
"ctfe-stress-5",
"cargo-0.60.0",
"token-stream-stress",
"match-stress",
"tuple-stress",
"diesel-1.4.8",
"bitmaps-3.1.0",
];
22 changes: 1 addition & 21 deletions src/tools/opt-dist/src/training.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,10 @@ use crate::exec::{cmd, CmdBuilder};
use crate::utils::io::{count_files, delete_directory};
use crate::utils::with_log_group;
use anyhow::Context;
use build_helper::{LLVM_PGO_CRATES, RUSTC_PGO_CRATES};
use camino::{Utf8Path, Utf8PathBuf};
use humansize::BINARY;

const LLVM_PGO_CRATES: &[&str] = &[
"syn-1.0.89",
"cargo-0.60.0",
"serde-1.0.136",
"ripgrep-13.0.0",
"regex-1.5.5",
"clap-3.1.6",
"hyper-0.14.18",
];

const RUSTC_PGO_CRATES: &[&str] = &[
"externs",
"ctfe-stress-5",
"cargo-0.60.0",
"token-stream-stress",
"match-stress",
"tuple-stress",
"diesel-1.4.8",
"bitmaps-3.1.0",
];

fn init_compiler_benchmarks(
env: &Environment,
profiles: &[&str],
Expand Down
36 changes: 36 additions & 0 deletions src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>)
//("src/tools/miri/test-cargo-miri", &[], None), // FIXME uncomment once all deps are vendored
//("src/tools/miri/test_dependencies", &[], None), // FIXME uncomment once all deps are vendored
("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None),
("src/tools/rustc-perf", EXCEPTIONS_RUSTC_PERF, None),
Copy link
Member Author

Choose a reason for hiding this comment

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

davidtwco and I talked about this today and we think it makes sense to add the EXCEPTIONS_RUSTC_PERF list and we're ok with the current set of crates/licenses in it. It would be best if we can avoid growing the list further, just to make maintenance easier but this seems like the correct approach to us.

I'm not sure if the list contains everything needed currently though. Does it contain just the dependencies of rustc-perf, or also the dependencies of the compile-time benchmarks that we use for the PGO workflow?

@wesleywiser @Kobzol

It currently only includes dependencies of rustc-perf, as those training data crates are not part of the rustc-perf workspace (also the reason we manually cargo vendor them). Should we license-check those training data crates as well?

(moved to a thread for following the discussion easier)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should check the licenses of everything that we vendor, but I'm not 100% sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

check the licenses of everything that we vendor

Including the web frontend code? 🤯
https://github.com/rust-lang/rustc-perf/blob/master/site/frontend/package.json

Copy link
Contributor

Choose a reason for hiding this comment

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

My assumption was that we should check licences of everything that we vendor, although I'm not sure if we currently do that. If that is needed, it would probably be the simplest if we just deleted the frontend code before packaging it in the source tarball.

("src/tools/x", &[], None),
// tidy-alphabetical-end
];
Expand Down Expand Up @@ -142,6 +143,22 @@ const EXCEPTIONS_RUST_ANALYZER: ExceptionList = &[
// tidy-alphabetical-end
];

const EXCEPTIONS_RUSTC_PERF: ExceptionList = &[
// tidy-alphabetical-start
("alloc-no-stdlib", "BSD-3-Clause"),
("alloc-stdlib", "BSD-3-Clause"),
("brotli", "BSD-3-Clause/MIT"),
("brotli-decompressor", "BSD-3-Clause/MIT"),
("encoding_rs", "(Apache-2.0 OR MIT) AND BSD-3-Clause"),
("inferno", "CDDL-1.0"),
("instant", "BSD-3-Clause"),
("ring", NON_STANDARD_LICENSE), // see EXCEPTIONS_NON_STANDARD_LICENSE_DEPS for more.
("ryu", "Apache-2.0 OR BSL-1.0"),
("snap", "BSD-3-Clause"),
("subtle", "BSD-3-Clause"),
// tidy-alphabetical-end
];

const EXCEPTIONS_CRANELIFT: ExceptionList = &[
// tidy-alphabetical-start
("cranelift-bforest", "Apache-2.0 WITH LLVM-exception"),
Expand Down Expand Up @@ -178,6 +195,20 @@ const EXCEPTIONS_UEFI_QEMU_TEST: ExceptionList = &[
("r-efi", "MIT OR Apache-2.0 OR LGPL-2.1-or-later"), // LGPL is not acceptible, but we use it under MIT OR Apache-2.0
];

/// Placeholder for non-standard license file.
const NON_STANDARD_LICENSE: &str = "NON_STANDARD_LICENSE";

/// These dependencies have non-standard licenses but are genenrally permitted.
const EXCEPTIONS_NON_STANDARD_LICENSE_DEPS: &[&str] = &[
// `ring` is included because it is an optional dependency of `hyper`,
// which is a training data in rustc-perf for optimized build.
// The license of it is generally `ISC AND MIT AND OpenSSL`,
// though the `package.license` field is not set.
//
// See https://github.com/briansmith/ring/issues/902
"ring",
];

/// These are the root crates that are part of the runtime. The licenses for
/// these and all their dependencies *must not* be in the exception list.
const RUNTIME_CRATES: &[&str] = &["std", "core", "alloc", "test", "panic_abort", "panic_unwind"];
Expand Down Expand Up @@ -610,6 +641,11 @@ fn check_license_exceptions(metadata: &Metadata, exceptions: &[(&str, &str)], ba
for pkg in metadata.packages.iter().filter(|p| p.name == *name) {
match &pkg.license {
None => {
if *license == NON_STANDARD_LICENSE
&& EXCEPTIONS_NON_STANDARD_LICENSE_DEPS.contains(&pkg.name.as_str())
{
continue;
}
tidy_error!(
bad,
"dependency exception `{}` does not declare a license expression",
Expand Down
6 changes: 5 additions & 1 deletion src/tools/tidy/src/extdeps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ use std::fs;
use std::path::Path;

/// List of allowed sources for packages.
const ALLOWED_SOURCES: &[&str] = &["\"registry+https://github.com/rust-lang/crates.io-index\""];
const ALLOWED_SOURCES: &[&str] = &[
r#""registry+https://github.com/rust-lang/crates.io-index""#,
// This is `rust_team_data` used by `site` in src/tools/rustc-perf,
r#""git+https://github.com/rust-lang/team#a5260e76d3aa894c64c56e6ddc8545b9a98043ec""#,
];

/// Checks for external package sources. `root` is the path to the directory that contains the
/// workspace `Cargo.toml`.
Expand Down
Loading