git cherry unkillable

git cherry unkillable

[Andrey Borzenkov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I mistakenly run git cherry on linus tree which was a bit too much for my poor 
old system. Trying to kill (^C) it I got

{pts/0}% git cherry ali1535 linus
external diff died, stopping at include/linux/sysctl.h.
external diff died, stopping at drivers/serial/suncore.c.
diff: /home/bor/tmp/.diff_Y6CvCw: No such file or directory
diff: /home/bor/tmp/.diff_tBqCCk: No such file or directory
external diff died, stopping at include/asm-arm/arch-pxa/pxa-regs.h.
external diff died, stopping at fs/cifs/file.c.
external diff died, stopping at MAINTAINERS.
diff: /home/bor/tmp/.diff_P12uX9: No such file or directory
diff: /home/bor/tmp/.diff_R4v0g0: No such file or directory
external diff died, stopping at net/ipv4/route.c.
....

and it jut goes on. This takes really some time here so ability to stop it 
would be really nice. It still runs with CPU temp rocketing.

Regards

- -andrey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFD010lR6LMutpd94wRArDIAKC4yNPpwZ2AEKpXwjeZREMRCL3VyACeJaYu
UMZUpX3D3UfHSm0m44pvAmY=
=0Cxg
-----END PGP SIGNATURE-----

Re: git cherry unkillable

[sean]

On Sun, 22 Jan 2006 13:23:26 +0300
Andrey Borzenkov <arvidjaar@mail.ru> wrote:

> Hash: SHA1
> 
> I mistakenly run git cherry on linus tree which was a bit too much for my poor 
> old system. Trying to kill (^C) it I got
> 
> {pts/0}% git cherry ali1535 linus
> external diff died, stopping at include/linux/sysctl.h.
> external diff died, stopping at drivers/serial/suncore.c.
> diff: /home/bor/tmp/.diff_Y6CvCw: No such file or directory
> diff: /home/bor/tmp/.diff_tBqCCk: No such file or directory
> external diff died, stopping at include/asm-arm/arch-pxa/pxa-regs.h.
> external diff died, stopping at fs/cifs/file.c.
> external diff died, stopping at MAINTAINERS.
> diff: /home/bor/tmp/.diff_P12uX9: No such file or directory
> diff: /home/bor/tmp/.diff_R4v0g0: No such file or directory
> external diff died, stopping at net/ipv4/route.c.
> ....
> 
> and it jut goes on. This takes really some time here so ability to stop it 
> would be really nice. It still runs with CPU temp rocketing.
> 

Ouch yeah, you'd have to explicitly kill -9 from another command line.   The
same problem exists in a few other scripts too, although probably none that
can be as long lived as git-cherry.   

The attached patch works for me but i'm concerned about it a bit.  With this
patch it looks like the signal handler is executed twice; once as normal and
a second time in response to the added "exit" statement.   By adding 
";trap - 0;exit" it is only executed once, but i'm not sure how universal that
is, and letting it run twice shouldn't be a big deal anyway.

The other minor concern is that exit should maybe return an error value
instead; but that isn't approprirate when handling signal zero.   So it
might be better to have a separate trap for signal zero, but in the end
it doesn't seem worth the bother.

Sean

	diff --git a/git-cherry.sh b/git-cherry.sh
index 1a62320..a1e4d70 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -49,7 +49,7 @@ ours=`git-rev-list $ours ^$limit` || exi
 tmp=.cherry-tmp$$
 patch=$tmp-patch
 mkdir $patch
-trap "rm -rf $tmp-*" 0 1 2 3 15
+trap "rm -rf $tmp-*;exit" 0 1 2 3 15
 
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-format-patch.sh b/git-format-patch.sh
index 7e67c4e..38adaad 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -77,7 +77,7 @@ tt)
 esac
 
 tmp=.tmp-series$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap 'rm -f $tmp-*;exit' 0 1 2 3 15
 
 series=$tmp-series
 commsg=$tmp-commsg
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index f699268..ba96969 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -38,7 +38,7 @@ peek_repo="$(get_remote_url "$@")"
 shift
 
 tmp=.ls-remote-$$
