git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] git-quiltimport.sh: disallow fuzz
@ 2014-09-24 21:35 Jörn Engel
  2014-09-25  5:09 ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jörn Engel @ 2014-09-24 21:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

git-quiltimport passed "-C1" to git-apply, supposedly to roughly match
the quilt default of --fuzz 2.  This is against the spirit of git.
Quoting Linus:
  Except unlike the standard "patch" program, "git apply" doesn't accept
  fuzz by default (which to me is a huge deal - I hate how "patch" tries
  to apply stuff that clearly isn't valid any more)

It also causes active harm when combining git-quiltimport with regular
quilt and quilt is used with --fuzz=0, as it should be by any prudent 
person.

Signed-off-by: Joern Engel <joern@logfs.org>
---
 git-quiltimport.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 167d79fea809..3eb2e2fd3648 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -130,7 +130,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
+		git apply --index ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-24 21:35 [PATCH] git-quiltimport.sh: disallow fuzz Jörn Engel
@ 2014-09-25  5:09 ` Junio C Hamano
  2014-09-25 22:08   ` Jörn Engel
  0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2014-09-25  5:09 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Jörn Engel <joern@logfs.org> writes:

> git-quiltimport passed "-C1" to git-apply, supposedly to roughly match
> the quilt default of --fuzz 2.  This is against the spirit of git.
> Quoting Linus:
>   Except unlike the standard "patch" program, "git apply" doesn't accept
>   fuzz by default (which to me is a huge deal - I hate how "patch" tries
>   to apply stuff that clearly isn't valid any more)
>
> It also causes active harm when combining git-quiltimport with regular
> quilt and quilt is used with --fuzz=0,...

This is fine for those who use quilt with --fuzz=0, but how are you
helping those who use quilt without --fuzz=0?

I agree that unconditionally passing -C1 is a bad thing, but
unconditionally passing -C2 is not that better.  Shouldn't this be
done by introducing a new --fuzz=<number> option to quiltimport?


>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
>  git-quiltimport.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 167d79fea809..3eb2e2fd3648 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -130,7 +130,7 @@ do
>  	fi
>  
>  	if [ -z "$dry_run" ] ; then
> -		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
> +		git apply --index ${level:+"$level"} "$tmp_patch" &&
>  		tree=$(git write-tree) &&
>  		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
>  		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-25  5:09 ` Junio C Hamano
@ 2014-09-25 22:08   ` Jörn Engel
  2014-09-25 22:48     ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jörn Engel @ 2014-09-25 22:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Wed, 24 September 2014 22:09:33 -0700, Junio C Hamano wrote:
> 
> This is fine for those who use quilt with --fuzz=0, but how are you
> helping those who use quilt without --fuzz=0?
> 
> I agree that unconditionally passing -C1 is a bad thing, but
> unconditionally passing -C2 is not that better.  Shouldn't this be
> done by introducing a new --fuzz=<number> option to quiltimport?

Maybe the patch below then?  Defaulting to no fuzz is both me
enforcing my (and Linus') preference and that alternatives are hard
and messy.  How would one specify fuzz=0?  -C3 would work for standard
patches, but be wrong for patches with more context.  -C0 would
arguably also be wrong.  There really is no good choice.  And I won't
add a fuzz parameter until git-apply has one, as it can only be
transformed to -C by either making assumptions about the context or
parsing the patches - ick!

Jörn

--
When I am working on a problem I never think about beauty.  I think
only how to solve the problem.  But when I have finished, if the
solution is not beautiful, I know it is wrong.
-- R. Buckminster Fuller

Subject: [PATCH] git-quiltimport.sh: disallow fuzz by default

git-quiltimport passed "-C1" to git-apply, supposedly to roughly match
the quilt default of --fuzz 2.  This is against the spirit of git.
Quoting Linus:
  Except unlike the standard "patch" program, "git apply" doesn't accept
  fuzz by default (which to me is a huge deal - I hate how "patch" tries
  to apply stuff that clearly isn't valid any more)

Users that want to want to emulate quilt defaults can pass "-C 1" to
git-quiltimport now.

Also note that -C1 and fuzz=2 is not identical.  Most patches have three
lines of context, so fuzz=2 leaves one relevant line of context.  But
for any patches with more or less context this is not true.  git-apply
has no option for fuzz, so any emulation will always be best-effort.

Signed-off-by: Joern Engel <joern@logfs.org>
---
 git-quiltimport.sh | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 167d79fea809..f45ee5ff6599 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -4,8 +4,9 @@ OPTIONS_STUCKLONG=
 OPTIONS_SPEC="\
 git quiltimport [options]
 --
-n,dry-run     dry run
 author=       author name and email address for patches without any
+C=            minimum context (see git apply)
+n,dry-run     dry run
 patches=      path to the quilt series and patches
 "
 SUBDIRECTORY_ON=Yes
@@ -13,6 +14,7 @@ SUBDIRECTORY_ON=Yes
 
 dry_run=""
 quilt_author=""
+cflag=""
 while test $# != 0
 do
 	case "$1" in
@@ -20,6 +22,15 @@ do
 		shift
 		quilt_author="$1"
 		;;
+	-C)
+		shift
+		# ensure numerical parameter
+		case $1 in
+			''|*[!0-9]*) usage;;
+			*) ;;
+		esac
+		cflag="-C$1"
+		;;
 	-n|--dry-run)
 		dry_run=1
 		;;
