On May 14, 2013, Jason Merrill <ja...@redhat.com> wrote: >> trap 'rmdir "$lockdir"; exit $status' 0 1 2 15
> In my testing I found that trapping signals other than 0 resulted in > trying to rmdir twice if the process is interrupted. Interesting... I guess it's one for the signal and one for the exit... I borrowed the general approach from libtool's shell locking machinery. I suppose it might be the case that lock files (in libtool's case) get removed twice, but I'm pretty sure there was broad portability testing in the process that led to that construct. Indeed, I vaguely recall some oddities about exit status preservation involving such obnoxious constructs as 'rm ....; (exit $status); exit', but I won't even pretend to recall the rationale for that ;-) > The extra exit here also seems unnecessary. Wouldn't the exit status of an interrupted or failed compilation be that of the successful rmdir rather than what you'd get without the wrapper? > And I added a check so that the script gives up and steals the lock > after waiting 5 minutes. I'm now a bit concerned about the portability of some constructs you used there, although the use of this script is only enabled manually. My concern is that, in spite of: > +#! /bin/bash the script is ran with $(SHELL), which IIRC is typically /bin/sh, not necessarily bash. > + if [ $((count++ % 30)) == 0 ]; then Let's make this more portable, shall we? How about count=`expr $count + 1` case $count in *0) this goes from 30 to 10 seconds, and it doesn't match the first iteration (should it?) > + # Reset if the lock has been renewed. > + if [ "$lockdir" -nt lock-stamp.$$ ]; then Hmm... I'm not sure how portable -nt is, but there's always case `ls -dt $lockdir lock-stamp.$$` in lock-stamp.$$*) ;; # Our stamp file is newer. *) ... > + echo waiting to acquire $lockdir Please make it >&2 > +echo $prog "$@" > +$prog "$@" I wonder if some day someone will be tempted to add an “exec” to the final line, without realizing that it will cause the lock dir to be released too early if at all. Perhaps a comment at the end noting the trap will release the lock would avoid this mistake and make the whole thing clearer. It would probably be wise to add lock-stamp.* to the make clean rules. Patch with the above change (the last one) is pre-approved, but if the other proposed portability changes make sense to you (even though you documented that bash is required), please put them in too. Thanks! -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer