Re: [PATCH] git-apply: allow empty patch text

From: Eric Wong <e@80x24.org>
Date: Apr 27, 2021, 05:46 UTC
Message-ID: <20210427054632.GA31347@dcvr>
In-Reply-To: <20210427011246.28054-1-jerry@skydio.com>

Jerry Zhang <jerry@skydio.com> wrote:

Show 8 quoted lines

> "git diff" produces no patch text if
> there is no diff, but "git apply" exits
> with code 128 if the patch text is empty.
> 
> Since every valid "git diff" should
> result in a successful patch application
> when applied to the same preimage as
> the diff,

Should it result in successful patch application? (Why?)

I fear this change can cause errors in pipelines to go undetected (since "set -o pipefail" is not POSIX). In my experience, zero-byte files is also a common failure mode for some filesystems, even after fsck marked them as clean.

Perhaps guarding this behavior with --allow-empty (as commit and cherry-pick do) is safer.

Show 14 quoted lines

> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
> index ceb6a79fe0..37351be609 100755
> --- a/t/t4126-apply-empty.sh
> +++ b/t/t4126-apply-empty.sh
> @@ -31,6 +31,13 @@ test_expect_success 'apply empty' '
>  	test_cmp expect empty
>  '
>  
> +test_expect_success 'apply empty diff' '
> +	git reset --hard &&
> +	git diff >empty.patch &&
> +	git apply empty.patch &&
> +	git diff | git apply -
> +'

It shouldn't be necessary to use "git diff" to generate empty output. Tests are too slow for me on Linux, even. Something like:

	>empty.patch &&
	git apply empty.patch &&
	git apply - <empty.patch

...ought to be realistic enough, but /dev/null in place of empty.patch is probably portable enough, too. Thanks.