From d7cd6e24264d75b7771e63fc15bfcb40a0678d92 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:36:06 +0200 Subject: [PATCH 1/4] Fix RESTRICTED_DEPENDENCY_CRATES to list rustc_driver instead of rustc_middle --- src/tools/tidy/src/deps.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 5a843ea61ec6f..4d9a19535ade9 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -49,7 +49,7 @@ const EXCEPTIONS: &[(&str, &str)] = &[ const RUNTIME_CRATES: &[&str] = &["std", "core", "alloc", "test", "panic_abort", "panic_unwind"]; /// Crates whose dependencies must be explicitly permitted. -const RESTRICTED_DEPENDENCY_CRATES: &[&str] = &["rustc_middle", "rustc_codegen_llvm"]; +const RESTRICTED_DEPENDENCY_CRATES: &[&str] = &["rustc_driver", "rustc_codegen_llvm"]; /// Crates rustc is allowed to depend on. Avoid adding to the list if possible. /// @@ -72,7 +72,10 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "cc", "cfg-if", "chalk-derive", + "chalk-engine", "chalk-ir", + "chalk-solve", + "chrono", "cmake", "compiler_builtins", "cpuid-bool", @@ -92,6 +95,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "expect-test", "fake-simd", "filetime", + "fixedbitset", "flate2", "fortanix-sgx-abi", "fuchsia-zircon", @@ -107,6 +111,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "indexmap", "instant", "itertools", + "itoa", "jobserver", "kernel32-sys", "lazy_static", @@ -114,6 +119,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "libz-sys", "lock_api", "log", + "matchers", "maybe-uninit", "md-5", "measureme", @@ -123,6 +129,8 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "memoffset", "miniz_oxide", "num_cpus", + "num-integer", + "num-traits", "object", "once_cell", "opaque-debug", @@ -130,6 +138,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "parking_lot_core", "pathdiff", "perf-event-open-sys", + "petgraph", "pin-project-lite", "pkg-config", "polonius-engine", @@ -147,22 +156,28 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "rand_xorshift", "redox_syscall", "regex", + "regex-automata", "regex-syntax", "remove_dir_all", + "rls-data", + "rls-span", "rustc-demangle", "rustc-hash", "rustc-rayon", "rustc-rayon-core", "rustc_version", + "ryu", "scoped-tls", "scopeguard", "semver", "semver-parser", "serde", "serde_derive", + "serde_json", "sha-1", "sha2", "smallvec", + "sharded-slab", "snap", "stable_deref_trait", "stacker", @@ -172,9 +187,15 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "termcolor", "termize", "thread_local", + "time", + "tinyvec", "tracing", "tracing-attributes", "tracing-core", + "tracing-log", + "tracing-serde", + "tracing-subscriber", + "tracing-tree", "typenum", "unicode-normalization", "unicode-script", From 2fa18b8864be68948fdd5df2170bf75e5bb0b158 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:36:38 +0200 Subject: [PATCH 2/4] Remove obsolete crate exceptions --- src/tools/tidy/src/deps.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 4d9a19535ade9..86231946bb725 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -244,13 +244,6 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } // Check that the license hasn't changed. for pkg in metadata.packages.iter().filter(|p| p.name == *name) { - if pkg.name == "fuchsia-cprng" { - // This package doesn't declare a license expression. Manual - // inspection of the license file is necessary, which appears - // to be BSD-3-Clause. - assert!(pkg.license.is_none()); - continue; - } match &pkg.license { None => { tidy_error!( @@ -261,14 +254,6 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } Some(pkg_license) => { if pkg_license.as_str() != *license { - if *name == "crossbeam-queue" - && *license == "MIT/Apache-2.0 AND BSD-2-Clause" - { - // We have two versions of crossbeam-queue and both - // are fine. - continue; - } - println!("dependency exception `{}` license has changed", name); println!(" previously `{}` now `{}`", license, pkg_license); println!(" update EXCEPTIONS for the new license"); From 5db01aae99a82675565c333ece511daa31d545cb Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:38:52 +0200 Subject: [PATCH 3/4] Take build dependencies into account during license checks The comment says that build dependencies shouldn't matter unless they do some kind of codegen. It is safer to always check it though. --- src/tools/tidy/src/deps.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 86231946bb725..91008fc9fb7f9 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -460,16 +460,7 @@ fn normal_deps_of_r<'a>( .iter() .find(|n| &n.id == pkg_id) .unwrap_or_else(|| panic!("could not find `{}` in resolve", pkg_id)); - // Don't care about dev-dependencies. - // Build dependencies *shouldn't* matter unless they do some kind of - // codegen. For now we'll assume they don't. - let deps = node.deps.iter().filter(|node_dep| { - node_dep - .dep_kinds - .iter() - .any(|kind_info| kind_info.kind == cargo_metadata::DependencyKind::Normal) - }); - for dep in deps { + for dep in &node.deps { normal_deps_of_r(resolve, &dep.pkg, result); } } From 24def637b36df2945f3a54d06ba5f791a3d78dd7 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:40:07 +0200 Subject: [PATCH 4/4] Wire up tidy dependency checks for cg_clif --- src/tools/tidy/src/deps.rs | 134 +++++++++++++++++++++++++++++++------ 1 file changed, 113 insertions(+), 21 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 91008fc9fb7f9..aa27ce6a72316 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -44,6 +44,23 @@ const EXCEPTIONS: &[(&str, &str)] = &[ ("fortanix-sgx-abi", "MPL-2.0"), // libstd but only for `sgx` target ]; +const EXCEPTIONS_CRANELIFT: &[(&str, &str)] = &[ + ("cranelift-bforest", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen-meta", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen-shared", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-entity", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-frontend", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-jit", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-module", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-native", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-object", "Apache-2.0 WITH LLVM-exception"), + ("libloading", "ISC"), + ("mach", "BSD-2-Clause"), + ("regalloc", "Apache-2.0 WITH LLVM-exception"), + ("target-lexicon", "Apache-2.0 WITH LLVM-exception"), +]; + /// 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"]; @@ -212,6 +229,59 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "winapi-x86_64-pc-windows-gnu", ]; +const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[ + "anyhow", + "ar", + "autocfg", + "bitflags", + "byteorder", + "cfg-if", + "cranelift-bforest", + "cranelift-codegen", + "cranelift-codegen-meta", + "cranelift-codegen-shared", + "cranelift-entity", + "cranelift-frontend", + "cranelift-jit", + "cranelift-module", + "cranelift-native", + "cranelift-object", + "crc32fast", + "errno", + "errno-dragonfly", + "gcc", + "gimli", + "hashbrown", + "indexmap", + "libc", + "libloading", + "log", + "mach", + "object", + "proc-macro2", + "quote", + "regalloc", + "region", + "rustc-hash", + "smallvec", + "syn", + "target-lexicon", + "thiserror", + "thiserror-impl", + "unicode-xid", + "winapi", + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +]; + +const FORBIDDEN_TO_HAVE_DUPLICATES: &[&str] = &[ + // These two crates take quite a long time to build, so don't allow two versions of them + // to accidentally sneak into our dependency graph, in order to ensure we keep our CI times + // under control. + "cargo", + "rustc-ap-rustc_ast", +]; + /// Dependency checks. /// /// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path @@ -222,17 +292,39 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) { .manifest_path(root.join("Cargo.toml")) .features(cargo_metadata::CargoOpt::AllFeatures); let metadata = t!(cmd.exec()); - check_exceptions(&metadata, bad); - check_dependencies(&metadata, bad); - check_crate_duplicate(&metadata, bad); + let runtime_ids = compute_runtime_crates(&metadata); + check_exceptions(&metadata, EXCEPTIONS, runtime_ids, bad); + check_dependencies(&metadata, PERMITTED_DEPENDENCIES, RESTRICTED_DEPENDENCY_CRATES, bad); + check_crate_duplicate(&metadata, FORBIDDEN_TO_HAVE_DUPLICATES, bad); + + // Check rustc_codegen_cranelift independently as it has it's own workspace. + let mut cmd = cargo_metadata::MetadataCommand::new(); + cmd.cargo_path(cargo) + .manifest_path(root.join("compiler/rustc_codegen_cranelift/Cargo.toml")) + .features(cargo_metadata::CargoOpt::AllFeatures); + let metadata = t!(cmd.exec()); + let runtime_ids = HashSet::new(); + check_exceptions(&metadata, EXCEPTIONS_CRANELIFT, runtime_ids, bad); + check_dependencies( + &metadata, + PERMITTED_CRANELIFT_DEPENDENCIES, + &["rustc_codegen_cranelift"], + bad, + ); + check_crate_duplicate(&metadata, &[], bad); } /// Check that all licenses are in the valid list in `LICENSES`. /// /// Packages listed in `EXCEPTIONS` are allowed for tools. -fn check_exceptions(metadata: &Metadata, bad: &mut bool) { +fn check_exceptions( + metadata: &Metadata, + exceptions: &[(&str, &str)], + runtime_ids: HashSet<&PackageId>, + bad: &mut bool, +) { // Validate the EXCEPTIONS list hasn't changed. - for (name, license) in EXCEPTIONS { + for (name, license) in exceptions { // Check that the package actually exists. if !metadata.packages.iter().any(|p| p.name == *name) { tidy_error!( @@ -264,8 +356,7 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } } - let exception_names: Vec<_> = EXCEPTIONS.iter().map(|(name, _license)| *name).collect(); - let runtime_ids = compute_runtime_crates(metadata); + let exception_names: Vec<_> = exceptions.iter().map(|(name, _license)| *name).collect(); // Check if any package does not have a valid license. for pkg in &metadata.packages { @@ -300,9 +391,14 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { /// `true` if a check failed. /// /// Specifically, this checks that the dependencies are on the `PERMITTED_DEPENDENCIES`. -fn check_dependencies(metadata: &Metadata, bad: &mut bool) { +fn check_dependencies( + metadata: &Metadata, + permitted_dependencies: &[&'static str], + restricted_dependency_crates: &[&'static str], + bad: &mut bool, +) { // Check that the PERMITTED_DEPENDENCIES does not have unused entries. - for name in PERMITTED_DEPENDENCIES { + for name in permitted_dependencies { if !metadata.packages.iter().any(|p| p.name == *name) { tidy_error!( bad, @@ -313,12 +409,12 @@ fn check_dependencies(metadata: &Metadata, bad: &mut bool) { } } // Get the list in a convenient form. - let permitted_dependencies: HashSet<_> = PERMITTED_DEPENDENCIES.iter().cloned().collect(); + let permitted_dependencies: HashSet<_> = permitted_dependencies.iter().cloned().collect(); // Check dependencies. let mut visited = BTreeSet::new(); let mut unapproved = BTreeSet::new(); - for &krate in RESTRICTED_DEPENDENCY_CRATES.iter() { + for &krate in restricted_dependency_crates.iter() { let pkg = pkg_from_name(metadata, krate); let mut bad = check_crate_dependencies(&permitted_dependencies, metadata, &mut visited, pkg); @@ -371,16 +467,12 @@ fn check_crate_dependencies<'a>( } /// Prevents multiple versions of some expensive crates. -fn check_crate_duplicate(metadata: &Metadata, bad: &mut bool) { - const FORBIDDEN_TO_HAVE_DUPLICATES: &[&str] = &[ - // These two crates take quite a long time to build, so don't allow two versions of them - // to accidentally sneak into our dependency graph, in order to ensure we keep our CI times - // under control. - "cargo", - "rustc-ap-rustc_ast", - ]; - - for &name in FORBIDDEN_TO_HAVE_DUPLICATES { +fn check_crate_duplicate( + metadata: &Metadata, + forbidden_to_have_duplicates: &[&str], + bad: &mut bool, +) { + for &name in forbidden_to_have_duplicates { let matches: Vec<_> = metadata.packages.iter().filter(|pkg| pkg.name == name).collect(); match matches.len() { 0 => {