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

Improve bootstrap git modified path handling #138706

Merged
merged 3 commits into from
Mar 21, 2025
Merged
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
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/format.rs
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
return Ok(None);
}

get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"])
get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"]).map(Some)
}

#[derive(serde_derive::Deserialize)]
94 changes: 49 additions & 45 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
@@ -1425,52 +1425,56 @@ impl Config {

// Infer the rest of the configuration.

// Infer the source directory. This is non-trivial because we want to support a downloaded bootstrap binary,
// running on a completely different machine from where it was compiled.
let mut cmd = helpers::git(None);
// NOTE: we cannot support running from outside the repository because the only other path we have available
// is set at compile time, which can be wrong if bootstrap was downloaded rather than compiled locally.
// We still support running outside the repository if we find we aren't in a git directory.

// NOTE: We get a relative path from git to work around an issue on MSYS/mingw. If we used an absolute path,
// and end up using MSYS's git rather than git-for-windows, we would get a unix-y MSYS path. But as bootstrap
// has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path.
cmd.arg("rev-parse").arg("--show-cdup");
// Discard stderr because we expect this to fail when building from a tarball.
let output = cmd
.as_command_mut()
.stderr(std::process::Stdio::null())
.output()
.ok()
.and_then(|output| if output.status.success() { Some(output) } else { None });
if let Some(output) = output {
let git_root_relative = String::from_utf8(output.stdout).unwrap();
// We need to canonicalize this path to make sure it uses backslashes instead of forward slashes,
// and to resolve any relative components.
let git_root = env::current_dir()
.unwrap()
.join(PathBuf::from(git_root_relative.trim()))
.canonicalize()
.unwrap();
let s = git_root.to_str().unwrap();

// Bootstrap is quite bad at handling /? in front of paths
let git_root = match s.strip_prefix("\\\\?\\") {
Some(p) => PathBuf::from(p),
None => git_root,
};
// If this doesn't have at least `stage0`, we guessed wrong. This can happen when,
// for example, the build directory is inside of another unrelated git directory.
// In that case keep the original `CARGO_MANIFEST_DIR` handling.
//
// NOTE: this implies that downloadable bootstrap isn't supported when the build directory is outside
// the source directory. We could fix that by setting a variable from all three of python, ./x, and x.ps1.
if git_root.join("src").join("stage0").exists() {
config.src = git_root;
}
if let Some(src) = flags.src {
config.src = src
} else {
// We're building from a tarball, not git sources.
// We don't support pre-downloaded bootstrap in this case.
// Infer the source directory. This is non-trivial because we want to support a downloaded bootstrap binary,
// running on a completely different machine from where it was compiled.
let mut cmd = helpers::git(None);
// NOTE: we cannot support running from outside the repository because the only other path we have available
// is set at compile time, which can be wrong if bootstrap was downloaded rather than compiled locally.
// We still support running outside the repository if we find we aren't in a git directory.

// NOTE: We get a relative path from git to work around an issue on MSYS/mingw. If we used an absolute path,
// and end up using MSYS's git rather than git-for-windows, we would get a unix-y MSYS path. But as bootstrap
// has already been (kinda-cross-)compiled to Windows land, we require a normal Windows path.
cmd.arg("rev-parse").arg("--show-cdup");
// Discard stderr because we expect this to fail when building from a tarball.
let output = cmd
.as_command_mut()
.stderr(std::process::Stdio::null())
.output()
.ok()
.and_then(|output| if output.status.success() { Some(output) } else { None });
if let Some(output) = output {
let git_root_relative = String::from_utf8(output.stdout).unwrap();
// We need to canonicalize this path to make sure it uses backslashes instead of forward slashes,
// and to resolve any relative components.
let git_root = env::current_dir()
.unwrap()
.join(PathBuf::from(git_root_relative.trim()))
.canonicalize()
.unwrap();
let s = git_root.to_str().unwrap();

// Bootstrap is quite bad at handling /? in front of paths
let git_root = match s.strip_prefix("\\\\?\\") {
Some(p) => PathBuf::from(p),
None => git_root,
};
// If this doesn't have at least `stage0`, we guessed wrong. This can happen when,
// for example, the build directory is inside of another unrelated git directory.
// In that case keep the original `CARGO_MANIFEST_DIR` handling.
//
// NOTE: this implies that downloadable bootstrap isn't supported when the build directory is outside
// the source directory. We could fix that by setting a variable from all three of python, ./x, and x.ps1.
if git_root.join("src").join("stage0").exists() {
config.src = git_root;
}
} else {
// We're building from a tarball, not git sources.
// We don't support pre-downloaded bootstrap in this case.
}
}

if cfg!(test) {
9 changes: 6 additions & 3 deletions src/build_helper/src/git.rs
Original file line number Diff line number Diff line change
@@ -173,7 +173,7 @@ pub fn get_git_modified_files(
config: &GitConfig<'_>,
git_dir: Option<&Path>,
extensions: &[&str],
) -> Result<Option<Vec<String>>, String> {
) -> Result<Vec<String>, String> {
let merge_base = get_closest_merge_commit(git_dir, config, &[])?;

let mut git = Command::new("git");
@@ -186,7 +186,10 @@ pub fn get_git_modified_files(
let (status, name) = f.trim().split_once(char::is_whitespace).unwrap();
if status == "D" {
None
} else if Path::new(name).extension().map_or(false, |ext| {
} else if Path::new(name).extension().map_or(extensions.is_empty(), |ext| {
// If there is no extension, we allow the path if `extensions` is empty
// If there is an extension, we allow it if `extension` is empty or it contains the
// extension.
extensions.is_empty() || extensions.contains(&ext.to_str().unwrap())
}) {
Some(name.to_owned())
@@ -195,7 +198,7 @@ pub fn get_git_modified_files(
}
})
.collect();
Ok(Some(files))
Ok(files)
}

/// Returns the files that haven't been added to git yet.
3 changes: 1 addition & 2 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
@@ -747,8 +747,7 @@ fn modified_tests(config: &Config, dir: &Path) -> Result<Vec<PathBuf>, String> {
}

let files =
get_git_modified_files(&config.git_config(), Some(dir), &vec!["rs", "stderr", "fixed"])?
.unwrap_or(vec![]);
get_git_modified_files(&config.git_config(), Some(dir), &vec!["rs", "stderr", "fixed"])?;
// Add new test cases to the list, it will be convenient in daily development.
let untracked_files = get_git_untracked_files(&config.git_config(), None)?.unwrap_or(vec![]);

6 changes: 1 addition & 5 deletions src/tools/suggest-tests/src/main.rs
Original file line number Diff line number Diff line change
@@ -14,11 +14,7 @@ fn main() -> ExitCode {
&Vec::new(),
);
let modified_files = match modified_files {
Ok(Some(files)) => files,
Ok(None) => {
eprintln!("git error");
return ExitCode::FAILURE;
}
Ok(files) => files,
Err(err) => {
eprintln!("Could not get modified files from git: \"{err}\"");
return ExitCode::FAILURE;
Loading