Some version strings in %Module::CoreList::delta contain trailing space and cause warning with version->parse

[p5pRT]

Migrated from rt.perl.org#124364 (status was 'resolved')

Searchable as RT124364$

[p5pRT]

From perlancar@cpan.org

Some version strings in %Module​::CoreList​::delta contain trailing space and cause warning with version->parse, e.g. there is one '1.15 ' which when fed to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e line 1.
1.15

Complete list​:

% perl -MModule​::CoreList -E'my $d = \%Module​::CoreList​::delta; for my $rel (keys %$d) { my $rd = $d->{$rel}; for my $t (keys %$rd) { my $h = $rd->{$t}; for my $mod (keys %$h) { my $v = $h->{$mod}; say "$rel -> $t -> $mod​: <$v>" if $v =~ /\s/ } } }'
5.007003 -> changed -> CPAN​::FirstTime​: <1.54 >
5.008001 -> changed -> CPAN​::FirstTime​: <1.60 >
5.00504 -> changed -> ExtUtils​::Liblist​: <1.25 >
5.00504 -> changed -> CPAN​::FirstTime​: <1.36 >
5.00504 -> changed -> ExtUtils​::Install​: <1.28 >
5.00504 -> changed -> ExtUtils​::Manifest​: <1.33 >
5.00504 -> changed -> ExtUtils​::MM_Unix​: <1.12602 >
5.00504 -> changed -> ExtUtils​::Mkbootstrap​: <1.14 >
5.00504 -> changed -> ExtUtils​::testlib​: <1.11 >
5.00504 -> changed -> ExtUtils​::Mksymlists​: <1.17 >
5.00307 -> changed -> ExtUtils​::Mksymlists​: <1.12 >
5.00307 -> changed -> ExtUtils​::MM_Unix​: <1.107 >
5.00307 -> changed -> ExtUtils​::Mkbootstrap​: <1.13 >
5.00307 -> changed -> ExtUtils​::testlib​: <1.11 >
5.00307 -> changed -> ExtUtils​::Liblist​: <1.20 >
5.00307 -> changed -> ExtUtils​::Install​: <1.15 >
5.008 -> changed -> CPAN​::FirstTime​: <1.56 >
5.004 -> changed -> ExtUtils​::Mksymlists​: <1.13 >
5.004 -> changed -> CPAN​::FirstTime​: <1.18 >
5.004 -> changed -> ExtUtils​::Manifest​: <1.33 >
5.004 -> changed -> ExtUtils​::Liblist​: <1.2201 >
5.004 -> changed -> ExtUtils​::MM_Unix​: <1.114 >
5.004 -> changed -> ExtUtils​::Install​: <1.16 >
5.006002 -> changed -> CPAN​::FirstTime​: <1.53 >
5.008000 -> changed -> CPAN​::FirstTime​: <1.56 >
5.00405 -> changed -> ExtUtils​::MM_Unix​: <1.118 >
5.00405 -> changed -> ExtUtils​::Mkbootstrap​: <1.14 >
5.00405 -> changed -> ExtUtils​::Install​: <1.28 >
5.00405 -> changed -> ExtUtils​::Liblist​: <1.25 >
5.00405 -> changed -> ExtUtils​::Mksymlists​: <1.16 >
5.00405 -> changed -> CPAN​::FirstTime​: <1.30 >
5.006000 -> changed -> CPAN​::FirstTime​: <1.38 >
5.006000 -> changed -> ExtUtils​::MM_Unix​: <1.12603 >
5.005 -> changed -> ExtUtils​::MM_Unix​: <1.12601 >
5.005 -> changed -> ExtUtils​::Mkbootstrap​: <1.13 >
5.005 -> changed -> CPAN​::FirstTime​: <1.29 >
5.005 -> changed -> ExtUtils​::Mksymlists​: <1.17 >
5.006 -> changed -> CPAN​::FirstTime​: <1.38 >
5.006 -> changed -> ExtUtils​::MM_Unix​: <1.12603 >

[p5pRT]

From @rjbs

* perlancar <perlbug-followup@​perl.org> [2015-04-23T02​:15​:31]

Some version strings in %Module​::CoreList​::delta contain trailing space and
cause warning with version->parse, e.g. there is one '1.15 ' which when fed
to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e line 1.
1.15

Thanks, I believe I have just fixed all this with b7c1f08.

I see that you probably found this while working on Module​::CoreList​::More,
which contains, among other things, a "faster is_core." Is there a reason we
shouldn't use your implementation in Module​::CoreList?

It seems a shame to leave it in an add-on module that many users would never
see.

--
rjbs

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @perlancar

I'd love to get it integrated, but we should probably get at least a couple
of other people looking at the code first to check its correctness first.

On Thu, Apr 23, 2015 at 7​:24 PM, Ricardo Signes via RT <
perlbug-followup@​perl.org> wrote​:

* perlancar <perlbug-followup@​perl.org> [2015-04-23T02​:15​:31]

Some version strings in %Module​::CoreList​::delta contain trailing space
and
cause warning with version->parse, e.g. there is one '1.15 ' which when
fed
to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e line
1.
1.15

