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

[Jim Meyering]

Eric Blake wrote:
...
>> 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?).

Fine with me.

___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Stefano Lattarini]

[SNIP]

Eric Blake wrote:
>>>
>>> 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?).

I replied:
>>
>> 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.

Eric replied:
>
> Would we move all .pm files, or just the .pm files that both automake
> and autoconf are sharing?  (The list of shared files is:
> Configure_ac.pm, Channels.pm, FileUtils.pm, Getopt.pm, XFile.pm)
> 
I'd move only the shared files as a first step, for sake of simplicity.
Once more syncing and testing infrastructure is in place, we might
decide to move other files as well (if that will be deemed to offer
some advantage).  No need to hurry or overdo, right?

Thanks,
  Stefano


___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Eric Blake]

On 04/26/2012 05:12 AM, Stefano Lattarini wrote:
> 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+"?

I hadn't thought about that - but we would indeed need to add some
smarts to XFile to handle it properly.

>>
>> 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.

Would we move all .pm files, or just the .pm files that both automake
and autoconf are sharing?  (The list of shared files is:
Configure_ac.pm, Channels.pm, FileUtils.pm, Getopt.pm, XFile.pm)

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Stefano Lattarini]

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

___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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  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   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Stefano Lattarini]

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  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:
> 
> 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 (@_))
>  {
>fatal "cannot open $file: $!";
>  }
> 
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.

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?

Thanks, and sorry for the confusion,
  Stefano

___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Eric Blake]

On 04/24/2012 04:50 PM, Russ Allbery wrote:
> Eric Blake  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:

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 (@_))
 {
   fatal "cannot open $file: $!";
 }

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf

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

[Russ Allbery]

Eric Blake  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 "-".

-- 
Russ Allbery (r...@stanford.edu) 

___
Autoconf mailing list
Autoconf@gnu.org
https://lists.gnu.org/mailman/listinfo/autoconf