git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Dec 6, 2019, 16:06 UTC
Message-ID
<20191206160614.631724-9-phillip.wood123@gmail.com>
In-Reply-To
<20191206160614.631724-1-phillip.wood123@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Add a specific message to advise the user what to do when a fixup or squash command would create an empty commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
    I'm slightly nervous of moving the call to determine_whence() to
    finalized_deferred_config(). I think it is ok but need to do a more
    thorough audit of the code to be sure.
 builtin/commit.c       | 32 ++++++++++++++++++++++++++++----
 sequencer.c            | 31 ++++++++++++++++++++++++++++++-
 sequencer.h            |  3 ++-
 t/t3403-rebase-skip.sh | 18 ++++++++++++------
 wt-status.h            |  5 +++--
 5 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c3b6b8a6bd..4ae984c470 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -52,6 +52,20 @@ N_("You asked to amend the most recent commit, but doing so would make\n"
 "it empty. You can repeat your command with --allow-empty, or you can\n"
 "remove the commit entirely with \"git reset HEAD^\".\n");
 
+static const char empty_rebase_fixup_advice[] =
+N_("The fixup would make the commit empty\n"
+"If you wish to commit it anyway use:\n"
+"\n"
+"    git commit --amend --allow-empty\n"
+"    git rebase --continue\n"
+"\n"
+"To remove the commit entirely use:\n"
+"\n"
+"    git reset HEAD^\n"
+"    git rebase --continue\n"
+"\n"
+"Otherwise, please use 'git rebase --skip' to skip it\n");
+
 static const char empty_cherry_pick_advice[] =
 N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
 "If you wish to commit it anyway, use:\n"
@@ -181,8 +195,14 @@ static void determine_whence(struct wt_status *s)
 {
 	if (file_exists(git_path_merge_head(the_repository)))
 		whence = FROM_MERGE;
-	else if (!sequencer_determine_whence(the_repository, &whence))
-		whence = FROM_COMMIT;
+	else {
+		int res = sequencer_determine_whence(the_repository, &whence,
+						     amend);
+		if (res < 0)
+			die(_("could not read sequencer state"));
+		else if (!res)
+			whence = FROM_COMMIT;
+	}
 	if (s)
 		s->whence = whence;
 }
@@ -192,7 +212,6 @@ static void status_init_config(struct wt_status *s, config_fn_t fn)
 	wt_status_prepare(the_repository, s);
 	init_diff_ui_defaults();
 	git_config(fn, s);
-	determine_whence(s);
 	s->hints = advice_status_hints; /* must come after git_config() */
 }
 
@@ -943,9 +962,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 */
 	if (!committable && whence != FROM_MERGE && !allow_empty &&
 	    !(amend && is_a_merge(current_head))) {
+		fprintf(stderr, "\nwhence = %d\n", whence);
 		s->display_comment_prefix = old_display_comment_prefix;
 		run_status(stdout, index_file, prefix, 0, s);
-		if (amend)
+		if (whence == FROM_REBASE_FIXUP)
+			fputs(_(empty_rebase_fixup_advice), stderr);
+		else if (amend)
 			fputs(_(empty_amend_advice), stderr);
 		else if (is_from_cherry_pick(whence) ||
 			 whence == FROM_REBASE_PICK) {
@@ -1114,6 +1136,8 @@ static void finalize_deferred_config(struct wt_status *s)
 
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
+
+	determine_whence(s);
 }
 
 static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/sequencer.c b/sequencer.c
index 6e153fea76..64242f4ce7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5270,7 +5270,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
 	return 0;
 }
 
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+			       int amending)
 {
 	if (file_exists(git_path_cherry_pick_head(r))) {
 		struct object_id cherry_pick_head, rebase_head;
@@ -5286,6 +5287,34 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
 			*whence = FROM_CHERRY_PICK_SINGLE;
 
 		return 1;
+	} else if (amending && file_exists(rebase_path_current_fixups()) &&
+		   (file_exists(git_path_squash_msg(r)) ||
+		    file_exists(git_path_merge_msg(r)))) {
+		/*
+		 * If rebase_path_amend() exists the user is running `git
+		 * commit`, if not we're committing a fixup/squash directly from
+		 * the sequencer
+		 */
+		if (file_exists(rebase_path_amend())) {
+			struct strbuf rev = STRBUF_INIT;
+			struct object_id to_amend, head;
+
+			if (get_oid("HEAD", &head))
+				return error(_("amending invalid head")); /* let commit deal with error */
+			if (!read_oneliner(&rev, rebase_path_amend(), 0))
+				return error(_("invalid file: '%s'"),
+					     rebase_path_amend());
+			if (get_oid_hex(rev.buf, &to_amend))
+				return error(_("invalid contents: '%s'"),
+					     rebase_path_amend());
+			if (oideq(&head, &to_amend)) {
+				*whence = FROM_REBASE_FIXUP;
+				return 1;
+			}
+		} else {
+			*whence = FROM_REBASE_FIXUP;
+			return 1;
+		}
 	}
 
 	return 0;
diff --git a/sequencer.h b/sequencer.h
index 8d451dbfcb..4e631900b4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -202,5 +202,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
 void sequencer_post_commit_cleanup(struct repository *r);
 int sequencer_get_last_command(struct repository* r,
 			       enum replay_action *action);
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+			       int amending);
 #endif /* SEQUENCER_H */
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index a927774910..bc110b66b3 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -164,22 +164,28 @@ test_expect_success 'correct advice upon multi cherry-pick picking an empty comm
 	test_i18ngrep "git cherry-pick --skip" err
 '
 
-test_expect_success 'fixup that empties commit fails' '
+test_expect_success 'correct advice when fixup empties commit' '
 	test_when_finished "git rebase --abort" &&
 	(
 		set_fake_editor &&
 		test_must_fail env FAKE_LINES="1 fixup 2" git rebase -i \
-			goodbye^ reverted-goodbye
-	)
+			goodbye^ reverted-goodbye 2>err
+	) &&
+	test_i18ngrep "git rebase --skip" err &&
+	test_must_fail git commit --amend --no-edit 2>err &&
+	test_i18ngrep "git rebase --skip" err
 '
 
-test_expect_success 'squash that empties commit fails' '
+test_expect_success 'correct advice when squash empties commit' '
 	test_when_finished "git rebase --abort" &&
 	(
 		set_fake_editor &&
 		test_must_fail env FAKE_LINES="1 squash 2" git rebase -i \
-			goodbye^ reverted-goodbye
-	)
+			goodbye^ reverted-goodbye 2>err
+	) &&
+	test_i18ngrep "git rebase --skip" err &&
+	test_must_fail git commit --amend --no-edit 2>err &&
+	test_i18ngrep "git rebase --skip" err
 '
 
 # Must be the last test in this file
diff --git a/wt-status.h b/wt-status.h
index 5f81bf7507..a4b7fe6de9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -41,7 +41,8 @@ enum commit_whence {
 	FROM_MERGE,      /* commit came from merge */
 	FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
 	FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
-	FROM_REBASE_PICK /* commit came from a pick/reword/edit */
+	FROM_REBASE_PICK, /* commit came from a pick/reword/edit */
+	FROM_REBASE_FIXUP /* commit came from fixup/squash */
 };
 
 static inline int is_from_cherry_pick(enum commit_whence whence)
@@ -52,7 +53,7 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
 
 static inline int is_from_rebase(enum commit_whence whence)
 {
-	return whence == FROM_REBASE_PICK;
+	return whence == FROM_REBASE_PICK || whence == FROM_REBASE_FIXUP;
 }
 
 struct wt_status_change_data {

-- 
2.24.0
Previous: Phillip WoodNext: Elijah Newren
Message 30 of 31 in “commit: fix advice for empty commits during rebases”
  1. 0/3 commit: fix advice for empty commits during rebasesJohannes Schindelin via GitGitGadget, Oct 22, 2019
  2. 2/3 sequencer: export the function to get the path of `.git/rebase-merge/`Johannes Schindelin via GitGitGadget, Oct 22, 2019
  3. 1/3 cherry-pick: add test for `--skip` advice in `git commit`Johannes Schindelin via GitGitGadget, Oct 22, 2019
  4. 3/3 commit: give correct advice for empty commit during a rebaseJohannes Schindelin via GitGitGadget, Oct 22, 2019
  5. Junio C HamanoOct 23, 2019
  6. Phillip WoodOct 24, 2019
  7. Johannes SchindelinOct 25, 2019
  8. Phillip WoodOct 25, 2019
  9. Junio C HamanoNov 8, 2019
  10. Johannes SchindelinNov 8, 2019
  11. Phillip WoodNov 11, 2019
  12. Junio C HamanoOct 23, 2019
  13. Johannes SchindelinOct 25, 2019
  14. Junio C HamanoOct 29, 2019
  15. Johannes SchindelinOct 29, 2019
  16. 0/9 commit: fix advice for empty commits during rebasesPhillip Wood, Dec 6, 2019
  17. 2/9 cherry-pick: add test for `--skip` advice in `git commit`Phillip Wood, Dec 6, 2019
  18. 4/9 sequencer: write CHERRY_PICK_HEAD for reword and editPhillip Wood, Dec 6, 2019
  19. 3/9 cherry-pick: check commit error messagesPhillip Wood, Dec 6, 2019
  20. 1/9 t3404: use test_cmp_revPhillip Wood, Dec 6, 2019
  21. Junio C HamanoDec 6, 2019
  22. 6/9 commit: encapsulate determine_whence() for sequencerPhillip Wood, Dec 6, 2019
  23. Junio C HamanoDec 6, 2019
  24. Phillip WoodDec 18, 2019
  25. 5/9 commit: use enum value for multiple cherry-picksPhillip Wood, Dec 6, 2019
  26. Junio C HamanoDec 6, 2019
  27. 9/9 [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflictsPhillip Wood, Dec 6, 2019
  28. Phillip WoodDec 18, 2019
  29. 7/9 commit: give correct advice for empty commit during a rebasePhillip Wood, Dec 6, 2019
  30. 8/9 [RFC] rebase: fix advice when a fixup creates an empty commitPhillip Wood, Dec 6, 2019
  31. Elijah NewrenFeb 26, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.