From a3bcd1a366e88d1da603dbf77d49ceb219b526b5 Mon Sep 17 00:00:00 2001 From: Jordan Torbiak Date: Tue, 5 Sep 2023 22:39:47 -0600 Subject: [PATCH] Choose upstream revisions automatically If an upstream revision isn't given on the command-line, try to find suitable ones automatically. This only works if the current branch has a tracking branch. Writing tests for this feature required refactoring the existing test helpers significantly, and it made sense to put things in different files and use packages other than main to make it obvious what's being called. --- .gitignore | 1 + MANIFEST | 3 + MANIFEST.SKIP | 3 + git-autofixup | 99 ++++++++++-- t/autofixup.t | 315 +++++--------------------------------- t/implicit_upstream.t | 349 ++++++++++++++++++++++++++++++++++++++++++ t/repo.pl | 152 ++++++++++++++++++ t/util.pl | 230 ++++++++++++++++++++++++++++ 8 files changed, 855 insertions(+), 297 deletions(-) create mode 100644 t/implicit_upstream.t create mode 100644 t/repo.pl create mode 100644 t/util.pl diff --git a/.gitignore b/.gitignore index 45472c6..206d188 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ MYMETA.* App-Git-Autofixup-*.tar.gz Makefile +/todo diff --git a/MANIFEST b/MANIFEST index 46b5c1e..77c5b14 100644 --- a/MANIFEST +++ b/MANIFEST @@ -6,3 +6,6 @@ Makefile.PL MANIFEST This list of files README.pod t/autofixup.t +t/implicit_upstream.t +t/repo.pl +t/util.pl diff --git a/MANIFEST.SKIP b/MANIFEST.SKIP index a45cc12..f151d1c 100644 --- a/MANIFEST.SKIP +++ b/MANIFEST.SKIP @@ -69,3 +69,6 @@ MANIFEST\.SKIP ^xt \.sw[pmno]$ +^\.github +^release$ +^todo$ diff --git a/git-autofixup b/git-autofixup index 93f7ad4..7821872 100755 --- a/git-autofixup +++ b/git-autofixup @@ -1,5 +1,5 @@ #!/usr/bin/perl -package main; +package App::Git::Autofixup; use 5.008004; use strict; use warnings FATAL => 'all'; @@ -20,7 +20,7 @@ my @GIT_OPTIONS; my ($CONTEXT, $ADJACENT, $SURROUNDED) = (0..10); my $usage =<<'END'; -usage: git-autofixup [] +usage: git-autofixup [] [] -h show usage --help show manpage @@ -119,10 +119,67 @@ sub git_cmd { return ('git', @GIT_OPTIONS, @_); } +# With a linear git history there'll be a single merge base that's easy to +# refer to with @{upstream}, but during an interactive rebase we need to get +# the "current" branch from the rebase metadata. +# +# Unusual cases: +# +# While there can be multiple merge bases if there have been criss-cross +# merges, there'll still be a single fork point unless the relevant reflog +# entries have already been garbage-collected. +# +# When multiple upstreams are configured via `branch..merge` in git's +# config the most correct approach is probably to find the fork-point for each +# merge value and return those. But it seems unlikely that someone is doing +# octopus merges and using git-autofixup, so we're not handling that specially +# currently. +sub find_merge_bases { + my $upstream = '@{upstream}'; + + # If an interactive rebase is in progress, derive the upstream from the + # rebase meatadata. + my $gitdir = qx(git rev-parse --git-dir) or die "git rev-parse: $!\n"; + chomp $gitdir; + if (-e "$gitdir/rebase-merge") { + my $branch = slurp("$gitdir/rebase-merge/head-name"); + chomp $branch; + $branch =~ s#^refs/heads/##; + $upstream = "$branch\@{upstream}"; + } + + # `git merge-base` will fail if there's no tracking branch. In that case + # redirect stderr and communicate failure by returning an empty list. Also, + # with the --fork-point option, no merge bases are returned if the relevant + # reflog entries have been GC'd, so fall back to normal merge-bases. + my @merge_bases; + for (qx(git merge-base --all --fork-point $upstream HEAD 2>/dev/null)) { + chomp; + push @merge_bases, $_; + } + if (!@merge_bases) { + for (qx(git merge-base --all $upstream HEAD 2>/dev/null)) { + chomp; + push @merge_bases, $_; + } + } + + return wantarray ? @merge_bases : \@merge_bases; +} + +sub slurp { + my $filename = shift; + open my $fh, '<', $filename or die "slurp $filename: $!"; + local $/; + my $content = readline $fh; + return $content; +} + sub summary_for_commits { - my $rev = shift; + my @upstreams = @_; my %commits; - for (qx(git log --no-merges --format=%H:%s $rev..)) { + my $negative = join(" ", map {"^$_"} @upstreams); + for (qx(git log --no-merges --format=%H:%s HEAD $negative)) { chomp; my ($sha, $msg) = split ':', $_, 2; $commits{$sha} = $msg; @@ -524,13 +581,14 @@ sub create_temp_index { # commit doesn't have any parents, resulting in blame only searching back as # far back as the upstream commit. sub create_grafts_file { - my $upstream = shift; + my @upstreams = @_; my $grafts_file = File::Temp->new( TEMPLATE => 'git-autofixup_grafts.XXXXXX', DIR => File::Spec->tmpdir()); - my $merge_base = qx(git merge-base $upstream HEAD) or return "/dev/null"; open(my $fh, '>', $grafts_file) or die "Can't open $grafts_file: $!\n"; - print $fh $merge_base, "\n"; + for (@upstreams) { + print $fh $_, "\n"; + } close($fh) or die "Error closing grafts file: $!\n"; return $grafts_file; } @@ -568,10 +626,19 @@ sub main { pod2usage(-exitval => 0, -verbose => 2); } - @ARGV == 1 or die "No upstream commit given.\n"; - my $upstream = shift @ARGV; - qx(git rev-parse --verify ${upstream}^{commit}); - $? == 0 or die "Can't resolve given commit.\n"; + # "upstream" revisions as 40 byte SHA1 hex hashes. + my @upstreams = (); + if (@ARGV == 1) { + my $raw_upstream = shift @ARGV; + my $upstream = qx(git rev-parse --verify --end-of-options ${raw_upstream}^{commit}) + or die "Can't resolve given commit.\n"; + push @upstreams, $upstream; + } else { + @upstreams = find_merge_bases(); + if (!@upstreams) { + die "Can't find tracking branch. Please specify a revision.\n"; + } + } if ($num_context_lines < 0) { die "invalid number of context lines: $num_context_lines\n"; @@ -590,9 +657,9 @@ sub main { chdir $toplevel or die $!; my $hunks = diff_hunks($num_context_lines); - my $summary_for = summary_for_commits($upstream); + my $summary_for = summary_for_commits(@upstreams); my $alias_for = sha_aliases($summary_for); - my $grafts_file = create_grafts_file($upstream); + my $grafts_file = create_grafts_file(@upstreams); my %blame_for = map {$_ => blame($_, $alias_for, $grafts_file)} @{$hunks}; my $hunks_for = fixup_hunks_by_sha({ hunks => $hunks, @@ -641,13 +708,11 @@ App::Git::Autofixup - create fixup commits for topic branches =head1 SYNOPSIS - git-autofixup [] + git-autofixup [] [] =head1 DESCRIPTION -F parses hunks of changes in the working directory out of C output and uses C to assign those hunks to commits in CrevisionE..HEAD>, which will typically represent a topic branch, and then creates fixup commits to be used with C. It is assumed that hunks near changes that were previously committed to the topic branch are related. - -C<@{upstream}> or C<@{u}> is likely a convenient value to use for CrevisionE> if the current branch has a tracking branch. See C for other ways to specify revisions. +F parses hunks of changes in the working directory out of C output and uses C to assign those hunks to commits in CrevisionE..HEAD>, which will typically represent a topic branch, and then creates fixup commits to be used with C. It is assumed that hunks near changes that were previously committed to the topic branch are related. If no revision is given and the current branch has a tracking branch, then C<@{upstream}> is used to find reasonable fork-points or merge-bases to use as upstream cutoffs. See C for info about how to specify revisions. If any changes have been staged to the index using C, then F will only consider staged hunks when trying to create fixup commits. A temporary index is used to create any resulting commits. diff --git a/t/autofixup.t b/t/autofixup.t index 496ac45..e66122f 100755 --- a/t/autofixup.t +++ b/t/autofixup.t @@ -2,259 +2,14 @@ use strict; use warnings FATAL => 'all'; -use Carp qw(croak); -use Cwd; -use English qw(-no_match_vars); -use File::Temp qw(tempdir); - -use Test::More; -if ($OSNAME eq 'MSWin32') { - plan skip_all => 'Run from Cygwin or Git Bash on Windows' -} elsif (!has_git()) { - plan skip_all => 'git version 1.7.4+ required' -} else { - plan tests => 42; -} +use Test::More tests => 42; +require './t/util.pl'; require './git-autofixup'; -$ENV{GIT_AUTHOR_NAME} = 'A U Thor'; -$ENV{GIT_AUTHOR_EMAIL} = 'author@example.com'; -$ENV{GIT_COMMITTER_NAME} = 'C O Mitter'; -$ENV{GIT_COMMITTER_EMAIL} = 'committer@example.com'; -$ENV{GIT_CONFIG_NOSYSTEM} = 'true'; - - -sub has_git { - my $stdout = qx{git --version}; - return if $? != 0; - my ($x, $y, $z) = $stdout =~ /(\d+)\.(\d+)(?:\.(\d+))?/; - defined $x or die "unexpected output from git: $stdout"; - $z = defined $z ? $z : 0; - my $cmp = $x <=> 1 || $y <=> 7 || $z <=> 4; - return $cmp >= 0; -} - -sub test_autofixup_strict { - my $params = shift; - my $strict_levels = $params->{strict} or croak "strictness levels not given"; - delete $params->{strict}; - my $autofixup_opts = $params->{autofixup_opts} || []; - if (grep /^(--strict|-s)/, @{$autofixup_opts}) { - croak "strict option already given"; - } - my $name = $params->{name} || croak "name not given"; - for my $strict (@{$strict_levels}) { - $params->{name} = "$name, strict=$strict"; - $params->{autofixup_opts} = ['-s' => $strict, @{$autofixup_opts}]; - test_autofixup($params); - } -} - -# test_autofixup initializes a git repo in a tempdir, creates given "upstream" -# and "topic" commits, applies changes to the working directory, runs -# autofixup, and compares the git log of the fixup commits to an expected log. -# -# The upstream_commits and topic_commits arguments are heterogeneous lists of -# sub and hash refs. Hash refs are interpreted as being maps of filenames to -# contents to be written. If more flexibility is needed a subref can be given -# to manipulate the working directory. -# -# Arguments given in a hashref: -# upstream_commits: sub or hash refs that must not be fixed up -# topic_commits: sub or hash refs representing commits that can be fixed up -# unstaged: sub or hash ref of working directory changes -# staged: sub or hash ref of index changes -# log_want: expected log output -# autofixup_opts: command-line options to pass thru to autofixup -sub test_autofixup { - my ($args) = shift; - my $name = defined($args->{name}) ? $args->{name} - : croak "no test name given"; - my $upstream_commits = $args->{upstream_commits} || []; - my $topic_commits = $args->{topic_commits} || []; - my $unstaged = defined($args->{unstaged}) ? $args->{unstaged} - : croak "no unstaged changes given"; - my $staged = $args->{staged}; - my $log_want = defined($args->{log_want}) ? $args->{log_want} - : croak "wanted log output not given"; - my $staged_want = $args->{staged_want}; - my $unstaged_want = $args->{unstaged_want}; - my $exit_code_want = $args->{exit_code}; - my $autofixup_opts = $args->{autofixup_opts} || []; - push @{$autofixup_opts}, '--exit-code'; - if (!$upstream_commits && !$topic_commits) { - croak "no upstream or topic commits given"; - } - if (exists $args->{strict}) { - croak "strict key given; use test_autofixup_strict instead"; - } - - my $exit_code_got; - my $log_got; - my $staged_got; - my $unstaged_got; - my $orig_dir = getcwd(); - my $dir = File::Temp::tempdir(CLEANUP => 1); - chdir $dir or die "$!"; - local $ENV{HOME} = $dir; - local $ENV{XDG_CONFIG_HOME} = "$dir/.config"; - eval { - - init_repo(); - - my $i = 0; - - for my $commit (@{$upstream_commits}) { - apply_change($commit); - commit_if_dirty("commit$i"); - $i++; - } - my $upstream_rev = get_revision_sha(); - - for my $commit (@{$topic_commits}) { - apply_change($commit); - commit_if_dirty("commit$i"); - $i++; - } - my $pre_fixup_rev = get_revision_sha(); - - if (defined($staged)) { - apply_change($staged); - # We're at the repo root, so using -A will change everything even - # in pre-v2 versions of git. See git commit 808d3d717e8. - run("git add -A"); - } - - apply_change($unstaged); +Util::check_test_deps(); - run("git --no-pager log --format='%h %s' ${upstream_rev}.."); - $exit_code_got = autofixup(@{$autofixup_opts}, $upstream_rev); - $log_got = git_log(${pre_fixup_rev}); - $staged_got = diff('--cached'); - if (defined($unstaged_want)) { - $unstaged_got = diff('HEAD'); - } - }; - my $err = $@; - chdir $orig_dir or die "$!"; - if ($err) { - diag($err); - fail($name); - return; - } - - my $failed = 0; - if ($log_got ne $log_want) { - diag("log_got=<', $filename) or croak "$!"; - print {$fh} $contents or croak "$!"; - close $fh or croak "$!"; -} - -sub git_log { - my $revision = shift; - my $log = qx{git -c diff.noprefix=false log -p --format=%s ${revision}..}; - if ($? != 0) { - croak "git log: $?\n"; - } - return $log; -} - -sub diff { - my $revision = shift; - my $diff = qx{git -c diff.noprefix=false diff ${revision}}; - if ($? != 0) { - croak "git diff $?\n"; - } - return $diff; -} - -sub get_revision_sha { - my $dir = shift; - my $revision = qx{git rev-parse HEAD}; - $? == 0 or croak "git rev-parse: $?"; - chomp $revision; - return $revision; -} - -sub autofixup { - local @ARGV = @_; - print "# git-autofixup ", join(' ', @ARGV), "\n"; - return main(); -} - - -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "single-line change gets autofixed", strict => [0..2], topic_commits => [{a => "a1\n"}], @@ -273,7 +28,7 @@ index da0f8ed..c1827f0 100644 EOF }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "adjacent change gets autofixed", strict => [0..1], upstream_commits => [{a => "a3\n"}], @@ -294,7 +49,7 @@ index 76642d4..2cdcdb0 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "adjacent change doesn't get autofixed if strict=2", upstream_commits => [{a => "a3\n"}], topic_commits => [{a => "a1\na3\n"}], @@ -304,7 +59,7 @@ test_autofixup({ exit_code => 2, }); -test_autofixup({ +Util::test_autofixup({ name => 'fixups are created for additions surrounded by topic commit lines when strict=2', topic_commits => [{a => "a1\na3\n", b => "b1\n", c => "c2\n"}], unstaged => {a => "a1\na2\na3\n", b => "b1\nb2\n", c => "c1\nc2\n"}, @@ -338,28 +93,28 @@ index 16f9ec0..d0aaf97 100644 EOF }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "removed file doesn't get autofixed", strict => [0..2], - topic_commits => [sub { write_file(a => "a1\n"); }], + topic_commits => [sub { Util::write_file(a => "a1\n"); }], unstaged => sub { unlink 'a'; }, exit_code => 3, log_want => '', }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "re-added file doesn't get autofixed", strict => [0..2], topic_commits => [ - sub { write_file(a => "a1\n"); }, + sub { Util::write_file(a => "a1\n"); }, sub { unlink 'a'; }, ], - unstaged => sub { write_file(a => "a1a\n"); }, + unstaged => sub { Util::write_file(a => "a1a\n"); }, exit_code => 3, log_want => '', }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "re-added line gets autofixed into the commit blamed for the adjacent context", # During rebase the line will just get removed again by the next commit. # --strict can be used to avoid creating a fixup in this case, where the @@ -388,7 +143,7 @@ index da0f8ed..0016606 100644 EOF }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "removed lines get autofixed", strict => [0..2], topic_commits => [{a => "a1\n", b => "b1\nb2\n"}], @@ -413,7 +168,7 @@ index 9b89cd5..e6bfff5 100644 EOF }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => 'no fixups are created for upstream commits', strict => [0..2], upstream_commits => [{a => "a1\n"}], @@ -422,7 +177,7 @@ test_autofixup_strict({ log_want => '', }); -test_autofixup({ +Util::test_autofixup({ name => 'fixups are created for hunks changing lines blamed by upstream if strict=0', # This depends on the number of context lines kept when creating diffs. git # keeps 3 by default. @@ -446,7 +201,7 @@ index 125d560..cc1aa32 100644 +a3b EOF }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => 'no fixups are created for hunks changing lines blamed by upstream if strict > 0', # This depends on the number of context lines kept when creating diffs. git # keeps 3 by default. @@ -458,14 +213,14 @@ test_autofixup_strict({ log_want => '', }); -test_autofixup_strict({ +Util::test_autofixup_strict({ name => "hunks blamed on a fixup! commit are assigned to that fixup's target", strict => [0..2], topic_commits => [ {a => "a1\n"}, sub { - write_file(a => "a2\n"); - run(qw(git commit -a --fixup=HEAD)); + Util::write_file(a => "a2\n"); + Util::run(qw(git commit -a --fixup=HEAD)); }, ], exit_code => 0, @@ -483,7 +238,7 @@ index c1827f0..b792f74 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "removed line gets autofixed when context=0", topic_commits => [{a => "a1\na2\n"}], unstaged => {a => "a1\n"}, @@ -502,7 +257,7 @@ index 0016606..da0f8ed 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "added line is ignored when context=0", topic_commits => [{a => "a1\n"}], unstaged => {a => "a1\na2\n"}, @@ -511,7 +266,7 @@ test_autofixup({ log_want => '', }); -test_autofixup({ +Util::test_autofixup({ name => "ADJACENCY assignment is used as a fallback for multiple context targets", topic_commits => [ {a => "a1\n"}, @@ -533,13 +288,13 @@ index 0016606..a0ef52c 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "Works when run in a subdir of the repo root", topic_commits => [ sub { mkdir 'sub' or die $!; chdir 'sub' or die $!; - write_file("a", "a1\n"); + Util::write_file("a", "a1\n"); } ], exit_code => 0, @@ -557,7 +312,7 @@ index da0f8ed..0016606 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "file without newline at EOF gets autofixed", topic_commits => [{a => "a1\na2"}], unstaged => {'a' => "a1\na2\n"}, @@ -577,7 +332,7 @@ index c928c51..0016606 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "multiple hunks in the same file get autofixed", topic_commits => [ {a => "a1.0\na2\na3\na4\na5\na6\na7\na8\na9.0\n"}, @@ -614,7 +369,7 @@ index 50de7e8..d9f44da 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "single-line change gets autofixed when mnemonic prefixes are enabled", topic_commits => [{a => "a1\n"}], unstaged => {a => "a2\n"}, @@ -633,7 +388,7 @@ index da0f8ed..c1827f0 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "single-line change gets autofixed when diff.external is set", topic_commits => [{a => "a1\n"}], unstaged => {a => "a2\n"}, @@ -652,7 +407,7 @@ index da0f8ed..c1827f0 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => 'exit code is 1 when some hunks are assigned', upstream_commits => [{a => "a1\n"}], topic_commits => [{b => "b1\n"}], @@ -671,7 +426,7 @@ index c9c6af7..e6bfff5 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "multiple hunks to the same commit get autofixed", topic_commits => [ {a => "a1.0\na2\na3\na4\na5\na6\na7\na8\na9.0\n"}, @@ -708,7 +463,7 @@ index 5d11004..0054137 100644 +a9.1 }}); -test_autofixup({ +Util::test_autofixup({ name => "only staged hunks get autofixed", topic_commits => [{a => "a1\n", b => "b1\n"}], staged => {a => "a2\n"}, @@ -736,7 +491,7 @@ index c9c6af7..e6bfff5 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "staged hunks that aren't autofixed remain in index", upstream_commits => [{b => "b1\n"}], topic_commits => [{a => "a1\n", , c => "c1\n"}], @@ -781,7 +536,7 @@ index c9c6af7..e6bfff5 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "filename with spaces", topic_commits => [{"filename with spaces" => "a1\n"}], unstaged => {"filename with spaces" => "a2\n"}, @@ -799,7 +554,7 @@ index da0f8ed..c1827f0 100644 EOF }); -test_autofixup({ +Util::test_autofixup({ name => "filename with unusual characters", topic_commits => [{"ff\f nak\025 dq\" hack\\ fei飞.txt" => "a1\n"}], unstaged => {"ff\f nak\025 dq\" hack\\ fei飞.txt" => "a2\n"}, diff --git a/t/implicit_upstream.t b/t/implicit_upstream.t new file mode 100644 index 0000000..de2386f --- /dev/null +++ b/t/implicit_upstream.t @@ -0,0 +1,349 @@ +#!/usr/bin/perl + +# Test finding "upstream" commits. When making fixups we want to minimize the +# number of commits we need to look at, and avoid making "fixups" (ie fixup +# commits) for commits that could be reasonably expected to be pushed or for +# commits that are likely to be considered outside of the topic branch. + +use strict; +use warnings FATAL => 'all'; + +use Test::More tests => 4; + +require './t/util.pl'; +require './t/repo.pl'; +require './git-autofixup'; + +Util::check_test_deps(); + +# fast-forward from upstream +# +# Probably the most common case. +# +# o upstream +# \ +# o--o topic +# +sub test_fast_forward { + my $name = 'fast-forward from upstream'; + + my $wants = { + fixup_log => <<'EOF' +fixup! commit1 + +diff --git a/a b/a +index 8737a60..ba81a56 100644 +--- a/a ++++ b/a +@@ -1,3 +1,3 @@ +-a1.1 +-a2 ++a1 ++a2.1 + a3 +EOF + , + staged => '', + unstaged => '', + }; + + eval { + my $repo = Repo->new(); + + $repo->create_commits({a => "a1\na2\na3\n"}); + my $upstream = $repo->current_commit_sha(); + + $repo->switch_to_downstream_branch('topic'); + $repo->create_commits({a => "a1.1\na2\na3\n"}); + my $topic = $repo->current_commit_sha(); + + $repo->write_change({a => "a1\na2.1\na3\n"}); + + my $upstreams = App::Git::Autofixup::find_merge_bases(); + my $ok = Util::upstreams_ok(want => [$upstream], got => $upstreams); + + if ($ok) { + my $exit_code = $repo->autofixup(); + $ok &&= Util::exit_code_ok(want => 0, got => $exit_code); + $ok &&= Util::repo_state_ok($repo, $topic, $wants); + } + + ok($ok, $name); + }; + if ($@) { + diag($@); + fail($name); + } + return; +} +test_fast_forward(); + + +# interactive rebase onto upstream, making a fixup from B for A +# +# o upstream +# \ +# A--B--C topic +# +sub test_interactive_rebase { + my $name = 'interactive rebase onto upstream'; + + my $wants = { + fixup_log => <<'EOF' +fixup! commit1 + +diff --git a/a b/a +index 8737a60..ba81a56 100644 +--- a/a ++++ b/a +@@ -1,3 +1,3 @@ +-a1.1 +-a2 ++a1 ++a2.1 + a3 +EOF + , + staged => '', + unstaged => '', + }; + + eval { + my $repo = Repo->new(); + + # upstream (commit0) + $repo->create_commits({a => "a1\na2\na3\n"}); + my $upstream = $repo->current_commit_sha(); + + # A (commit1) + $repo->switch_to_downstream_branch('topic'); + $repo->create_commits({a => "a1.1\na2\na3\n"}); + my $topic = $repo->current_commit_sha(); + + # B (commit2) + $repo->create_commits({a => "a1\na2.1\na3\n"}); + + # C + $repo->create_commits({b => "b1\n"}); + + # Start an interactive rebase to edit commit B (which'll have commit2 + # in its message). + local $ENV{GIT_SEQUENCE_EDITOR} = q(perl -i -pe "/commit2/ && s/^pick/edit/"); + Util::with_stderr_to_null(sub { + Util::run(qw(git rebase -q -i), $upstream); + }); + Util::run(qw(git reset HEAD^)); + + my $upstreams = App::Git::Autofixup::find_merge_bases(); + my $ok = Util::upstreams_ok(want => [$upstream], got => $upstreams); + + if ($ok) { + my $exit_code = $repo->autofixup(); + $ok &&= Util::exit_code_ok(want => 0, got => $exit_code); + $ok &&= Util::repo_state_ok($repo, $topic, $wants); + } + + ok($ok, $name); + }; + if ($@) { + diag($@); + fail($name); + } + return; +} +test_interactive_rebase(); + + +# fork-point and merge-base are different +# +# Here the upstream commit that topic originally diverged from is different +# from the first ancestor that currently belongs to both topic and upstream, +# due to upstream being rewound and rebuilt. We don't want to make fixups for +# the fork-point since the user probably doesn't consider it part of the topic +# branch at a conceptual level and by default `git rebase` excludes the +# fork-point from the set of commits to be rewritten. +# +# B1--o upstream +# \ +# B0 fork-point: was previously part of upstream +# \ +# T0 topic +# +sub test_fork_point_and_merge_base_are_different { + my $name = 'fork-point and merge-base are different'; + + my $wants = { + fixup_log => <<'EOF' +fixup! commit2 + +diff --git a/a b/a +index 8737a60..472f448 100644 +--- a/a ++++ b/a +@@ -1,3 +1,3 @@ + a1.1 +-a2 ++a2.1 + a3 +EOF + , + staged => '', + unstaged => '', + }; + + eval { + my $repo = Repo->new(); + + local $ENV{GIT_SEQUENCE_EDITOR}; + + # upstream + # + # Create a commit to use as the fork-point for the topic branch, save + # the SHA, then amend it so that the fork-point is no longer reachable + # from master and create another commit on top. + $repo->create_commits({a => "a1\na2\na3\n"}); + my $fork_point = $repo->current_commit_sha(); + Util::run(qw(git commit --amend -m), 'commit0, reworded'); # B1 + $repo->create_commits({b => "b1\n"}); # o (commit1) + + # topic + Util::run(qw(git checkout -q -b topic), $fork_point); + Util::run(qw(git branch --set-upstream-to master)); + $repo->create_commits({a => "a1.1\na2\na3\n"}); + + $repo->write_change({a => "a1.1\na2.1\na3\n"}); + my $topic = $repo->current_commit_sha(); + + my $upstreams = App::Git::Autofixup::find_merge_bases(); + my $ok = Util::upstreams_ok(want => [$fork_point], got => $upstreams); + + if ($ok) { + my $exit_code = $repo->autofixup(); + $ok &&= Util::exit_code_ok(want => 0, got => $exit_code); + $ok &&= Util::repo_state_ok($repo, $topic, $wants); + } + + ok($ok, $name); + }; + if ($@) { + diag($@); + fail($name); + } + return; +} +test_fork_point_and_merge_base_are_different(); + + +# criss-cross merge and gc'd fork-point +# +# Here's one way to get a criss-cross merge. If you have two branches (A and B, +# here) that include the same merge commit, M0: +# +# C1-M0 A, B +# / +# C2 +# +# And then you amend that merge commit from one of the branches (A, in this +# case), creating M1, you get the following topology. The important implication +# for us is that commits 1 and 2 are both equally good merge bases of A and B, +# and we don't want to create fixups for either of them or their ancestors. +# (Here 'X' represents overlapping graph edges, not another commit.) +# +# C1---M1 A +# \ / +# X +# / \ +# C2---M0 B +# +# For this test we'll also amend B's merge commit and garbage-collect the +# reflog so that M0 isn't simply used as B's fork-point from its tracking +# branch A, forcing git-autofixup to fall back on the merge-bases C1 and C2. +# +# C1---M1 A +# \ / +# X +# / \ +# C2---M2---o B +# +sub test_criss_cross_merge_and_gc_fork_point { + my $name = "criss-cross merge and gc'd fork point"; + + my $wants = { + fixup_log => <<'EOF' +fixup! commit2 + +diff --git a/a b/a +index 8737a60..472f448 100644 +--- a/a ++++ b/a +@@ -1,3 +1,3 @@ + a1.1 +-a2 ++a2.1 + a3 +diff --git a/b b/b +index 0ef8a8e..b1710a1 100644 +--- a/b ++++ b/b +@@ -1,3 +1,3 @@ + b1.1 +-b2 ++b2.1 + b3 +EOF + , + staged => '', + unstaged => '', + }; + + eval { + my $repo = Repo->new(); + + # C1 + Util::run(qw(git checkout -q -b A)); + $repo->create_commits({a => "a1\na2\na3\n"}); + my $c1 = $repo->current_commit_sha(); + + # C2 + Util::run(qw(git checkout -q -b B master)); + Util::run(qw(git branch --set-upstream-to A)); + $repo->create_commits({b => "b1\nb2\nb3\n"}); + my $c2 = $repo->current_commit_sha(); + + # Merge A and B, so they're both pointing to the same merge commit. + Util::run(qw(git merge --no-ff), '-m' => 'Merge A into B', 'A'); # M0 + Util::run(qw(git checkout -q A)); + Util::run(qw(git merge --ff-only B)); # fast-forward to M0 + + # Then ammend the merge commits for both branches and gc the reflog so + # git can't tell what the original fork-point of B from A is. + Util::run(qw(git commit --amend -m), 'Merge A into B, reworded for A'); # M1 + Util::run(qw(git checkout -q B)); + Util::run(qw(git commit --amend -m), 'Merge A into B, reworded for B'); # M2 + Util::run(qw(git -c gc.reflogExpire=now gc)); + + # topic + $repo->create_commits({a => "a1.1\na2\na3\n", b => "b1.1\nb2\nb3\n"}); + my $topic = $repo->current_commit_sha(); + + $repo->write_change({a => "a1.1\na2.1\na3\n", b => "b1.1\nb2.1\nb3\n"}); + + my @upstreams_got = sort(App::Git::Autofixup::find_merge_bases()); + my @upstreams_want = sort $c1, $c2; + my $ok = Util::upstreams_ok(want => \@upstreams_want, got => \@upstreams_got); + + if ($ok) { + my $exit_code = $repo->autofixup(); + $ok &&= Util::exit_code_ok(want => 0, got => $exit_code); + $ok &&= Util::repo_state_ok($repo, $topic, $wants); + } + + ok($ok, $name); + }; + if ($@) { + diag($@); + fail($name); + } + return; +} +test_criss_cross_merge_and_gc_fork_point(); diff --git a/t/repo.pl b/t/repo.pl new file mode 100644 index 0000000..1c620ca --- /dev/null +++ b/t/repo.pl @@ -0,0 +1,152 @@ +package Repo; + +use Carp qw(croak); + +require './t/util.pl'; + +# Return a new Repo, which is a git repo initialized in a temp dir. +# +# By default the temp dir will be removed when it goes out of scope, so if you +# want to be able to inspect a repo after a test fails, give `cleanup => 0`. +sub new { + my ($class, %args) = @_; + my $self = {}; + $self->{cleanup} = defined($args{cleanup}) ? $args{cleanup} : 1; + + bless $self, $class; + + $self->_init_env(); + $self->_init_repo(); + + return $self; +} + +sub _init_env { + my $self = shift; + + my $orig_dir = Cwd::getcwd(); + my $dir = File::Temp::tempdir(CLEANUP => self->{cleanup}); + chdir $dir or die "$!"; + + my %env = ( + # Avoid loading user or global git config, since lots of options can + # break our tests. + HOME => $dir, + XDG_CONFIG_HOME => $dir, + GIT_CONFIG_NOSYSTEM => 'true', + + # In order to make commits, git requires an author identity. + GIT_AUTHOR_NAME => 'A U Thor', + GIT_AUTHOR_EMAIL => 'author@example.com', + GIT_COMMITTER_NAME => 'C O Mitter', + GIT_COMMITTER_EMAIL => 'committer@example.com', + ); + my %orig_env = (); + for my $key (keys %env) { + my $val = $env{$key}; + $orig_env{$key} = $ENV{$key}; + $ENV{$key} = $val; + } + + $self->{dir} = $dir; + $self->{orig_dir} = $orig_dir; + $self->{orig_env} = \%orig_env; +} + +sub _init_repo { + my $self = shift; + $self->{n_commits} = 0; # Number of commits created using create_commits() + local $ENV{GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME} = 'master'; + Util::run('git init'); + # git-autofixup needs a commit to exclude, since it uses the REVISION.. + # syntax. This is that commit. + my $filename = 'README'; + Util::write_file($filename, "init\n"); + Util::run("git add $filename"); + Util::run(qw(git commit -m), "add $filename"); +} + +sub DESTROY { + local $!; + my $self = shift; + + chdir $self->{orig_dir} or die "change to orig working dir: $!"; + File::Temp::cleanup(); + + for my $key (keys %{$self->{orig_env}}) { + my $val = $self->{orig_env}{key}; + $ENV{$key} = $val; + } +} + +sub create_commits { + my ($self, @commits) = @_; + for my $commit (@commits) { + $self->write_change($commit); + $self->commit_if_dirty("commit" . $self->{n_commits}); + $self->{n_commits} += 1; + } +} + +sub write_change { + my ($self, $change) = @_; + if (ref $change eq 'HASH') { + while (my ($file, $contents) = each %{$change}) { + Util::write_file($file, $contents); + } + } elsif (ref $change eq 'CODE') { + &{$change}(); + } +} + +sub commit_if_dirty { + my ($self, $msg) = @_; + my $is_dirty = qx(git status -s); + if ($is_dirty) { + Util::run('git add -A'); + Util::run(qw(git commit -am), $msg); + } +} + +sub log_since { + my ($self, $revision) = @_; + my $log = qx{git -c diff.noprefix=false log -p --format=%s ${revision}..}; + if ($? != 0) { + croak "git log: $!\n"; + } + return $log; +} + +sub diff { + my ($self, $revision) = @_; + my $diff = qx{git -c diff.noprefix=false diff ${revision}}; + if ($? != 0) { + croak "git diff $!\n"; + } + return $diff; +} + +sub current_commit_sha { + my ($self, $dir) = @_; + my $revision = qx{git rev-parse HEAD}; + $? == 0 or croak "git rev-parse: $!"; + chomp $revision; + return $revision; +} + +sub autofixup { + my $self = shift; + local @ARGV = @_; + print "# git-autofixup ", join(' ', @ARGV), "\n"; + return App::Git::Autofixup::main(); +} + +sub switch_to_downstream_branch { + my ($self, $branch) = @_; + my $tracking_branch = qx(git rev-parse --abbrev-ref HEAD) + or croak "get tracking branch: $!"; + chomp $tracking_branch; + Util::run(qw(git checkout -q -b), $branch, '--track', $tracking_branch); +} + +1; diff --git a/t/util.pl b/t/util.pl new file mode 100644 index 0000000..2221088 --- /dev/null +++ b/t/util.pl @@ -0,0 +1,230 @@ +#!/usr/bin/perl +package Util; + +use strict; +use warnings FATAL => 'all'; + +use Carp qw(croak); +use Cwd; +use English qw(-no_match_vars); +use File::Temp qw(tempdir); +use Test::More; + +require './t/repo.pl'; +require './git-autofixup'; + +sub check_test_deps { + if ($OSNAME eq 'MSWin32') { + plan skip_all => 'Run from Cygwin or Git Bash on Windows' + } elsif (!has_git()) { + plan skip_all => 'git version 1.7.4+ required' + } +} + +sub has_git { + my $stdout = qx{git --version}; + return if $? != 0; + my ($x, $y, $z) = $stdout =~ /(\d+)\.(\d+)(?:\.(\d+))?/; + defined $x or die "unexpected output from git: $stdout"; + $z = defined $z ? $z : 0; + my $cmp = $x <=> 1 || $y <=> 7 || $z <=> 4; + return $cmp >= 0; +} + +sub test_autofixup_strict { + my $params = shift; + my $strict_levels = $params->{strict} or croak "strictness levels not given"; + delete $params->{strict}; + my $autofixup_opts = $params->{autofixup_opts} || []; + if (grep /^(--strict|-s)/, @{$autofixup_opts}) { + croak "strict option already given"; + } + my $name = $params->{name} || croak "name not given"; + for my $strict (@{$strict_levels}) { + $params->{name} = "$name, strict=$strict"; + $params->{autofixup_opts} = ['-s' => $strict, @{$autofixup_opts}]; + test_autofixup($params); + } +} + +# test_autofixup initializes a git repo in a tempdir, creates given "upstream" +# and "topic" commits, applies changes to the working directory, runs +# autofixup, and compares wanted `git log` and `git diff` outputs to actual +# ones. +# +# Arguments must be given in a hashref: +# name: test name or description +# upstream_commits: sub or hash refs that must not be fixed up +# topic_commits: sub or hash refs representing commits that can be fixed up +# unstaged: sub or hash ref of working directory changes +# staged: sub or hash ref of index changes +# log_want: expected log output for new fixup commited +# staged_want: expected log output for the staging area +# unstaged_want: expected diff output for the working tree +# autofixup_opts: command-line options to pass thru to autofixup +# +# The upstream_commits and topic_commits arguments are heterogeneous lists of +# sub and hash refs. Hash refs are interpreted as being maps of filenames to +# contents to be written. If more flexibility is needed a subref can be given +# to manipulate the working directory. +sub test_autofixup { + my ($args) = shift; + my $name = defined($args->{name}) ? $args->{name} + : croak "no test name given"; + my $upstream_commits = $args->{upstream_commits} || []; + my $topic_commits = $args->{topic_commits} || []; + my $unstaged = defined($args->{unstaged}) ? $args->{unstaged} + : croak "no unstaged changes given"; + my $staged = $args->{staged}; + my $log_want = defined($args->{log_want}) ? $args->{log_want} + : croak "wanted log output not given"; + my $staged_want = $args->{staged_want}; + my $unstaged_want = $args->{unstaged_want}; + my $exit_code_want = $args->{exit_code}; + my $autofixup_opts = $args->{autofixup_opts} || []; + push @{$autofixup_opts}, '--exit-code'; + if (!$upstream_commits && !$topic_commits) { + croak "no upstream or topic commits given"; + } + if (exists $args->{strict}) { + croak "strict key given; use test_autofixup_strict instead"; + } + + eval { + my $repo = Repo->new(); + + $repo->create_commits(@$upstream_commits); + my $upstream_rev = $repo->current_commit_sha(); + + $repo->create_commits(@$topic_commits); + my $pre_fixup_rev = $repo->current_commit_sha(); + + if (defined($staged)) { + $repo->write_change($staged); + # We're at the repo root, so using -A will change everything even + # in pre-v2 versions of git. See git commit 808d3d717e8. + run(qw(git add -A)); + } + + $repo->write_change($unstaged); + + run('git', '--no-pager', 'log', "--format='%h %s'", "${upstream_rev}.."); + my $exit_code_got = $repo->autofixup(@{$autofixup_opts}, $upstream_rev); + + my $ok = exit_code_ok(want => $exit_code_want, got => $exit_code_got); + my $wants = { + fixup_log => $log_want, + staged => $staged_want, + unstaged => $unstaged_want, + }; + $ok &&= repo_state_ok($repo, $pre_fixup_rev, $wants); + ok($ok, $name); + }; + if ($@) { + diag($@); + fail($name); + } + return; +} + +# Take wanted and actual autofixup exit codes as a hash with keys ('want', +# 'got') and return true if want and got are equal or if want is undefined. +# +# eg: exit_code_got(want => 3, got => 2) +# +# Params are taken as a hash since the order matters and it seems difficult to +# get the order right if the args aren't named. +sub exit_code_ok { + my %args = @_; + defined $args{got} or croak "got exit code is undefined"; + if (defined $args{want} && $args{want} != $args{got}) { + diag("exit_code_want=$args{want},exit_code_got=$args{got}"); + return 0; + } + return 1; +} + +# Take wanted and actual listrefs of upstream SHAs as a hash with keys ('want', +# 'got') and return true if want and got are equal. +# +# eg: exit_code_got(want => 3, got => 2) +sub upstreams_ok { + my %args = @_; + defined $args{want} or croak 'wanted upstream list must be given'; + defined $args{got} or croak 'actual upstream list must be given'; + my @wants = @{$args{want}}; + my @gots = @{$args{got}}; + my $max_len = @wants > @gots ? @wants : @gots; + my $ok = 1; + for my $i (0..$max_len - 1) { + my $want = defined $wants[$i] ? $wants[$i] : ''; + my $got = defined $gots[$i] ? $gots[$i] : ''; + if (!$want || !$got || $want ne $got) { + diag("upstream mismatch,i=$i,want=$want,got=$got"); + $ok = 0; + } + } + return $ok; +} + +sub repo_state_ok { + my ($repo, $pre_fixup_rev, $wants) = @_; + my $ok = 1; + + for my $key (qw(fixup_log staged unstaged)) { + next if (!defined $wants->{$key}); + + my $want = $wants->{$key}; + + my $got; + if ($key eq 'fixup_log') { + $got = $repo->log_since($pre_fixup_rev); + } elsif ($key eq 'staged') { + $got = $repo->diff('--cached'); + } elsif ($key eq 'unstaged') { + $got = $repo->diff('HEAD'); + } + + if ($got ne $want) { + diag("${key}_got=<{staged})) { + my $got = $repo->diff('--cached'); + if ($got) { + diag("staged_got=<', $filename) or croak "$!"; + print {$fh} $contents or croak "$!"; + close $fh or croak "$!"; +} + +sub with_stderr_to_null { + my $funcref = shift; + + open(my $stderr_orig, ">&STDERR") or die "dup stderr: $!"; + open(STDERR, '>', '/dev/null') or die "redirect stderr: $!"; + + eval { &$funcref(); }; + my $err = $@; + + open(STDERR, '>&', $stderr_orig) or die "restore stderr: $!"; + die $err if $err; +} + +1;