Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[gregor herrmann]

On Fri, 14 Apr 2017 06:38:44 +1000, Ben Finney wrote:

> On 13-Apr-2017, gregor herrmann wrote:
> > So basically new({'Vcs_Git'.. and new({'vCs-GiT'.. both create a
> > working object, both actions have a working Vcs-Git() method which
> > returns the correct value, and both don't have a vCs_GiT() method.
> Thank you for writing test cases! Yes, that behaviour sounds right to
> me.

Great, thanks for the confirmation! 

Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Tom Waits: Temptation


signature.asc
Description: Digital Signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Ben Finney]

On 13-Apr-2017, gregor herrmann wrote:

> So basically new({'Vcs_Git'.. and new({'vCs-GiT'.. both create a
> working object, both actions have a working Vcs-Git() method which
> returns the correct value, and both don't have a vCs_GiT() method.

Thank you for writing test cases! Yes, that behaviour sounds right to
me.

-- 
 \“My house is made out of balsa wood, so when I want to scare |
  `\ the neighborhood kids I lift it over my head and tell them to |
_o__) get out of my yard or I'll throw it at them.” —Steven Wright |
Ben Finney 


signature.asc
Description: PGP signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Ben Finney]

On 13-Apr-2017, Alex Muntada wrote:

> The patch provided below accepts any valid field ignoring case
> by using the canonical name instead, so VCS-Git, vCS-gIT, etc.
> will end up being Vcs-Git. Would that be compliant with Policy?

My Perl-fu is not up to the task of understanding the changes there.

Your description of the behaviour does conform with my reading of
Policy §5.1.

-- 
 \   “The best ad-libs are rehearsed.” —Graham Kennedy |
  `\   |
_o__)  |
Ben Finney 


signature.asc
Description: PGP signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[gregor herrmann]

Control: tag -1 + patch pending

On Thu, 13 Apr 2017 15:54:19 +0200, Alex Muntada wrote:

> > So, with the above example, if each those field names were accepted,
> > and they all map to the Perl object accessor ‘Vcs_Git’, and other Perl
> > code still needed to spell the *accessor* name that way, that would
> > IMO be compliant with Policy and would resolve this bug report.
> The patch provided below accepts any valid field ignoring case
> by using the canonical name instead, so VCS-Git, vCS-gIT, etc.
> will end up being Vcs-Git. Would that be compliant with Policy?

Cool, this looks good to me.
(Now committed in git.)

To check what it's doing and that it's doing what we want I added
some tests to t/Control.t:

+# canonical / case-insensitive field names/accessors
+lives_ok { $s = Debian::Control::Stanza::Source->new({'Vcs_Git' => 
'git://example.org'}) } 'Source constructur with Vcs_Git';
+can_ok($s, qw(Vcs_Git));
+ok($s->Vcs_Git eq 'git://example.org', 'Vcs_Git returns correct value');
+throws_ok { $s->vCs_GiT } qr/Can't locate object method "vCs_GiT" via package 
"Debian::Control::Stanza::Source"/, 'No method vCs_GiT';
+
+lives_ok { $s = Debian::Control::Stanza::Source->new({'vCs-GiT' => 
'git://example.net'}) } 'Source constructur with vCs-GiT';
+can_ok($s, qw(Vcs_Git));
+ok($s->Vcs_Git eq 'git://example.net', 'Vcs_Git returns correct value');
+throws_ok { $s->vCs_GiT } qr/Can't locate object method "vCs_GiT" via package 
"Debian::Control::Stanza::Source"/, 'No method vCs_GiT';


So basically new({'Vcs_Git'.. and new({'vCs-GiT'.. both create a
working object, both actions have a working Vcs-Git() method which
returns the correct value, and both don't have a vCs_GiT() method.

If this is what we want, and I think see, we seem to be settled :)


Cheers,
gregor


-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Don McLean: Wonderful baby


signature.asc
Description: Digital Signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Alex Muntada]

Ben Finney:

> So, with the above example, if each those field names were accepted,
> and they all map to the Perl object accessor ‘Vcs_Git’, and other Perl
> code still needed to spell the *accessor* name that way, that would
> IMO be compliant with Policy and would resolve this bug report.

The patch provided below accepts any valid field ignoring case
by using the canonical name instead, so VCS-Git, vCS-gIT, etc.
will end up being Vcs-Git. Would that be compliant with Policy?

Cheers!
Alex

diff --git a/lib/Debian/Control/Stanza.pm b/lib/Debian/Control/Stanza.pm
index f6642e6..3036e14 100644
--- a/lib/Debian/Control/Stanza.pm
+++ b/lib/Debian/Control/Stanza.pm
@@ -48,9 +48,17 @@ L class.
 
 use constant fields => ();
 
+my %canonical;
+
 sub import {
 my( $class ) = @_;
 
+# map the accessor name for the lower case equivalent
+%canonical = map (
+( lc($_) => $_ ),
+$class->fields,
+);
+
 $class->mk_accessors( $class->fields );
 }
 
@@ -87,6 +95,8 @@ sub new {
 
 while( my($k,$v) = each %$init ) {
 $k =~ s/-/_/g;
+# translate field name into the accessor canonical name
+$k = $canonical{ lc $k } || $k;
 $self->can($k)
 or croak "Invalid field given ($k)";
 $self->$k($v);


signature.asc
Description: Digital signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Ben Finney]

On 12-Apr-2017, gregor herrmann wrote:
> Maybe someone has an idea how we can simulate something like
> case-insensitive method names?

For resolving this bug, I think the method names don't need to change
and don't need to be case-insensitive.

The only behaviour needing resolution for this bug report is that
valid field names from the control file are rejected.

Policy does not mandate the way a Perl library makes accessors on its
objects, for example. It merely mandates that the names from the
control files must be interpreted case-insensitive.

> On Mon, 10 Apr 2017 22:37:28 +1000, Ben Finney wrote:
> > For example, the names “vcs-git”, “Vcs-Git”, “VCS-Git”, “vcS-gIt”
> > should all be interpreted as the same field name by
> > ‘libdebian-source-perl’.

So, with the above example, if each those field names were accepted,
and they all map to the Perl object accessor ‘Vcs_Git’, and other Perl
code still needed to spell the *accessor* name that way, that would
IMO be compliant with Policy and would resolve this bug report.

-- 
 \“I fly Air Bizarre. You buy a combination one-way round-trip |
  `\ticket. Leave any Monday, and they bring you back the previous |
_o__) Friday. That way you still have the weekend.” —Steven Wright |
Ben Finney 


signature.asc
Description: PGP signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Jonas Smedegaard]

Quoting gregor herrmann (2017-04-12 23:30:08)
> Control: tag -1 + confirmed
> 
> On Mon, 10 Apr 2017 22:37:28 +1000, Ben Finney wrote:
> 
> > Given a Perl module::
> > 
> > use Debian::Control::Stanza::Source;
> > 
> > my $s = Debian::Control::Stanza::Source->new( {
> > 'Build-Depends' => "debhelper",
> > 'VCS-Git' => "https://example.org/;,
> > } );
> > 
> > The above code causes the error message::
> > 
> > Invalid field given (VCS_Git) at ./foo.pl line 6.
> > 
> > The spelling “VCS-Git” is not invalid. Debian Policy §5.1 specifies
> > that “Field names are not case-sensitive […]”.
> 
> Thank you, that's indeed an issue.
>  
> > The library should allow field names without regard to their
> > capitalisation. For example, the names “vcs-git”, “Vcs-Git”,
> > “VCS-Git”, “vcS-gIt” should all be interpreted as the same field name
> > by ‘libdebian-source-perl’.
> 
> Right. Unfortunately that's not so easy, as these field names are not
> just strings but they are also automatically generated accessors; the
> error message above is in fact the user-friendly version of the
> underlying problem:
> 
> 
> % perl -MDebian::Control::Stanza::Source -E '$s = 
> Debian::Control::Stanza::Source->new(); say $s->Vcs_Git; say $s->VCS_Git;'
> 
> Can't locate object method "VCS_Git" via package 
> "Debian::Control::Stanza::Source" at -e line 1.
> 
> 
> Maybe someone has an idea how we can simulate something like
> case-insensitive method names?

A dirty but (I guess) effective approach would be to compute all 
upper- and lowercase combinations, and create aliases for the accessors.

For Moo there is MooX::Aliases to help with the latter part - but seems 
Debian::Source does not use Moo.


 - Jonas

-- 
 * Jonas Smedegaard - idealist & Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private


signature.asc
Description: signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Alex Muntada]

gregor herrmann:


Maybe someone has an idea how we can simulate something
like case-insensitive method names?


AUTOLOAD comes to mind, but that's something we'd probably like to avoid.
Normalizing the method names is the alternative: replace dashes with
underscores, make the whole method name smallcase, then call it.

I can take a look at the actual code tomorrow and see how difficult it'd be.

Cheers!
Alex

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[gregor herrmann]

Control: tag -1 + confirmed

On Mon, 10 Apr 2017 22:37:28 +1000, Ben Finney wrote:

> Given a Perl module::
> 
> use Debian::Control::Stanza::Source;
> 
> my $s = Debian::Control::Stanza::Source->new( {
> 'Build-Depends' => "debhelper",
> 'VCS-Git' => "https://example.org/;,
> } );
> 
> The above code causes the error message::
> 
> Invalid field given (VCS_Git) at ./foo.pl line 6.
> 
> The spelling “VCS-Git” is not invalid. Debian Policy §5.1 specifies
> that “Field names are not case-sensitive […]”.

Thank you, that's indeed an issue.
 
> The library should allow field names without regard to their
> capitalisation. For example, the names “vcs-git”, “Vcs-Git”,
> “VCS-Git”, “vcS-gIt” should all be interpreted as the same field name
> by ‘libdebian-source-perl’.

Right. Unfortunately that's not so easy, as these field names are not
just strings but they are also automatically generated accessors; the
error message above is in fact the user-friendly version of the
underlying problem:


% perl -MDebian::Control::Stanza::Source -E '$s = 
Debian::Control::Stanza::Source->new(); say $s->Vcs_Git; say $s->VCS_Git;'

Can't locate object method "VCS_Git" via package 
"Debian::Control::Stanza::Source" at -e line 1.


Maybe someone has an idea how we can simulate something like
case-insensitive method names?


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Van Morrison


signature.asc
Description: Digital Signature

Bug#860023: libdebian-source-perl: Rejects valid field names based on letter case

[Ben Finney]

Package: libdebian-source-perl
Version: 0.94
Severity: normal

The ‘Debian::Control::Stanza’ constructor assumes that field names
must have the exact case of the names as defined in that library. This
assumption is incorrect.

Given a Perl module::

use Debian::Control::Stanza::Source;

my $s = Debian::Control::Stanza::Source->new( {
'Build-Depends' => "debhelper",
'VCS-Git' => "https://example.org/;,
} );

The above code causes the error message::

Invalid field given (VCS_Git) at ./foo.pl line 6.

The spelling “VCS-Git” is not invalid. Debian Policy §5.1 specifies
that “Field names are not case-sensitive […]”.

The library should allow field names without regard to their
capitalisation. For example, the names “vcs-git”, “Vcs-Git”,
“VCS-Git”, “vcS-gIt” should all be interpreted as the same field name
by ‘libdebian-source-perl’.

-- 
 \ “I have yet to see any problem, however complicated, which, |
  `\  when you looked at it in the right way, did not become still |
_o__)more complicated.” —Paul Anderson |
Ben Finney 


signature.asc
Description: PGP signature