-trap "rm -fr $tmp-*" 0 1 2 3 15
+trap "rm -fr $tmp-*;exit" 0 1 2 3 15
 tmpdir=$tmp-d
 
 case "$peek_repo" in
diff --git a/git-reset.sh b/git-reset.sh
index 6c9e58a..1b6781f 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -4,7 +4,7 @@ USAGE='[--mixed | --soft | --hard]  [<co
 . git-sh-setup
 
 tmp=${GIT_DIR}/reset.$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap 'rm -f $tmp-*;exit' 0 1 2 3 15
 
 reset_type=--mixed
 case "$1" in

Re: git cherry unkillable

[sean]

On Sun, 22 Jan 2006 06:39:04 -0500
sean <seanlkml@sympatico.ca> wrote:

> The attached patch works for me but i'm concerned about it a bit.  

Below is a better version that doesn't obscure the proper exit value in the normal
case and exits with 1 if interrupted by the user.   It also ensures that the cleanup
isn't executed twice when the script is interrupted.

However, for Bash (at least) none of this is necessary; all the traps could just 
be changed to only trap 0 and the cleanup will be executed for all cases.   However,
I don't know how compatible that is with other shells.   If other shells behave
the same, the best fix is just to strip the 1,2,3 and 15 from each of the existing
trap lines and not bother with the patch given below.

Sean


diff --git a/git-cherry.sh b/git-cherry.sh
index 1a62320..0b2ef1f 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -49,7 +49,8 @@ ours=`git-rev-list $ours ^$limit` || exi
 tmp=.cherry-tmp$$
 patch=$tmp-patch
 mkdir $patch
-trap "rm -rf $tmp-*" 0 1 2 3 15
+trap "rm -rf $tmp-*" 0
+trap "exit 1" 1 2 3 15
 
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-format-patch.sh b/git-format-patch.sh
index 7e67c4e..6ba6339 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -77,7 +77,8 @@ tt)
 esac
 
 tmp=.tmp-series$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap "rm -f $tmp-*" 0
+trap "exit 1" 1 2 3 15
 
 series=$tmp-series
 commsg=$tmp-commsg
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index f699268..7dc224b 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -38,7 +38,8 @@ peek_repo="$(get_remote_url "$@")"
 shift
 
 tmp=.ls-remote-$$
-trap "rm -fr $tmp-*" 0 1 2 3 15
+trap "rm -rf $tmp-*" 0
+trap "exit 1" 1 2 3 15
 tmpdir=$tmp-d
 
 case "$peek_repo" in
diff --git a/git-reset.sh b/git-reset.sh
index 6c9e58a..166b635 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -4,7 +4,8 @@ USAGE='[--mixed | --soft | --hard]  [<co
 . git-sh-setup
 
 tmp=${GIT_DIR}/reset.$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap "rm -f $tmp-*" 0
+trap "exit 1" 1 2 3 15
 
 reset_type=--mixed
 case "$1" in

Re: git cherry unkillable

[Andrey Borzenkov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sunday 22 January 2006 14:39, sean wrote:
>
> The attached patch works for me 

ack at least for git-cherry

> but i'm concerned about it a bit.  With 
> this patch it looks like the signal handler is executed twice; once as
> normal and a second time in response to the added "exit" statement.   By
> adding ";trap - 0;exit" it is only executed once, but i'm not sure how
> universal that is, and letting it run twice shouldn't be a big deal anyway.
>

why not

trap "rm -rf $tmp-*" 0
trap "exit" 1 2 3 15

?

- -andrey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFD055JR6LMutpd94wRAn06AJ0RrLXwND67hvhRrSeaQvUjgrfGrACeK1cs
JA0jLNs5/8FPwpZqzKRd3eo=
=8QbF
-----END PGP SIGNATURE-----

Re: git cherry unkillable

[Andrey Borzenkov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sunday 22 January 2006 17:51, sean wrote:
> On Sun, 22 Jan 2006 06:39:04 -0500
>
> sean <seanlkml@sympatico.ca> wrote:
> > The attached patch works for me but i'm concerned about it a bit.
>
> Below is a better version that doesn't obscure the proper exit value in the
> normal case and exits with 1 if interrupted by the user.   It also ensures
> that the cleanup isn't executed twice when the script is interrupted.
>
> However, for Bash (at least) none of this is necessary; all the traps could
> just be changed to only trap 0 and the cleanup will be executed for all
> cases.   However, I don't know how compatible that is with other shells.  
> If other shells behave the same, the best fix is just to strip the 1,2,3
> and 15 from each of the existing trap lines and not bother with the patch
> given below.
>

this patch fails on zsh; trap 0 never executed after signal (SIGINT to be 
sure) is caught:

#!/bin/zsh

trap "exit 0" 1 2 3 15
trap "echo  exiting" 0

sleep 100000000

{pts/0}% /tmp/timout.zsh ^C
{pts/0}% 

- -andrey

> Sean
>
>
> diff --git a/git-cherry.sh b/git-cherry.sh
> index 1a62320..0b2ef1f 100755
> --- a/git-cherry.sh
> +++ b/git-cherry.sh
> @@ -49,7 +49,8 @@ ours=`git-rev-list $ours ^$limit` || exi
>  tmp=.cherry-tmp$$
>  patch=$tmp-patch
>  mkdir $patch
> -trap "rm -rf $tmp-*" 0 1 2 3 15
> +trap "rm -rf $tmp-*" 0
> +trap "exit 1" 1 2 3 15
>
>  _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
>  _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> diff --git a/git-format-patch.sh b/git-format-patch.sh
> index 7e67c4e..6ba6339 100755
> --- a/git-format-patch.sh
> +++ b/git-format-patch.sh
> @@ -77,7 +77,8 @@ tt)
>  esac
>
>  tmp=.tmp-series$$
> -trap 'rm -f $tmp-*' 0 1 2 3 15
> +trap "rm -f $tmp-*" 0
> +trap "exit 1" 1 2 3 15
>
>  series=$tmp-series
>  commsg=$tmp-commsg
> diff --git a/git-ls-remote.sh b/git-ls-remote.sh
> index f699268..7dc224b 100755
> --- a/git-ls-remote.sh
> +++ b/git-ls-remote.sh
> @@ -38,7 +38,8 @@ peek_repo="$(get_remote_url "$@")"
>  shift
>
>  tmp=.ls-remote-$$
> -trap "rm -fr $tmp-*" 0 1 2 3 15
> +trap "rm -rf $tmp-*" 0
> +trap "exit 1" 1 2 3 15
>  tmpdir=$tmp-d
>
>  case "$peek_repo" in
> diff --git a/git-reset.sh b/git-reset.sh
> index 6c9e58a..166b635 100755
> --- a/git-reset.sh
> +++ b/git-reset.sh
> @@ -4,7 +4,8 @@ USAGE='[--mixed | --soft | --hard]  [<co
>  . git-sh-setup
>
>  tmp=${GIT_DIR}/reset.$$
> -trap 'rm -f $tmp-*' 0 1 2 3 15
> +trap "rm -f $tmp-*" 0
> +trap "exit 1" 1 2 3 15
>
>  reset_type=--mixed
>  case "$1" in
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFD06MSR6LMutpd94wRApgAAJ0e0AUIS83XTa4BA1rEY1XriHKZmQCgnXZd
1sElZOLAgyuxgi3EHiaMGFc=
=xwjP
-----END PGP SIGNATURE-----

Re: git cherry unkillable

[sean]

On Sun, 22 Jan 2006 18:21:53 +0300
Andrey Borzenkov <arvidjaar@mail.ru> wrote:

> this patch fails on zsh; trap 0 never executed after signal (SIGINT to be 
> sure) is caught:
> 
> #!/bin/zsh
> 
> trap "exit 0" 1 2 3 15
> trap "echo  exiting" 0
> 
> sleep 100000000
> 
> {pts/0}% /tmp/timout.zsh ^C
> {pts/0}% 
> 

Damn, would be so much nicer to get this stuff out of shell scripts.   Anyway,
your discovery kills the idea of being able to just ignore the higher signal
traps...   The following implements the same idea as my second patch
in hopefully a slightly more cross-shell compatible way;  it works on bash
and zsh at least.

diff --git a/git-cherry.sh b/git-cherry.sh
index 1a62320..686210b 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -49,7 +49,8 @@ ours=`git-rev-list $ours ^$limit` || exi
 tmp=.cherry-tmp$$
 patch=$tmp-patch
 mkdir $patch
-trap "rm -rf $tmp-*" 0 1 2 3 15
+trap "rm -rf $tmp-*" 0
+trap "kill -0 $$" 1 2 3 15
 
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-format-patch.sh b/git-format-patch.sh
index 7e67c4e..9f93bbb 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -77,7 +77,8 @@ tt)
 esac
 
 tmp=.tmp-series$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap "rm -f $tmp-*" 0
