From e8ef886962720fc9b5fe3b66e2d14401bb62d9e5 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 23 Jul 2024 17:44:40 -0700 Subject: [PATCH] Fix tidy check if book submodule is not checked out --- src/tools/tidy/src/deps.rs | 47 +++++++++++++++++++---------------- src/tools/tidy/src/extdeps.rs | 15 +++-------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index ea03662c584..6d61e0dd3ef 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -48,31 +48,33 @@ /// * Optionally a tuple of: /// * A list of crates for which dependencies need to be explicitly allowed. /// * The list of allowed dependencies. +/// * Submodules required for the workspace. // FIXME auto detect all cargo workspaces -pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>)] = &[ +pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>, &[&str])] = &[ // The root workspace has to be first for check_rustfix to work. - (".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES))), + (".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES)), &[]), // Outside of the alphabetical section because rustfmt formats it using multiple lines. ( "compiler/rustc_codegen_cranelift", EXCEPTIONS_CRANELIFT, Some((&["rustc_codegen_cranelift"], PERMITTED_CRANELIFT_DEPENDENCIES)), + &[], ), // tidy-alphabetical-start - ("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None), + ("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None, &[]), //("library/backtrace", &[], None), // FIXME uncomment once rust-lang/backtrace#562 has been synced back to the rust repo //("library/portable-simd", &[], None), // FIXME uncomment once rust-lang/portable-simd#363 has been synced back to the rust repo //("library/stdarch", EXCEPTIONS_STDARCH, None), // FIXME uncomment once rust-lang/stdarch#1462 has been synced back to the rust repo - ("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None), - ("src/ci/docker/host-x86_64/test-various/uefi_qemu_test", EXCEPTIONS_UEFI_QEMU_TEST, None), - ("src/etc/test-float-parse", EXCEPTIONS, None), - ("src/tools/cargo", EXCEPTIONS_CARGO, None), + ("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None, &[]), + ("src/ci/docker/host-x86_64/test-various/uefi_qemu_test", EXCEPTIONS_UEFI_QEMU_TEST, None, &[]), + ("src/etc/test-float-parse", EXCEPTIONS, None, &[]), + ("src/tools/cargo", EXCEPTIONS_CARGO, None, &["src/tools/cargo"]), //("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/rustbook", EXCEPTIONS_RUSTBOOK, None), - ("src/tools/rustc-perf", EXCEPTIONS_RUSTC_PERF, None), - ("src/tools/x", &[], None), + ("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None, &[]), + ("src/tools/rustbook", EXCEPTIONS_RUSTBOOK, None, &["src/doc/book"]), + ("src/tools/rustc-perf", EXCEPTIONS_RUSTC_PERF, None, &["src/tools/rustc-perf"]), + ("src/tools/x", &[], None, &[]), // tidy-alphabetical-end ]; @@ -531,16 +533,8 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) { let mut checked_runtime_licenses = false; - let submodules = build_helper::util::parse_gitmodules(root); - for &(workspace, exceptions, permitted_deps) in WORKSPACES { - // Skip if it's a submodule, not in a CI environment, and not initialized. - // - // This prevents enforcing developers to fetch submodules for tidy. - if submodules.contains(&workspace.into()) - && !CiEnv::is_ci() - // If the directory is empty, we can consider it as an uninitialized submodule. - && read_dir(root.join(workspace)).unwrap().next().is_none() - { + for &(workspace, exceptions, permitted_deps, submodules) in WORKSPACES { + if has_missing_submodule(root, submodules) { continue; } @@ -573,6 +567,17 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) { assert!(checked_runtime_licenses); } +/// Used to skip a check if a submodule is not checked out, and not in a CI environment. +/// +/// This helps prevent enforcing developers to fetch submodules for tidy. +pub fn has_missing_submodule(root: &Path, submodules: &[&str]) -> bool { + !CiEnv::is_ci() + && submodules.iter().any(|submodule| { + // If the directory is empty, we can consider it as an uninitialized submodule. + read_dir(root.join(submodule)).unwrap().next().is_none() + }) +} + /// Check that all licenses of runtime dependencies are in the valid list in `LICENSES`. /// /// Unlike for tools we don't allow exceptions to the `LICENSES` list for the runtime with the sole diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index 8bb80f11711..55f937aeacf 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -1,7 +1,6 @@ //! Check for external package sources. Allow only vendorable packages. -use build_helper::ci::CiEnv; -use std::fs::{self, read_dir}; +use std::fs; use std::path::Path; /// List of allowed sources for packages. @@ -14,16 +13,8 @@ /// Checks for external package sources. `root` is the path to the directory that contains the /// workspace `Cargo.toml`. pub fn check(root: &Path, bad: &mut bool) { - let submodules = build_helper::util::parse_gitmodules(root); - for &(workspace, _, _) in crate::deps::WORKSPACES { - // Skip if it's a submodule, not in a CI environment, and not initialized. - // - // This prevents enforcing developers to fetch submodules for tidy. - if submodules.contains(&workspace.into()) - && !CiEnv::is_ci() - // If the directory is empty, we can consider it as an uninitialized submodule. - && read_dir(root.join(workspace)).unwrap().next().is_none() - { + for &(workspace, _, _, submodules) in crate::deps::WORKSPACES { + if crate::deps::has_missing_submodule(root, submodules) { continue; }