Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist.

[Gary V. Vaughan]

On 21 Sep 2010, at 12:07, Ralf Wildenhues wrote:
> Hi Gary,

Hallo Ralf,

Thanks again for the review.

> * Gary V. Vaughan wrote on Tue, Sep 21, 2010 at 03:05:46AM CEST:
>> Well, it does at least show that the script interacts correctly with
>> an error for make to help catch the case where someone commits a change
>> to the first paragraph of README without a matching edit to the sed
>> expressions in edit-readme-alpha.
>> 
>> Here is (an overkill) patch to not exit with an error, so that `make
>> distcheck' can complete.
>> 
>> Okay to push?
> 
> Well, I don't want to appear overly nitpicky for this issue,
> but if you don't absolutely need to check and skip non-writable files,
> I wouldn't do it, and rather error out in that case.

Well, the thing that is screwing up distcheck is when the distribution
tarball is unpacked into a read-only tree, and edit-readme-alpha wants
to rewrite the file again...

...although, the "don't re-edit when the alpha text is already there"
check later on should catch this already.  Although I feel happier with
both checks in place, I'll remove the writeable test if you'd prefer.

>> * libltdl/config/edit-readme-alpha: If README is non-writable
>> assume that it is being run from distcheck, and bail out with
>> a warning
> 
> FWIW, exiting 0 is not what I'd call "bail out".

I mean "bail out" as in: don't do the rest of the processing.

>> for file in "$@"; do
>> +  # Assume that read-only README indicates that we are running inside
>> +  # the latter part of a `make distcheck'.
>> +  test -w $file || {
>> +    echo "$progname: not editing non-writeable \`$file' (distcheck?)"
> 
> If you prefer to, or need to keep the warning+skip, I suggest to
> print warnings on stderr.

Oops.  Well caught, thanks.

>> +    continue
>> +  }
>> +
>>   # Make sure the paragraph we are matching has not been edited since
>>   # this script was written.
>> -  matched=`sed -n -e '/^This is GNU Libtool,/,/^interface.$/p' $file \
>> +  matched=`sed -n -e '/^This is GNU Libtool,/,/^interface\.$/p' $file \
>>       |wc -l |sed 's|^ *||'`
>> +
>> +  # Unless, of course, it was edited by this script already.
>> +  test 3 = "$matched" \
>> +      || matched=`sed -n -e '/^This is an alpha testing release/,/behind a 
>> consistent, portable interface\.$/p' $file \
>> +      |wc -l |sed 's|^ *||'`
> 
> Indentation is a bit weird here, I'd have expected the | to align one
> after the ` in the line above.

Okay.  I was lining it up with previous clause, but it is ugly.  I'll fix
that too.

With those addressed, and another successful distcheck, okay to push?

Cheers,
-- 
Gary V. Vaughan (address@hidden)

PGP.sig
Description: This is a digitally signed message part