From b1b576da4df8341c07ba99124613570b829b0607 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 31 Dec 2024 16:24:07 +0300 Subject: [PATCH 1/2] handle submodules automatically on `doc` steps Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/doc.rs | 47 ++++++++--------------- src/bootstrap/src/utils/helpers.rs | 6 +++ 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 8a9321f8e79b6..68bb483be5457 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -18,19 +18,10 @@ use crate::core::builder::{ self, Alias, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, crate_description, }; use crate::core::config::{Config, TargetSelection}; -use crate::utils::helpers::{symlink_dir, t, up_to_date}; - -macro_rules! submodule_helper { - ($path:expr, submodule) => { - $path - }; - ($path:expr, submodule = $submodule:literal) => { - $submodule - }; -} +use crate::helpers::{is_path_in_submodule, symlink_dir, t, up_to_date}; macro_rules! book { - ($($name:ident, $path:expr, $book_name:expr, $lang:expr $(, submodule $(= $submodule:literal)? )? ;)+) => { + ($($name:ident, $path:expr, $book_name:expr, $lang:expr ;)+) => { $( #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct $name { @@ -53,10 +44,10 @@ macro_rules! book { } fn run(self, builder: &Builder<'_>) { - $( - let path = submodule_helper!( $path, submodule $( = $submodule )? ); - builder.require_submodule(path, None); - )? + if is_path_in_submodule(&builder, $path) { + builder.require_submodule($path, None); + } + builder.ensure(RustbookSrc { target: self.target, name: $book_name.to_owned(), @@ -77,12 +68,12 @@ macro_rules! book { // FIXME: Make checking for a submodule automatic somehow (maybe by having a list of all submodules // and checking against it?). book!( - CargoBook, "src/tools/cargo/src/doc", "cargo", &[], submodule = "src/tools/cargo"; + CargoBook, "src/tools/cargo/src/doc", "cargo", &[]; ClippyBook, "src/tools/clippy/book", "clippy", &[]; - EditionGuide, "src/doc/edition-guide", "edition-guide", &[], submodule; - EmbeddedBook, "src/doc/embedded-book", "embedded-book", &[], submodule; - Nomicon, "src/doc/nomicon", "nomicon", &[], submodule; - RustByExample, "src/doc/rust-by-example", "rust-by-example", &["ja", "zh"], submodule; + EditionGuide, "src/doc/edition-guide", "edition-guide", &[]; + EmbeddedBook, "src/doc/embedded-book", "embedded-book", &[]; + Nomicon, "src/doc/nomicon", "nomicon", &[]; + RustByExample, "src/doc/rust-by-example", "rust-by-example", &["ja", "zh"]; RustdocBook, "src/doc/rustdoc", "rustdoc", &[]; StyleGuide, "src/doc/style-guide", "style-guide", &[]; ); @@ -910,7 +901,6 @@ macro_rules! tool_doc { $(rustc_tool = $rustc_tool:literal, )? $(is_library = $is_library:expr,)? $(crates = $crates:expr)? - $(, submodule $(= $submodule:literal)? )? ) => { #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct $tool { @@ -938,14 +928,12 @@ macro_rules! tool_doc { /// we do not merge it with the other documentation from std, test and /// proc_macros. This is largely just a wrapper around `cargo doc`. fn run(self, builder: &Builder<'_>) { - let source_type = SourceType::InTree; - $( - let _ = source_type; // silence the "unused variable" warning - let source_type = SourceType::Submodule; + let mut source_type = SourceType::InTree; - let path = submodule_helper!( $path, submodule $( = $submodule )? ); - builder.require_submodule(path, None); - )? + if is_path_in_submodule(&builder, $path) { + source_type = SourceType::Submodule; + builder.require_submodule($path, None); + } let stage = builder.top_stage; let target = self.target; @@ -1054,8 +1042,7 @@ tool_doc!( "crates-io", "mdman", "rustfix", - ], - submodule = "src/tools/cargo" + ] ); tool_doc!(Tidy, "src/tools/tidy", rustc_tool = false, crates = ["tidy"]); tool_doc!( diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 923cc2dfc28ce..985d733a41f74 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -60,6 +60,12 @@ pub fn is_dylib(path: &Path) -> bool { }) } +/// Returns `true` if the given path is part of a submodule. +pub fn is_path_in_submodule(builder: &Builder<'_>, path: &str) -> bool { + let submodule_paths = build_helper::util::parse_gitmodules(&builder.src); + submodule_paths.iter().any(|submodule_path| path.starts_with(submodule_path)) +} + fn is_aix_shared_archive(path: &Path) -> bool { let file = match fs::File::open(path) { Ok(file) => file, From 6eb9ebf1d61131df43f18e379493e23a193c3a7c Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Tue, 31 Dec 2024 16:34:21 +0300 Subject: [PATCH 2/2] add test coverage for `helpers::is_path_in_submodule` Signed-off-by: onur-ozkan --- src/bootstrap/src/utils/helpers/tests.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/utils/helpers/tests.rs b/src/bootstrap/src/utils/helpers/tests.rs index f6fe6f47aa4fa..7bd2a47c63c1c 100644 --- a/src/bootstrap/src/utils/helpers/tests.rs +++ b/src/bootstrap/src/utils/helpers/tests.rs @@ -3,8 +3,8 @@ use std::io::Write; use std::path::PathBuf; use crate::utils::helpers::{ - check_cfg_arg, extract_beta_rev, hex_encode, make, program_out_of_date, set_file_times, - symlink_dir, + check_cfg_arg, extract_beta_rev, hex_encode, is_path_in_submodule, make, program_out_of_date, + set_file_times, symlink_dir, }; use crate::{Config, Flags}; @@ -115,3 +115,18 @@ fn test_set_file_times_sanity_check() { assert_eq!(found_metadata.accessed().unwrap(), unix_epoch); assert_eq!(found_metadata.modified().unwrap(), unix_epoch) } + +#[test] +fn test_is_path_in_submodule() { + let config = Config::parse_inner(Flags::parse(&["build".into(), "--dry-run".into()]), |&_| { + Ok(Default::default()) + }); + + let build = crate::Build::new(config.clone()); + let builder = crate::core::builder::Builder::new(&build); + assert!(!is_path_in_submodule(&builder, "invalid/path")); + assert!(is_path_in_submodule(&builder, "src/tools/cargo")); + assert!(is_path_in_submodule(&builder, "src/llvm-project")); + // Make sure subdirs are handled properly + assert!(is_path_in_submodule(&builder, "src/tools/cargo/random-subdir")); +}