Merge pull request #91 from rbartlensky/one-fixup-per-commit

Add feature to generate one fixup per commit.
This commit is contained in:
Stephen Jung 2023-07-25 17:52:27 -04:00 committed by GitHub
commit bca5f29ec4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 197 additions and 8 deletions

View file

@ -14,13 +14,17 @@ pub struct Config<'a> {
pub base: Option<&'a str>,
pub and_rebase: bool,
pub whole_file: bool,
pub one_fixup_per_commit: bool,
pub logger: &'a slog::Logger,
}
pub fn run(config: &Config) -> Result<()> {
let repo = git2::Repository::open_from_env()?;
debug!(config.logger, "repository found"; "path" => repo.path().to_str());
run_with_repo(config, &repo)
}
fn run_with_repo(config: &Config, repo: &git2::Repository) -> Result<()> {
let stack = stack::working_stack(&repo, config.base, config.force, config.logger)?;
if stack.is_empty() {
crit!(config.logger, "No commits available to fix up, exiting");
@ -80,6 +84,8 @@ pub fn run(config: &Config) -> Result<()> {
.or_else(|_| git2::Signature::now("nobody", "nobody@example.com"))?;
let mut head_commit = repo.head()?.peel_to_commit()?;
let mut hunks_with_commit = vec![];
let mut patches_considered = 0usize;
'patch: for index_patch in index.iter() {
let old_path = index_patch.new_path.as_slice();
@ -174,7 +180,10 @@ pub fn run(config: &Config) -> Result<()> {
// cases, might be helpful to just match the first commit touching the same
// file as the current hunk. Use this option with care!
if config.whole_file {
debug!(c_logger, "Commit touches the hunk file and match whole file is enabled");
debug!(
c_logger,
"Commit touches the hunk file and match whole file is enabled"
);
dest_commit = Some(commit);
break 'commit;
}
@ -223,18 +232,56 @@ pub fn run(config: &Config) -> Result<()> {
}
};
let hunk_with_commit = HunkWithCommit {
hunk_to_apply,
dest_commit,
index_patch,
};
hunks_with_commit.push(hunk_with_commit);
applied_hunks_offset += hunk_offset;
}
}
hunks_with_commit.sort_by_key(|h| h.dest_commit.id());
// * apply all hunks that are going to be fixed up into `dest_commit`
// * commit the fixup
// * repeat for all `dest_commit`s
//
// the `.zip` here will gives us something similar to `.windows`, but with
// an extra iteration for the last element (otherwise we would have to
// special case the last element and commit it separately)
for (current, next) in hunks_with_commit
.iter()
.zip(hunks_with_commit.iter().skip(1).map(Some).chain([None]))
{
head_tree = apply_hunk_to_tree(
&repo,
&head_tree,
&current.hunk_to_apply,
&current.index_patch.old_path,
)?;
// whether there are no more hunks to apply to `dest_commit`
let commit_fixup = next.map_or(true, |next| {
// if the next hunk is for a different commit -- commit what we have so far
!config.one_fixup_per_commit || next.dest_commit.id() != current.dest_commit.id()
});
if commit_fixup {
// TODO: the git2 api only supports utf8 commit messages,
// so it's okay to use strings instead of bytes here
// https://docs.rs/git2/0.7.5/src/git2/repo.rs.html#998
// https://libgit2.org/libgit2/#HEAD/group/commit/git_commit_create
let dest_commit_id = dest_commit.id().to_string();
let dest_commit_locator = dest_commit
let dest_commit_id = current.dest_commit.id().to_string();
let dest_commit_locator = current
.dest_commit
.summary()
.filter(|&msg| summary_counts[msg] == 1)
.unwrap_or(&dest_commit_id);
let diff = repo
.diff_tree_to_tree(Some(&head_commit.tree()?), Some(&head_tree), None)?
.stats()?;
if !config.dry_run {
head_tree =
apply_hunk_to_tree(&repo, &head_tree, &hunk_to_apply, &index_patch.old_path)?;
head_commit = repo.find_commit(repo.commit(
Some("HEAD"),
&signature,
@ -245,15 +292,14 @@ pub fn run(config: &Config) -> Result<()> {
)?)?;
info!(config.logger, "committed";
"commit" => head_commit.id().to_string(),
"header" => hunk_to_apply.header(),
"header" => format!("+{},-{}", diff.insertions(), diff.deletions()),
);
} else {
info!(config.logger, "would have committed";
"fixup" => dest_commit_locator,
"header" => hunk_to_apply.header(),
"header" => format!("+{},-{}", diff.insertions(), diff.deletions()),
);
}
applied_hunks_offset += hunk_offset;
}
}
@ -290,6 +336,12 @@ pub fn run(config: &Config) -> Result<()> {
Ok(())
}
struct HunkWithCommit<'c, 'r, 'p> {
hunk_to_apply: owned::Hunk,
dest_commit: &'c git2::Commit<'r>,
index_patch: &'p owned::Patch,
}
fn apply_hunk_to_tree<'repo>(
repo: &'repo git2::Repository,
base: &git2::Tree,
@ -364,3 +416,132 @@ fn split_lines_after(content: &[u8], n: usize) -> (&[u8], &[u8]) {
};
content.split_at(split_index)
}
#[cfg(test)]
mod tests {
use std::path::{Path, PathBuf};
use super::*;
struct Context {
repo: git2::Repository,
dir: tempfile::TempDir,
}
impl Context {
fn join(&self, p: &Path) -> PathBuf {
self.dir.path().join(p)
}
}
/// Prepare a fresh git repository with an initial commit and a file.
fn prepare_repo() -> (Context, PathBuf) {
let dir = tempfile::tempdir().unwrap();
let repo = git2::Repository::init(dir.path()).unwrap();
let path = PathBuf::from("test-file.txt");
std::fs::write(
dir.path().join(&path),
br#"
line
line
more
lines
"#,
)
.unwrap();
// make the borrow-checker happy by introducing a new scope
{
let tree = add(&repo, &path);
let signature = repo
.signature()
.or_else(|_| git2::Signature::now("nobody", "nobody@example.com"))
.unwrap();
repo.commit(
Some("HEAD"),
&signature,
&signature,
"Initial commit.",
&tree,
&[],
)
.unwrap();
}
(Context { repo, dir }, path)
}
/// Stage the changes made to `path`.
fn add<'r>(repo: &'r git2::Repository, path: &Path) -> git2::Tree<'r> {
let mut index = repo.index().unwrap();
index.add_path(&path).unwrap();
index.write().unwrap();
let tree_id = index.write_tree_to(&repo).unwrap();
repo.find_tree(tree_id).unwrap()
}
/// Prepare an empty repo, and stage some changes.
fn prepare_and_stage() -> Context {
let (ctx, file_path) = prepare_repo();
// add some lines to our file
let path = ctx.join(&file_path);
let contents = std::fs::read_to_string(&path).unwrap();
let modifications = format!("new_line1\n{contents}\nnew_line2");
std::fs::write(&path, &modifications).unwrap();
// stage it
add(&ctx.repo, &file_path);
ctx
}
#[test]
fn multiple_fixups_per_commit() {
let ctx = prepare_and_stage();
// run 'git-absorb'
let drain = slog::Discard;
let logger = slog::Logger::root(drain, o!());
let config = Config {
dry_run: false,
force: false,
base: None,
and_rebase: false,
whole_file: false,
one_fixup_per_commit: false,
logger: &logger,
};
run_with_repo(&config, &ctx.repo).unwrap();
let mut revwalk = ctx.repo.revwalk().unwrap();
revwalk.push_head().unwrap();
assert_eq!(revwalk.count(), 3);
}
#[test]
fn one_fixup_per_commit() {
let ctx = prepare_and_stage();
// run 'git-absorb'
let drain = slog::Discard;
let logger = slog::Logger::root(drain, o!());
let config = Config {
dry_run: false,
force: false,
base: None,
and_rebase: false,
whole_file: false,
one_fixup_per_commit: true,
logger: &logger,
};
run_with_repo(&config, &ctx.repo).unwrap();
let mut revwalk = ctx.repo.revwalk().unwrap();
revwalk.push_head().unwrap();
assert_eq!(revwalk.count(), 2);
}
}

View file

@ -59,6 +59,13 @@ fn main() {
.short("w")
.long("whole-file")
.takes_value(false),
)
.arg(
clap::Arg::with_name("one-fixup-per-commit")
.help("Only generate one fixup per commit")
.short("F")
.long("one-fixup-per-commit")
.takes_value(false),
);
let mut args_clone = args.clone();
let args = args.get_matches();
@ -112,6 +119,7 @@ fn main() {
base: args.value_of("base"),
and_rebase: args.is_present("and-rebase"),
whole_file: args.is_present("whole-file"),
one_fixup_per_commit: args.is_present("one-fixup-per-commit"),
logger: &logger,
}) {
crit!(logger, "absorb failed"; "err" => e.to_string());