+trap "kill -0 $$" 1 2 3 15
 
 series=$tmp-series
 commsg=$tmp-commsg
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index f699268..0cd1f07 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -38,7 +38,8 @@ peek_repo="$(get_remote_url "$@")"
 shift
 
 tmp=.ls-remote-$$
-trap "rm -fr $tmp-*" 0 1 2 3 15
+trap "rm -rf $tmp-*" 0
+trap "kill -0 $$" 1 2 3 15
 tmpdir=$tmp-d
 
 case "$peek_repo" in
diff --git a/git-reset.sh b/git-reset.sh
index 6c9e58a..597b36e 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -4,7 +4,8 @@ USAGE='[--mixed | --soft | --hard]  [<co
 . git-sh-setup
 
 tmp=${GIT_DIR}/reset.$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+trap "rm -f $tmp-*" 0
+trap "kill -0 $$" 1 2 3 15
 
 reset_type=--mixed
 case "$1" in

Re: git cherry unkillable

[sean]

On Sun, 22 Jan 2006 10:32:04 -0500
sean <seanlkml@sympatico.ca> wrote:

> Damn, would be so much nicer to get this stuff out of shell scripts.   Anyway,
> your discovery kills the idea of being able to just ignore the higher signal
> traps...   The following implements the same idea as my second patch
> in hopefully a slightly more cross-shell compatible way;  it works on bash
> and zsh at least.

Ooops, not even close on that attempt :o/   Here's a version that really does
work on zsh and bash; and should work on all shells.

Sean

diff --git a/git-cherry.sh b/git-cherry.sh
index 1a62320..4925f1f 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -49,7 +49,9 @@ ours=`git-rev-list $ours ^$limit` || exi
 tmp=.cherry-tmp$$
 patch=$tmp-patch
 mkdir $patch
-trap "rm -rf $tmp-*" 0 1 2 3 15
+cleanup() { rm -rf $tmp-*; }
+trap cleanup 0
+trap "cleanup;trap 0;exit 1" 1 2 3 15
 
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-format-patch.sh b/git-format-patch.sh
index 7e67c4e..574a79c 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -77,7 +77,9 @@ tt)
 esac
 
 tmp=.tmp-series$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+cleanup() { rm -f $tmp-*; }
+trap cleanup 0
+trap "cleanup;trap 0;exit 1" 1 2 3 15
 
 series=$tmp-series
 commsg=$tmp-commsg
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index f699268..0259a88 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -38,7 +38,9 @@ peek_repo="$(get_remote_url "$@")"
 shift
 
 tmp=.ls-remote-$$
-trap "rm -fr $tmp-*" 0 1 2 3 15
+cleanup() { rm -rf $tmp-*; }
+trap cleanup 0
+trap "cleanup;trap 0;exit 1" 1 2 3 15
 tmpdir=$tmp-d
 
 case "$peek_repo" in
diff --git a/git-reset.sh b/git-reset.sh
index 6c9e58a..3336690 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -4,7 +4,9 @@ USAGE='[--mixed | --soft | --hard]  [<co
 . git-sh-setup
 
 tmp=${GIT_DIR}/reset.$$
-trap 'rm -f $tmp-*' 0 1 2 3 15
+cleanup() { rm -f $tmp-*; }
+trap cleanup 0
+trap "cleanup;trap 0;exit 1" 1 2 3 15
 
 reset_type=--mixed
 case "$1" in