Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] fix Module::CoreList::is_core bounds checking for specific module versions #15308

Closed
p5pRT opened this issue May 6, 2016 · 12 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented May 6, 2016

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

Searchable as RT128089$

@p5pRT
Copy link
Author

p5pRT commented May 6, 2016

From @karenetheridge

  $ corelist File​::Temp

  Data for 2016-04-29
  File​::Temp was first released with perl v5.6.1
  $ perl -MModule​::CoreList -wle'print Module​::CoreList​::is_core("File​::Temp", "0", "5.006001")'
  0
  $ perl -MModule​::CoreList -wle'print Module​::CoreList​::is_core("File​::Temp", "0", "5.006002")'
  1

A fix is attached, containing tests that demonstrate the issue.

@p5pRT
Copy link
Author

p5pRT commented May 6, 2016

From @karenetheridge

0001-fix-Module-CoreList-is_core-bounds-checking-for-spec.patch
From fc469c554eb27e3657db236d2bdc5c45a89a7daa Mon Sep 17 00:00:00 2001
From: Karen Etheridge <ether@cpan.org>
Date: Fri, 6 May 2016 13:36:17 -0700
Subject: [PATCH] fix Module::CoreList::is_core bounds checking for specific
 module versions

---
 dist/Module-CoreList/Changes                | 6 +++++-
 dist/Module-CoreList/lib/Module/CoreList.pm | 2 +-
 dist/Module-CoreList/t/is_core.t            | 9 ++++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/dist/Module-CoreList/Changes b/dist/Module-CoreList/Changes
index b254182..0c20e2c 100644
--- a/dist/Module-CoreList/Changes
+++ b/dist/Module-CoreList/Changes
@@ -1,8 +1,12 @@
+
+ - fixed edge-case checking in is_core() (checking for a specific version of
+   a module returned false for the first perl release that contained it)
+
 5.20160429
  - Updated for v5.22.2
 
 5.20160320
- - Updated vor v5.23.9
+ - Updated for v5.23.9
 
 5.20160228
   - [perl #127624] corelist: wrong Digest::SHA version in 5.18.4
diff --git a/dist/Module-CoreList/lib/Module/CoreList.pm b/dist/Module-CoreList/lib/Module/CoreList.pm
index 1fcae7b..3decefa 100644
--- a/dist/Module-CoreList/lib/Module/CoreList.pm
+++ b/dist/Module-CoreList/lib/Module/CoreList.pm
@@ -12449,7 +12449,7 @@ sub is_core
         }
         RELEASE:
         foreach my $prn (@releases) {
-            next RELEASE if $prn <= $first_release;
+            next RELEASE if $prn < $first_release;
             last RELEASE if $prn > $perl_version;
             next unless defined(my $next_module_version
                                    = $delta{$prn}->{changed}->{$module});
diff --git a/dist/Module-CoreList/t/is_core.t b/dist/Module-CoreList/t/is_core.t
index cc06a21..1ddc260 100644
--- a/dist/Module-CoreList/t/is_core.t
+++ b/dist/Module-CoreList/t/is_core.t
@@ -1,7 +1,7 @@
 #!perl -w
 use strict;
 use Module::CoreList;
-use Test::More tests => 38;
+use Test::More tests => 43;
 
 BEGIN { require_ok('Module::CoreList'); }
 
@@ -28,6 +28,13 @@ ok(Module::CoreList::is_core('attributes', undef, '5.006001') == 1, "attributes
 ok(Module::CoreList::is_core('Pod::Plainer', undef, '5.012001') == 1, "Pod::Plainer was core in 5.12.1");
 ok(Module::CoreList::is_core('Pod::Plainer', undef, '5.016003') == 0, "Pod::Plainer was removed in 5.13.1");
 
+ok(!Module::CoreList::is_core('File::Temp', 0, '5.006'), 'File::Temp is not in 5.006000');
+ok(Module::CoreList::is_core('File::Temp', 0, '5.006001'), 'File::Temp is in 5.006001');
+ok(!Module::CoreList::is_core('File::Temp', '0.12', '5.006'), 'File::Temp 0.12 is not in 5.006000');
+ok(Module::CoreList::is_core('File::Temp', '0.12', '5.006001'), 'File::Temp 0.12 is in 5.006001');
+ok(Module::CoreList::is_core('File::Temp', '0.12', '5.006002'), 'File::Temp 0.12 is in 5.006002');
+
+
 # history of module 'encoding' in core
 #   version 1.00 included in 5.007003
 #   version 1.35 included in 5.008
-- 
2.6.2

@p5pRT
Copy link
Author

p5pRT commented May 6, 2016

From @jkeenan

On Fri May 06 14​:20​:15 2016, ether wrote​:

$ corelist File​::Temp

Data for 2016-04-29
File​::Temp was first released with perl v5.6.1
$ perl -MModule​::CoreList -wle'print
Module​::CoreList​::is_core("File​::Temp", "0", "5.006001")'
0
$ perl -MModule​::CoreList -wle'print
Module​::CoreList​::is_core("File​::Temp", "0", "5.006002")'
1

A fix is attached, containing tests that demonstrate the issue.

Thanks. Smoking in branch​:

smoke-me/jkeenan/128089-corelist-is-core

I was a bit surprised that I did *not* have to increment the $VERSION in CoreList.pm.

I'll leave it to the pumpking to decide whether this can go into 5.24.0 or should wait for 5.25.1.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented May 6, 2016

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

@p5pRT
Copy link
Author

p5pRT commented May 7, 2016

From @karenetheridge

On Fri May 06 16​:52​:15 2016, jkeenan wrote​:

Thanks. Smoking in branch​:

smoke-me/jkeenan/128089-corelist-is-core

I was a bit surprised that I did *not* have to increment the $VERSION
in CoreList.pm.

Yes, I left the $VERSION untouched as it is set when it is released.

I'll let the pumpking(s) worry about what releases it should go in (i.e. after the MCL release that contains the 5.24.0 bump, or before) :)

@p5pRT
Copy link
Author

p5pRT commented May 7, 2016

From @rjbs

* James E Keenan via RT <perlbug-followup@​perl.org> [2016-05-06T19​:52​:15]

I'll leave it to the pumpking to decide whether this can go into 5.24.0 or
should wait for 5.25.1.

It'll keep. ;)

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

From @karenetheridge

On Fri May 06 17​:15​:28 2016, perl.p5p@​rjbs.manxome.org wrote​:

* James E Keenan via RT <perlbug-followup@​perl.org> [2016-05-06T19​:52​:15]

I'll leave it to the pumpking to decide whether this can go into 5.24.0 or
should wait for 5.25.1.

It'll keep. ;)

Could this be merged now, please? I would like to get a fixed Module​::CoreList out to the CPAN soon. thanks!

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

From @khwilliamson

Thanks,

Merged as 6af6e59
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented May 17, 2016

@khwilliamson - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2016

From @karenetheridge

On Tue May 17 13​:41​:27 2016, khw wrote​:

Thanks,

Merged as 6af6e59

This was in v5.25.1.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT p5pRT closed this as completed May 30, 2017
@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant