Re: [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2018, 18:50 UTC
Message-ID: <xmqq7er74ht2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180219112910.24471-10-phillip.wood@talktalk.net>

Phillip Wood <phillip.wood@talktalk.net> writes:

Show 12 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Now that add -p counts patches properly it should be possible to turn
> off the '--recount' option when invoking 'git apply'
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>
> Notes:
>     I can't think of a reason why this shouldn't be OK but I can't help
>     feeling slightly nervous about it. I've made it a separate patch so it
>     can be easily dropped or reverted if I've missed something.

It is a lot more preferrable approach to produce a patch with information as precise as the producer (i.e. add -p) can and feed it to the consumer (i.e. apply) than to rely on the --recount and the --allow-overlap options that are mainly ways to force the consumer make imprecise guesses and accepting potential garbage with looser consistency checks. So I very much like the direction (and the next step may be to make sure we coalesce correctly so that we do not have to use "--allow-overlap").

Thanks for working on this. Let's see if the updated code really "counts patches properly" as the log message claims to ;-)

Show 16 quoted lines

>  git-add--interactive.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 3226c2c4f02d5f8679d77b8eede984fc727b422d..a64c0db57d62ab02ef718b8c8f821105132d9920 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -677,7 +677,7 @@ sub add_untracked_cmd {
>  sub run_git_apply {
>  	my $cmd = shift;
>  	my $fh;
> -	open $fh, '| git ' . $cmd . " --recount --allow-overlap";
> +	open $fh, '| git ' . $cmd . " --allow-overlap";
>  	print $fh @_;
>  	return close $fh;
>  }