Skip to content

Commit

Permalink
Auto merge of rust-lang#135096 - jieyouxu:fix-doc-submodule-handling,…
Browse files Browse the repository at this point in the history
… r=onur-ozkan

bootstrap: correctly handle doc paths within submodules

Fixes rust-lang#135041 by passing the correct submodule path when requiring submodules. This PR changes `is_path_in_submodule` to `submodule_path_of`. `submodule_path_of` returns the path of the containing submodule when given a path nested inside a submodule we handle, and `None` otherwise.

I tested this manually locally by unregistering the `src/tools/cargo` submodule, then running `./x doc src/tools/cargo/src/doc`. This command fails on master with

```
thread 'main' panicked at src/bootstrap/src/utils/helpers.rs:441:5:
std::fs::read_dir(dir) failed with No such file or directory (os error 2)
```

since the require submodule fails as `src/tools/cargo/src/doc` is not a known submodule. Now we use the submodule path if such a nested-in-submodule-path is passed, and thus running this command with cargo submodule unregistered still succeeds:

```
Rustbook (x86_64-unknown-linux-gnu) - cargo
Doc path: /home/joe/repos/rust/build/x86_64-unknown-linux-gnu/doc/cargo/index.html
Build completed successfully in 0:00:11
```

r? `@onur-ozkan`
  • Loading branch information
bors committed Jan 4, 2025
2 parents 2a8af4f + 4406f42 commit ead4a8f
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 15 deletions.
10 changes: 5 additions & 5 deletions src/bootstrap/src/core/build_steps/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::core::builder::{
self, Alias, Builder, Compiler, Kind, RunConfig, ShouldRun, Step, crate_description,
};
use crate::core::config::{Config, TargetSelection};
use crate::helpers::{is_path_in_submodule, symlink_dir, t, up_to_date};
use crate::helpers::{submodule_path_of, symlink_dir, t, up_to_date};

macro_rules! book {
($($name:ident, $path:expr, $book_name:expr, $lang:expr ;)+) => {
Expand All @@ -44,8 +44,8 @@ macro_rules! book {
}

fn run(self, builder: &Builder<'_>) {
if is_path_in_submodule(&builder, $path) {
builder.require_submodule($path, None);
if let Some(submodule_path) = submodule_path_of(&builder, $path) {
builder.require_submodule(&submodule_path, None)
}

builder.ensure(RustbookSrc {
Expand Down Expand Up @@ -933,9 +933,9 @@ macro_rules! tool_doc {
fn run(self, builder: &Builder<'_>) {
let mut source_type = SourceType::InTree;

if is_path_in_submodule(&builder, $path) {
if let Some(submodule_path) = submodule_path_of(&builder, $path) {
source_type = SourceType::Submodule;
builder.require_submodule($path, None);
builder.require_submodule(&submodule_path, None);
}

let stage = builder.top_stage;
Expand Down
8 changes: 5 additions & 3 deletions src/bootstrap/src/utils/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +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 {
/// Return the path to the containing submodule if available.
pub fn submodule_path_of(builder: &Builder<'_>, path: &str) -> Option<String> {
let submodule_paths = build_helper::util::parse_gitmodules(&builder.src);
submodule_paths.iter().any(|submodule_path| path.starts_with(submodule_path))
submodule_paths.iter().find_map(|submodule_path| {
if path.starts_with(submodule_path) { Some(submodule_path.to_string()) } else { None }
})
}

fn is_aix_shared_archive(path: &Path) -> bool {
Expand Down
20 changes: 13 additions & 7 deletions src/bootstrap/src/utils/helpers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::io::Write;
use std::path::PathBuf;

use crate::utils::helpers::{
check_cfg_arg, extract_beta_rev, hex_encode, is_path_in_submodule, make, program_out_of_date,
set_file_times, symlink_dir,
check_cfg_arg, extract_beta_rev, hex_encode, make, program_out_of_date, set_file_times,
submodule_path_of, symlink_dir,
};
use crate::{Config, Flags};

Expand Down Expand Up @@ -117,16 +117,22 @@ fn test_set_file_times_sanity_check() {
}

#[test]
fn test_is_path_in_submodule() {
fn test_submodule_path_of() {
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"));
assert_eq!(submodule_path_of(&builder, "invalid/path"), None);
assert_eq!(submodule_path_of(&builder, "src/tools/cargo"), Some("src/tools/cargo".to_string()));
assert_eq!(
submodule_path_of(&builder, "src/llvm-project"),
Some("src/llvm-project".to_string())
);
// Make sure subdirs are handled properly
assert!(is_path_in_submodule(&builder, "src/tools/cargo/random-subdir"));
assert_eq!(
submodule_path_of(&builder, "src/tools/cargo/random-subdir"),
Some("src/tools/cargo".to_string())
);
}

0 comments on commit ead4a8f

Please sign in to comment.