threads / patch / 15711

patchfix guilt-pop and push to fail if no relevant patches

Subject: [PATCH] fix guilt-pop and push to fail if no relevant patches

## tl;dr

4 messages between Sep 29, 2008 and Oct 17, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Scott Moser· Sep 29, 2008, 18:51 UTC · lore

currently guilt-pop and guilt-push will exit with '0' if there are no more relevant patches in the series (ie, if you've pushed or popped all of them)

This means that you cannot do something like:
  while guilt-push; do
    guilt refresh || break
  done
for reference, quilt does exit with non-zero in those cases:
  $ quilt push -a && quilt push
  File series fully applied, ends at patch my.patch
  $ echo $?
  1
  $ quilt pop -a; quilt pop
  No patch removed
  $ echo $?
  2
Signed-off-by: Scott Moser <smoser@brickies.net>
---
 guilt-pop            |    3 +--
 guilt-push           |   43 ++++++++++++++++++++++++-------------------
 regression/t-021.out |    3 +++
 3 files changed, 28 insertions(+), 21 deletions(-)
Show changes to 3 files +28 −21

guilt-pop, guilt-push, regression/t-021.out

diff --git a/guilt-pop b/guilt-pop
index db8473e..8a83fdb 100755
--- a/guilt-pop
+++ b/guilt-pop
@@ -45,8 +45,7 @@ patch="$1"
 [ ! -z "$all" ] && patch="-a"
 
 if [ ! -s "$applied" ]; then
-	disp "No patches applied."
-	exit 0
+	die "No patches applied."
 elif [ "$patch" = "-a" ]; then
 	# we are supposed to pop all patches
 
diff --git a/guilt-push b/guilt-push
index 018f9ac..48f886b 100755
--- a/guilt-push
+++ b/guilt-push
@@ -97,22 +97,27 @@ fi
 sidx=`wc -l < $applied`
 sidx=`expr $sidx + 1`
 
-get_series | sed -n -e "${sidx},${eidx}p" | while read p
-do
-	disp "Applying patch..$p"
-	if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
-		die "Patch $p does not exist. Aborting."
-	fi
-
-	push_patch "$p" $abort_flag
-
-	# bail if necessary
-	if [ $? -eq 0 ]; then
-		disp "Patch applied."
-	elif [ -z "$abort_flag" ]; then
-		die "Patch applied with rejects. Fix it up, and refresh."
-	else
-		die "To force apply this patch, use 'guilt push -f'"
-	fi
-done
-
+get_series | sed -n -e "${sidx},${eidx}p" |
+	{
+	did_patch=0
+	while read p
+	do
+		disp "Applying patch..$p"
+		if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
+			die "Patch $p does not exist. Aborting."
+		fi
+
+		push_patch "$p" $abort_flag
+
+		# bail if necessary
+		if [ $? -eq 0 ]; then
+			disp "Patch applied."
+		elif [ -z "$abort_flag" ]; then
+			die "Patch applied with rejects. Fix it up, and refresh."
+		else
+			die "To force apply this patch, use 'guilt push -f'"
+		fi
+		did_patch=1
+	done
+	[ $did_patch -ge 1 ] || die "no patches to apply"
+	}
diff --git a/regression/t-021.out b/regression/t-021.out
index cd8ae96..44771cb 100644
--- a/regression/t-021.out
+++ b/regression/t-021.out
@@ -822,6 +822,7 @@ index 0000000..8baef1b
 @@ -0,0 +1 @@
 +abc
 % guilt-push --all
+no patches to apply
 % guilt-pop -n -1
 Invalid number of patches to pop.
 % list_files
@@ -908,6 +909,7 @@ index 0000000..8baef1b
 @@ -0,0 +1 @@
 +abc
 % guilt-push --all
+no patches to apply
 % guilt-pop -n 0
 No patches requested to be removed.
 % list_files
@@ -994,6 +996,7 @@ index 0000000..8baef1b
 @@ -0,0 +1 @@
 +abc
 % guilt-push --all
+no patches to apply
 % guilt-pop -n 1
 Now at remove.
 % list_files
-- 
1.5.6.3
Scott Moser· Oct 17, 2008, 11:37 UTC · re: Scott Moser · lore

Re: [PATCH] fix guilt-pop and push to fail if no relevant patches

Jeff,
   Did you not like the patch below for some reason ?
   It seemed fairly straightforward to me that guilt-pop and guilt-push
should exit failure if they did not do anything due to having nothing to
do.
On Mon, 29 Sep 2008, Scott Moser wrote:
Show 126 quoted lines
> currently guilt-pop and guilt-push will exit with '0' if there are no more
> relevant patches in the series (ie, if you've pushed or popped all of them)
>
> This means that you cannot do something like:
>   while guilt-push; do
>     guilt refresh || break
>   done
>
> for reference, quilt does exit with non-zero in those cases:
>   $ quilt push -a && quilt push
>   File series fully applied, ends at patch my.patch
>   $ echo $?
>   1
>
>   $ quilt pop -a; quilt pop
>   No patch removed
>   $ echo $?
>   2
>
> Signed-off-by: Scott Moser <smoser@brickies.net>
> ---
>  guilt-pop            |    3 +--
>  guilt-push           |   43 ++++++++++++++++++++++++-------------------
>  regression/t-021.out |    3 +++
>  3 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/guilt-pop b/guilt-pop
> index db8473e..8a83fdb 100755
> --- a/guilt-pop
> +++ b/guilt-pop
> @@ -45,8 +45,7 @@ patch="$1"
>  [ ! -z "$all" ] && patch="-a"
>
>  if [ ! -s "$applied" ]; then
> -	disp "No patches applied."
> -	exit 0
> +	die "No patches applied."
>  elif [ "$patch" = "-a" ]; then
>  	# we are supposed to pop all patches
>
> diff --git a/guilt-push b/guilt-push
> index 018f9ac..48f886b 100755
> --- a/guilt-push
> +++ b/guilt-push
> @@ -97,22 +97,27 @@ fi
>  sidx=`wc -l < $applied`
>  sidx=`expr $sidx + 1`
>
> -get_series | sed -n -e "${sidx},${eidx}p" | while read p
> -do
> -	disp "Applying patch..$p"
> -	if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
> -		die "Patch $p does not exist. Aborting."
> -	fi
> -
> -	push_patch "$p" $abort_flag
> -
> -	# bail if necessary
> -	if [ $? -eq 0 ]; then
> -		disp "Patch applied."
> -	elif [ -z "$abort_flag" ]; then
> -		die "Patch applied with rejects. Fix it up, and refresh."
> -	else
> -		die "To force apply this patch, use 'guilt push -f'"
> -	fi
> -done
> -
> +get_series | sed -n -e "${sidx},${eidx}p" |
> +	{
> +	did_patch=0
> +	while read p
> +	do
> +		disp "Applying patch..$p"
> +		if [ ! -f "$GUILT_DIR/$branch/$p" ]; then
> +			die "Patch $p does not exist. Aborting."
> +		fi
> +
> +		push_patch "$p" $abort_flag
> +
> +		# bail if necessary
> +		if [ $? -eq 0 ]; then
> +			disp "Patch applied."
> +		elif [ -z "$abort_flag" ]; then
> +			die "Patch applied with rejects. Fix it up, and refresh."
> +		else
> +			die "To force apply this patch, use 'guilt push -f'"
> +		fi
> +		did_patch=1
> +	done
> +	[ $did_patch -ge 1 ] || die "no patches to apply"
> +	}
> diff --git a/regression/t-021.out b/regression/t-021.out
> index cd8ae96..44771cb 100644
> --- a/regression/t-021.out
> +++ b/regression/t-021.out
> @@ -822,6 +822,7 @@ index 0000000..8baef1b
>  @@ -0,0 +1 @@
>  +abc
>  % guilt-push --all
> +no patches to apply
>  % guilt-pop -n -1
>  Invalid number of patches to pop.
>  % list_files
> @@ -908,6 +909,7 @@ index 0000000..8baef1b
>  @@ -0,0 +1 @@
>  +abc
>  % guilt-push --all
> +no patches to apply
>  % guilt-pop -n 0
>  No patches requested to be removed.
>  % list_files
> @@ -994,6 +996,7 @@ index 0000000..8baef1b
>  @@ -0,0 +1 @@
>  +abc
>  % guilt-push --all
> +no patches to apply
>  % guilt-pop -n 1
>  Now at remove.
>  % list_files
> --
> 1.5.6.3
>
>
> !DSPAM:48e123ca138521410093335!
>
>
Josef 'Jeff' Sipek· Oct 17, 2008, 14:28 UTC · re: Scott Moser · lore

Re: [PATCH] fix guilt-pop and push to fail if no relevant patches

On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:
> Jeff,
>    Did you not like the patch below for some reason ?

I don't remember my train of thought, but I ended up making a simpler patch to address the push-pushing-more-than-it-should bug. I completely missed the part about the exit codes.

>    It seemed fairly straightforward to me that guilt-pop and guilt-push
> should exit failure if they did not do anything due to having nothing to
> do.
I'd actually say that it's not obvious, but...see below :)
Show 19 quoted lines
> On Mon, 29 Sep 2008, Scott Moser wrote:
> > currently guilt-pop and guilt-push will exit with '0' if there are no more
> > relevant patches in the series (ie, if you've pushed or popped all of them)
> >
> > This means that you cannot do something like:
> >   while guilt-push; do
> >     guilt refresh || break
> >   done
> >
> > for reference, quilt does exit with non-zero in those cases:
> >   $ quilt push -a && quilt push
> >   File series fully applied, ends at patch my.patch
> >   $ echo $?
> >   1
> >
> >   $ quilt pop -a; quilt pop
> >   No patch removed
> >   $ echo $?
> >   2
Who am I to argue against compatibility.
...
> > diff --git a/guilt-push b/guilt-push
> > index 018f9ac..48f886b 100755
> > --- a/guilt-push
> > +++ b/guilt-push
[snipped long diff]
With my fix, this should be a 2-liner :)
Sorry for missing the return code part...
Josef 'Jeff' Sipek.
-- 
We have joy, we have fun, we have Linux on a Sun...
Scott Moser· Oct 17, 2008, 14:40 UTC · re: Josef 'Jeff' Sipek · lore

Re: [PATCH] fix guilt-pop and push to fail if no relevant patches

On Fri, 17 Oct 2008, Josef 'Jeff' Sipek wrote:
Show 8 quoted lines
> On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:
> > Jeff,
> >    Did you not like the patch below for some reason ?
>
> I don't remember my train of thought, but I ended up making a simpler patch
> to address the push-pushing-more-than-it-should bug. I completely missed the
> part about the exit codes.
>

good enough. I hadn't pulled in a while and didn't realize you'd made a change. I never actually saw a problem with "pushing too much" , but only in the exit codes.

Show 7 quoted lines
> > > diff --git a/guilt-push b/guilt-push
> > > index 018f9ac..48f886b 100755
> > > --- a/guilt-push
> > > +++ b/guilt-push
> [snipped long diff]
>
> With my fix, this should be a 2-liner :)

for what its worth, the patch i sent only added a couple lines. It really just changed indentation. So, the patch was long, only 2 new lines that did anything.

anyway... I'm happy if you make it exit failure.
Thanks,

← back to recent threads