git-quiltimport and non-existent patches

git-quiltimport and non-existent patches

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1071 bytes --]

	Hi,

Unlike quilt itself, git-quiltimport doesn't ignore non-existent patches.
Instead it bails out badly, leaving a .dotest directory that must be removed
manually. This is with git 1.5.3.2.

It would be nice if git-quiltimport would just warn about non-existent patches,
just like quilt.  This will make it work with `markers' we put in our quilt
series files. Commenting out the markers is no solution as `quilt series'
doesn't show commented-out patches.

Thanks!

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: git-quiltimport and non-existent patches

[Junio C Hamano]

Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> writes:

> It would be nice if git-quiltimport would just warn about
> non-existent patches, just like quilt.  This will make it work
> with `markers' we put in our quilt series files. Commenting
> out the markers is no solution as `quilt series' doesn't show
> commented-out patches.

I cannot decide if this should be the default (that's up to
heavy users of git-quiltimport script), but something along this
line should do.  Care to test it and ack?

---

 git-quiltimport.sh |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 74a54d5..3c38959 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -4,6 +4,7 @@ SUBDIRECTORY_ON=Yes
 . git-sh-setup
 
 dry_run=""
+error_empty=t
 quilt_author=""
 while test $# != 0
 do
@@ -25,6 +26,11 @@ do
 		dry_run=1
 		;;
 
+	--expect-marker)
+		shift
+		error_empty=
+		;;
+
 	--pa=*|--pat=*|--patc=*|--patch=*|--patche=*|--patches=*)
 		QUILT_PATCHES=$(expr "z$1" : 'z-[^=]*\(.*\)')
 		shift
@@ -74,10 +80,17 @@ for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
 	echo $patch_name
 	git mailinfo "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
-	test -s "$tmp_patch" || {
-		echo "Patch is empty.  Was it split wrong?"
-		exit 1
-	}
+	if test ! -s "$tmp_patch"
+	then
+		if test -z "$error_empty"
+		then
+			echo "Patch is empty.  Was it split wrong?"
+			exit 1
+		else
+			echo "Marker seen."
+			continue
+		fi
+	fi
 
 	# Parse the author information
 	export GIT_AUTHOR_NAME=$(sed -ne 's/Author: //p' "$tmp_info")

[PATCH] quiltimport: Skip non-existent patches

[Dan Nicholson]

When quiltimport encounters a non-existent patch in the series file,
just skip to the next patch. This matches the behavior of quilt.

Signed-off-by: Dan Nicholson <dbn.lists@gmail.com>
---
 git-quiltimport.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 74a54d5..880c81d 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
 
 mkdir $tmp_dir || exit 2
 for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
+	if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
+		echo "$patch_name doesn't exist. Skipping."
+		continue
+	fi
 	echo $patch_name
 	git mailinfo "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
-- 
1.5.3.2

Re: [PATCH] quiltimport: Skip non-existent patches

[Dan Nicholson]

Dan Nicholson <dbn.lists <at> gmail.com> writes:
> 
> When quiltimport encounters a non-existent patch in the series file,
> just skip to the next patch. This matches the behavior of quilt.
> 
> Signed-off-by: Dan Nicholson <dbn.lists <at> gmail.com>
> ---
>  git-quiltimport.sh |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 74a54d5..880c81d 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
> 
>  mkdir $tmp_dir || exit 2
>  for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
> +	if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
> +		echo "$patch_name doesn't exist. Skipping."
> +		continue
> +	fi
>  	echo $patch_name
>  	git mailinfo "$tmp_msg" "$tmp_patch" \
>  		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3


I forgot to mention the rationale for this patch vs. what Junio sent. The issue
with Junio's patch is that the failure will occur before $tmp_patch is created
because the script tries to feed git-mailinfo a non-existent patch
($patch_name). You'll only get past the mailinfo if $patch_name exists.

The marker setting may still be useful in this context, though, to suppress the
"doesn't exist" message.

--
Dan

Re: [PATCH] quiltimport: Skip non-existent patches

[Junio C Hamano]

Dan Nicholson <dbn.lists@gmail.com> writes:

> Dan Nicholson <dbn.lists <at> gmail.com> writes:
>> 
>> When quiltimport encounters a non-existent patch in the series file,
>> just skip to the next patch. This matches the behavior of quilt.
>> 
>> Signed-off-by: Dan Nicholson <dbn.lists <at> gmail.com>
>> ---
>>  git-quiltimport.sh |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>> 
>> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
>> index 74a54d5..880c81d 100755
>> --- a/git-quiltimport.sh
>> +++ b/git-quiltimport.sh
>> @@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
>> 
>>  mkdir $tmp_dir || exit 2
>>  for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
>> +	if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
>> +		echo "$patch_name doesn't exist. Skipping."
>> +		continue
>> +	fi
>>  	echo $patch_name
>>  	git mailinfo "$tmp_msg" "$tmp_patch" \
>>  		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
>
>
> I forgot to mention the rationale for this patch vs. what Junio sent. The issue
> with Junio's patch is that the failure will occur before $tmp_patch is created
> because the script tries to feed git-mailinfo a non-existent patch
> ($patch_name). You'll only get past the mailinfo if $patch_name exists.
>
> The marker setting may still be useful in this context, though, to suppress the
> "doesn't exist" message.

Thanks.  I did not know what "marker" meant by the original
context and assumed there is a file referred to by the series
file but there is no patch in that file.  Instead it seems that
a series file can contain something that is _not_ a file and
that is called the marker, right?

Re: [PATCH] quiltimport: Skip non-existent patches