@@ -130,7 +141,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
+		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-25 22:08   ` Jörn Engel
@ 2014-09-25 22:48     ` Junio C Hamano
  2014-09-25 22:59       ` Junio C Hamano
                         ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Junio C Hamano @ 2014-09-25 22:48 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Jörn Engel <joern@logfs.org> writes:

> When I am working on a problem I never think about beauty.  I think
> only how to solve the problem.  But when I have finished, if the
> solution is not beautiful, I know it is wrong.
> -- R. Buckminster Fuller

Nice quote, but does not belong to above --- lines, unless you have
a -- >8 -- line below or something.

Now is the patch beautiful? ;-)

> Subject: [PATCH] git-quiltimport.sh: disallow fuzz by default
>
> git-quiltimport passed "-C1" to git-apply, supposedly to roughly match
> the quilt default of --fuzz 2.  This is against the spirit of git.
> Quoting Linus:
>   Except unlike the standard "patch" program, "git apply" doesn't accept
>   fuzz by default (which to me is a huge deal - I hate how "patch" tries
>   to apply stuff that clearly isn't valid any more)
> Users that want to want to emulate quilt defaults can pass "-C 1" to
> git-quiltimport now.

Things might have been different if this were mid 2006 or early
2007, but I am afraid that "the spirit of git" with a quote from
Linus no longer carries much weight on this particular issue.  A
backward incompatible change is backward incompatible change that
breaks existing users no matter how loudly you (and I) shout that it
is the right thing to do in the long run.

Let's have it the other way around, keep the same behaviour for
those who run the command without the new option, while allowing
people who know better and are aligned with the spirit of git to
pass the parameter, at least for now, with a note in the
documentation to warn that the default may change in the future to
allow no fuzz, or something.

> Also note that -C1 and fuzz=2 is not identical.  Most patches have three
> lines of context, so fuzz=2 leaves one relevant line of context.  But
> for any patches with more or less context this is not true.  git-apply
> has no option for fuzz, so any emulation will always be best-effort.
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
>  git-quiltimport.sh | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 167d79fea809..f45ee5ff6599 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -4,8 +4,9 @@ OPTIONS_STUCKLONG=
>  OPTIONS_SPEC="\
>  git quiltimport [options]
>  --
> -n,dry-run     dry run
>  author=       author name and email address for patches without any
> +C=            minimum context (see git apply)
> +n,dry-run     dry run
>  patches=      path to the quilt series and patches
>  "
>  SUBDIRECTORY_ON=Yes
> @@ -13,6 +14,7 @@ SUBDIRECTORY_ON=Yes
>  
>  dry_run=""
>  quilt_author=""
> +cflag=""
>  while test $# != 0
>  do
>  	case "$1" in
> @@ -20,6 +22,15 @@ do
>  		shift
>  		quilt_author="$1"
>  		;;
> +	-C)
> +		shift
> +		# ensure numerical parameter
> +		case $1 in
> +			''|*[!0-9]*) usage;;
> +			*) ;;
> +		esac
> +		cflag="-C$1"
> +		;;
>  	-n|--dry-run)
>  		dry_run=1
>  		;;
> @@ -130,7 +141,7 @@ do
>  	fi
>  
>  	if [ -z "$dry_run" ] ; then
> -		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
> +		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
>  		tree=$(git write-tree) &&
>  		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
>  		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-25 22:48     ` Junio C Hamano
@ 2014-09-25 22:59       ` Junio C Hamano
  2014-09-26 21:02         ` Junio C Hamano
  2014-10-21 21:32       ` Junio C Hamano
  2014-10-21 21:38       ` [PATCH 2/2] git-quiltimport: flip the default not to allow fuzz Junio C Hamano
  2 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2014-09-25 22:59 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Junio C Hamano <gitster@pobox.com> writes:

> Let's have it the other way around, keep the same behaviour for
> those who run the command without the new option, while allowing
> people who know better and are aligned with the spirit of git to
> pass the parameter, at least for now, with a note in the
> documentation to warn that the default may change in the future to
> allow no fuzz, or something.

Perhaps like this, with some documentation added (do we have/need
any test???).

-- >8 --
To: Jörn Engel <joern@logfs.org>
Date: Thu, 25 Sep 2014 18:08:31 -0400
Subject: [PATCH] git-quiltimport.sh: allow declining fuzz with --exact option

git-quiltimport unconditionally passes "-C1" to "git apply",
supposedly to roughly match the quilt default of --fuzz 2.  Allow
users to pass --exact option to disable it, requiring the patch to
apply without any fuzz.

Also note that -C1 and fuzz=2 is not identical.  Most patches have
three lines of context, so fuzz=2 leaves one relevant line of
context.  But for any patches with more or less context this is not
true.  git-apply has no option for fuzz, so any emulation will
always be best-effort.

Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-quiltimport.txt | 12 ++++++++++++
 git-quiltimport.sh                | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
index a356196..109918d 100644
--- a/Documentation/git-quiltimport.txt
+++ b/Documentation/git-quiltimport.txt
@@ -49,6 +49,18 @@ The default for the patch directory is patches
 or the value of the $QUILT_PATCHES environment
 variable.
 
+-C <number>::
+	Pass `-C<number>` to underlying `git apply` when applying
+	the patch, to reduce number of context lines to be matched.
+	By default, `-C1` is passed to `git apply` to emulate the
+	`--fuzz=2` behaviour of quilt (assuming the standard 3
+	context lines).
+
+--exact::
+	Do not pass any `-C<number>` option to `git apply` when
+	applying the patch, to require context lines to fully match.
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 167d79f..2d2c377 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -6,6 +6,8 @@ git quiltimport [options]
 --
 n,dry-run     dry run
 author=       author name and email address for patches without any
+C=            minimum context (see git apply)
+exact         allow no-fuzz
 patches=      path to the quilt series and patches
 "
 SUBDIRECTORY_ON=Yes
@@ -13,6 +15,7 @@ SUBDIRECTORY_ON=Yes
 
 dry_run=""
 quilt_author=""
+cflag=-C1
 while test $# != 0
 do
 	case "$1" in
@@ -20,6 +23,18 @@ do
 		shift
 		quilt_author="$1"
 		;;
+	-C)
+		shift
+		# ensure numerical parameter
+		case "$1" in
+		''|*[!0-9]*) usage;;
+		*) ;;
+		esac
+		cflag="-C$1"
+		;;
+	--exact)
+		cflag=
+		;;
 	-n|--dry-run)
 		dry_run=1
 		;;
@@ -130,7 +145,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
+		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.1-394-g5293c25

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-25 22:59       ` Junio C Hamano
@ 2014-09-26 21:02         ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2014-09-26 21:02 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Junio C Hamano <gitster@pobox.com> writes:

> Perhaps like this, with some documentation added (do we have/need
> any test???).
>
> -- >8 --
> Date: Thu, 25 Sep 2014 18:08:31 -0400
> Subject: [PATCH] git-quiltimport.sh: allow declining fuzz with --exact option

And on top of that change, if somebody really wants to enforce
stricter check by default, I think we could do something like this.

Obviously not tested at all, as I do not care too deeply myself ;-)

-- >8 --
Subject: [PATCH] git-quiltimport: flip the default not to allow fuzz

Trying to be as strict as possible when applying the patch may be a
good discipline, so let's flip the default but we can be helpful to
those who do rely on the original behaviour thanks to the previous
change to add -C$n option.

Suggest using -C1 when:

 - "git apply" without fuzz fails to apply; and
 - the user did not specify a -C$n or --exact option; and
 - "git apply -C1" (old behaviour) would have succeeded.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-quiltimport.sh | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 929365f..01de26d 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -15,7 +15,8 @@ SUBDIRECTORY_ON=Yes
 
 dry_run=""
 quilt_author=""
-cflag=-C1
+cflag=
+fuzz_specified=
 while test $# != 0
 do
 	case "$1" in
@@ -31,9 +32,11 @@ do
 		*) ;;
 		esac
 		cflag="-C$1"
+		fuzz_specified=yes
 		;;
 	--exact)
 		cflag=
+		fuzz_specified=yes
 		;;
 	-n|--dry-run)
 		dry_run=1
@@ -74,6 +77,25 @@ tmp_msg="$tmp_dir/msg"
 tmp_patch="$tmp_dir/patch"
 tmp_info="$tmp_dir/info"
 
+# Helper to warn about -C$n option
+do_apply () {
+	if git apply --index $cflag "$@"
+	then
+		return
+	fi
+	if test -z "$fuzz_specified" &&
+	   git apply --check --index -C1 "$@" >/dev/null 2>&1
+	then
+		cat >&2 <<-\EOM
+		'git quiltimport' by default no longer attempts to apply
+		patches with reduced context lines to allow fuzz; if you
+		want the old 'unsafe' behaviour, run the command with -C1
+		option.
+		EOM
+
+	fi
+	return 1
+}
 
 # Find the initial commit
 commit=$(git rev-parse HEAD)
@@ -145,7 +167,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
+		do_apply ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.1-394-g5293c25

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] git-quiltimport.sh: disallow fuzz
  2014-09-25 22:48     ` Junio C Hamano
  2014-09-25 22:59       ` Junio C Hamano
@ 2014-10-21 21:32       ` Junio C Hamano
  2014-10-21 21:38       ` [PATCH 2/2] git-quiltimport: flip the default not to allow fuzz Junio C Hamano
  2 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2014-10-21 21:32 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Junio C Hamano <gitster@pobox.com> writes:

> Things might have been different if this were mid 2006 or early
> 2007, but I am afraid that "the spirit of git" with a quote from
> Linus no longer carries much weight on this particular issue.  A
> backward incompatible change is backward incompatible change that
> breaks existing users no matter how loudly you (and I) shout that it
> is the right thing to do in the long run.
>
> Let's have it the other way around, keep the same behaviour for
> those who run the command without the new option, while allowing
> people who know better and are aligned with the spirit of git to
> pass the parameter, at least for now, with a note in the
> documentation to warn that the default may change in the future to
> allow no fuzz, or something.

I was waiting to hear an Ack or some comments and then forgot about
this topic.

I'll send two patches, one is essentially yours *but* does not
flip the default (i.e. those who want to be safe have to be explicit
about it), and then the other is to flip the default but more gently.

Here is the first one.  I'll send the "default flipping" as a reply
to this message.

-- >8 --
From: Joern Engel <joern@logfs.org>
Date: Thu, 25 Sep 2014 18:08:31 -0400
Subject: [PATCH 1/2] git-quiltimport.sh: allow declining fuzz with --exact option

git-quiltimport unconditionally passes "-C1" to "git apply",
supposedly to roughly match the quilt default of --fuzz 2.  Allow
users to pass --exact option to disable it, requiring the patch to
apply without any fuzz.

Also note that -C1 and fuzz=2 is not identical.  Most patches have
three lines of context, so fuzz=2 leaves one relevant line of
context.  But for any patches with more or less context this is not
true.  git-apply has no option for fuzz, so any emulation will
always be best-effort.

Signed-off-by: Joern Engel <joern@logfs.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-quiltimport.txt | 12 ++++++++++++
 git-quiltimport.sh                | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
index a356196..109918d 100644
--- a/Documentation/git-quiltimport.txt
+++ b/Documentation/git-quiltimport.txt
@@ -49,6 +49,18 @@ The default for the patch directory is patches
 or the value of the $QUILT_PATCHES environment
 variable.
 
+-C <number>::
+	Pass `-C<number>` to underlying `git apply` when applying
+	the patch, to reduce number of context lines to be matched.
+	By default, `-C1` is passed to `git apply` to emulate the
+	`--fuzz=2` behaviour of quilt (assuming the standard 3
+	context lines).
+
+--exact::
+	Do not pass any `-C<number>` option to `git apply` when
+	applying the patch, to require context lines to fully match.
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 167d79f..929365f 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -6,6 +6,8 @@ git quiltimport [options]
 --
 n,dry-run     dry run
 author=       author name and email address for patches without any
+C=            minimum context (see git apply)
+exact         allow no-fuzz
 patches=      path to the quilt series and patches
 "
 SUBDIRECTORY_ON=Yes
@@ -13,6 +15,7 @@ SUBDIRECTORY_ON=Yes
 
 dry_run=""
 quilt_author=""
+cflag=-C1
 while test $# != 0
 do
 	case "$1" in
@@ -20,6 +23,18 @@ do
 		shift
 		quilt_author="$1"
 		;;
+	-C)
+		shift
+		# ensure numerical parameter
+		case "$1" in
+		''|*[!0-9]*) usage;;
+		*) ;;
+		esac
+		cflag="-C$1"
+		;;
+	--exact)
+		cflag=
+		;;
 	-n|--dry-run)
 		dry_run=1
 		;;
@@ -130,7 +145,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index -C1 ${level:+"$level"} "$tmp_patch" &&
+		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.2-583-g325e495

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] git-quiltimport: flip the default not to allow fuzz
  2014-09-25 22:48     ` Junio C Hamano
  2014-09-25 22:59       ` Junio C Hamano
  2014-10-21 21:32       ` Junio C Hamano