Thanks, I believe I have just fixed all this with b7c1f08.

I see that you probably found this while working on Module​::CoreList​::More,
which contains, among other things, a "faster is_core." Is there a reason
we
shouldn't use your implementation in Module​::CoreList?

It seems a shame to leave it in an add-on module that many users would
never
see.

--
rjbs

[p5pRT]

From @jkeenan

On Thu Apr 23 21​:45​:43 2015, perlancar@​gmail.com wrote​:

I'd love to get it integrated, but we should probably get at least a
couple
of other people looking at the code first to check its correctness
first.

On Thu, Apr 23, 2015 at 7​:24 PM, Ricardo Signes via RT <
perlbug-followup@​perl.org> wrote​:

* perlancar <perlbug-followup@​perl.org> [2015-04-23T02​:15​:31]

Some version strings in %Module​::CoreList​::delta contain trailing
space
and
cause warning with version->parse, e.g. there is one '1.15 ' which
when
fed
to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e
line
1.
1.15

Thanks, I believe I have just fixed all this with b7c1f08.

I see that you probably found this while working on
Module​::CoreList​::More,
which contains, among other things, a "faster is_core." Is there a
reason
we
shouldn't use your implementation in Module​::CoreList?

It seems a shame to leave it in an add-on module that many users
would
never
see.

--
rjbs

I've created a project to evaluate Module-CoreList-More at the New York Perl Hackathon, coming up on May 2.

https://github.com/nyperlmongers/nyperlhackathon2015/wiki/Evaluate-Module-CoreList-More

So perhaps we'll get some eyeballs on this there.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @haarg

On Wed Apr 22 23​:15​:30 2015, perlancar@​cpan.org wrote​:

Some version strings in %Module​::CoreList​::delta contain trailing
space and cause warning with version->parse, e.g. there is one '1.15 '
which when fed to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e line
1.
1.15

Complete list​:

% perl -MModule​::CoreList -E'my $d = \%Module​::CoreList​::delta; for my
$rel (keys %$d) { my $rd = $d->{$rel}; for my $t (keys %$rd) { my $h =
$rd->{$t}; for my $mod (keys %$h) { my $v = $h->{$mod}; say "$rel ->
$t -> $mod​: <$v>" if $v =~ /\s/ } } }'
5.007003 -> changed -> CPAN​::FirstTime​: <1.54 >
5.008001 -> changed -> CPAN​::FirstTime​: <1.60 >
5.00504 -> changed -> ExtUtils​::Liblist​: <1.25 >
5.00504 -> changed -> CPAN​::FirstTime​: <1.36 >
5.00504 -> changed -> ExtUtils​::Install​: <1.28 >
5.00504 -> changed -> ExtUtils​::Manifest​: <1.33 >
5.00504 -> changed -> ExtUtils​::MM_Unix​: <1.12602 >
5.00504 -> changed -> ExtUtils​::Mkbootstrap​: <1.14 >
5.00504 -> changed -> ExtUtils​::testlib​: <1.11 >
5.00504 -> changed -> ExtUtils​::Mksymlists​: <1.17 >
5.00307 -> changed -> ExtUtils​::Mksymlists​: <1.12 >
5.00307 -> changed -> ExtUtils​::MM_Unix​: <1.107 >
5.00307 -> changed -> ExtUtils​::Mkbootstrap​: <1.13 >
5.00307 -> changed -> ExtUtils​::testlib​: <1.11 >
5.00307 -> changed -> ExtUtils​::Liblist​: <1.20 >
5.00307 -> changed -> ExtUtils​::Install​: <1.15 >
5.008 -> changed -> CPAN​::FirstTime​: <1.56 >
5.004 -> changed -> ExtUtils​::Mksymlists​: <1.13 >
5.004 -> changed -> CPAN​::FirstTime​: <1.18 >
5.004 -> changed -> ExtUtils​::Manifest​: <1.33 >
5.004 -> changed -> ExtUtils​::Liblist​: <1.2201 >
5.004 -> changed -> ExtUtils​::MM_Unix​: <1.114 >
5.004 -> changed -> ExtUtils​::Install​: <1.16 >
5.006002 -> changed -> CPAN​::FirstTime​: <1.53 >
5.008000 -> changed -> CPAN​::FirstTime​: <1.56 >
5.00405 -> changed -> ExtUtils​::MM_Unix​: <1.118 >
5.00405 -> changed -> ExtUtils​::Mkbootstrap​: <1.14 >
5.00405 -> changed -> ExtUtils​::Install​: <1.28 >
5.00405 -> changed -> ExtUtils​::Liblist​: <1.25 >
5.00405 -> changed -> ExtUtils​::Mksymlists​: <1.16 >
5.00405 -> changed -> CPAN​::FirstTime​: <1.30 >
5.006000 -> changed -> CPAN​::FirstTime​: <1.38 >
5.006000 -> changed -> ExtUtils​::MM_Unix​: <1.12603 >
5.005 -> changed -> ExtUtils​::MM_Unix​: <1.12601 >
5.005 -> changed -> ExtUtils​::Mkbootstrap​: <1.13 >
5.005 -> changed -> CPAN​::FirstTime​: <1.29 >
5.005 -> changed -> ExtUtils​::Mksymlists​: <1.17 >
5.006 -> changed -> CPAN​::FirstTime​: <1.38 >
5.006 -> changed -> ExtUtils​::MM_Unix​: <1.12603 >

These versions with spaces are an accurate representation of what the versions were defined as. As an example, in perl 5.006, lib/ExtUtils/MM_Unix.pm, line 11​:

$VERSION = substr q$Revision​: 1.12603 $, 10;

This leaves a trailing space.

[p5pRT]

From @Leont

On Mon, Apr 27, 2015 at 4​:27 PM, Graham Knop via RT <
perlbug-followup@​perl.org> wrote​:

These versions with spaces are an accurate representation of what the
versions were defined as. As an example, in perl 5.006,
lib/ExtUtils/MM_Unix.pm, line 11​:

$VERSION = substr q$Revision​: 1.12603 $, 10;

This leaves a trailing space.

I would consider that an unhelpful artifact. Historically correct as it may
be, leaving it in only causes headaches and doesn't solve any problem I can
imagine.

Leon

[p5pRT]

From spam-bitcard@yary.ack.org

These versions with spaces are an accurate representation of what the
versions were defined as. As an example, in perl 5.006,
lib/ExtUtils/MM_Unix.pm, line 11​:

$VERSION = substr q$Revision​: 1.12603 $, 10;

This leaves a trailing space.

I would consider that an unhelpful artifact. Historically correct as it may
be, leaving it in only causes headaches and doesn't solve any problem I can
imagine.

The "version.pm" docs say that the above looks like a CVS revision, and "CVS $Revision$ increments differently from Decimal versions (i.e. 1.10 follows 1.9), so it must be handled as if it were a Dotted-Decimal Version" like "v1.12603"

which leads me to believe that a large number of version strings in Module​::CoreList should have a leading "v" on them so they will compare correctly. For example 'Module​::Pluggable' is at version '3.8', if it were to go to '3.10' then this would be a problem, whereas 'v3.10' would compare properly against 'v3.8'.

On the other hand, when I look at the actual version numbers, the authors seem to be sane about this.

That's beyond what I can do in today's Hackathon, but I will have a smaller pull request by the end of the day. It will also handle "CGI​::Fast" which has a letter in its version "1.00a", and letters are just plain wrong, according to version.pm. Full comments will be in the pull request.

[p5pRT]

From spam-bitcard@yary.ack.org

I've added a pull request to perl-Module-CoreList-More via github

Basically, the trailing space could probably be stripped from the core Module​::CoreList, but I don't think it's needed. As for Module​::CoreList​::Core handling these, I submitted a patch to strip the trailing space (and handle other trailing junk which version.pm complains about). Also this patch resumes using "warnings" which might alert us to issues in the future. The patch also adds a test to look for warnings.

And I think Module-CoreList-More could be folded into Module-CoreList. I would like to factor out common code from is_core/is_still_core, and list_core_modules/list_still_core_modules, but that's not a big deal.

[p5pRT]

From [Unknown Contact. See original ticket]

I've added a pull request to perl-Module-CoreList-More via github

Basically, the trailing space could probably be stripped from the core Module​::CoreList, but I don't think it's needed. As for Module​::CoreList​::Core handling these, I submitted a patch to strip the trailing space (and handle other trailing junk which version.pm complains about). Also this patch resumes using "warnings" which might alert us to issues in the future. The patch also adds a test to look for warnings.

And I think Module-CoreList-More could be folded into Module-CoreList. I would like to factor out common code from is_core/is_still_core, and list_core_modules/list_still_core_modules, but that's not a big deal.

[p5pRT]

From @jkeenan

On Thu Apr 23 05​:24​:18 2015, perl.p5p@​rjbs.manxome.org wrote​:

* perlancar <perlbug-followup@​perl.org> [2015-04-23T02​:15​:31]

Some version strings in %Module​::CoreList​::delta contain trailing
space and
cause warning with version->parse, e.g. there is one '1.15 ' which
when fed
to version->parse under warnings​:

% perl -wE'say version->parse("1.15 ")'
Version string '1.15 ' contains invalid data; ignoring​: ' ' at -e
line 1.
1.15

Thanks, I believe I have just fixed all this with b7c1f08.

I see that you probably found this while working on
Module​::CoreList​::More,
which contains, among other things, a "faster is_core." Is there a
reason we
shouldn't use your implementation in Module​::CoreList?

It seems a shame to leave it in an add-on module that many users would
never
see.

rjbs​: If we've addressed the problem which the OP reported, then I'd recommend closing this ticket and moving any discussion about a revision to Module-CoreList to a new RT.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

[p5pRT]

From @rjbs

So resolved.

--
rjbs

[p5pRT]

@rjbs - Status changed from 'open' to 'resolved'