Subject: [PATCH v2 00/11] rebase: dereference tags

From: "Phillip Wood via GitGitGadget" <[email-masked]>
Date: Mon, 13 Sep 2021 15:19:11 +0000

Thanks to Ævar and Johannes for their comments.

 * Changed "! test_cmp_rev" to "test_cmp_rev !" (suggested by Ævar)
 * Fixed the quoting for the title of the "rebase --quit" tests.
 * Reworked the last commit to handle the error case first (suggested by
   Ævar)
 * Tweaked the commit messages for patches 8 & 11
 * Rebased onto 31e4a0db03 ("Merge branch 'ab/rebase-fatal-fatal-fix'",
   2021-09-08) to avoid a merge conflict that upset gitgitgadget

Cover letter for V1:

Aborting a rebase stated with git rebase <upstream> <tag-object> should
checkout the commit pointed to by . Instead it gives

    error: update_ref failed for ref 'HEAD': cannot update ref 'HEAD': trying to write non-commit object       710d743b2b9892457fdcc3970f397e6ec07447e0 to branch 'HEAD'

The fix for that is in the last patch, the rest of the patches are cleanups
to t3407 and builtin/rebase.c

Phillip Wood (11):
  t3407: run tests in $TEST_DIRECTORY
  t3407: use test_commit
  t3407: use test_cmp_rev
  t3407: rename a variable
  t3407: use test_path_is_missing
  t3407: strengthen rebase --abort tests
  t3407: rework rebase --quit tests
  rebase: remove redundant strbuf
  rebase: use our standard error return value
  rebase: use lookup_commit_reference_by_name()
  rebase: dereference tags

 builtin/rebase.c        |  63 +++++++++++-------------
 t/t3407-rebase-abort.sh | 105 ++++++++++++++++++----------------------
 2 files changed, 73 insertions(+), 95 deletions(-)

base-commit: 31e4a0db0337e2aa972d9b9f11a332dff7c4cbcb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1033%2Fphillipwood%2Fwip%2Frebase-handle-tags-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1033/phillipwood/wip/rebase-handle-tags-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1033

Range-diff vs v1:

  1:  0f5992e8cab =  1:  bac009d8543 t3407: run tests in $TEST_DIRECTORY
  2:  79b6dec910e =  2:  abfffb31a56 t3407: use test_commit
  3:  dfa27d7a8e7 !  3:  7755ce17fef t3407: use test_cmp_rev
     @@ t/t3407-rebase-abort.sh: testrebase() {
       		git add a &&
       		test_must_fail git rebase --continue &&
      -		test $(git rev-parse HEAD) != $(git rev-parse main) &&
     -+		! test_cmp_rev HEAD main &&
     ++		test_cmp_rev ! HEAD main &&
       		git rebase --abort &&
      -		test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&
      +		test_cmp_rev to-rebase pre-rebase &&
  4:  bef70d86d53 !  4:  38eee11baf5 t3407: rename a variable
     @@ t/t3407-rebase-abort.sh: test_expect_success setup '
       		echo d >> a &&
       		git add a &&
      @@ t/t3407-rebase-abort.sh: testrebase() {
     - 		! test_cmp_rev HEAD main &&
     + 		test_cmp_rev ! HEAD main &&
       		git rebase --abort &&
       		test_cmp_rev to-rebase pre-rebase &&
      -		test ! -d "$dotest"
  5:  d9376fe0818 !  5:  61a37c89f1e t3407: use test_path_is_missing
     @@ t/t3407-rebase-abort.sh: testrebase() {
       
       	test_expect_success "rebase$type --abort after --continue" '
      @@ t/t3407-rebase-abort.sh: testrebase() {
     - 		! test_cmp_rev HEAD main &&
     + 		test_cmp_rev ! HEAD main &&
       		git rebase --abort &&
       		test_cmp_rev to-rebase pre-rebase &&
      -		test ! -d "$state_dir"
  6:  87d7e9bf2d4 !  6:  6866630528b t3407: strengthen rebase --abort tests
     @@ t/t3407-rebase-abort.sh: testrebase() {
       
      @@ t/t3407-rebase-abort.sh: testrebase() {
       		test_must_fail git rebase --continue &&
     - 		! test_cmp_rev HEAD main &&
     + 		test_cmp_rev ! HEAD main &&
       		git rebase --abort &&
      -		test_cmp_rev to-rebase pre-rebase &&
      +		check_head &&
  7:  9a4f6ea73c5 !  7:  fd55a3196b1 t3407: rework rebase --quit tests
     @@ t/t3407-rebase-abort.sh: testrebase() {
       		git rebase --abort
       	'
      +
     -+	test_expect_success 'rebase$type --quit' '
     ++	test_expect_success "rebase$type --quit" '
      +		test_when_finished "git symbolic-ref HEAD refs/heads/to-rebase" &&
      +		# Clean up the state from the previous one
      +		git reset --hard pre-rebase &&
  8:  35b6c26c8f9 =  8:  ad3c4efc027 rebase: remove redundant strbuf
  9:  f0644cde725 =  9:  ad940b633d0 rebase: use our standard error return value
 10:  c537897006c = 10:  bc103e703e8 rebase: use lookup_commit_reference_by_name()
 11:  e87ce4fe253 ! 11:  951de6bb199 rebase: dereference tags
     @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
      -		} else if (!get_oid(branch_name, &options.orig_head) &&
      -			   lookup_commit_reference(the_repository,
      -						   &options.orig_head))
     --			options.head_name = NULL;
     --		else
     --			die(_("fatal: no such branch/commit '%s'"),
     --			    branch_name);
      +		} else {
      +			struct commit *commit =
      +				lookup_commit_reference_by_name(branch_name);
     -+			if (commit) {
     -+				oidcpy(&options.orig_head, &commit->object.oid);
     -+				options.head_name = NULL;
     -+			} else {
     -+				die(_("fatal: no such branch/commit '%s'"),
     ++			if (!commit)
     ++				die(_("no such branch/commit '%s'"),
      +				    branch_name);
     -+			}
     ++			oidcpy(&options.orig_head, &commit->object.oid);
     + 			options.head_name = NULL;
     +-		else
     +-			die(_("no such branch/commit '%s'"),
     +-			    branch_name);
      +		}
       	} else if (argc == 0) {
       		/* Do not need to switch branches, we are already on it. */

-- 
gitgitgadget