threads / discuss / 10043

git-quiltimport and non-existent patches

Subject: git-quiltimport and non-existent patches

## tl;dr

9 messages between Sep 27, 2007 and Sep 28, 2007.

replies: 8people: 3as markdown or json

Geert Uytterhoeven· Sep 27, 2007, 09:59 UTC · lore

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

Junio C Hamano· Sep 27, 2007, 19:41 UTC · re: Geert Uytterhoeven · lore

Re: git-quiltimport and non-existent patches

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

Show 5 quoted lines

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

Dan Nicholson· Sep 27, 2007, 20:30 UTC · re: Geert Uytterhoeven · lore

[PATCH] quiltimport: Skip non-existent patches

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

Dan Nicholson· Sep 27, 2007, 20:39 UTC · re: Dan Nicholson · lore

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

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

Show 24 quoted lines

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

Junio C Hamano· Sep 27, 2007, 20:50 UTC · re: Dan Nicholson · lore

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

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

Show 34 quoted lines

> 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?

Dan Nicholson· Sep 27, 2007, 21:45 UTC · re: Junio C Hamano · lore

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

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:

Show 42 quoted lines

> 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

Junio C Hamano· Sep 27, 2007, 22:02 UTC · re: Dan Nicholson · lore

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

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

Show 9 quoted lines

> 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

Dan Nicholson· Sep 27, 2007, 22:20 UTC · re: Junio C Hamano · lore

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

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:

Show 35 quoted lines

> "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

Geert Uytterhoeven· Sep 28, 2007, 14:06 UTC · re: Dan Nicholson · lore

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

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>

Show 21 quoted lines

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