From da9dad212dded5aa4d36daf03004b9e0ad34ca2a Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Sun, 2 Feb 2025 06:59:11 -0500 Subject: [PATCH 1/3] Test foreign author with and without force flag --- src/lib.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 8929c85..a58332f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -476,6 +476,12 @@ fn nothing_left_in_index(repo: &git2::Repository) -> Result { Ok(nothing) } +fn something_left_in_index(repo: &git2::Repository) -> Result { + let stats = index_stats(repo)?; + let nothing = stats.files_changed() != 0; + Ok(nothing) +} + fn index_stats(repo: &git2::Repository) -> Result { let head = repo.head()?.peel_to_tree()?; let diff = repo.diff_tree_to_index(Some(&head), Some(&repo.index()?), None)?; @@ -565,6 +571,12 @@ lines ctx } + fn become_new_author(ctx: &Context) { + let mut config = ctx.repo.config().unwrap(); + config.set_str("user.name", "nobody2").unwrap(); + config.set_str("user.email", "nobody2@example.com").unwrap(); + } + #[test] fn multiple_fixups_per_commit() { let ctx = prepare_and_stage(); @@ -615,6 +627,60 @@ lines assert!(nothing_left_in_index(&ctx.repo).unwrap()); } + #[test] + fn foreign_author() { + let ctx = prepare_and_stage(); + + become_new_author(&ctx); + + // 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(), 1); + + assert!(something_left_in_index(&ctx.repo).unwrap()); + } + + #[test] + fn foreign_author_with_force_flag() { + let ctx = prepare_and_stage(); + + become_new_author(&ctx); + + // run 'git-absorb' + let drain = slog::Discard; + let logger = slog::Logger::root(drain, o!()); + let config = Config { + dry_run: false, + force: true, + 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); + + assert!(nothing_left_in_index(&ctx.repo).unwrap()); + } + fn autostage_common(ctx: &Context, file_path: &PathBuf) -> (PathBuf, PathBuf) { // 1 modification w/o staging let path = ctx.join(&file_path); From 95acb4ded3380760413224099226418a0149406a Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Sun, 2 Feb 2025 08:07:14 -0500 Subject: [PATCH 2/3] Add --force-author flag to generate fixup commits for foreign authors The --force flag continues to enable this behavior --- Documentation/git-absorb.1 | 11 +++-- Documentation/git-absorb.txt | 6 ++- README.md | 1 - src/lib.rs | 82 ++++++++++++++++++++++++++++-------- src/main.rs | 7 ++- src/stack.rs | 14 +++--- 6 files changed, 91 insertions(+), 30 deletions(-) diff --git a/Documentation/git-absorb.1 b/Documentation/git-absorb.1 index e58d344..36e7950 100644 --- a/Documentation/git-absorb.1 +++ b/Documentation/git-absorb.1 @@ -2,12 +2,12 @@ .\" Title: git-absorb .\" Author: [see the "AUTHOR" section] .\" Generator: DocBook XSL Stylesheets vsnapshot -.\" Date: 04/06/2023 +.\" Date: 02/02/2025 .\" Manual: git absorb .\" Source: git-absorb 0.5.0 .\" Language: English .\" -.TH "GIT\-ABSORB" "1" "04/06/2023" "git\-absorb 0\&.5\&.0" "git absorb" +.TH "GIT\-ABSORB" "1" "02/02/2025" "git\-absorb 0\&.5\&.0" "git absorb" .\" ----------------------------------------------------------------- .\" * Define some portability stuff .\" ----------------------------------------------------------------- @@ -66,9 +66,14 @@ Run rebase if successful Don\(cqt make any actual changes .RE .PP +\-\-force\-author +.RS 4 +Generate fixups to commits not made by you +.RE +.PP \-f, \-\-force .RS 4 -Skip safety checks +Skip all safety checks\&. Generate fixups to commits not made by you (as if by \-\-force\-author) and to non\-branch HEADs .RE .PP \-w, \-\-whole\-file diff --git a/Documentation/git-absorb.txt b/Documentation/git-absorb.txt index 8db6983..066f25f 100644 --- a/Documentation/git-absorb.txt +++ b/Documentation/git-absorb.txt @@ -50,9 +50,13 @@ FLAGS --dry-run:: Don't make any actual changes +--force-author:: + Generate fixups to commits not made by you + -f:: --force:: - Skip safety checks + Skip all safety checks. + Generate fixups to commits not made by you (as if by --force-author) and to non-branch HEADs -w:: --whole-file:: diff --git a/README.md b/README.md index 6215f37..cf2c1ac 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,6 @@ By default, git-absorb will create fixup commits with their messages pointing to ## TODO -- implement force flag - implement remote default branch check - add smaller force flags to disable individual safety checks - stop using `failure::err_msg` and ensure all error output is actionable by the user diff --git a/src/lib.rs b/src/lib.rs index a58332f..e88c544 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ use std::io::Write; pub struct Config<'a> { pub dry_run: bool, + pub force_author: bool, pub force: bool, pub base: Option<&'a str>, pub and_rebase: bool, @@ -38,8 +39,17 @@ pub fn run(config: &mut Config) -> Result<()> { 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)?; +fn run_with_repo(config: &mut Config, repo: &git2::Repository) -> Result<()> { + // have force flag enable all force* flags + config.force_author |= config.force; + + let stack = stack::working_stack( + repo, + config.base, + config.force_author, + config.force, + config.logger, + )?; if stack.is_empty() { crit!(config.logger, "No commits available to fix up, exiting"); return Ok(()); @@ -584,8 +594,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -593,7 +604,7 @@ lines one_fixup_per_commit: false, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -609,8 +620,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -618,7 +630,7 @@ lines one_fixup_per_commit: true, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -636,8 +648,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -645,7 +658,7 @@ lines one_fixup_per_commit: true, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -654,6 +667,34 @@ lines assert!(something_left_in_index(&ctx.repo).unwrap()); } + #[test] + fn foreign_author_with_force_author_flag() { + let ctx = prepare_and_stage(); + + become_new_author(&ctx); + + // run 'git-absorb' + let drain = slog::Discard; + let logger = slog::Logger::root(drain, o!()); + let mut config = Config { + dry_run: false, + force_author: true, + force: false, + base: None, + and_rebase: false, + whole_file: false, + one_fixup_per_commit: true, + logger: &logger, + }; + run_with_repo(&mut config, &ctx.repo).unwrap(); + + let mut revwalk = ctx.repo.revwalk().unwrap(); + revwalk.push_head().unwrap(); + assert_eq!(revwalk.count(), 2); + + assert!(nothing_left_in_index(&ctx.repo).unwrap()); + } + #[test] fn foreign_author_with_force_flag() { let ctx = prepare_and_stage(); @@ -663,8 +704,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: true, base: None, and_rebase: false, @@ -672,7 +714,7 @@ lines one_fixup_per_commit: true, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -711,8 +753,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -720,7 +763,7 @@ lines one_fixup_per_commit: false, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -747,8 +790,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -756,7 +800,7 @@ lines one_fixup_per_commit: false, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -781,8 +825,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -790,7 +835,7 @@ lines one_fixup_per_commit: false, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); let mut revwalk = ctx.repo.revwalk().unwrap(); revwalk.push_head().unwrap(); @@ -812,8 +857,9 @@ lines // run 'git-absorb' let drain = slog::Discard; let logger = slog::Logger::root(drain, o!()); - let config = Config { + let mut config = Config { dry_run: false, + force_author: false, force: false, base: None, and_rebase: false, @@ -821,7 +867,7 @@ lines one_fixup_per_commit: true, logger: &logger, }; - run_with_repo(&config, &ctx.repo).unwrap(); + run_with_repo(&mut config, &ctx.repo).unwrap(); assert!(nothing_left_in_index(&ctx.repo).unwrap()); let mut revwalk = ctx.repo.revwalk().unwrap(); diff --git a/src/main.rs b/src/main.rs index e54c0dc..8103cca 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,10 @@ struct Cli { /// Don't make any actual changes #[clap(long, short = 'n')] dry_run: bool, - /// Skip safety checks + /// Generate fixups to commits not made by you + #[clap(long)] + force_author: bool, + /// Skip all safety checks; generate fixups to commits not made by you (as if by --force-author) and to non-branch HEADs #[clap(long, short)] force: bool, /// Display more output @@ -41,6 +44,7 @@ fn main() { let Cli { base, dry_run, + force_author, force, verbose, and_rebase, @@ -87,6 +91,7 @@ fn main() { if let Err(e) = git_absorb::run(&mut git_absorb::Config { dry_run, + force_author, force, base: base.as_deref(), and_rebase, diff --git a/src/stack.rs b/src/stack.rs index 9beafd2..2eef3bb 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -7,6 +7,7 @@ use crate::config; pub fn working_stack<'repo>( repo: &'repo git2::Repository, user_provided_base: Option<&str>, + force_author: bool, force: bool, logger: &slog::Logger, ) -> Result>> { @@ -69,11 +70,11 @@ pub fn working_stack<'repo>( break; } if let Ok(ref sig) = sig { - if !force + if !force_author && (commit.author().name_bytes() != sig.name_bytes() || commit.author().email_bytes() != sig.email_bytes()) { - warn!(logger, "Will not fix up past commits not authored by you, use --force to override"; + warn!(logger, "Will not fix up past commits not authored by you, use --force-author to override"; "commit" => commit.id().to_string()); break; } @@ -194,7 +195,7 @@ mod tests { assert_stack_matches_chain( 1, - &working_stack(&repo, None, false, &empty_slog()).unwrap(), + &working_stack(&repo, None, false, false, &empty_slog()).unwrap(), &commits, ); } @@ -211,6 +212,7 @@ mod tests { &repo, Some(&commits[0].id().to_string()), false, + false, &empty_slog(), ) .unwrap(), @@ -232,7 +234,7 @@ mod tests { assert_stack_matches_chain( config::MAX_STACK + 1, - &working_stack(&repo, None, false, &empty_slog()).unwrap(), + &working_stack(&repo, None, false, false, &empty_slog()).unwrap(), &commits, ); } @@ -249,7 +251,7 @@ mod tests { assert_stack_matches_chain( 2, - &working_stack(&repo, None, false, &empty_slog()).unwrap(), + &working_stack(&repo, None, false, false, &empty_slog()).unwrap(), &new_commits, ); } @@ -267,7 +269,7 @@ mod tests { assert_stack_matches_chain( 2, - &working_stack(&repo, None, false, &empty_slog()).unwrap(), + &working_stack(&repo, None, false, false, &empty_slog()).unwrap(), &commits, ); } From 8dc45990d108e473d890c0958f59d31ad031af1d Mon Sep 17 00:00:00 2001 From: Blair Conrad Date: Sun, 2 Feb 2025 14:53:50 -0500 Subject: [PATCH 3/3] Add forceAuthor configuration option to always generate fixup commits for foreign authors --- Documentation/git-absorb.1 | 14 +++++++++++ Documentation/git-absorb.txt | 12 +++++++++ README.md | 11 +++++++++ src/config.rs | 13 ++++++++++ src/lib.rs | 48 ++++++++++++++++++++++++++++++------ 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/Documentation/git-absorb.1 b/Documentation/git-absorb.1 index 36e7950..7047ca0 100644 --- a/Documentation/git-absorb.1 +++ b/Documentation/git-absorb.1 @@ -199,6 +199,20 @@ edit your local or global \&.gitconfig and add the following section: .if n \{\ .RE .\} +.SS "GENERATE FIXUPS FOR COMMITS NOT AUTHORED BY YOU" +.sp +By default, git\-absorb will only generate fixup commits for commits that were authored by you\&. To always generate fixups for any author\(cqs commits, edit your local or global \&.gitconfig and add the following section: +.sp +.if n \{\ +.RS 4 +.\} +.nf +[absorb] + forceAuthor = true +.fi +.if n \{\ +.RE +.\} .SH "GITHUB PROJECT" .sp https://github\&.com/tummychow/git\-absorb diff --git a/Documentation/git-absorb.txt b/Documentation/git-absorb.txt index 066f25f..fbb2234 100644 --- a/Documentation/git-absorb.txt +++ b/Documentation/git-absorb.txt @@ -128,6 +128,18 @@ edit your local or global `.gitconfig` and add the following section: maxStack=50 # Or any other reasonable value for your project ............................................................................. +GENERATE FIXUPS FOR COMMITS NOT AUTHORED BY YOU +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +By default, git-absorb will only generate fixup commits for commits that were +authored by you. To always generate fixups for any author's commits, +edit your local or global `.gitconfig` and add the following section: + +............................................................................. +[absorb] + forceAuthor = true +............................................................................. + GITHUB PROJECT -------------- diff --git a/README.md b/README.md index cf2c1ac..fa98206 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,17 @@ edit your local or global `.gitconfig` and add the following section maxStack=50 # Or any other reasonable value for your project ``` +### Generate fixups for commits not authored by you + +By default, git-absorb will only generate fixup commits for commits that were authored by you. +Instead, use the `--force-author` flag to generate fixup commits for commits written by any author. +To always have this behavior, set + +```ini +[absorb] + forceAuthor = true +``` + ### One fixup per fixable commit By default, git-absorb will generate separate fixup commits for every absorbable hunk. Instead, can use the `-F` flag to create only 1 fixup commit for all hunks that absorb into the same commit. diff --git a/src/config.rs b/src/config.rs index 726fac6..d476e68 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,9 @@ pub const MAX_STACK_CONFIG_NAME: &str = "absorb.maxStack"; pub const MAX_STACK: usize = 10; +pub const FORCE_AUTHOR_CONFIG_NAME: &str = "absorb.forceAuthor"; +pub const FORCE_AUTHOR_DEFAULT: bool = false; + pub const ONE_FIXUP_PER_COMMIT_CONFIG_NAME: &str = "absorb.oneFixupPerCommit"; pub const ONE_FIXUP_PER_COMMIT_DEFAULT: bool = false; @@ -20,6 +23,16 @@ pub fn max_stack(repo: &git2::Repository) -> usize { } } +pub fn force_author(repo: &git2::Repository) -> bool { + match repo + .config() + .and_then(|config| config.get_bool(FORCE_AUTHOR_CONFIG_NAME)) + { + Ok(force_author) => force_author, + _ => FORCE_AUTHOR_DEFAULT, + } +} + pub fn one_fixup_per_commit(repo: &git2::Repository) -> bool { match repo .config() diff --git a/src/lib.rs b/src/lib.rs index e88c544..1fcc33c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,13 @@ pub fn run(config: &mut 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: &mut Config, repo: &git2::Repository) -> Result<()> { + // have force flag enable all force* flags + config.force_author |= config.force; + // here, we default to the git config value, // if the flag was not provided in the CLI. // @@ -35,13 +42,7 @@ pub fn run(config: &mut Config) -> Result<()> { // like we do here is no longer sufficient. but until then, this is fine. // config.one_fixup_per_commit |= config::one_fixup_per_commit(&repo); - - run_with_repo(config, &repo) -} - -fn run_with_repo(config: &mut Config, repo: &git2::Repository) -> Result<()> { - // have force flag enable all force* flags - config.force_author |= config.force; + config.force_author |= config::force_author(&repo); let stack = stack::working_stack( repo, @@ -723,6 +724,39 @@ lines assert!(nothing_left_in_index(&ctx.repo).unwrap()); } + #[test] + fn foreign_author_with_force_author_config() { + let ctx = prepare_and_stage(); + + become_new_author(&ctx); + + ctx.repo.config() + .unwrap() + .set_str("absorb.forceAuthor", "true") + .unwrap(); + + // run 'git-absorb' + let drain = slog::Discard; + let logger = slog::Logger::root(drain, o!()); + let mut config = Config { + dry_run: false, + force_author: false, + force: false, + base: None, + and_rebase: false, + whole_file: false, + one_fixup_per_commit: true, + logger: &logger, + }; + run_with_repo(&mut config, &ctx.repo).unwrap(); + + let mut revwalk = ctx.repo.revwalk().unwrap(); + revwalk.push_head().unwrap(); + assert_eq!(revwalk.count(), 2); + + assert!(nothing_left_in_index(&ctx.repo).unwrap()); + } + fn autostage_common(ctx: &Context, file_path: &PathBuf) -> (PathBuf, PathBuf) { // 1 modification w/o staging let path = ctx.join(&file_path);