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

Re: [PATCH v5 00/44] Make git-am a builtin

From
Paul Tan <pyokagan@gmail.com>
Date
Jul 14, 2015, 10:08 UTC
Message-ID
<20150714100803.GA23901@yoshi.chippynet.com>
In-Reply-To
<xmqqh9p7y9iu.fsf@gitster.dls.corp.google.com>
On Mon, Jul 13, 2015 at 03:31:37PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> A fix is to edit the patch to replace that "flags);" line with full
> "return delete_ref()" line and save it.  Then running
> 
>   $ git am
> 
> (no argument) is supposed to read from the corrected patch file and
> continue the application.
> 
> This no longer works with the version with this series, it seems.

Ah, this is actually the same bug as the previous one, and it actually all stems from the fact that I completely overlooked the code path change introduced by 271440e (git-am: make it easier after fixing up an unapplicable patch., 2005-10-25). When "git am" is run again while paused, the mail message should not be re-parsed at all.

So, I'm thinking of something like the following as a separate patch in this series to re-implement the feature:

diff a/builtin/am.c b/builtin/am.c
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1714,6 +1692,21 @@ static void do_commit(const struct am_state *state)
 }
 
 /**
+ * Validates the am_state for resuming -- the "msg" and authorship fields must
+ * be filled up.
+ */
+static void validate_resume_state(const struct am_state *state)
+{
+	if (!state->msg)
+		die(_("cannot resume: %s does not exist."),
+			am_path(state, "final-commit"));
+
+	if (!state->author_name || !state->author_email || !state->author_date)
+		die(_("cannot resume: %s does not exist."),
+			am_path(state, "author-script"));
+}
+
+/**
  * Interactively prompt the user on whether the current patch should be
  * applied.
  *
@@ -1774,8 +1767,12 @@ static int do_interactive(struct am_state *state)
 
 /**
  * Applies all queued mail.
+ *
+ * If `resume` is true, we are "resuming". The "msg" and authorship fields, as
+ * well as the state directory's "patch" file is used as-is for applying the
+ * patch and committing it.
  */
