Re: RFC: PATCH to avoid linking multiple front ends at once with parallel make

[Alexandre Oliva <aoliva at redhat dot com>]

On May 14, 2013, Jason Merrill <jason@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