@ 2014-10-21 21:38       ` Junio C Hamano
  2 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2014-10-21 21:38 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Git Mailing List

Trying to be as strict as possible when applying the patch may be a
good discipline, so let's flip the default but we can be helpful to
those who do rely on the original behaviour thanks to the previous
change to add -C$n option.

Suggest using -C1 when (and only when):

 - "git apply" without fuzz fails to apply; and
 - the user did not specify a -C$n or --exact option; and
 - "git apply -C1" (old behaviour) would have succeeded.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * And this is the "flipping of the default"

 git-quiltimport.sh | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 929365f..1190eb9 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -15,7 +15,8 @@ SUBDIRECTORY_ON=Yes
 
 dry_run=""
 quilt_author=""
-cflag=-C1
+cflag=
+fuzz_specified=
 while test $# != 0
 do
 	case "$1" in
@@ -31,9 +32,11 @@ do
 		*) ;;
 		esac
 		cflag="-C$1"
+		fuzz_specified=yes
 		;;
 	--exact)
 		cflag=
+		fuzz_specified=yes
 		;;
 	-n|--dry-run)
 		dry_run=1
@@ -74,6 +77,25 @@ tmp_msg="$tmp_dir/msg"
 tmp_patch="$tmp_dir/patch"
 tmp_info="$tmp_dir/info"
 
+# Helper to warn about -C$n option
+do_apply () {
+	if git apply --index ${cflag+"$cflag"} "$@"
+	then
+		return
+	fi
+	if test -z "$fuzz_specified" &&
+	   git apply --check --index -C1 "$@" >/dev/null 2>&1
+	then
+		cat >&2 <<-\EOM
+		'git quiltimport' by default no longer attempts to apply
+		patches with reduced context lines to allow fuzz; if you
+		want the old 'unsafe' behaviour, run the command with -C1
+		option.
+		EOM
+
+	fi
+	return 1
+}
 
 # Find the initial commit
 commit=$(git rev-parse HEAD)
@@ -145,7 +167,7 @@ do
 	fi
 
 	if [ -z "$dry_run" ] ; then
-		git apply --index $cflag ${level:+"$level"} "$tmp_patch" &&
+		do_apply ${level:+"$level"} "$tmp_patch" &&
 		tree=$(git write-tree) &&
 		commit=$( (echo "$SUBJECT"; echo; cat "$tmp_msg") | git commit-tree $tree -p $commit) &&
 		git update-ref -m "quiltimport: $patch_name" HEAD $commit || exit 4
-- 
2.1.2-583-g325e495

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-10-21 21:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-24 21:35 [PATCH] git-quiltimport.sh: disallow fuzz Jörn Engel
2014-09-25  5:09 ` Junio C Hamano
2014-09-25 22:08   ` Jörn Engel
2014-09-25 22:48     ` Junio C Hamano
2014-09-25 22:59       ` Junio C Hamano
2014-09-26 21:02         ` Junio C Hamano
2014-10-21 21:32       ` Junio C Hamano
2014-10-21 21:38       ` [PATCH 2/2] git-quiltimport: flip the default not to allow fuzz Junio C Hamano

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).