[Dan Nicholson]

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> Dan Nicholson <dbn.lists@gmail.com> writes:
>
> > Dan Nicholson <dbn.lists <at> gmail.com> writes:
> >>
> >> When quiltimport encounters a non-existent patch in the series file,
> >> just skip to the next patch. This matches the behavior of quilt.
> >>
> >> Signed-off-by: Dan Nicholson <dbn.lists <at> gmail.com>
> >> ---
> >>  git-quiltimport.sh |    4 ++++
> >>  1 files changed, 4 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> >> index 74a54d5..880c81d 100755
> >> --- a/git-quiltimport.sh
> >> +++ b/git-quiltimport.sh
> >> @@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
> >>
> >>  mkdir $tmp_dir || exit 2
> >>  for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
> >> +    if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
> >> +            echo "$patch_name doesn't exist. Skipping."
> >> +            continue
> >> +    fi
> >>      echo $patch_name
> >>      git mailinfo "$tmp_msg" "$tmp_patch" \
> >>              <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
> >
> >
> > I forgot to mention the rationale for this patch vs. what Junio sent. The issue
> > with Junio's patch is that the failure will occur before $tmp_patch is created
> > because the script tries to feed git-mailinfo a non-existent patch
> > ($patch_name). You'll only get past the mailinfo if $patch_name exists.
> >
> > The marker setting may still be useful in this context, though, to suppress the
> > "doesn't exist" message.
>
> Thanks.  I did not know what "marker" meant by the original
> context and assumed there is a file referred to by the series
> file but there is no patch in that file.  Instead it seems that
> a series file can contain something that is _not_ a file and
> that is called the marker, right?

I'm actually not a quilt user, but I tested out that patch on a repo
with a series containing a non-existent patch. I'm not sure what's
actually in Geerd's "marker", but I believe it's just random text.

When you run the command `quilt series', it just lists what's in the
series file (minus any comments). And when you run `quilt push' with a
non-existent patch, it says "Patch foo.patch does not exist; applied
empty patch"

So, I think the consistent thing to do is what's in my patch: just
skip the patch with a message to the user. Maybe the message can be
tailored to match quilt's output. Actually, it would be best to also
skip on empty files since quiltimport will bomb in that case as well.

--
Dan

Re: [PATCH] quiltimport: Skip non-existent patches

[Junio C Hamano]

"Dan Nicholson" <dbn.lists@gmail.com> writes:

> When you run the command `quilt series', it just lists what's in the
> series file (minus any comments). And when you run `quilt push' with a
> non-existent patch, it says "Patch foo.patch does not exist; applied
> empty patch"
>
> So, I think the consistent thing to do is what's in my patch: just
> skip the patch with a message to the user. Maybe the message can be
> tailored to match quilt's output. Actually, it would be best to also
> skip on empty files since quiltimport will bomb in that case as well.

Thanks for your helpful explanation.  So perhaps we can do this
on top of yours to be safer and more consistent.

---

 git-quiltimport.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 880c81d..627e023 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -79,8 +79,8 @@ for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
 	git mailinfo "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
 	test -s "$tmp_patch" || {
-		echo "Patch is empty.  Was it split wrong?"
-		exit 1
+		echo "Patch is empty. Skipping."
+		continue
 	}
 
 	# Parse the author information

Re: [PATCH] quiltimport: Skip non-existent patches

[Dan Nicholson]

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Dan Nicholson" <dbn.lists@gmail.com> writes:
>
> > When you run the command `quilt series', it just lists what's in the
> > series file (minus any comments). And when you run `quilt push' with a
> > non-existent patch, it says "Patch foo.patch does not exist; applied
> > empty patch"
> >
> > So, I think the consistent thing to do is what's in my patch: just
> > skip the patch with a message to the user. Maybe the message can be
> > tailored to match quilt's output. Actually, it would be best to also
> > skip on empty files since quiltimport will bomb in that case as well.
>
> Thanks for your helpful explanation.  So perhaps we can do this
> on top of yours to be safer and more consistent.
>
> ---
>
>  git-quiltimport.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 880c81d..627e023 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -79,8 +79,8 @@ for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
>         git mailinfo "$tmp_msg" "$tmp_patch" \
>                 <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
>         test -s "$tmp_patch" || {
> -               echo "Patch is empty.  Was it split wrong?"
> -               exit 1
> +               echo "Patch is empty. Skipping."
> +               continue
>         }
>
>         # Parse the author information

That's seems fine. IIUC, mailinfo will only create an empty patch if
there's no actual patch content in the original mail/patch. In that
case, you probably do want to skip and not bomb. I'd changed my patch
to do 'if ! [ -s "$patch" ]' to catch an empty file, but this is
probably better. Hmm, checking `quilt push' on a patch with no actual
patch bombs. Here's the output:

$ quilt push
Applying patch foo.patch
patch: **** Only garbage was found in the patch input.
Patch foo.patch does not apply (enforce with -f)
$ echo $?
1
$ cat patches/foo.patch
Here's info about an empty patch.

So, it might be better to leave the original behavior there to match quilt.

--
Dan

Re: [PATCH] quiltimport: Skip non-existent patches

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1548 bytes --]

On Thu, 27 Sep 2007, Dan Nicholson wrote:
> When quiltimport encounters a non-existent patch in the series file,
> just skip to the next patch. This matches the behavior of quilt.
> 
> Signed-off-by: Dan Nicholson <dbn.lists@gmail.com>

Acked-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

> ---
>  git-quiltimport.sh |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 74a54d5..880c81d 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
>  
>  mkdir $tmp_dir || exit 2
>  for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
> +	if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
> +		echo "$patch_name doesn't exist. Skipping."
> +		continue
> +	fi
>  	echo $patch_name
>  	git mailinfo "$tmp_msg" "$tmp_patch" \
>  		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
> -- 
> 1.5.3.2

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619