Re: [FYI] {master} refactor: use modern semantics of 'open'

[Eric Blake]

On 04/24/2012 05:17 PM, Stefano Lattarini wrote:
> Hi Eric, Russ.
> 
> On 04/25/2012 01:08 AM, Eric Blake wrote:
>> On 04/24/2012 04:50 PM, Russ Allbery wrote:
>>> Eric Blake <address@hidden> writes:
>>>
>>>> Help!  I can't release autoconf 2.69 until I figure out how to work
>>>> around this patch.  After updating to the latest shared files, as well
>>>> as applying this patch, I'm now stuck with output going to a literal
>>>> file named '-' instead of going to stdout.  I suspect that the
>>>> conversion to the 2-arg form is mishandling our idiom of '-' as standard
>>>> in/out.
>>>
>>> If you call open with three arguments, "-" has no special meaning and
>>> refers to a file named "-" (since the whole point of three-argument open
>>> is to remove all magic interpretations of the filename string).  The
>>> easiest way to work around this is probably to change the Automake helper
>>> functions that sit between the code and the Perl open command and have
>>> them switch to calling open with two arguments if the file name is "-".
>>
>> Indeed, this hack gets me further, but still not complete success:

Actually, this appears to make _all_ the XFile uses work; all that
remains broken is places such as bin/autoupdate.in calling raw open
instead of using XFile.

>>
>> diff --git i/lib/Autom4te/XFile.pm w/lib/Autom4te/XFile.pm
>> index 19b73aa..95a452b 100644
>> --- i/lib/Autom4te/XFile.pm
>> +++ w/lib/Autom4te/XFile.pm
>> @@ -138,7 +138,14 @@ sub open
>>    # comment in IO::Handle.
>>    ${*$fh}{'autom4te_xfile_file'} = "$file";
>>
>> -  if (!$fh->SUPER::open (@_))
>> +  if (defined $mode && "$file" eq '-')
>> +    {
>> +      if (!$fh->SUPER::open ("$mode$file"))
>> +    {
>> +      fatal "cannot open $file: $!";
>> +    }
>> +    }
>> +  elsif (!$fh->SUPER::open (@_))

In other words, since the point of XFile is to ensure binmode, and to
make open nicer to use, making XFile the owner of '-' magic seems like
the right thing to do, and the real remaining autoconf bug is that we
aren't using XFile everywhere.

> My hope is that you'll manage to quickly find a patch (preferably to
> be applied to the Autoconf repository only) that can work around the
> issue effectively.  If you don't, I will (after Automake 1.12 is out)
> accept a commit reverting my patch, since keeping Autoconf sane and
> working is definitely, absolutely more worth than a having a ten-line
> simplification in Automake.

I don't think we need to revert your patch, so much as add the '-'
handling enhancement.  But I figured out a solution: release autoconf
2.69 _without_ fetching the latest from automake, which in turn will let
you release automake 1.12 depending on autoconf 2.69, and _then_ we fix
the fallout.

> 
> If anything, this issue shows that the Automake::XFile module (and
> other similar modules as well) should live in their own git
> repository, which both Autoconf and Automake should use as a
> submodule -- and which (for $DEITY's sake!), should be unit-tested.
> Any volunteer?

Alas, my perl-fu is weak enough that I'm not sure how good my
unit-testing attempts would be.  But moving the files to a common
repository seems doable (how about gnulib?).

> 
> Thanks, and sorry for the confusion,

And sorry I didn't review your automake patch queue sooner, since I
wasn't thinking about the impact to autoconf.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature