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

[PATCH 2/9] revert: make commit subjects in insn sheet optional

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Dec 9, 2011, 15:41 UTC
Message-ID
<1323445326-24637-3-git-send-email-artagnon@gmail.com>
In-Reply-To
<1323445326-24637-1-git-send-email-artagnon@gmail.com>

Change the instruction sheet format subtly so that the subject of the commit message that follows the object name is optional. As a result, an instruction sheet like this is now perfectly valid:

  pick 35b0426
  pick fbd5bbcbc2e
  pick 7362160f

While at it, also fix a bug: currently, we use a commit-id-shaped buffer to store the word after "pick" in '.git/sequencer/todo'. This is both wasteful and wrong because it places an artificial limit on the line length. In addition to literal SHA-1 hexes, expressions like the following are valid object names in the instruction sheet:

  featurebranch~4
  rr/revert-cherry-pick-continue^2~12@{12 days ago}

So, eliminate the need for the buffer to keep the object name altogether, and add a test demonstrating this.

[jc: simplify parsing]
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/revert.c                |   37 ++++++++++++++++---------------------
 t/t3510-cherry-pick-sequence.sh |   28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0c6d3d8..70055e5 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -711,31 +711,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
 {
 	unsigned char commit_sha1[20];
-	char sha1_abbrev[40];
 	enum replay_action action;
-	int insn_len = 0;
-	char *p, *q;
+	char *end_of_object_name;
+	int saved, status;
 
-	if (!prefixcmp(start, "pick ")) {
+	if (!prefixcmp(bol, "pick ")) {
 		action = CHERRY_PICK;
-		insn_len = strlen("pick");
-		p = start + insn_len + 1;
-	} else if (!prefixcmp(start, "revert ")) {
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ")) {
 		action = REVERT;
-		insn_len = strlen("revert");
-		p = start + insn_len + 1;
+		bol += strlen("revert ");
 	} else
 		return NULL;
 
-	q = strchr(p, ' ');
-	if (!q)
-		return NULL;
-	q++;
-
-	strlcpy(sha1_abbrev, p, q - p);
+	end_of_object_name = bol + strcspn(bol, " \n");
+	saved = *end_of_object_name;
+	*end_of_object_name = '\0';
+	status = get_sha1(bol, commit_sha1);
+	*end_of_object_name = saved;
 
 	/*
 	 * Verify that the action matches up with the one in
@@ -748,7 +744,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 		return NULL;
 	}
 
-	if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+	if (status < 0)
 		return NULL;
 
 	return lookup_commit_reference(commit_sha1);
@@ -763,13 +759,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 	int i;
 
 	for (i = 1; *p; i++) {
-		commit = parse_insn_line(p, opts);
+		char *eol = strchrnul(p, '\n');
+		commit = parse_insn_line(p, eol, opts);
 		if (!commit)
 			return error(_("Could not parse line %d."), i);
 		next = commit_list_append(commit, next);
-		p = strchrnul(p, '\n');
-		if (*p)
-			p++;
+		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
 		return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2c4c1c8..6390f2a 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -13,6 +13,9 @@ test_description='Test cherry-pick continuation features
 
 . ./test-lib.sh
 
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
 pristine_detach () {
 	git cherry-pick --quit &&
 	git checkout -f "$1^0" &&
@@ -328,4 +331,29 @@ test_expect_success 'malformed instruction sheet 2' '
 	test_must_fail git cherry-pick --continue
 '
 
+test_expect_success 'malformed instruction sheet 3' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "resolved" >foo &&
+	git add foo &&
+	git commit &&
+	sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	test_must_fail git cherry-pick --continue
+'
+
+test_expect_success 'commit descriptions in insn sheet are optional' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick base..anotherpick &&
+	echo "c" >foo &&
+	git add foo &&
+	git commit &&
+	cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+	cp new_sheet .git/sequencer/todo &&
+	git cherry-pick --continue &&
+	test_path_is_missing .git/sequencer &&
+	git rev-list HEAD >commits &&
+	test_line_count = 4 commits
+'
+
 test_done

-- 
1.7.7.3
Previous: Ramkumar RamachandraNext: Jonathan Nieder
Message 3 of 45 in “Re-roll rr/revert-cherry-pick v2”
  1. 0/9 Re-roll rr/revert-cherry-pick v2Ramkumar Ramachandra, Dec 9, 2011
  2. 1/9 revert: free msg in format_todo()Ramkumar Ramachandra, Dec 9, 2011
  3. 2/9 revert: make commit subjects in insn sheet optionalRamkumar Ramachandra, Dec 9, 2011
  4. Jonathan NiederDec 9, 2011
  5. 3/9 revert: tolerate extra spaces, tabs in insn sheetRamkumar Ramachandra, Dec 9, 2011
  6. Jonathan NiederDec 9, 2011
  7. Ramkumar RamachandraDec 9, 2011
  8. Junio C HamanoDec 9, 2011
  9. Ramkumar RamachandraDec 9, 2011
  10. 4/9 revert: simplify getting commit subject in format_todo()Ramkumar Ramachandra, Dec 9, 2011
  11. Jonathan NiederDec 9, 2011
  12. Ramkumar RamachandraDec 9, 2011
  13. 5/9 t3510 (cherry-pick-sequencer): use exit statusRamkumar Ramachandra, Dec 9, 2011
  14. Jonathan NiederDec 9, 2011
  15. Ramkumar RamachandraDec 9, 2011
  16. Jonathan NiederDec 9, 2011
  17. 6/9 t3510 (cherry-pick-sequencer): remove malformed sheet 2Ramkumar Ramachandra, Dec 9, 2011
  18. Jonathan NiederDec 9, 2011
  19. Ramkumar RamachandraDec 9, 2011
  20. Jonathan NiederDec 9, 2011
  21. 7/9 revert: allow mixed pick and revert instructionsRamkumar Ramachandra, Dec 9, 2011
  22. Jonathan NiederDec 9, 2011
  23. 8/9 revert: report fine-grained error messages from insn parserRamkumar Ramachandra, Dec 9, 2011
  24. Jonathan NiederDec 9, 2011
  25. 9/9 revert: simplify communicating command-line argumentsRamkumar Ramachandra, Dec 9, 2011
  26. Jonathan NiederDec 9, 2011
  27. Ramkumar RamachandraDec 9, 2011
  28. Jonathan NiederDec 9, 2011
  29. Ramkumar RamachandraDec 9, 2011
  30. Jonathan NiederDec 9, 2011
  31. Jonathan NiederDec 9, 2011
  32. Ramkumar RamachandraDec 9, 2011
  33. 00/10 Re-roll rr/revert-cherry-pick v3Ramkumar Ramachandra, Dec 14, 2011
  34. 01/10 revert: free msg in format_todo()Ramkumar Ramachandra, Dec 14, 2011
  35. 02/10 revert: make commit subjects in insn sheet optionalRamkumar Ramachandra, Dec 14, 2011
  36. 03/10 revert: tolerate extra spaces, tabs in insn sheetRamkumar Ramachandra, Dec 14, 2011
  37. 04/10 revert: simplify getting commit subject in format_todo()Ramkumar Ramachandra, Dec 14, 2011
  38. 05/10 t3510 (cherry-pick-sequencer): use exit statusRamkumar Ramachandra, Dec 14, 2011
  39. 06/10 t3502, t3510: clarify cherry-pick -m failureRamkumar Ramachandra, Dec 14, 2011
  40. 07/10 t3510 (cherry-pick-sequencer): remove malformed sheet 2Ramkumar Ramachandra, Dec 14, 2011
  41. 08/10 revert: move replay_action, replay_subcommand to headerRamkumar Ramachandra, Dec 14, 2011
  42. 09/10 revert: allow mixed pick and revert instructionsRamkumar Ramachandra, Dec 14, 2011
  43. 10/10 revert: report fine-grained error messages from insn parserRamkumar Ramachandra, Dec 14, 2011
  44. Junio C HamanoDec 15, 2011
  45. Ramkumar RamachandraDec 15, 2011

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.