Re: [PATCH v4 2/2] am: support --empty-commit option to handle empty patches
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 16, 2021, 10:07 UTC
- Message-ID
- <23de56a6-5b41-d8be-d59c-5a4e025ccd73@gmail.com>
- In-Reply-To
- <b7e30c9b7abecdc871ddc38122ca042e940cb190.1637039888.git.gitgitgadget@gmail.com>
Hi Aleen
Thanks for working on this
On 16/11/2021 05:18, Aleen via GitGitGadget wrote:
Show 32 quoted lines
> From: Aleen <aleen42@vip.qq.com> > > Signed-off-by: Aleen <aleen42@vip.qq.com> > --- > Documentation/git-am.txt | 9 +++++ > builtin/am.c | 48 +++++++++++++++++++++++--- > t/t4150-am.sh | 73 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 125 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt > index 0a4a984dfde..d8d3bf202d7 100644 > --- a/Documentation/git-am.txt > +++ b/Documentation/git-am.txt > @@ -16,6 +16,7 @@ SYNOPSIS > [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet] > [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>] > [--quoted-cr=<action>] > + [--empty-commit=(die|skip|asis)] > [(<mbox> | <Maildir>)...] > 'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=(diff|raw)]) > > @@ -63,6 +64,14 @@ OPTIONS > --quoted-cr=<action>:: > This flag will be passed down to 'git mailinfo' (see linkgit:git-mailinfo[1]). > > +--empty-commit=(die|skip|asis):: > + The command usually errors out when seeing an input e-mail > + message that lacks a patch. When this option is set to > + 'skip', skip such an e-mail message without outputting error. > + When this option is set to 'asis', create an empty commit, > + recording the contents of the e-mail message as its log. > + 'die' is specified by default.
This feels sufficiently similar to the case of handling empty commits in 'git rebase' that it is worth trying to have a similar user interface. Otherwise the two commands have two different option names doing more or less the same thing. 'git rebase' has --empty=[drop,keep,ask] where drop is the default. If am were to accept --empty=[drop,keep,die] it would offer a similar user experience.
Best Wishes
Phillip
Show 194 quoted lines
> -m::
> --message-id::
> Pass the `-m` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]),
> diff --git a/builtin/am.c b/builtin/am.c
> index 8677ea2348a..e7755c1377e 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -87,6 +87,12 @@ enum show_patch_type {
> SHOW_PATCH_DIFF = 1,
> };
>
> +enum empty_commit_action {
> + DIE_EMPTY_COMMIT = 0, /* output errors */
> + SKIP_EMPTY_COMMIT, /* skip without outputting errors */
> + ASIS_EMPTY_COMMIT /* keep recording as empty commits */
> +};
> +
> struct am_state {
> /* state directory path */
> char *dir;
> @@ -118,6 +124,7 @@ struct am_state {
> int message_id;
> int scissors; /* enum scissors_type */
> int quoted_cr; /* enum quoted_cr_action */
> + int empty_commit; /* enum empty_commit_action */
> struct strvec git_apply_opts;
> const char *resolvemsg;
> int committer_date_is_author_date;
> @@ -178,6 +185,23 @@ static int am_option_parse_quoted_cr(const struct option *opt,
> return 0;
> }
>
> +static int am_option_parse_empty_commit(const struct option *opt,
> + const char *arg, int unset)
> +{
> + int *opt_value = opt->value;
> +
> + if (unset || !strcmp(arg, "die"))
> + *opt_value = DIE_EMPTY_COMMIT;
> + else if (!strcmp(arg, "skip"))
> + *opt_value = SKIP_EMPTY_COMMIT;
> + else if (!strcmp(arg, "asis"))
> + *opt_value = ASIS_EMPTY_COMMIT;
> + else
> + return error(_("Invalid value for --empty-commit: %s"), arg);
> +
> + return 0;
> +}
> +
> /**
> * Returns path relative to the am_state directory.
> */
> @@ -1248,11 +1272,6 @@ static int parse_mail(struct am_state *state, const char *mail)
> goto finish;
> }
>
> - if (is_empty_or_missing_file(am_path(state, "patch"))) {
> - printf_ln(_("Patch is empty."));
> - die_user_resolve(state);
> - }
> -
> strbuf_addstr(&msg, "\n\n");
> strbuf_addbuf(&msg, &mi.log_message);
> strbuf_stripspace(&msg, 0);
> @@ -1792,6 +1811,20 @@ static void am_run(struct am_state *state, int resume)
> if (state->interactive && do_interactive(state))
> goto next;
>
> + if (is_empty_or_missing_file(am_path(state, "patch"))) {
> + if (state->empty_commit == SKIP_EMPTY_COMMIT)
> + goto next;
> + else if (state->empty_commit == ASIS_EMPTY_COMMIT) {
> + if (run_applypatch_msg_hook(state))
> + exit(1);
> + else
> + goto commit;
> + } else if (state->empty_commit == DIE_EMPTY_COMMIT) {
> + printf_ln(_("Patch is empty."));
> + die_user_resolve(state);
> + }
> + }
> +
> if (run_applypatch_msg_hook(state))
> exit(1);
>
> @@ -1827,6 +1860,7 @@ static void am_run(struct am_state *state, int resume)
> die_user_resolve(state);
> }
>
> +commit:
> do_commit(state);
>
> next:
> @@ -2357,6 +2391,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
> { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"),
> N_("GPG-sign commits"),
> PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> + { OPTION_CALLBACK, 0, "empty-commit", &state.empty_commit,
> + "(die|skip|asis)",
> + N_("specify how to handle empty patches"),
> + PARSE_OPT_OPTARG, am_option_parse_empty_commit },
> OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
> N_("(internal use for git-rebase)")),
> OPT_END()
> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> index 2aaaa0d7ded..e657180c201 100755
> --- a/t/t4150-am.sh
> +++ b/t/t4150-am.sh
> @@ -196,6 +196,13 @@ test_expect_success setup '
>
> git format-patch -M --stdout lorem^ >rename-add.patch &&
>
> + git checkout -b empty-commit &&
> + git commit -m "empty commit" --allow-empty &&
> +
> + git format-patch --stdout empty-commit^ >empty.patch &&
> + git format-patch --stdout --cover-letter empty-commit^ >cover-letter.patch &&
> + git format-patch --always --stdout empty-commit^ >empty-commit.patch &&
> +
> # reset time
> sane_unset test_tick &&
> test_tick
> @@ -1152,4 +1159,70 @@ test_expect_success 'apply binary blob in partial clone' '
> git -C client am ../patch
> '
>
> +test_expect_success 'still output error with --empty-commit when meeting empty files' '
> + test_must_fail git am --empty-commit=skip empty.patch 2>actual &&
> + echo Patch format detection failed. >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'error when meeting e-mail message that lacks a patch by default' '
> + git checkout empty-commit^ &&
> + test_must_fail git am empty-commit.patch >err &&
> + test_path_is_dir .git/rebase-apply &&
> + test_i18ngrep "Patch is empty." err &&
> + rm -fr .git/rebase-apply &&
> +
> + test_must_fail git am --empty-commit=die empty-commit.patch >err &&
> + test_path_is_dir .git/rebase-apply &&
> + test_i18ngrep "Patch is empty." err &&
> + rm -fr .git/rebase-apply &&
> +
> + test_must_fail git am --empty-commit=die cover-letter.patch >err &&
> + test_path_is_dir .git/rebase-apply &&
> + test_i18ngrep "Patch is empty." err &&
> + rm -fr .git/rebase-apply
> +'
> +
> +test_expect_success 'skip without error when meeting e-mail message that lacks a patch' '
> + git am --empty-commit=skip empty-commit.patch >err &&
> + test_path_is_missing .git/rebase-apply &&
> + git rev-parse empty-commit^ >expected &&
> + git rev-parse HEAD >actual &&
> + test_cmp expected actual &&
> +
> + git am --empty-commit=skip cover-letter.patch >err &&
> + test_path_is_missing .git/rebase-apply &&
> + test_cmp_rev empty-commit^ HEAD
> +'
> +
> +test_expect_success 'record as an empty commit when meeting e-mail message that lacks a patch' '
> + git am --empty-commit=asis empty-commit.patch &&
> + test_path_is_missing .git/rebase-apply &&
> + {
> + git show empty-commit --format="%B" &&
> + echo "--" &&
> + git version | sed -e "s/^git version //" &&
> + echo
> + } >expected &&
> + git show HEAD --format="%B" >actual &&
> + test_cmp actual expected &&
> +
> + git am --empty-commit=asis cover-letter.patch &&
> + test_path_is_missing .git/rebase-apply &&
> + {
> + echo "*** SUBJECT HERE ***" &&
> + echo &&
> + echo "*** BLURB HERE ***" &&
> + echo &&
> + echo "A U Thor (1):" &&
> + printf " " &&
> + git show empty-commit --format="%B" &&
> + echo "--" &&
> + git version | sed -e "s/^git version //" &&
> + echo
> + } >expected &&
> + git show HEAD --format="%B" >actual &&
> + test_cmp actual expected
> +'
> +
> test_done
>