Re: [PATCH 1/2] am -3: allow nonstandard -p<num> option

From: Jeff King <peff@peff.net>
Date: Feb 29, 2012, 02:58 UTC
Message-ID: <20120229025842.GA3585@sigill.intra.peff.net>
In-Reply-To: <1330471495-12013-2-git-send-email-gitster@pobox.com>

On Tue, Feb 28, 2012 at 03:24:54PM -0800, Junio C Hamano wrote:

Show 5 quoted lines

> When falling back to 3-way merge, we run "git apply" to synthesize the
> fake ancestor tree by parsing the incoming patch, and another "git apply"
> to apply the patch to the fake ancestor tree.  Both invocation need to
> be aware of the custom -p<num> setting to parse patches that were prepared
> with non-standard src/dst prefix.

Makes sense. One question:

Show 13 quoted lines

> diff --git a/git-am.sh b/git-am.sh
> index 1c13b13..d5d168f 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -127,15 +127,18 @@ fall_back_3way () {
>      mkdir "$dotest/patch-merge-tmp-dir"
>  
>      # First see if the patch records the index info that we can use.
> -    git apply --build-fake-ancestor "$dotest/patch-merge-tmp-index" \
> -	"$dotest/patch" &&
> +    cmd="git apply $git_apply_opt --build-fake-ancestor" &&
> +    cmd="$cmd "'"$dotest/patch-merge-tmp-index" "$dotest/patch"' &&
> +    eval "$cmd" &&

$git_apply_opt can have other stuff in it, too (from my cursory reading, it looks like --whitespace, --directory, --exclude, -C, --reject, --ignore-whitespace, and --ignore-space-change). Those options are now passed, too.

Naively, I don't think it should be a problem. Many of them will do nothing (because the patch _should_ apply cleanly to the blobs it mentions). Some seem like an obvious improvement (e.g., "--directory" should be just as necessary as "-p", I would think). For something like "--whitespace=error", I would think we would have errored out already when we first tried to apply the patch. Or maybe not. I didn't test.

-Peff