[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [FYI] {master} refactor: use modern semantics of 'open'
From: |
Stefano Lattarini |
Subject: |
Re: [FYI] {master} refactor: use modern semantics of 'open' |
Date: |
Thu, 26 Apr 2012 13:12:48 +0200 |
Hi Eric, sorry for the delay.
On 04/25/2012 01:25 AM, Eric Blake wrote:
>
> 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"))
>>>
What if "mode" is something like "w" or "r+"?
>>> + {
>>> + 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.
>
Makes sense.
>>
>> 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?).
>
This could be a good "interim" solution, as I the paperwork in place for
Gnulib, as well as pushing rights for its repository. Unit tests can be
added later, as the need arise, and/or slowly step-by-step when we feel
like doing so.
Thanks,
Stefano