From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jerry Zhang <jerry@skydio.com>
Cc: git@vger.kernel.org, gitster@pobox.com, ross@skydio.com,
abe@skydio.com, brian.kubisiak@skydio.com
Subject: Re: [PATCH V2] git-apply: add --allow-empty flag
Date: Thu, 29 Apr 2021 02:32:53 +0200 [thread overview]
Message-ID: <878s52ylko.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20210427194049.14399-1-jerry@skydio.com>
On Tue, Apr 27 2021, Jerry Zhang wrote:
> Some users or scripts will pipe "git diff"
> output to "git apply" when replaying diffs
> or commits. In these cases, they will rely
> on the return value of "git apply" to know
> whether the diff was applied successfully.
>
> However, for empty commits, "git apply" will
> fail. This complicates scripts since they
> have to either buffer the diff and check
> its length, or run diff again with "exit-code",
> essentially doing the diff twice.
>
> Add the "--allow-empty" flag to "git apply"
> which allows it to handle both empty diffs
> and empty commits created by "git format-patch
> --always" by doing nothing and returning 0.
>
> Add tests for both with and without --allow-empty.
>
> Signed-off-by: Jerry Zhang <jerry@skydio.com>
> ---
> This patch applies on top of "git-apply: add --quiet flag".
> The conflict is in Documentation -> Synopsis and is
> trivial to solve.
>
> V1 -> V2:
> - Moved behavior under a flag
> - Added tests for both cases
>
> Documentation/git-apply.txt | 6 +++++-
> apply.c | 8 ++++++--
> apply.h | 1 +
> t/t4126-apply-empty.sh | 16 ++++++++++++++++
> 4 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index a32ad64718..b6d77f4206 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -16,7 +16,7 @@ SYNOPSIS
> [--ignore-space-change | --ignore-whitespace]
> [--whitespace=(nowarn|warn|fix|error|error-all)]
> [--exclude=<path>] [--include=<path>] [--directory=<root>]
> - [--verbose | --quiet] [--unsafe-paths] [<patch>...]
> + [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [<patch>...]
>
> DESCRIPTION
> -----------
> @@ -256,6 +256,10 @@ When `git apply` is used as a "better GNU patch", the user can pass
> the `--unsafe-paths` option to override this safety check. This option
> has no effect when `--index` or `--cached` is in use.
>
> +--allow-empty::
> + Don't return error for patches containing no diff. This includes
> + empty patches and patches with commit text only.
> +
> CONFIGURATION
> -------------
>
> diff --git a/apply.c b/apply.c
> index 918e0988bb..0ddde5e8a8 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4732,8 +4732,10 @@ static int apply_patch(struct apply_state *state,
> }
>
> if (!list && !skipped_patch) {
> - error(_("unrecognized input"));
> - res = -128;
> + if (!state->allow_empty) {
> + error(_("unrecognized input"));
> + res = -128;
> + }
> goto end;
> }
>
> @@ -5061,6 +5063,8 @@ int apply_parse_options(int argc, const char **argv,
> OPT_CALLBACK(0, "directory", state, N_("root"),
> N_("prepend <root> to all filenames"),
> apply_option_parse_directory),
> + OPT_BOOL(0, "allow-empty", &state->allow_empty,
> + N_("don't return error for empty patches")),
> OPT_END()
> };
>
> diff --git a/apply.h b/apply.h
> index da3d95fa50..16202da160 100644
> --- a/apply.h
> +++ b/apply.h
> @@ -66,6 +66,7 @@ struct apply_state {
> int threeway;
> int unidiff_zero;
> int unsafe_paths;
> + int allow_empty;
>
> /* Other non boolean parameters */
> struct repository *repo;
> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
> index ceb6a79fe0..f89c53c5f6 100755
> --- a/t/t4126-apply-empty.sh
> +++ b/t/t4126-apply-empty.sh
> @@ -31,6 +31,22 @@ test_expect_success 'apply empty' '
> test_cmp expect empty
> '
>
> +test_expect_success 'apply empty patch fails' '
> + git reset --hard &&
> + git commit --allow-empty -m "empty commit" &&
> + git format-patch --always HEAD~ >empty.patch &&
> + test_must_fail git apply empty.patch &&
> + test_must_fail git apply - </dev/null
> +'
> +
> +test_expect_success 'apply with --allow-empty succeeds' '
> + git reset --hard &&
> + git commit --allow-empty -m "empty commit" &&
Rather than making two --allow-empty commits, shouldn't this be added to
the earlier "setup" routine, so both of these tests can use the same
empty commit?
The setup routine sets up a patch0 and patch1, just add an empty-patch ?
And for the "git reset --hard" I see you're copy/pasting the existing
convention in that file, but FWIW it's better to use 'test_when_finished
"git reset --hard"' in the appropriate spot in the test (i.e. just
before we'd need the reset if we fail), so a test cleans up after
itself. As opposed to "I'm cleaning in case an earlier test didn't clean
up".
> + git format-patch --always HEAD~ >empty.patch &&
> + git apply --allow-empty empty.patch &&
> + git apply --allow-empty - </dev/null
> +'
> +
> test_expect_success 'apply --index empty' '
> git reset --hard &&
> rm -f missing &&
next prev parent reply other threads:[~2021-04-29 0:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-27 1:12 [PATCH] git-apply: allow empty patch text Jerry Zhang
2021-04-27 5:46 ` Eric Wong
2021-04-27 6:28 ` Junio C Hamano
2021-04-27 19:40 ` [PATCH V2] git-apply: add --allow-empty flag Jerry Zhang
2021-04-28 5:08 ` Junio C Hamano
2021-04-28 18:40 ` Jerry Zhang
2021-04-29 0:32 ` Ævar Arnfjörð Bjarmason [this message]
2021-12-11 3:13 ` [PATCH V3] " Jerry Zhang
2021-12-11 7:36 ` Bagas Sanjaya
2021-12-13 20:28 ` [PATCH V4] " Jerry Zhang
2021-12-13 21:44 ` Junio C Hamano
2021-12-13 21:52 ` Jerry Zhang
2021-12-13 21:57 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878s52ylko.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=abe@skydio.com \
--cc=brian.kubisiak@skydio.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jerry@skydio.com \
--cc=ross@skydio.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).