Re: [PATCH] bootstrap: don't exit with status 0 on errors

[Stefano Lattarini]

On 07/21/2012 09:20 PM, Eric Blake wrote:
> On 07/21/2012 12:02 PM, Stefano Lattarini wrote:
>> * bootstrap (EXIT_FAILURE): Actually initialize to '1'.  Otherwise,
>> the function 'func_fatal_error' will actually and unconditionally
>> exit with status 0 (yikes).
>> (EXIT_SUCCESS): Initialize to 0.  This missing initialization wasn't
>> causing any actual bug for now, but could easily start doing so in
>> the future.
> 
> Eww.  Looks like this has been around since commit 2d8b924, in Jul 2006.
>  :(  And it's not the only bug; we were unconditionally invoking:
> 
> $exit_cmd $EXIT_FAILURE
> 
> which happens to be a no-op in the common case where both variables are
> undefined, but would be a syntax error with your patch (unless a script
> named '1' exists in your PATH) or unexpected behavior if exit_cmd were
> pre-populated in the calling environment.
>
Oh, I missed that.  Should have done better testing, sorry.

>> diff --git a/bootstrap b/bootstrap
>> index b5d740c..db37209 100755
>> --- a/bootstrap
>> +++ b/bootstrap
>> @@ -49,6 +49,9 @@
>>  : ${RM='rm -f'}
>>  : ${SED=sed}
>>  
>> +EXIT_SUCCESS=0
>> +EXIT_FAILURE=1
> 
> This isn't C - rather than go through indirection, we might as well just
> directly use the exit values.
>
Fine by me.  I just took what seemed to me the shortest path out of the
predicament.

> I'm planning on pushing this instead to branch-1.4, and I really need to
> find time to bring the branch-1.6 and master branches up to speed.
>
Looks good, thanks.  Just a (non-rethorical) question: while you are at
it, wouldn't it be worth converting m4 to the use of the bootstrap script
provided by gnulib?  That would improve consistency with several others
GNU packages.

Regards,
  Stefano