Skip to content
Open
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
75 changes: 46 additions & 29 deletions src/build_helper/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,13 @@ pub fn check_path_modifications(

/// Returns true if any of the passed `paths` have changed since the `base` commit.
pub fn has_changed_since(git_dir: &Path, base: &str, paths: &[&str]) -> bool {
let mut git = Command::new("git");
git.current_dir(git_dir);
run_git_diff_index(Some(git_dir), |cmd| {
cmd.args(["--quiet", base, "--"]).args(paths);

git.args(["diff-index", "--quiet", base, "--"]).args(paths);

// Exit code 0 => no changes
// Exit code 1 => some changes were detected
!git.status().expect("cannot run git diff-index").success()
// Exit code 0 => no changes
// Exit code 1 => some changes were detected
!cmd.status().expect("cannot run git diff-index").success()
})
}

/// Returns the latest upstream commit that modified `target_paths`, or `None` if no such commit
Expand Down Expand Up @@ -267,31 +266,49 @@ pub fn get_git_modified_files(
return Err("No upstream commit was found".to_string());
};

let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
let files = output_result(git.args(["diff-index", "--name-status", merge_base.trim()]))?
.lines()
.filter_map(|f| {
let (status, name) = f.trim().split_once(char::is_whitespace).unwrap();
if status == "D" {
None
} 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())
} else {
None
}
})
.collect();
let files = run_git_diff_index(git_dir, |cmd| {
output_result(cmd.args(["--name-status", merge_base.trim()]))
})?
.lines()
.filter_map(|f| {
let (status, name) = f.trim().split_once(char::is_whitespace).unwrap();
if status == "D" {
None
} 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())
} else {
None
}
})
.collect();
Ok(files)
}

/// diff-index can return outdated information, because it does not update the git index.
/// This function uses `update-index` to update the index first, and then provides `func` with a
/// command prepared to run `git diff-index`.
fn run_git_diff_index<F, T>(git_dir: Option<&Path>, func: F) -> T
where
F: FnOnce(&mut Command) -> T,
{
let git = || {
let mut git = Command::new("git");
if let Some(git_dir) = git_dir {
git.current_dir(git_dir);
}
git
};

// We ignore the exit code, as it errors out when some files are modified.
let _ = git().args(["update-index", "--refresh", "-q"]).output();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this discards stderr as well as stdout — you probably only want to discard stdout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to discard all crap from git 😆 But you're right that stderr might be useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say keep the stderr, otherwise if the user has any git shenanigans it will be extremely confusing

func(git().arg("diff-index"))
}

/// Returns the files that haven't been added to git yet.
pub fn get_git_untracked_files(git_dir: Option<&Path>) -> Result<Option<Vec<String>>, String> {
let mut git = Command::new("git");
Expand Down
Loading