{"thread":{"id":"55565","subject":"[PATCH] git-apply: allow empty patch text","startedAt":"2021-04-27T01:12:55Z","lastAt":"2021-12-13T21:57:35Z","messageCount":13,"participants":["Jerry Zhang","Eric Wong","Junio C Hamano","Ævar Arnfjörð Bjarmason","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"423050","messageId":"20210427011246.28054-1-jerry@skydio.com","threadId":"55565","inReplyTo":null,"subject":"[PATCH] git-apply: allow empty patch text","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-04-27T01:12:46Z","receivedAt":"2021-04-27T01:12:55Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"\"git diff\" produces no patch text if\nthere is no diff, but \"git apply\" exits\nwith code 128 if the patch text is empty.\n\nSince every valid \"git diff\" should\nresult in a successful patch application\nwhen applied to the same preimage as\nthe diff, allow \"git apply\" to handle\nthese empty diffs by doing nothing and\nreturning 0.\n\nSigned-off-by: Jerry Zhang \n---\n apply.c | 2 ++\n t/t4126-apply-empty.sh | 7 +++++++\n t/t7006-pager.sh | 2 +-\n 3 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex 5ea237232d..2abf2e8a61 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4685,6 +4685,8 @@ static int apply_patch(struct apply_state *state,\n \tstate->patch_input_file = filename;\n \tif (read_patch_file(&buf, fd) < 0)\n \t\treturn -128;\n+\tif (!buf.len)\n+\t\tgoto end;\n \toffset = 0;\n \twhile (offset < buf.len) {\n \t\tstruct patch *patch;\ndiff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\nindex ceb6a79fe0..37351be609 100755\n--- a/t/t4126-apply-empty.sh\n+++ b/t/t4126-apply-empty.sh\n@@ -31,6 +31,13 @@ test_expect_success 'apply empty' '\n \ttest_cmp expect empty\n '\n \n+test_expect_success 'apply empty diff' '\n+\tgit reset --hard &&\n+\tgit diff >empty.patch &&\n+\tgit apply empty.patch &&\n+\tgit diff | git apply -\n+'\n+\n test_expect_success 'apply --index empty' '\n \tgit reset --hard &&\n \trm -f missing &&\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 0e7cf75435..2c0ada6bf6 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -567,7 +567,7 @@ test_default_pager expect_success 'git -p shortlog'\n test_core_pager_subdir expect_success 'git -p shortlog'\n \n test_core_pager_subdir expect_success test_must_fail \\\n-\t\t\t\t\t 'git -p apply wrote:\n> \"git diff\" produces no patch text if\n> there is no diff, but \"git apply\" exits\n> with code 128 if the patch text is empty.\n> \n> Since every valid \"git diff\" should\n> result in a successful patch application\n> when applied to the same preimage as\n> the diff,\n\nShould it result in successful patch application? (Why?)\n\nI fear this change can cause errors in pipelines to go\nundetected (since \"set -o pipefail\" is not POSIX).\nIn my experience, zero-byte files is also a common failure mode\nfor some filesystems, even after fsck marked them as clean.\n\nPerhaps guarding this behavior with --allow-empty (as commit and\ncherry-pick do) is safer.\n\n> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\n> index ceb6a79fe0..37351be609 100755\n> --- a/t/t4126-apply-empty.sh\n> +++ b/t/t4126-apply-empty.sh\n> @@ -31,6 +31,13 @@ test_expect_success 'apply empty' '\n> \ttest_cmp expect empty\n> '\n> \n> +test_expect_success 'apply empty diff' '\n> +\tgit reset --hard &&\n> +\tgit diff >empty.patch &&\n> +\tgit apply empty.patch &&\n> +\tgit diff | git apply -\n> +'\n\nIt shouldn't be necessary to use \"git diff\" to generate empty\noutput. Tests are too slow for me on Linux, even. Something\nlike:\n\n\t>empty.patch &&\n\tgit apply empty.patch &&\n\tgit apply - writes:\n\n> I fear this change can cause errors in pipelines to go\n> undetected (since \"set -o pipefail\" is not POSIX).\n> In my experience, zero-byte files is also a common failure mode\n> for some filesystems, even after fsck marked them as clean.\n\nThanks for saving me time to say it.\n\nIt would be a grave regression to any workflow automation to change\nthe behaviour to silently succeed a non-patch application as a\nnoop. And it does not take filesystem corruption.\n\nSome people seem to gpg sign their patches sent to the list, which\nis not very useful at this point as I don't bother to collect their\npublic keys anyway, but feeding such a piece of e-mail from GNUS to\n\"git am -s\" takes \"\\M-i r |\" prefix instead of the regular \"|\" to\n\"pipe the message to an external command\" to properly get it\nprocessed. I was saved by the \"empty input is wrong\" behaviour a\nnumber of times.\n"},{"id":"423126","messageId":"20210427194049.14399-1-jerry@skydio.com","threadId":"55565","inReplyTo":"20210427011246.28054-1-jerry@skydio.com","subject":"[PATCH V2] git-apply: add --allow-empty flag","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-04-27T19:40:49Z","receivedAt":"2021-04-27T19:40:58Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"Some users or scripts will pipe \"git diff\"\noutput to \"git apply\" when replaying diffs\nor commits. In these cases, they will rely\non the return value of \"git apply\" to know\nwhether the diff was applied successfully.\n\nHowever, for empty commits, \"git apply\" will\nfail. This complicates scripts since they\nhave to either buffer the diff and check\nits length, or run diff again with \"exit-code\",\nessentially doing the diff twice.\n\nAdd the \"--allow-empty\" flag to \"git apply\"\nwhich allows it to handle both empty diffs\nand empty commits created by \"git format-patch\n--always\" by doing nothing and returning 0.\n\nAdd tests for both with and without --allow-empty.\n\nSigned-off-by: Jerry Zhang \n---\nThis patch applies on top of \"git-apply: add --quiet flag\".\nThe conflict is in Documentation -> Synopsis and is\ntrivial to solve.\n\nV1 -> V2:\n- Moved behavior under a flag\n- Added tests for both cases\n\n Documentation/git-apply.txt | 6 +++++-\n apply.c | 8 ++++++--\n apply.h | 1 +\n t/t4126-apply-empty.sh | 16 ++++++++++++++++\n 4 files changed, 28 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex a32ad64718..b6d77f4206 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n \t [--ignore-space-change | --ignore-whitespace]\n \t [--whitespace=(nowarn|warn|fix|error|error-all)]\n \t [--exclude=] [--include=] [--directory=]\n-\t [--verbose | --quiet] [--unsafe-paths] [...]\n+\t [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n \n DESCRIPTION\n -----------\n@@ -256,6 +256,10 @@ When `git apply` is used as a \"better GNU patch\", the user can pass\n the `--unsafe-paths` option to override this safety check. This option\n has no effect when `--index` or `--cached` is in use.\n \n+--allow-empty::\n+\tDon't return error for patches containing no diff. This includes\n+\tempty patches and patches with commit text only.\n+\n CONFIGURATION\n -------------\n \ndiff --git a/apply.c b/apply.c\nindex 918e0988bb..0ddde5e8a8 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4732,8 +4732,10 @@ static int apply_patch(struct apply_state *state,\n \t}\n \n \tif (!list && !skipped_patch) {\n-\t\terror(_(\"unrecognized input\"));\n-\t\tres = -128;\n+\t\tif (!state->allow_empty) {\n+\t\t\terror(_(\"unrecognized input\"));\n+\t\t\tres = -128;\n+\t\t}\n \t\tgoto end;\n \t}\n \n@@ -5061,6 +5063,8 @@ int apply_parse_options(int argc, const char **argv,\n \t\tOPT_CALLBACK(0, \"directory\", state, N_(\"root\"),\n \t\t\tN_(\"prepend to all filenames\"),\n \t\t\tapply_option_parse_directory),\n+\t\tOPT_BOOL(0, \"allow-empty\", &state->allow_empty,\n+\t\t\tN_(\"don't return error for empty patches\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/apply.h b/apply.h\nindex da3d95fa50..16202da160 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -66,6 +66,7 @@ struct apply_state {\n \tint threeway;\n \tint unidiff_zero;\n \tint unsafe_paths;\n+\tint allow_empty;\n \n \t/* Other non boolean parameters */\n \tstruct repository *repo;\ndiff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\nindex ceb6a79fe0..f89c53c5f6 100755\n--- a/t/t4126-apply-empty.sh\n+++ b/t/t4126-apply-empty.sh\n@@ -31,6 +31,22 @@ test_expect_success 'apply empty' '\n \ttest_cmp expect empty\n '\n \n+test_expect_success 'apply empty patch fails' '\n+\tgit reset --hard &&\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit format-patch --always HEAD~ >empty.patch &&\n+\ttest_must_fail git apply empty.patch &&\n+\ttest_must_fail git apply - empty.patch &&\n+\tgit apply --allow-empty empty.patch &&\n+\tgit apply --allow-empty - writes:\n\n> Some users or scripts will pipe \"git diff\"\n> output to \"git apply\" when replaying diffs\n> or commits. In these cases, they will rely\n> on the return value of \"git apply\" to know\n> whether the diff was applied successfully.\n>\n> However, for empty commits, \"git apply\" will\n> fail. This complicates scripts since they\n> have to either buffer the diff and check\n> its length, or run diff again with \"exit-code\",\n> essentially doing the diff twice.\n>\n> Add the \"--allow-empty\" flag to \"git apply\"\n> which allows it to handle both empty diffs\n> and empty commits created by \"git format-patch\n> --always\" by doing nothing and returning 0.\n>\n> Add tests for both with and without --allow-empty.\n>\n> Signed-off-by: Jerry Zhang \n> ---\n> This patch applies on top of \"git-apply: add --quiet flag\".\n> The conflict is in Documentation -> Synopsis and is\n> trivial to solve.\n\n> V1 -> V2:\n> - Moved behavior under a flag\n> - Added tests for both cases\n\nWhen people add a flag (a boolean option), it becomes tempting to\nadd a corresponding configuration variable.\n\nI wonder it this step should start calling\n\n\tgit apply --no-allow-empty\n\nwhen \"git am\" drives it, so that a futre end-user configuration\nvariable would not break it?\n\n\n"},{"id":"423211","messageId":"CAMKO5CvHWDQNFoPeon99K97vUY0Y8WCbBcJghGyJT9XUr=nYqA@mail.gmail.com","threadId":"55565","inReplyTo":"xmqqmttjate1.fsf@gitster.g","subject":"Re: [PATCH V2] git-apply: add --allow-empty flag","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-04-28T18:40:37Z","receivedAt":"2021-04-28T18:40:52Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"On Tue, Apr 27, 2021 at 10:08 PM Junio C Hamano wrote:\n>\n> Jerry Zhang writes:\n>\n> > Some users or scripts will pipe \"git diff\"\n> > output to \"git apply\" when replaying diffs\n> > or commits. In these cases, they will rely\n> > on the return value of \"git apply\" to know\n> > whether the diff was applied successfully.\n> >\n> > However, for empty commits, \"git apply\" will\n> > fail. This complicates scripts since they\n> > have to either buffer the diff and check\n> > its length, or run diff again with \"exit-code\",\n> > essentially doing the diff twice.\n> >\n> > Add the \"--allow-empty\" flag to \"git apply\"\n> > which allows it to handle both empty diffs\n> > and empty commits created by \"git format-patch\n> > --always\" by doing nothing and returning 0.\n> >\n> > Add tests for both with and without --allow-empty.\n> >\n> > Signed-off-by: Jerry Zhang \n> > ---\n> > This patch applies on top of \"git-apply: add --quiet flag\".\n> > The conflict is in Documentation -> Synopsis and is\n> > trivial to solve.\n>\n> > V1 -> V2:\n> > - Moved behavior under a flag\n> > - Added tests for both cases\n>\n> When people add a flag (a boolean option), it becomes tempting to\n> add a corresponding configuration variable.\n>\n> I wonder it this step should start calling\n>\n> git apply --no-allow-empty\n>\n> when \"git am\" drives it, so that a futre end-user configuration\n> variable would not break it?\n>\nThat's currently not necessary, since \"git am\" can catch empty patches\nbefore it ever calls into apply.c:\n\n if (is_empty_or_missing_file(am_path(state, \"patch\"))) {\n printf_ln(_(\"Patch is empty.\"));\n die_user_resolve(state);\n }\n\nIf someone does make a config variable I think it might be convenient to\nteach \"git am\" to use \"allow-empty\" as well, since applying empty commits\ncan be useful for documenting the history. The config variable could then\ncontrol the behavior of both \"am\" and \"apply\" since they are used for very\nsimilar things. Of course that's up to the decision of that hypothetical person\nsince I don't currently have a need to change \"am\".\n"},{"id":"423223","messageId":"878s52ylko.fsf@evledraar.gmail.com","threadId":"55565","inReplyTo":"20210427194049.14399-1-jerry@skydio.com","subject":"Re: [PATCH V2] git-apply: add --allow-empty flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-29T00:32:53Z","receivedAt":"2021-04-29T00:35:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Apr 27 2021, Jerry Zhang wrote:\n\n> Some users or scripts will pipe \"git diff\"\n> output to \"git apply\" when replaying diffs\n> or commits. In these cases, they will rely\n> on the return value of \"git apply\" to know\n> whether the diff was applied successfully.\n>\n> However, for empty commits, \"git apply\" will\n> fail. This complicates scripts since they\n> have to either buffer the diff and check\n> its length, or run diff again with \"exit-code\",\n> essentially doing the diff twice.\n>\n> Add the \"--allow-empty\" flag to \"git apply\"\n> which allows it to handle both empty diffs\n> and empty commits created by \"git format-patch\n> --always\" by doing nothing and returning 0.\n>\n> Add tests for both with and without --allow-empty.\n>\n> Signed-off-by: Jerry Zhang \n> ---\n> This patch applies on top of \"git-apply: add --quiet flag\".\n> The conflict is in Documentation -> Synopsis and is\n> trivial to solve.\n>\n> V1 -> V2:\n> - Moved behavior under a flag\n> - Added tests for both cases\n>\n> Documentation/git-apply.txt | 6 +++++-\n> apply.c | 8 ++++++--\n> apply.h | 1 +\n> t/t4126-apply-empty.sh | 16 ++++++++++++++++\n> 4 files changed, 28 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n> index a32ad64718..b6d77f4206 100644\n> --- a/Documentation/git-apply.txt\n> +++ b/Documentation/git-apply.txt\n> @@ -16,7 +16,7 @@ SYNOPSIS\n> \t [--ignore-space-change | --ignore-whitespace]\n> \t [--whitespace=(nowarn|warn|fix|error|error-all)]\n> \t [--exclude=] [--include=] [--directory=]\n> -\t [--verbose | --quiet] [--unsafe-paths] [...]\n> +\t [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n> \n> DESCRIPTION\n> -----------\n> @@ -256,6 +256,10 @@ When `git apply` is used as a \"better GNU patch\", the user can pass\n> the `--unsafe-paths` option to override this safety check. This option\n> has no effect when `--index` or `--cached` is in use.\n> \n> +--allow-empty::\n> +\tDon't return error for patches containing no diff. This includes\n> +\tempty patches and patches with commit text only.\n> +\n> CONFIGURATION\n> -------------\n> \n> diff --git a/apply.c b/apply.c\n> index 918e0988bb..0ddde5e8a8 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -4732,8 +4732,10 @@ static int apply_patch(struct apply_state *state,\n> \t}\n> \n> \tif (!list && !skipped_patch) {\n> -\t\terror(_(\"unrecognized input\"));\n> -\t\tres = -128;\n> +\t\tif (!state->allow_empty) {\n> +\t\t\terror(_(\"unrecognized input\"));\n> +\t\t\tres = -128;\n> +\t\t}\n> \t\tgoto end;\n> \t}\n> \n> @@ -5061,6 +5063,8 @@ int apply_parse_options(int argc, const char **argv,\n> \t\tOPT_CALLBACK(0, \"directory\", state, N_(\"root\"),\n> \t\t\tN_(\"prepend to all filenames\"),\n> \t\t\tapply_option_parse_directory),\n> +\t\tOPT_BOOL(0, \"allow-empty\", &state->allow_empty,\n> +\t\t\tN_(\"don't return error for empty patches\")),\n> \t\tOPT_END()\n> \t};\n> \n> diff --git a/apply.h b/apply.h\n> index da3d95fa50..16202da160 100644\n> --- a/apply.h\n> +++ b/apply.h\n> @@ -66,6 +66,7 @@ struct apply_state {\n> \tint threeway;\n> \tint unidiff_zero;\n> \tint unsafe_paths;\n> +\tint allow_empty;\n> \n> \t/* Other non boolean parameters */\n> \tstruct repository *repo;\n> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\n> index ceb6a79fe0..f89c53c5f6 100755\n> --- a/t/t4126-apply-empty.sh\n> +++ b/t/t4126-apply-empty.sh\n> @@ -31,6 +31,22 @@ test_expect_success 'apply empty' '\n> \ttest_cmp expect empty\n> '\n> \n> +test_expect_success 'apply empty patch fails' '\n> +\tgit reset --hard &&\n> +\tgit commit --allow-empty -m \"empty commit\" &&\n> +\tgit format-patch --always HEAD~ >empty.patch &&\n> +\ttest_must_fail git apply empty.patch &&\n> +\ttest_must_fail git apply - +'\n> +\n> +test_expect_success 'apply with --allow-empty succeeds' '\n> +\tgit reset --hard &&\n> +\tgit commit --allow-empty -m \"empty commit\" &&\n\nRather than making two --allow-empty commits, shouldn't this be added to\nthe earlier \"setup\" routine, so both of these tests can use the same\nempty commit?\n\nThe setup routine sets up a patch0 and patch1, just add an empty-patch ?\n\nAnd for the \"git reset --hard\" I see you're copy/pasting the existing\nconvention in that file, but FWIW it's better to use 'test_when_finished\n\"git reset --hard\"' in the appropriate spot in the test (i.e. just\nbefore we'd need the reset if we fail), so a test cleans up after\nitself. As opposed to \"I'm cleaning in case an earlier test didn't clean\nup\".\n\n> +\tgit format-patch --always HEAD~ >empty.patch &&\n> +\tgit apply --allow-empty empty.patch &&\n> +\tgit apply --allow-empty - +'\n> +\n> test_expect_success 'apply --index empty' '\n> \tgit reset --hard &&\n> \trm -f missing &&\n\n"},{"id":"443862","messageId":"20211211031343.15818-1-jerry@skydio.com","threadId":"55565","inReplyTo":"20210427194049.14399-1-jerry@skydio.com","subject":"[PATCH V3] git-apply: add --allow-empty flag","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-12-11T03:13:43Z","receivedAt":"2021-12-11T03:14:10Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"Some users or scripts will pipe \"git diff\"\noutput to \"git apply\" when replaying diffs\nor commits. In these cases, they will rely\non the return value of \"git apply\" to know\nwhether the diff was applied successfully.\n\nHowever, for empty commits, \"git apply\" will\nfail. This complicates scripts since they\nhave to either buffer the diff and check\nits length, or run diff again with \"exit-code\",\nessentially doing the diff twice.\n\nAdd the \"--allow-empty\" flag to \"git apply\"\nwhich allows it to handle both empty diffs\nand empty commits created by \"git format-patch\n--always\" by doing nothing and returning 0.\n\nAdd tests for both with and without --allow-empty.\n\nSigned-off-by: Jerry Zhang \n---\nV2->V3:\n- moved lines into setup routine\n- switched all tests to using \"test_when_finished\" rather than resetting at the end\n\n Documentation/git-apply.txt | 6 +++++-\n apply.c | 8 ++++++--\n apply.h | 1 +\n t/t4126-apply-empty.sh | 22 ++++++++++++++++++----\n 4 files changed, 30 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex a32ad64718..b6d77f4206 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -14,11 +14,11 @@ SYNOPSIS\n \t [--allow-binary-replacement | --binary] [--reject] [-z]\n \t [-p] [-C] [--inaccurate-eof] [--recount] [--cached]\n \t [--ignore-space-change | --ignore-whitespace]\n \t [--whitespace=(nowarn|warn|fix|error|error-all)]\n \t [--exclude=] [--include=] [--directory=]\n-\t [--verbose | --quiet] [--unsafe-paths] [...]\n+\t [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n \n DESCRIPTION\n -----------\n Reads the supplied diff output (i.e. \"a patch\") and applies it to files.\n When running from a subdirectory in a repository, patched paths\n@@ -254,10 +254,14 @@ running `git apply --directory=modules/git-gui`.\n +\n When `git apply` is used as a \"better GNU patch\", the user can pass\n the `--unsafe-paths` option to override this safety check. This option\n has no effect when `--index` or `--cached` is in use.\n \n+--allow-empty::\n+\tDon't return error for patches containing no diff. This includes\n+\tempty patches and patches with commit text only.\n+\n CONFIGURATION\n -------------\n \n apply.ignoreWhitespace::\n \tSet to 'change' if you want changes in whitespace to be ignored by default.\ndiff --git a/apply.c b/apply.c\nindex 9f00f882a2..f6029fd935 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4752,12 +4752,14 @@ static int apply_patch(struct apply_state *state,\n \t\t}\n \t\toffset += nr;\n \t}\n \n \tif (!list && !skipped_patch) {\n-\t\terror(_(\"unrecognized input\"));\n-\t\tres = -128;\n+\t\tif (!state->allow_empty) {\n+\t\t\terror(_(\"unrecognized input\"));\n+\t\t\tres = -128;\n+\t\t}\n \t\tgoto end;\n \t}\n \n \tif (state->whitespace_error && (state->ws_error_action == die_on_ws_error))\n \t\tstate->apply = 0;\n@@ -5081,10 +5083,12 @@ int apply_parse_options(int argc, const char **argv,\n \t\t\tN_(\"do not trust the line counts in the hunk headers\"),\n \t\t\tAPPLY_OPT_RECOUNT),\n \t\tOPT_CALLBACK(0, \"directory\", state, N_(\"root\"),\n \t\t\tN_(\"prepend to all filenames\"),\n \t\t\tapply_option_parse_directory),\n+\t\tOPT_BOOL(0, \"allow-empty\", &state->allow_empty,\n+\t\t\tN_(\"don't return error for empty patches\")),\n \t\tOPT_END()\n \t};\n \n \treturn parse_options(argc, argv, state->prefix, builtin_apply_options, apply_usage, 0);\n }\ndiff --git a/apply.h b/apply.h\nindex da3d95fa50..16202da160 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -64,10 +64,11 @@ struct apply_state {\n \tint apply_with_reject;\n \tint no_add;\n \tint threeway;\n \tint unidiff_zero;\n \tint unsafe_paths;\n+\tint allow_empty;\n \n \t/* Other non boolean parameters */\n \tstruct repository *repo;\n \tconst char *index_file;\n \tenum apply_verbosity apply_verbosity;\ndiff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\nindex ceb6a79fe0..949e284d14 100755\n--- a/t/t4126-apply-empty.sh\n+++ b/t/t4126-apply-empty.sh\n@@ -7,10 +7,12 @@ test_description='apply empty'\n test_expect_success setup '\n \t>empty &&\n \tgit add empty &&\n \ttest_tick &&\n \tgit commit -m initial &&\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit format-patch --always HEAD~ >empty.patch &&\n \tfor i in a b c d e\n \tdo\n \t\techo $i\n \tdone >empty &&\n \tcat empty >expect &&\n@@ -23,34 +25,46 @@ test_expect_success setup '\n \t>empty &&\n \tgit update-index --refresh\n '\n \n test_expect_success 'apply empty' '\n-\tgit reset --hard &&\n \trm -f missing &&\n+\ttest_when_finished \"git reset --hard\" &&\n \tgit apply patch0 &&\n \ttest_cmp expect empty\n '\n \n+test_expect_success 'apply empty patch fails' '\n+\ttest_when_finished \"git reset --hard\" &&\n+\ttest_must_fail git apply empty.patch &&\n+\ttest_must_fail git apply - +--allow-empty::\n> +\tDon't return error for patches containing no diff. This includes\n> +\tempty patches and patches with commit text only.\n> +\n\nLooks OK.\n\n> \tif (!list && !skipped_patch) {\n> -\t\terror(_(\"unrecognized input\"));\n> -\t\tres = -128;\n> +\t\tif (!state->allow_empty) {\n> +\t\t\terror(_(\"unrecognized input\"));\n> +\t\t\tres = -128;\n> +\t\t}\n> \t\tgoto end;\n> \t}\n\nWhen we apply empty patch but we forget --allow-empty, we got generic\n\"unrecognized input\". So for this case, we should disambiguate the\nformer case and case when malformed patch input is provided.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"444028","messageId":"20211213202826.7051-1-jerry@skydio.com","threadId":"55565","inReplyTo":"20211211031343.15818-1-jerry@skydio.com","subject":"[PATCH V4] git-apply: add --allow-empty flag","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-12-13T20:28:26Z","receivedAt":"2021-12-13T20:28:36Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"Some users or scripts will pipe \"git diff\"\noutput to \"git apply\" when replaying diffs\nor commits. In these cases, they will rely\non the return value of \"git apply\" to know\nwhether the diff was applied successfully.\n\nHowever, for empty commits, \"git apply\" will\nfail. This complicates scripts since they\nhave to either buffer the diff and check\nits length, or run diff again with \"exit-code\",\nessentially doing the diff twice.\n\nAdd the \"--allow-empty\" flag to \"git apply\"\nwhich allows it to handle both empty diffs\nand empty commits created by \"git format-patch\n--always\" by doing nothing and returning 0.\n\nAdd tests for both with and without --allow-empty.\n\nSigned-off-by: Jerry Zhang \n---\nV3->V4:\n- Add detail to error text, and suggest --allow-empty\nin case user runs into the empty patch error.\n\n Documentation/git-apply.txt | 6 +++++-\n apply.c | 8 ++++++--\n apply.h | 1 +\n t/t4126-apply-empty.sh | 22 ++++++++++++++++++----\n 4 files changed, 30 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex a32ad64718..b6d77f4206 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -14,11 +14,11 @@ SYNOPSIS\n \t [--allow-binary-replacement | --binary] [--reject] [-z]\n \t [-p] [-C] [--inaccurate-eof] [--recount] [--cached]\n \t [--ignore-space-change | --ignore-whitespace]\n \t [--whitespace=(nowarn|warn|fix|error|error-all)]\n \t [--exclude=] [--include=] [--directory=]\n-\t [--verbose | --quiet] [--unsafe-paths] [...]\n+\t [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n \n DESCRIPTION\n -----------\n Reads the supplied diff output (i.e. \"a patch\") and applies it to files.\n When running from a subdirectory in a repository, patched paths\n@@ -254,10 +254,14 @@ running `git apply --directory=modules/git-gui`.\n +\n When `git apply` is used as a \"better GNU patch\", the user can pass\n the `--unsafe-paths` option to override this safety check. This option\n has no effect when `--index` or `--cached` is in use.\n \n+--allow-empty::\n+\tDon't return error for patches containing no diff. This includes\n+\tempty patches and patches with commit text only.\n+\n CONFIGURATION\n -------------\n \n apply.ignoreWhitespace::\n \tSet to 'change' if you want changes in whitespace to be ignored by default.\ndiff --git a/apply.c b/apply.c\nindex 9f00f882a2..afc1c6510e 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4752,12 +4752,14 @@ static int apply_patch(struct apply_state *state,\n \t\t}\n \t\toffset += nr;\n \t}\n \n \tif (!list && !skipped_patch) {\n-\t\terror(_(\"unrecognized input\"));\n-\t\tres = -128;\n+\t\tif (!state->allow_empty) {\n+\t\t\terror(_(\"No valid patches in input (allow with \\\"--allow-empty\\\")\"));\n+\t\t\tres = -128;\n+\t\t}\n \t\tgoto end;\n \t}\n \n \tif (state->whitespace_error && (state->ws_error_action == die_on_ws_error))\n \t\tstate->apply = 0;\n@@ -5081,10 +5083,12 @@ int apply_parse_options(int argc, const char **argv,\n \t\t\tN_(\"do not trust the line counts in the hunk headers\"),\n \t\t\tAPPLY_OPT_RECOUNT),\n \t\tOPT_CALLBACK(0, \"directory\", state, N_(\"root\"),\n \t\t\tN_(\"prepend to all filenames\"),\n \t\t\tapply_option_parse_directory),\n+\t\tOPT_BOOL(0, \"allow-empty\", &state->allow_empty,\n+\t\t\tN_(\"don't return error for empty patches\")),\n \t\tOPT_END()\n \t};\n \n \treturn parse_options(argc, argv, state->prefix, builtin_apply_options, apply_usage, 0);\n }\ndiff --git a/apply.h b/apply.h\nindex da3d95fa50..16202da160 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -64,10 +64,11 @@ struct apply_state {\n \tint apply_with_reject;\n \tint no_add;\n \tint threeway;\n \tint unidiff_zero;\n \tint unsafe_paths;\n+\tint allow_empty;\n \n \t/* Other non boolean parameters */\n \tstruct repository *repo;\n \tconst char *index_file;\n \tenum apply_verbosity apply_verbosity;\ndiff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh\nindex ceb6a79fe0..949e284d14 100755\n--- a/t/t4126-apply-empty.sh\n+++ b/t/t4126-apply-empty.sh\n@@ -7,10 +7,12 @@ test_description='apply empty'\n test_expect_success setup '\n \t>empty &&\n \tgit add empty &&\n \ttest_tick &&\n \tgit commit -m initial &&\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit format-patch --always HEAD~ >empty.patch &&\n \tfor i in a b c d e\n \tdo\n \t\techo $i\n \tdone >empty &&\n \tcat empty >expect &&\n@@ -23,34 +25,46 @@ test_expect_success setup '\n \t>empty &&\n \tgit update-index --refresh\n '\n \n test_expect_success 'apply empty' '\n-\tgit reset --hard &&\n \trm -f missing &&\n+\ttest_when_finished \"git reset --hard\" &&\n \tgit apply patch0 &&\n \ttest_cmp expect empty\n '\n \n+test_expect_success 'apply empty patch fails' '\n+\ttest_when_finished \"git reset --hard\" &&\n+\ttest_must_fail git apply empty.patch &&\n+\ttest_must_fail git apply - writes:\n\n> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n> index a32ad64718..b6d77f4206 100644\n> --- a/Documentation/git-apply.txt\n> +++ b/Documentation/git-apply.txt\n> @@ -14,11 +14,11 @@ SYNOPSIS\n> \t [--allow-binary-replacement | --binary] [--reject] [-z]\n> \t [-p] [-C] [--inaccurate-eof] [--recount] [--cached]\n> \t [--ignore-space-change | --ignore-whitespace]\n> \t [--whitespace=(nowarn|warn|fix|error|error-all)]\n> \t [--exclude=] [--include=] [--directory=]\n> -\t [--verbose | --quiet] [--unsafe-paths] [...]\n> +\t [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n> \n> DESCRIPTION\n> -----------\n> Reads the supplied diff output (i.e. \"a patch\") and applies it to files.\n> When running from a subdirectory in a repository, patched paths\n\nApplying: git-apply: add --allow-empty flag\nerror: patch failed: Documentation/git-apply.txt:14\nerror: Documentation/git-apply.txt: patch does not apply\nPatch failed at 0001 git-apply: add --allow-empty flag\n\nHmph.... Where did that \"| --quiet\" thing come from?\n\n\n"},{"id":"444034","messageId":"CAMKO5Ctq8kzAvHAXxPVd06_Ko7-GW4CQbUmw7g7Eb7Su_Cp8RQ@mail.gmail.com","threadId":"55565","inReplyTo":"xmqqlf0ob2o0.fsf@gitster.g","subject":"Re: [PATCH V4] git-apply: add --allow-empty flag","fromName":"Jerry Zhang","fromEmail":"jerry@skydio.com","sentAt":"2021-12-13T21:52:42Z","receivedAt":"2021-12-13T21:52:59Z","isPatch":true,"sender":{"key":"jerry@skydio.com","avatar":"https://avatars.githubusercontent.com/u/81337184?v=4"},"body":"On Mon, Dec 13, 2021 at 1:44 PM Junio C Hamano wrote:\n>\n> Jerry Zhang writes:\n>\n> > diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n> > index a32ad64718..b6d77f4206 100644\n> > --- a/Documentation/git-apply.txt\n> > +++ b/Documentation/git-apply.txt\n> > @@ -14,11 +14,11 @@ SYNOPSIS\n> > [--allow-binary-replacement | --binary] [--reject] [-z]\n> > [-p] [-C] [--inaccurate-eof] [--recount] [--cached]\n> > [--ignore-space-change | --ignore-whitespace]\n> > [--whitespace=(nowarn|warn|fix|error|error-all)]\n> > [--exclude=] [--include=] [--directory=]\n> > - [--verbose | --quiet] [--unsafe-paths] [...]\n> > + [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n> >\n> > DESCRIPTION\n> > -----------\n> > Reads the supplied diff output (i.e. \"a patch\") and applies it to files.\n> > When running from a subdirectory in a repository, patched paths\n>\n> Applying: git-apply: add --allow-empty flag\n> error: patch failed: Documentation/git-apply.txt:14\n> error: Documentation/git-apply.txt: patch does not apply\n> Patch failed at 0001 git-apply: add --allow-empty flag\n>\n> Hmph.... Where did that \"| --quiet\" thing come from?\n>\n>\nApologies, I overlooked a dependence on my other patch here:\nhttps://patchwork.kernel.org/project/git/patch/20210427194106.14500-1-jerry@skydio.com/\n\nLet me know if you'd like a clean patch off of master. Please give the\nother patch\na look though, as I've addressed outstanding comments.\n"},{"id":"444036","messageId":"xmqq5yrsb22e.fsf@gitster.g","threadId":"55565","inReplyTo":"CAMKO5Ctq8kzAvHAXxPVd06_Ko7-GW4CQbUmw7g7Eb7Su_Cp8RQ@mail.gmail.com","subject":"Re: [PATCH V4] git-apply: add --allow-empty flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-13T21:57:29Z","receivedAt":"2021-12-13T21:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jerry Zhang writes:\n\n> On Mon, Dec 13, 2021 at 1:44 PM Junio C Hamano wrote:\n>>\n>> Jerry Zhang writes:\n>>\n>> > diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\n>> > index a32ad64718..b6d77f4206 100644\n>> > --- a/Documentation/git-apply.txt\n>> > +++ b/Documentation/git-apply.txt\n>> > @@ -14,11 +14,11 @@ SYNOPSIS\n>> > [--allow-binary-replacement | --binary] [--reject] [-z]\n>> > [-p] [-C] [--inaccurate-eof] [--recount] [--cached]\n>> > [--ignore-space-change | --ignore-whitespace]\n>> > [--whitespace=(nowarn|warn|fix|error|error-all)]\n>> > [--exclude=] [--include=] [--directory=]\n>> > - [--verbose | --quiet] [--unsafe-paths] [...]\n>> > + [--verbose | --quiet] [--unsafe-paths] [--allow-empty] [...]\n>> >\n>> > DESCRIPTION\n>> > -----------\n>> > Reads the supplied diff output (i.e. \"a patch\") and applies it to files.\n>> > When running from a subdirectory in a repository, patched paths\n>>\n>> Applying: git-apply: add --allow-empty flag\n>> error: patch failed: Documentation/git-apply.txt:14\n>> error: Documentation/git-apply.txt: patch does not apply\n>> Patch failed at 0001 git-apply: add --allow-empty flag\n>>\n>> Hmph.... Where did that \"| --quiet\" thing come from?\n>>\n>>\n> Apologies, I overlooked a dependence on my other patch here:\n> https://patchwork.kernel.org/project/git/patch/20210427194106.14500-1-jerry@skydio.com/\n>\n> Let me know if you'd like a clean patch off of master. Please give the\n> other patch\n> a look though, as I've addressed outstanding comments.\n\nI am OK with either a single patch without the other one at all, or\nN-patch series if they all have to go in together.\n\nThanks.\n\n"}]}