{"thread":{"id":"15711","subject":"[PATCH] fix guilt-pop and push to fail if no relevant patches","startedAt":"2008-09-29T18:51:33Z","lastAt":"2008-10-17T14:40:43Z","messageCount":4,"participants":["Scott Moser","Josef 'Jeff' Sipek"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"91875","messageId":"1222714293-9680-1-git-send-email-smoser@brickies.net","threadId":"15711","inReplyTo":null,"subject":"[PATCH] fix guilt-pop and push to fail if no relevant patches","fromName":"Scott Moser","fromEmail":"smoser@brickies.net","sentAt":"2008-09-29T18:51:33Z","receivedAt":"2008-09-29T18:51:33Z","isPatch":true,"sender":{"key":"smoser@brickies.net","avatar":"https://gravatar.com/avatar/72ac028bd8501889de97ffa6dda78e0f3c4cd15830aa09c2c11cdc117130f83c?d=mp&s=160"},"body":"currently guilt-pop and guilt-push will exit with '0' if there are no more\nrelevant patches in the series (ie, if you've pushed or popped all of them)\n\nThis means that you cannot do something like:\n  while guilt-push; do\n    guilt refresh || break\n  done\n\nfor reference, quilt does exit with non-zero in those cases:\n  $ quilt push -a && quilt push\n  File series fully applied, ends at patch my.patch\n  $ echo $?\n  1\n\n  $ quilt pop -a; quilt pop\n  No patch removed\n  $ echo $?\n  2\n\nSigned-off-by: Scott Moser <smoser@brickies.net>\n---\n guilt-pop            |    3 +--\n guilt-push           |   43 ++++++++++++++++++++++++-------------------\n regression/t-021.out |    3 +++\n 3 files changed, 28 insertions(+), 21 deletions(-)\n\ndiff --git a/guilt-pop b/guilt-pop\nindex db8473e..8a83fdb 100755\n--- a/guilt-pop\n+++ b/guilt-pop\n@@ -45,8 +45,7 @@ patch=\"$1\"\n [ ! -z \"$all\" ] && patch=\"-a\"\n \n if [ ! -s \"$applied\" ]; then\n-\tdisp \"No patches applied.\"\n-\texit 0\n+\tdie \"No patches applied.\"\n elif [ \"$patch\" = \"-a\" ]; then\n \t# we are supposed to pop all patches\n \ndiff --git a/guilt-push b/guilt-push\nindex 018f9ac..48f886b 100755\n--- a/guilt-push\n+++ b/guilt-push\n@@ -97,22 +97,27 @@ fi\n sidx=`wc -l < $applied`\n sidx=`expr $sidx + 1`\n \n-get_series | sed -n -e \"${sidx},${eidx}p\" | while read p\n-do\n-\tdisp \"Applying patch..$p\"\n-\tif [ ! -f \"$GUILT_DIR/$branch/$p\" ]; then\n-\t\tdie \"Patch $p does not exist. Aborting.\"\n-\tfi\n-\n-\tpush_patch \"$p\" $abort_flag\n-\n-\t# bail if necessary\n-\tif [ $? -eq 0 ]; then\n-\t\tdisp \"Patch applied.\"\n-\telif [ -z \"$abort_flag\" ]; then\n-\t\tdie \"Patch applied with rejects. Fix it up, and refresh.\"\n-\telse\n-\t\tdie \"To force apply this patch, use 'guilt push -f'\"\n-\tfi\n-done\n-\n+get_series | sed -n -e \"${sidx},${eidx}p\" |\n+\t{\n+\tdid_patch=0\n+\twhile read p\n+\tdo\n+\t\tdisp \"Applying patch..$p\"\n+\t\tif [ ! -f \"$GUILT_DIR/$branch/$p\" ]; then\n+\t\t\tdie \"Patch $p does not exist. Aborting.\"\n+\t\tfi\n+\n+\t\tpush_patch \"$p\" $abort_flag\n+\n+\t\t# bail if necessary\n+\t\tif [ $? -eq 0 ]; then\n+\t\t\tdisp \"Patch applied.\"\n+\t\telif [ -z \"$abort_flag\" ]; then\n+\t\t\tdie \"Patch applied with rejects. Fix it up, and refresh.\"\n+\t\telse\n+\t\t\tdie \"To force apply this patch, use 'guilt push -f'\"\n+\t\tfi\n+\t\tdid_patch=1\n+\tdone\n+\t[ $did_patch -ge 1 ] || die \"no patches to apply\"\n+\t}\ndiff --git a/regression/t-021.out b/regression/t-021.out\nindex cd8ae96..44771cb 100644\n--- a/regression/t-021.out\n+++ b/regression/t-021.out\n@@ -822,6 +822,7 @@ index 0000000..8baef1b\n @@ -0,0 +1 @@\n +abc\n % guilt-push --all\n+no patches to apply\n % guilt-pop -n -1\n Invalid number of patches to pop.\n % list_files\n@@ -908,6 +909,7 @@ index 0000000..8baef1b\n @@ -0,0 +1 @@\n +abc\n % guilt-push --all\n+no patches to apply\n % guilt-pop -n 0\n No patches requested to be removed.\n % list_files\n@@ -994,6 +996,7 @@ index 0000000..8baef1b\n @@ -0,0 +1 @@\n +abc\n % guilt-push --all\n+no patches to apply\n % guilt-pop -n 1\n Now at remove.\n % list_files\n-- \n1.5.6.3\n"},{"id":"93291","messageId":"alpine.DEB.1.00.0810170736240.27798@brickies","threadId":"15711","inReplyTo":"1222714293-9680-1-git-send-email-smoser@brickies.net","subject":"Re: [PATCH] fix guilt-pop and push to fail if no relevant patches","fromName":"Scott Moser","fromEmail":"smoser@brickies.net","sentAt":"2008-10-17T11:37:33Z","receivedAt":"2008-10-17T11:37:33Z","isPatch":true,"sender":{"key":"smoser@brickies.net","avatar":"https://gravatar.com/avatar/72ac028bd8501889de97ffa6dda78e0f3c4cd15830aa09c2c11cdc117130f83c?d=mp&s=160"},"body":"Jeff,\n   Did you not like the patch below for some reason ?\n   It seemed fairly straightforward to me that guilt-pop and guilt-push\nshould exit failure if they did not do anything due to having nothing to\ndo.\n\n\nOn Mon, 29 Sep 2008, Scott Moser wrote:\n\n> currently guilt-pop and guilt-push will exit with '0' if there are no more\n> relevant patches in the series (ie, if you've pushed or popped all of them)\n>\n> This means that you cannot do something like:\n>   while guilt-push; do\n>     guilt refresh || break\n>   done\n>\n> for reference, quilt does exit with non-zero in those cases:\n>   $ quilt push -a && quilt push\n>   File series fully applied, ends at patch my.patch\n>   $ echo $?\n>   1\n>\n>   $ quilt pop -a; quilt pop\n>   No patch removed\n>   $ echo $?\n>   2\n>\n> Signed-off-by: Scott Moser <smoser@brickies.net>\n> ---\n>  guilt-pop            |    3 +--\n>  guilt-push           |   43 ++++++++++++++++++++++++-------------------\n>  regression/t-021.out |    3 +++\n>  3 files changed, 28 insertions(+), 21 deletions(-)\n>\n> diff --git a/guilt-pop b/guilt-pop\n> index db8473e..8a83fdb 100755\n> --- a/guilt-pop\n> +++ b/guilt-pop\n> @@ -45,8 +45,7 @@ patch=\"$1\"\n>  [ ! -z \"$all\" ] && patch=\"-a\"\n>\n>  if [ ! -s \"$applied\" ]; then\n> -\tdisp \"No patches applied.\"\n> -\texit 0\n> +\tdie \"No patches applied.\"\n>  elif [ \"$patch\" = \"-a\" ]; then\n>  \t# we are supposed to pop all patches\n>\n> diff --git a/guilt-push b/guilt-push\n> index 018f9ac..48f886b 100755\n> --- a/guilt-push\n> +++ b/guilt-push\n> @@ -97,22 +97,27 @@ fi\n>  sidx=`wc -l < $applied`\n>  sidx=`expr $sidx + 1`\n>\n> -get_series | sed -n -e \"${sidx},${eidx}p\" | while read p\n> -do\n> -\tdisp \"Applying patch..$p\"\n> -\tif [ ! -f \"$GUILT_DIR/$branch/$p\" ]; then\n> -\t\tdie \"Patch $p does not exist. Aborting.\"\n> -\tfi\n> -\n> -\tpush_patch \"$p\" $abort_flag\n> -\n> -\t# bail if necessary\n> -\tif [ $? -eq 0 ]; then\n> -\t\tdisp \"Patch applied.\"\n> -\telif [ -z \"$abort_flag\" ]; then\n> -\t\tdie \"Patch applied with rejects. Fix it up, and refresh.\"\n> -\telse\n> -\t\tdie \"To force apply this patch, use 'guilt push -f'\"\n> -\tfi\n> -done\n> -\n> +get_series | sed -n -e \"${sidx},${eidx}p\" |\n> +\t{\n> +\tdid_patch=0\n> +\twhile read p\n> +\tdo\n> +\t\tdisp \"Applying patch..$p\"\n> +\t\tif [ ! -f \"$GUILT_DIR/$branch/$p\" ]; then\n> +\t\t\tdie \"Patch $p does not exist. Aborting.\"\n> +\t\tfi\n> +\n> +\t\tpush_patch \"$p\" $abort_flag\n> +\n> +\t\t# bail if necessary\n> +\t\tif [ $? -eq 0 ]; then\n> +\t\t\tdisp \"Patch applied.\"\n> +\t\telif [ -z \"$abort_flag\" ]; then\n> +\t\t\tdie \"Patch applied with rejects. Fix it up, and refresh.\"\n> +\t\telse\n> +\t\t\tdie \"To force apply this patch, use 'guilt push -f'\"\n> +\t\tfi\n> +\t\tdid_patch=1\n> +\tdone\n> +\t[ $did_patch -ge 1 ] || die \"no patches to apply\"\n> +\t}\n> diff --git a/regression/t-021.out b/regression/t-021.out\n> index cd8ae96..44771cb 100644\n> --- a/regression/t-021.out\n> +++ b/regression/t-021.out\n> @@ -822,6 +822,7 @@ index 0000000..8baef1b\n>  @@ -0,0 +1 @@\n>  +abc\n>  % guilt-push --all\n> +no patches to apply\n>  % guilt-pop -n -1\n>  Invalid number of patches to pop.\n>  % list_files\n> @@ -908,6 +909,7 @@ index 0000000..8baef1b\n>  @@ -0,0 +1 @@\n>  +abc\n>  % guilt-push --all\n> +no patches to apply\n>  % guilt-pop -n 0\n>  No patches requested to be removed.\n>  % list_files\n> @@ -994,6 +996,7 @@ index 0000000..8baef1b\n>  @@ -0,0 +1 @@\n>  +abc\n>  % guilt-push --all\n> +no patches to apply\n>  % guilt-pop -n 1\n>  Now at remove.\n>  % list_files\n> --\n> 1.5.6.3\n>\n>\n> !DSPAM:48e123ca138521410093335!\n>\n>\n"},{"id":"93295","messageId":"20081017142832.GF27647@josefsipek.net","threadId":"15711","inReplyTo":"alpine.DEB.1.00.0810170736240.27798@brickies","subject":"Re: [PATCH] fix guilt-pop and push to fail if no relevant patches","fromName":"Josef 'Jeff' Sipek","fromEmail":"jeffpc@josefsipek.net","sentAt":"2008-10-17T14:28:32Z","receivedAt":"2008-10-17T14:28:32Z","isPatch":true,"sender":{"key":"jeffpc@josefsipek.net","avatar":null},"body":"On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:\n> Jeff,\n>    Did you not like the patch below for some reason ?\n\nI don't remember my train of thought, but I ended up making a simpler patch\nto address the push-pushing-more-than-it-should bug. I completely missed the\npart about the exit codes.\n\n>    It seemed fairly straightforward to me that guilt-pop and guilt-push\n> should exit failure if they did not do anything due to having nothing to\n> do.\n\nI'd actually say that it's not obvious, but...see below :)\n\n> On Mon, 29 Sep 2008, Scott Moser wrote:\n> > currently guilt-pop and guilt-push will exit with '0' if there are no more\n> > relevant patches in the series (ie, if you've pushed or popped all of them)\n> >\n> > This means that you cannot do something like:\n> >   while guilt-push; do\n> >     guilt refresh || break\n> >   done\n> >\n> > for reference, quilt does exit with non-zero in those cases:\n> >   $ quilt push -a && quilt push\n> >   File series fully applied, ends at patch my.patch\n> >   $ echo $?\n> >   1\n> >\n> >   $ quilt pop -a; quilt pop\n> >   No patch removed\n> >   $ echo $?\n> >   2\n\nWho am I to argue against compatibility.\n\n...\n> > diff --git a/guilt-push b/guilt-push\n> > index 018f9ac..48f886b 100755\n> > --- a/guilt-push\n> > +++ b/guilt-push\n[snipped long diff]\n\nWith my fix, this should be a 2-liner :)\n\nSorry for missing the return code part...\n\nJosef 'Jeff' Sipek.\n\n-- \nWe have joy, we have fun, we have Linux on a Sun...\n"},{"id":"93296","messageId":"alpine.DEB.1.00.0810171036410.27798@brickies","threadId":"15711","inReplyTo":"20081017142832.GF27647@josefsipek.net","subject":"Re: [PATCH] fix guilt-pop and push to fail if no relevant patches","fromName":"Scott Moser","fromEmail":"smoser@brickies.net","sentAt":"2008-10-17T14:40:43Z","receivedAt":"2008-10-17T14:40:43Z","isPatch":true,"sender":{"key":"smoser@brickies.net","avatar":"https://gravatar.com/avatar/72ac028bd8501889de97ffa6dda78e0f3c4cd15830aa09c2c11cdc117130f83c?d=mp&s=160"},"body":"On Fri, 17 Oct 2008, Josef 'Jeff' Sipek wrote:\n\n> On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:\n> > Jeff,\n> >    Did you not like the patch below for some reason ?\n>\n> I don't remember my train of thought, but I ended up making a simpler patch\n> to address the push-pushing-more-than-it-should bug. I completely missed the\n> part about the exit codes.\n>\n\ngood enough. I hadn't pulled in a while and didn't realize you'd made a\nchange.  I never actually saw a problem with \"pushing too much\" , but\nonly in the exit codes.\n\n> > > diff --git a/guilt-push b/guilt-push\n> > > index 018f9ac..48f886b 100755\n> > > --- a/guilt-push\n> > > +++ b/guilt-push\n> [snipped long diff]\n>\n> With my fix, this should be a 2-liner :)\n\nfor what its worth, the patch i sent only added a couple lines.  It\nreally just changed indentation.  So, the patch was long, only 2 new\nlines that did anything.\n\nanyway... I'm happy if you make it exit failure.\n\nThanks,\n"}]}