-static void am_run(struct am_state *state)
+static void am_run(struct am_state *state, int resume)
 {
 	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	struct strbuf sb = STRBUF_INIT;
@@ -1793,21 +1790,28 @@ static void am_run(struct am_state *state)
 
 	while (state->cur <= state->last) {
 		const char *mail = am_path(state, msgnum(state));
-		int apply_status, skip;
+		int apply_status;
 
 		if (!file_exists(mail))
 			goto next;
 
-		if (state->rebasing)
-			skip = parse_mail_rebase(state, mail);
-		else
-			skip = parse_mail(state, mail);
+		if (resume) {
+			validate_resume_state(state);
+			resume = 0;
+		} else {
+			int skip;
+
+			if (state->rebasing)
+				skip = parse_mail_rebase(state, mail);
+			else
+				skip = parse_mail(state, mail);
 
-		if (skip)
-			goto next; /* mail should be skipped */
+			if (skip)
+				goto next; /* mail should be skipped */
 
-		write_author_script(state);
-		write_commit_msg(state);
+			write_author_script(state);
+			write_commit_msg(state);
+		}
 
 		if (state->interactive && do_interactive(state))
 			goto next;
@@ -1880,13 +1884,7 @@ next:
  */
 static void am_resolve(struct am_state *state)
 {
-	if (!state->msg)
-		die(_("cannot resume: %s does not exist."),
-			am_path(state, "final-commit"));
-
-	if (!state->author_name || !state->author_email || !state->author_date)
-		die(_("cannot resume: %s does not exist."),
-			am_path(state, "author-script"));
+	validate_resume_state(state);
 
 	say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg);
 
@@ -1915,7 +1913,7 @@ static void am_resolve(struct am_state *state)
 
 next:
 	am_next(state);
-	am_run(state);
+	am_run(state, 0);
 }
 
 /**
@@ -2040,7 +2038,7 @@ static void am_skip(struct am_state *state)
 		die(_("failed to clean index"));
 
 	am_next(state);
-	am_run(state);
+	am_run(state, 0);
 }
 
 /**
@@ -2136,6 +2134,7 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int
 
 enum resume_mode {
 	RESUME_FALSE = 0,
+	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
 	RESUME_ABORT
@@ -2271,6 +2270,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 			die(_("previous rebase directory %s still exists but mbox given."),
 				state.dir);
 
+		if (resume == RESUME_FALSE)
+			resume = RESUME_APPLY;
+
 		am_load(&state);
 	} else {
 		struct argv_array paths = ARGV_ARRAY_INIT;
@@ -2310,7 +2312,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
 	switch (resume) {
 	case RESUME_FALSE:
-		am_run(&state);
+		am_run(&state, 0);
+		break;
+	case RESUME_APPLY:
+		am_run(&state, 1);
 		break;
 	case RESUME_RESOLVED:
 		am_resolve(&state);

At this point I've accumulated a lot of changes on my end so I'll do a
re-roll.

Thanks,
Paul
Previous: Junio C Hamano
Message 63 of 63 in “Make git-am a builtin”
  1. 00/44 Make git-am a builtinPaul Tan, Jul 7, 2015
  2. 01/44 wrapper: implement xopen()Paul Tan, Jul 7, 2015
  3. 02/44 wrapper: implement xfopen()Paul Tan, Jul 7, 2015
  4. 03/44 builtin-am: implement skeletal builtin amPaul Tan, Jul 7, 2015
  5. 04/44 builtin-am: implement patch queue mechanismPaul Tan, Jul 7, 2015
  6. 05/44 builtin-am: split out mbox/maildir patches with git-mailsplitPaul Tan, Jul 7, 2015
  7. 06/44 builtin-am: auto-detect mbox patchesPaul Tan, Jul 7, 2015
  8. 07/44 builtin-am: extract patch and commit info with git-mailinfoPaul Tan, Jul 7, 2015
  9. 08/44 builtin-am: apply patch with git-applyPaul Tan, Jul 7, 2015
  10. 09/44 builtin-am: implement committing applied patchPaul Tan, Jul 7, 2015
  11. 10/44 builtin-am: refuse to apply patches if index is dirtyPaul Tan, Jul 7, 2015
  12. 11/44 builtin-am: implement --resolved/--continuePaul Tan, Jul 7, 2015
  13. 12/44 builtin-am: implement --skipPaul Tan, Jul 7, 2015
  14. Stefan BellerJul 13, 2015
  15. Paul TanJul 14, 2015
  16. Stefan BellerJul 14, 2015
  17. Paul TanJul 18, 2015
  18. 13/44 builtin-am: implement --abortPaul Tan, Jul 7, 2015
  19. 14/44 builtin-am: reject patches when there's a session in progressPaul Tan, Jul 7, 2015
  20. 15/44 builtin-am: implement -q/--quietPaul Tan, Jul 7, 2015
  21. 16/44 builtin-am: exit with user friendly message on failurePaul Tan, Jul 7, 2015
  22. 17/44 builtin-am: implement -s/--signoffPaul Tan, Jul 7, 2015
  23. 18/44 cache-tree: introduce write_index_as_tree()Paul Tan, Jul 7, 2015
  24. Junio C HamanoJul 7, 2015
  25. 19/44 builtin-am: implement --3way, am.threeWayPaul Tan, Jul 7, 2015
  26. Junio C HamanoJul 7, 2015
  27. Paul TanJul 14, 2015
  28. 20/44 builtin-am: implement --rebasing modePaul Tan, Jul 7, 2015
  29. 21/44 builtin-am: bypass git-mailinfo when --rebasingPaul Tan, Jul 7, 2015
  30. 22/44 builtin-am: handle stray state directoryPaul Tan, Jul 7, 2015
  31. 23/44 builtin-am: implement -u/--utf8Paul Tan, Jul 7, 2015
  32. 24/44 builtin-am: implement -k/--keep, --keep-non-patchPaul Tan, Jul 7, 2015
  33. 25/44 builtin-am: implement --[no-]message-id, am.messageidPaul Tan, Jul 7, 2015
  34. 26/44 builtin-am: support --keep-cr, am.keepcrPaul Tan, Jul 7, 2015
  35. 27/44 builtin-am: implement --[no-]scissorsPaul Tan, Jul 7, 2015
  36. 28/44 builtin-am: pass git-apply's options to git-applyPaul Tan, Jul 7, 2015
  37. 29/44 builtin-am: implement --ignore-datePaul Tan, Jul 7, 2015
  38. 30/44 builtin-am: implement --committer-date-is-author-datePaul Tan, Jul 7, 2015
  39. 31/44 builtin-am: implement -S/--gpg-sign, commit.gpgsignPaul Tan, Jul 7, 2015
  40. 32/44 builtin-am: invoke post-rewrite hookPaul Tan, Jul 7, 2015
  41. 33/44 builtin-am: support automatic notes copyingPaul Tan, Jul 7, 2015
  42. 34/44 builtin-am: invoke applypatch-msg hookPaul Tan, Jul 7, 2015
  43. 35/44 builtin-am: invoke pre-applypatch hookPaul Tan, Jul 7, 2015
  44. 36/44 builtin-am: invoke post-applypatch hookPaul Tan, Jul 7, 2015
  45. 37/44 builtin-am: rerere supportPaul Tan, Jul 7, 2015
  46. 38/44 builtin-am: support and auto-detect StGit patchesPaul Tan, Jul 7, 2015
  47. 39/44 builtin-am: support and auto-detect StGit series filesPaul Tan, Jul 7, 2015
  48. 40/44 builtin-am: support and auto-detect mercurial patchesPaul Tan, Jul 7, 2015
  49. 41/44 builtin-am: implement -i/--interactivePaul Tan, Jul 7, 2015
  50. 42/44 builtin-am: implement legacy -b/--binary optionPaul Tan, Jul 7, 2015
  51. 43/44 builtin-am: check for valid committer identPaul Tan, Jul 7, 2015
  52. 44/44 builtin-am: remove redirection to git-am.shPaul Tan, Jul 7, 2015
  53. Junio C HamanoJul 7, 2015
  54. Paul TanJul 7, 2015
  55. Junio C HamanoJul 8, 2015
  56. Paul TanJul 8, 2015
  57. Junio C HamanoJul 8, 2015
  58. Paul TanJul 8, 2015
  59. Junio C HamanoJul 9, 2015
  60. Paul TanJul 12, 2015
  61. Junio C HamanoJul 12, 2015
  62. Junio C HamanoJul 13, 2015
  63. Paul TanJul 14, 2015

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.