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

supply patches to upstream to fix install location #12722

Closed
p5pRT opened this issue Jan 21, 2013 · 48 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jan 21, 2013

Migrated from rt.perl.org#116479 (status was 'open')

Searchable as RT116479$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 26, 2012

From @Leont

Historically, dual-life modules have upgraded to the "perl"/"core"
installdirs (as opposed to site, where most modules go). This kind of
made sense before 5.12, when we made sense out of our default loading
order, but really doesn't anymore right now. We should make all dual
life modules install to site when installed from CPAN on 5.12+.

Currently, It seems at least these modules fail to do that​:

Storable
Module-CoreList
lib
ExtUtils-CBuilder
IO
Encode
List-Util
Digest-SHA
Module-Plugable
CPANPLUS
Math-Complex
Time-Piece
Unicode-Normalize
Unicode-Collate
IPC-SysV
Compress-Raw-Bzip2
Win32API-File
DB_File
IO-Compress
Compress-Raw-Zlib
Mime-Base64
Time-HiRes
Devel-PPPort
Win32
ExtUtils-MakeMaker
Module-Build

The last two didn't show up when grepping (since they don't have a
Makefile.PL in core), I may have missed some more.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 26, 2012

From @xdg

On Thu, Jul 26, 2012 at 10​:17 AM, Leon Timmermans
<perlbug-followup@​perl.org> wrote​:

Historically, dual-life modules have upgraded to the "perl"/"core"
installdirs (as opposed to site, where most modules go). This kind of
made sense before 5.12, when we made sense out of our default loading
order, but really doesn't anymore right now. We should make all dual
life modules install to site when installed from CPAN on 5.12+.

I wonder if we should patch EU​::MM and M​::B to detect dual-life
modules from Module​::CoreList and override the install location if
incorrect for a given version of Perl.

That might be too draconian, as someone might have reasons to install
in the wrong place -- but they could prompt first.

-- David

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 26, 2012

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 26, 2012

From @Leont

On Thu, Jul 26, 2012 at 5​:17 PM, Leon Timmermans
<perlbug-followup@​perl.org> wrote​:

The last two didn't show up when grepping (since they don't have a
Makefile.PL in core), I may have missed some more.

That indeed seems to be the case. PathTools, AutoLoader, SelfLoader,
parent, Archive​::Tar, Filter​::Simple, Text​::Balanced,
Package-Constants, NEXT, base, Text-ParseWords, File-Fetch, Net-Ping.
And that's just from the first page of grep.cpan.me.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 29, 2012

From @Leont

On Thu, Jul 26, 2012 at 6​:02 PM, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Jul 26, 2012 at 5​:17 PM, Leon Timmermans
<perlbug-followup@​perl.org> wrote​:

The last two didn't show up when grepping (since they don't have a
Makefile.PL in core), I may have missed some more.

That indeed seems to be the case. PathTools, AutoLoader, SelfLoader,
parent, Archive​::Tar, Filter​::Simple, Text​::Balanced,
Package-Constants, NEXT, base, Text-ParseWords, File-Fetch, Net-Ping.
And that's just from the first page of grep.cpan.me.

See http​://grep.cpan.me/?q=INSTALLDIRS\s*%3D%3E.*perl and
http​://grep.cpan.me/?q=installdirs\s*%3D%3E.*core for more. Quite
frankly, it seems we should assume any dual-life module is always
installing to 'perl' unless prove otherwise is given.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @rjbs

It used to be that the search order of `@​INC` was perl, then site. If you
installed a new version of something from CPAN, it would be installed into
site, by default. That meant that if it was in core, you'd never see the new
version, because the core code was in the "perl" lib.

To work around this, libraries found in both core and CPAN overrode their
install location to go into perl lib. This was a lousy situation. Now it's
over! Perl now looks in site first.

...but lots of dual-life libraries have not been updated to install to site as
usual! They need fixing.

Libraries that are released primarily on CPAN (i.e., "upstream CPAN" libraries)
need to have support requests filed against them. Include a patch! For most
libraries, this means using rt.cpan.org, but don't use it blindly. Do what the
author wants. Also, check to see that it hasn't already been filed or resolved
first.

Libraries that are released primarily via the core perl distribution need to
have patches sent to the core. Use perlbug or contact perl5-porters@​perl.org

In most cases, libraries will be using ExtUtils​::MakeMaker. They should end up
with a line something like this​:

  INSTALLDIRS => ($] < 5.011 ? 'perl' : 'site'),

They're broken if they currently just say `INSTALLDIRS => 'perl'`

## Partial List

Here are some seemingly broken ones​:

* CPANPLUS
* Compress-Raw-Bzip2
* Compress-Raw-Zlib
* DB_File
* Devel-PPPort
* Digest-SHA
* Encode
* ExtUtils-CBuilder
* ExtUtils-MakeMaker
* IO
* IO-Compress
* IPC-SysV
* List-Util
* Math-Complex
* Mime-Base64
* Module-Build
* Module-CoreList
* Module-Plugable
* Storable
* Time-HiRes
* Time-Piece
* Unicode-Collate
* Unicode-Normalize
* Win32
* Win32API-File
* lib

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @jkeenan

On Mon Jan 21 08​:05​:04 2013, rjbs wrote​:

Libraries that are released primarily on CPAN (i.e., "upstream CPAN"
libraries)
need to have support requests filed against them. Include a patch!
For most
libraries, this means using rt.cpan.org, but don't use it blindly. Do
what the
author wants. Also, check to see that it hasn't already been filed or
resolved
first.

Libraries that are released primarily via the core perl distribution
need to
have patches sent to the core. Use perlbug or contact perl5-
porters@​perl.org

Attaching a file which takes the list of libraries provided by rjbs and
adds what's listed for each as UPSTREAM in Porting/Maintainers.pl. I
changed the way a few of the libraries are spelled to conform to that file.

Note​: We should get a better UPSTREAM value for Time-Piece than 'undef'.

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @jkeenan

On Mon Jan 21 13​:11​:49 2013, jkeenan wrote​:

On Mon Jan 21 08​:05​:04 2013, rjbs wrote​:

Attachment

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @jkeenan

CPANPLUS,cpan
Compress-Raw-Bzip2,cpan
Compress-Raw-Zlib,cpan
DB_File,cpan
Devel-PPPort,cpan
Digest-SHA,cpan
Encode,cpan
ExtUtils-CBuilder,blead
ExtUtils-MakeMaker,first-come
IO,blead
IO-Compress,cpan
IPC-SysV,cpan
Math-Complex,cpan
MIME-Base64,cpan
Module-Build,cpan
Module-CoreList,blead
Module-Pluggable,cpan
Scalar-List-Utils,cpan
Storable,blead
Time-HiRes,cpan
Time-Piece,undef
Unicode-Collate,first-come
Unicode-Normalize,first-come
Win32,cpan
Win32API-File,cpan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @Leont

On Mon, Jan 21, 2013 at 5​:05 PM, Ricardo SIGNES
<perlbug-followup@​perl.org> wrote​:

## Partial List

Here are some seemingly broken ones​:

* CPANPLUS
* Compress-Raw-Bzip2
* Compress-Raw-Zlib
* DB_File
* Devel-PPPort
* Digest-SHA
* Encode
* ExtUtils-CBuilder
* ExtUtils-MakeMaker
* IO
* IO-Compress
* IPC-SysV
* List-Util
* Math-Complex
* Mime-Base64
* Module-Build
* Module-CoreList
* Module-Plugable
* Storable
* Time-HiRes
* Time-Piece
* Unicode-Collate
* Unicode-Normalize
* Win32
* Win32API-File
* lib

I created that list mostly from distributions in core that contain a
customized Makefile.PL/Build.PL. In general, all dual-life
distributions should be considered suspect until proven otherwise.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2013

From @rjbs

* James E Keenan via RT <perlbug-followup@​perl.org> [2013-01-21T16​:11​:50]

Note​: We should get a better UPSTREAM value for Time-Piece than 'undef'.

I've already got something of an open thread with M. Sergeant about Time-Piece,
I'll get this clarified, too, ASAP.

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 1, 2013

From @nwc10

On Mon, Jan 21, 2013 at 04​:27​:12PM -0500, Ricardo Signes wrote​:

* James E Keenan via RT <perlbug-followup@​perl.org> [2013-01-21T16​:11​:50]

Note​: We should get a better UPSTREAM value for Time-Piece than 'undef'.

I've already got something of an open thread with M. Sergeant about Time-Piece,
I'll get this clarified, too, ASAP.

Cool.

You're aware that the XS code has forked? And that blead's is portable to all
platforms, whereas last I knew, the CPAN code wasn't.

The core has more platform coverage than CPAN testers.

Nicholas Clark

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 2, 2013

From @rjbs

* Nicholas Clark <nick@​ccl4.org> [2013-02-01T11​:52​:27]

You're aware that the XS code has forked? And that blead's is portable to all
platforms, whereas last I knew, the CPAN code wasn't.

Yeah, my goal was to get blead's changes backported to CPAN, if not blead made
upstream.

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 3, 2013

From @jkeenan

On Mon Jan 21 13​:12​:35 2013, jkeenan wrote​:

On Mon Jan 21 13​:11​:49 2013, jkeenan wrote​:

On Mon Jan 21 08​:05​:04 2013, rjbs wrote​:

Attachment

I'm attaching a patch to fix 3 of the 4 distributions named above which
are maintained in blead.

The patch bumps the version number for each distribution in the file
named in its Makefile.PL VERSION_FROM entry. However, I did not bump
the distro numbers in Porting/Maintainers.pl, as I don't know how those
are determined.

Before we apply the patch, however, we should take note of this​: In
dist/Storable/Makefile.PL, we *already* have this​:

#####
  INSTALLDIRS => $] >= 5.007 ? 'perl' : 'site',
#####

This places the beginning of the 'site' era at 5.7 rather than 5.11.
Which is correct?

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 3, 2013

From @jkeenan

0001-Update-INSTALLDIRS-to-favor-installation-under-site.patch
From cbdb3b72da671cbe687f5f9fcae26223f0d8b698 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Sun, 3 Feb 2013 11:52:44 -0500
Subject: [PATCH] Update INSTALLDIRS to favor installation under 'site'.

For: RT #116479
---
 dist/ExtUtils-CBuilder/Makefile.PL              |    1 +
 dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder.pm |    2 +-
 dist/IO/IO.pm                                   |    2 +-
 dist/IO/Makefile.PL                             |    2 +-
 dist/Module-CoreList/Makefile.PL                |    1 +
 dist/Module-CoreList/lib/Module/CoreList.pm     |    2 +-
 6 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/dist/ExtUtils-CBuilder/Makefile.PL b/dist/ExtUtils-CBuilder/Makefile.PL
index e6b1d16..57014a3 100644
--- a/dist/ExtUtils-CBuilder/Makefile.PL
+++ b/dist/ExtUtils-CBuilder/Makefile.PL
@@ -15,6 +15,7 @@ my %WriteMakefileArgs = (
     "ExtUtils::MakeMaker" => "6.30"
   },
   "EXE_FILES" => [],
+  "INSTALLDIRS" => ($] < 5.011 ? 'perl' : 'site'),
   "LICENSE" => "perl",
   "PREREQ_PM" => {
     "Cwd" => 0,
diff --git a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder.pm b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder.pm
index ad2ea0d..1caba9c 100644
--- a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder.pm
+++ b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder.pm
@@ -6,7 +6,7 @@ use File::Basename ();
 use Perl::OSType qw/os_type/;
 
 use vars qw($VERSION @ISA);
-$VERSION = '0.280209';
+$VERSION = '0.280210';
 $VERSION = eval $VERSION;
 
 # We only use this once - don't waste a symbol table entry on it.
diff --git a/dist/IO/IO.pm b/dist/IO/IO.pm
index 522aaab..2e021c4 100644
--- a/dist/IO/IO.pm
+++ b/dist/IO/IO.pm
@@ -7,7 +7,7 @@ use Carp;
 use strict;
 use warnings;
 
-our $VERSION = "1.26";
+our $VERSION = "1.27";
 XSLoader::load 'IO', $VERSION;
 
 sub import {
diff --git a/dist/IO/Makefile.PL b/dist/IO/Makefile.PL
index 70ffe12..7783cf9 100644
--- a/dist/IO/Makefile.PL
+++ b/dist/IO/Makefile.PL
@@ -39,7 +39,7 @@ WriteMakefile(
   ( $PERL_CORE
     ? ()
     : (
-      INSTALLDIRS => 'perl',
+      INSTALLDIRS => ($] < 5.011 ? 'perl' : 'site'),
       clean       => {FILES => 'typemap'},
     )
   ),
diff --git a/dist/Module-CoreList/Makefile.PL b/dist/Module-CoreList/Makefile.PL
index 6235c47..9f86c61 100644
--- a/dist/Module-CoreList/Makefile.PL
+++ b/dist/Module-CoreList/Makefile.PL
@@ -13,6 +13,7 @@ WriteMakefile
         'List::Util' => 0,
     },
     'EXE_FILES' => [ _scripts() ],
+    'INSTALLDIRS' => ($] < 5.011 ? 'perl' : 'site'),
     'PL_FILES' => {},
     LICENSE => 'perl',
     @extra,
diff --git a/dist/Module-CoreList/lib/Module/CoreList.pm b/dist/Module-CoreList/lib/Module/CoreList.pm
index 354fb83..eea2abc 100644
--- a/dist/Module-CoreList/lib/Module/CoreList.pm
+++ b/dist/Module-CoreList/lib/Module/CoreList.pm
@@ -3,7 +3,7 @@ use strict;
 use vars qw/$VERSION %released %version %families %upstream
 	    %bug_tracker %deprecated/;
 use Module::CoreList::TieHashDelta;
-$VERSION = '2.81';
+$VERSION = '2.82';
 
 my $dumpinc = 0;
 sub import {
-- 
1.6.3.2

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 3, 2013

From @jkeenan

On Sun Feb 03 09​:01​:34 2013, jkeenan wrote​:

On Mon Jan 21 13​:12​:35 2013, jkeenan wrote​:

On Mon Jan 21 13​:11​:49 2013, jkeenan wrote​:

On Mon Jan 21 08​:05​:04 2013, rjbs wrote​:

Attachment

I'm attaching a patch to fix 3 of the 4 distributions named above which
are maintained in blead.

The patch bumps the version number for each distribution in the file
named in its Makefile.PL VERSION_FROM entry. However, I did not bump
the distro numbers in Porting/Maintainers.pl, as I don't know how those
are determined.

Before we apply the patch, however, we should take note of this​: In
dist/Storable/Makefile.PL, we *already* have this​:

#####
INSTALLDIRS => $] >= 5.007 ? 'perl' : 'site',
#####

This places the beginning of the 'site' era at 5.7 rather than 5.11.
Which is correct?

Having subsequently built perl in a branch with the patch applied, I
wonder whether the patch is actually correct.

For example, here's a grep on the Makefile for ExtUtils-CBuilder​:

#####
$ grep -n INSTALLDIRS dist/ExtUtils-CBuilder/Makefile
9​:# MakeMaker ARGV​: (q[INSTALLDIRS=perl], q[INSTALLMAN1DIR=none],
q[INSTALLMAN3DIR=none], q[PERL_CORE=1], q[LIBPERL_A=libperl.a])
19​:# INSTALLDIRS => q[site]
78​:INSTALLDIRS = perl
598​: cd $(DISTVNAME) && $(ABSPERLRUN) Makefile.PL "INSTALLDIRS=perl"
"INSTALLMAN1DIR=none" "INSTALLMAN3DIR=none" "PERL_CORE=1"
"LIBPERL_A=libperl.a"
647​:pure_install :​: pure_$(INSTALLDIRS)_install
650​:doc_install :​: doc_$(INSTALLDIRS)_install
654​: $(NOECHO) $(ECHO) INSTALLDIRS not defined, defaulting to
INSTALLDIRS=site
657​: $(NOECHO) $(ECHO) INSTALLDIRS not defined, defaulting to
INSTALLDIRS=site
731​:uninstall :​: uninstall_from_$(INSTALLDIRS)dirs
771​: $(PERLRUN) Makefile.PL "INSTALLDIRS=perl" "INSTALLMAN1DIR=none"
"INSTALLMAN3DIR=none" "PERL_CORE=1" "LIBPERL_A=libperl.a"
793​: INSTALLDIRS=perl \
#####

I read that as saying, "Install into 'perl', not 'site'." Can anyone
clarify?

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 2, 2013

From @rjbs

* Jim wrote​:

I read that as saying, "Install into 'perl', not 'site'." Can anyone
clarify?

Jim, your reading is correct.

Storable entered core in 5.7.3, when core modules still had to install to "perl" to replace old
versions from core. That's why the line says​:

  INSTALLDIRS => $] >= 5.007 ? 'perl' : 'site',

It's saying, "If you're installing me on a perl that had me in core, you better install over that
version."

The need to install "over" the core's copy in the "perl" dirs was eliminated in 5.12.0 by the "@​INC
reorganization." After that, site came first, so normal installs work. The upper bound on
installing to 'perl' instead of 'site' should be 5.12.0, exclusive. So the Storable INSTALLDIRS
should be​:

  INSTALLDIRS => ($] >= 5.007 && $] < 5.012) ? 'perl' : 'site',

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 14, 2013

From @jkeenan

On Fri Mar 01 18​:54​:06 2013, rjbs wrote​:

* Jim wrote​:

I read that as saying, "Install into 'perl', not 'site'." Can
anyone
clarify?

Jim, your reading is correct.

Storable entered core in 5.7.3, when core modules still had to install
to "perl" to replace old
versions from core. That's why the line says​:

INSTALLDIRS => $] >= 5.007 ? 'perl' : 'site',

It's saying, "If you're installing me on a perl that had me in core,
you better install over that
version."

The need to install "over" the core's copy in the "perl" dirs was
eliminated in 5.12.0 by the "@​INC
reorganization." After that, site came first, so normal installs
work. The upper bound on
installing to 'perl' instead of 'site' should be 5.12.0, exclusive.
So the Storable INSTALLDIRS
should be​:

INSTALLDIRS => \($\] >= 5\.007 && $\] \< 5\.012\) ? 'perl' : 'site'\,

Following discussion with rjbs at Perl QA Hackathon, I re-created the
patch -- some of the modules had had other changes in the interim which
boosted their version numbers -- added the Storable changes, and applied
all in 262731e. Closing ticket.

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 14, 2013

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

@p5pRT p5pRT closed this Apr 14, 2013
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 14, 2013

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 14, 2013

From @jkeenan

I just realized that this RT should remain open. The patches applied
only pertain to modules maintained by P5P in blead. We still have to
deal with modules maintained elsewhere.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From gottreu@gmail.com

Here's a patch for lib which seems to be the only module in dist/ that
still had this problem.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From gottreu@gmail.com

0001-dist-lib-Makefile.PL-change-INSTALLDIRS-to-site-for-.patch
From 567619a8bc7802dfe6aae5c4468bb23184c15d95 Mon Sep 17 00:00:00 2001
From: Brian Gottreu <gottreu@gmail.com>
Date: Mon, 1 Jul 2013 19:54:03 -0500
Subject: [PATCH] dist/lib/Makefile.PL: change INSTALLDIRS to 'site' for 5.12
 and later

---
 dist/lib/Makefile.PL |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dist/lib/Makefile.PL b/dist/lib/Makefile.PL
index 1f80b02..129a703 100644
--- a/dist/lib/Makefile.PL
+++ b/dist/lib/Makefile.PL
@@ -31,7 +31,7 @@ WriteMakefile(
   'PREREQ_PM' => {},
   'ABSTRACT_FROM' => 'lib_pm.PL',
   'AUTHOR' => 'Steffen Mueller <smueller@cpan.org>',
-  'INSTALLDIRS' => 'perl',
+  'INSTALLDIRS' => ($] < 5.012 ? 'perl' : 'site'),
   'PL_FILES' => {'lib_pm.PL' => 'lib.pm'},
   'PM' => {'lib.pm' => '$(INST_LIBDIR)/lib.pm'},
   'clean' => {FILES => 'lib.pm'},
-- 
1.7.10.4

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From @jkeenan

On Mon Jul 01 18​:13​:08 2013, gottreu wrote​:

Here's a patch for lib which seems to be the only module in dist/ that
still had this problem.

Applied to blead in commit 2ed0bf6. This RT remains open because
similar patches presumably need to be submitted to dual-lifed
distributions under the cpan/ directory.

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From @rjbs

On Mon Jul 01 19​:15​:03 2013, jkeenan wrote​:

Applied to blead in commit 2ed0bf6. This RT remains open because
similar patches presumably need to be submitted to dual-lifed
distributions under the cpan/ directory.

A useful task for a would-be contributor​: supplying a list of not-yet-patched dists under ./cpan,
which can serve as a checklist toward closing this bug.

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From gottreu@gmail.com

On Mon Jul 01 19​:30​:05 2013, rjbs wrote​:

A useful task for a would-be contributor​: supplying a list of not-yet-
patched dists under ./cpan,
which can serve as a checklist toward closing this bug.

There are 8 remaining modules that install to the wrong place.

Devel​::PPPort and Time​::Piece have UPSTREAM set to 'undef' and undef respectively in
Porting/Maintainers.pl. I've attached a patch for them if blead turns out to be
upstream.

For the remaining 6, all but one had open tickets with patches already. I submitted a
patch for the other one.

DB_File https://rt.cpan.org/Public/Bug/Display.html?id=70420
Digest​::MD5 https://rt.cpan.org/Ticket/Display.html?id=86621
IPC​::SysV https://rt.cpan.org/Ticket/Display.html?id=79821
Math​::Complex https://rt.cpan.org/Ticket/Display.html?id=79795
MIME​::Base64 https://rt.cpan.org/Ticket/Display.html?id=79796
Time​::HiRes https://rt.cpan.org/Ticket/Display.html?id=79797

Brian Gottreu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 2, 2013

From gottreu@gmail.com

0001-Change-INSTALLDIRS-to-site-for-5.12-and-later.patch
From d02cd94cf15155eeedd4d8e8eb1b227d817cda35 Mon Sep 17 00:00:00 2001
From: Brian Gottreu <gottreu@gmail.com>
Date: Mon, 1 Jul 2013 22:04:05 -0500
Subject: [PATCH] Change INSTALLDIRS to 'site' for 5.12 and later

---
 cpan/Devel-PPPort/Makefile.PL |    2 +-
 cpan/Time-Piece/Makefile.PL   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpan/Devel-PPPort/Makefile.PL b/cpan/Devel-PPPort/Makefile.PL
index 8a77ae5..3c1d479 100644
--- a/cpan/Devel-PPPort/Makefile.PL
+++ b/cpan/Devel-PPPort/Makefile.PL
@@ -72,7 +72,7 @@ sub configure
   }
   else {
     # Devel::PPPort is in the core since 5.7.3
-    push @moreopts, INSTALLDIRS => ($] >= 5.007003 ? 'perl' : 'site');
+    push @moreopts, INSTALLDIRS => ($] >= 5.007003 and $] < 5.012 ? 'perl' : 'site');
   }
 
   if ($opt{'apicheck'}) {
diff --git a/cpan/Time-Piece/Makefile.PL b/cpan/Time-Piece/Makefile.PL
index 9b2a964..f4e944d 100644
--- a/cpan/Time-Piece/Makefile.PL
+++ b/cpan/Time-Piece/Makefile.PL
@@ -7,5 +7,5 @@ WriteMakefile(
     'VERSION_FROM' => 'Piece.pm', # finds $VERSION
     'AUTHOR' => 'Matt Sergeant',
     'ABSTRACT_FROM' => 'Piece.pm',
-    'INSTALLDIRS' => ( $] >= 5.009005 ? 'perl' : 'site' ),
+    'INSTALLDIRS' => ( $] >= 5.009005 && $] < 5.012 ? 'perl' : 'site' ),
 );
-- 
1.7.10.4

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From gottreu@gmail.com

Digest​::MD5 and MIME​::Base64 have been patched and the updates are on
CPAN.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From [Unknown Contact. See original ticket]

Digest​::MD5 and MIME​::Base64 have been patched and the updates are on
CPAN.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From @rjbs

* Brian Gottreu via RT <perlbug-followup@​perl.org> [2013-07-01T23​:27​:49]

Devel​::PPPort and Time​::Piece have UPSTREAM set to 'undef' and undef
respectively in Porting/Maintainers.pl. I've attached a patch for them if
blead turns out to be upstream.

Thanks, Brian. I will be taking over Time​::Piece this week and will apply the
patch.

For the remaining 6, all but one had open tickets with patches already. I
submitted a patch for the other one.

DB_File https://rt.cpan.org/Public/Bug/Display.html?id=70420
Digest​::MD5 https://rt.cpan.org/Ticket/Display.html?id=86621
IPC​::SysV https://rt.cpan.org/Ticket/Display.html?id=79821
Math​::Complex https://rt.cpan.org/Ticket/Display.html?id=79795
MIME​::Base64 https://rt.cpan.org/Ticket/Display.html?id=79796
Time​::HiRes https://rt.cpan.org/Ticket/Display.html?id=79797

Thanks again. That is a *very* useful list for me.

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From @pmqs

There seem to be a number of variations on the value used for the Perl lower version for this test in both this thread and in existing modules, so can I confirm the absolute lower bound version for this test is 5.007? The lower bound actually used then needs to be the version when the module was first included with core (assuming that is prior to 5.012)?

Paul

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From gottreu@gmail.com

On Jul 4, 2013 12​:42 PM, "Paul Marquess" <Paul.Marquess@​ntlworld.com> wrote​:

can I confirm the absolute lower bound version for this test is 5.007?

IPC​::SysV checks against 5.005. It seems to be the only one checking lower
than 5.007 (though I looked quickly and from a phone). (From the root of
the repo I used​: find . -name Makefile.PL|xargs grep -l INSTALLDIR|xargs
grep -F '$]')

The lower bound actually used then needs to be the version when the
module was first included with core (assuming that is prior to 5.012)?

Yes, that is my understanding.

Brian Gottreu

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From @Leont

On Thu, Jul 4, 2013 at 8​:42 PM, Brian Gottreu <gottreu@​gmail.com> wrote​:

IPC​::SysV checks against 5.005. It seems to be the only one checking lower
than 5.007 (though I looked quickly and from a phone). (From the root of
the repo I used​: find . -name Makefile.PL|xargs grep -l INSTALLDIR|xargs
grep -F '$]')

Most dists don't have their CPAN Makefile.PL, for simple distributions
it's generated during build. So grepping blead isn't enough.

Wih a minicpan checkout, someone could use Porting/Maintainers.pl to
find all dists proper and then check those instead.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 4, 2013

From @pmqs

From​: Brian Gottreu [mailto​:gottreu@​gmail.com]

Sent​: 04 July 2013 19​:42

To​: perl5-porters@​perl.org

Subject​: RE​: [perl #116479] supply patches to upstream to fix install
location

On Jul 4, 2013 12​:42 PM, "Paul Marquess" <Paul.Marquess@​ntlworld.com>
wrote​:

can I confirm the absolute lower bound version for this test is 5.007?

IPC​::SysV checks against 5.005. It seems to be the only one checking
lower than 5.007 (though I looked quickly and from a phone). (From the root
of the repo I used​: find . -name Makefile.PL|xargs grep -l INSTALLDIR|xargs
grep -F '$]')

The lower bound actually used then needs to be the version when the

module was first included with core (assuming that is prior to 5.012)?

Yes, that is my understanding.

Thanks Brian.

Paul

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 7, 2013

From gottreu@gmail.com

These are the modules that need to be updated on CPAN. I downloaded all the distributions listed in
Porting/Maintainers.pl (even the commented out ExtUtils-Constant) and checked Makefile.PL and Build.PL.
This list of distributions was current as of commit 0c0d42f.

These sixty (60) distributions create a Makefile from Makefile.PL that sets INSTALLDIRS to 'perl' instead
of 'site'.
Archive-Tar-1.92
Attribute-Handlers-0.93
AutoLoader-5.73
base-2.18
B-Debug-1.18
bignum-0.32
CGI.pm-3.63
constant-1.27
CPAN-2.00
Data-Dumper-2.145
Devel-PPPort-3.20
Digest-1.17
ExtUtils-Constant-0.16
ExtUtils-Install-1.54
ExtUtils-MakeMaker-6.68
File-Fetch-0.42
File-Path-2.09
Filter-1.49
Filter-Simple-0.88
Getopt-Long-2.40
I18N-LangTags-0.35
if-0.0601
IO-1.25
IO-Zlib-1.10
IPC-Cmd-0.82
IPC-SysV-2.03
lib-0.63
libnet-1.22
Locale-Codes-3.26
Locale-Maketext-1.23
Locale-Maketext-Simple-0.21
Math-BigInt-1.997
Math-BigInt-FastCalc-0.30
Math-BigRat-0.2602
Math-Complex-1.59
Module-Load-0.24
Module-Loaded-0.08
Net-Ping-2.41
NEXT-0.65
Package-Constants-0.02
Params-Check-0.38
parent-0.225
PathTools-3.40
Pod-Checker-1.60
podlators-2.5.1
Pod-Parser-1.61
Pod-Perldoc-3.20
SelfLoader-1.20
Storable-2.39
Term-ANSIColor-4.02
Term-Cap-1.12
Test-1.26
Test-Harness-3.28
Test-Simple-0.98
Text-Balanced-2.02
Text-ParseWords-3.29
Text-Tabs+Wrap-2013.0523
Tie-File-0.98
Tie-RefHash-1.39
Time-HiRes-1.9725

In addition, these three (3) distributions set installdirs in Build.PL to 'core' rather than 'site'​:
Locale-Codes-3.26
constant-1.27
ExtUtils-Install-1.54

These twenty (20) are in dist/ in the core, the rest are in cpan/.
Attribute-Handlers-0.93
base-2.18
bignum-0.32
constant-1.27
PathTools-3.40
Data-Dumper-2.145
ExtUtils-Install-1.54
Filter-Simple-0.88
I18N-LangTags-0.35
if-0.0601
IO-1.25
lib-0.63
Locale-Maketext-1.23
Math-BigInt-FastCalc-0.30
Math-BigInt-1.997
Math-BigRat-0.2602
Net-Ping-2.41
SelfLoader-1.20
Storable-2.39
Tie-File-0.98

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 8, 2013

From @Leont

On Sun, Jul 7, 2013 at 11​:27 PM, Brian Gottreu via RT
<perlbug-followup@​perl.org> wrote​:

Test-Harness-3.28

I just patched that in repo.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 8, 2013

From @sciurius

Fixed in Getopt​::Long 2.41 and on its way to CPAN.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 24, 2013

From @jkeenan

On Sun Jul 29 05​:39​:50 2012, LeonT wrote​:

On Thu, Jul 26, 2012 at 6​:02 PM, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Jul 26, 2012 at 5​:17 PM, Leon Timmermans
<perlbug-followup@​perl.org> wrote​:

The last two didn't show up when grepping (since they don't have a
Makefile.PL in core), I may have missed some more.

That indeed seems to be the case. PathTools, AutoLoader, SelfLoader,
parent, Archive​::Tar, Filter​::Simple, Text​::Balanced,
Package-Constants, NEXT, base, Text-ParseWords, File-Fetch, Net-Ping.
And that's just from the first page of grep.cpan.me.

See http​://grep.cpan.me/?q=INSTALLDIRS\s*%3D%3E.*perl and
http​://grep.cpan.me/?q=installdirs\s*%3D%3E.*core for more. Quite
frankly, it seems we should assume any dual-life module is always
installing to 'perl' unless prove otherwise is given.

Leon

Leon,

Is this the same problem as is being tracked in
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=116479?

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 24, 2013

From @Leont

On Wed, Jul 24, 2013 at 3​:17 AM, James E Keenan via RT
<perlbug-followup@​perl.org> wrote​:

Leon,

Is this the same problem as is being tracked in
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=116479?

Thank you very much.
Jim Keenan

Yes it is.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2013

From @jkeenan

LeonT has confirmed that RT #114278 and #116479 describe the same
problem. Since we have a list we're working on in the latter, I'm
merging the former into the latter.

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 28, 2013

From @dolmen

Le Dim. Jul. 07 14​:27​:12 2013, gottreu a �crit�​:

File-Fetch-0.42

Patch submitted​:
Perl-Toolchain-Gang/File-Fetch#4

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 28, 2013

From @dolmen

Le Dim. Jul. 07 14​:27​:12 2013, gottreu a �crit�​:

Pod-Perldoc-3.20

Patch submitted​: mrallen1/Pod-Perldoc#9

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 24, 2013

From @jkeenan

On Sun Jul 07 14​:27​:12 2013, gottreu wrote​:

These are the modules that need to be updated on CPAN. I downloaded
all the distributions listed in
Porting/Maintainers.pl (even the commented out ExtUtils-Constant) and
checked Makefile.PL and Build.PL.
This list of distributions was current as of commit
0c0d42f.

[snip]

These twenty (20) are in dist/ in the core, the rest are in cpan/.
Attribute-Handlers-0.93
base-2.18
bignum-0.32
constant-1.27
PathTools-3.40
Data-Dumper-2.145
ExtUtils-Install-1.54
Filter-Simple-0.88
I18N-LangTags-0.35
if-0.0601
IO-1.25
lib-0.63
Locale-Maketext-1.23
Math-BigInt-FastCalc-0.30
Math-BigInt-1.997
Math-BigRat-0.2602
Net-Ping-2.41
SelfLoader-1.20
Storable-2.39
Tie-File-0.98

I had occasion to look at this ticket again while looking at https://rt.cpan.org/Ticket/Display.html?id=89664 this evening.

Only a subset of these distributions have their own Makefile.PL.

1. Two of these distros use 5.012 as the cut-off rather than 5.011. Is that acceptable?

dist/lib/Makefile.PL
dist/Module-CoreList/Makefile.PL

2. I have prepared a patch for dist/Cwd/Makefile.PL.
The tests in dist/Cwd/t/ pass, but I get a failure in t/porting/utils.t​:

#####
# Failed test 38 - Porting/makerel compiles at porting/utils.t line 89
# got "Subroutine File​::Spec​::Unix​::canonpath redefined at lib/Cwd.pm line 249.\nSubroutine File​::Spec​::Unix​::catdir redefined at lib/Cwd.pm line 249.\nSubroutine File​::Spec​::Unix​::catfile redefined at lib/Cwd.pm line 249.\nPorting/makerel syntax OK\n"
# expected "Porting/makerel syntax OK\n"
not ok 38 - Porting/makerel compiles
#####

Can anyone diagnose this?

Thank you very much.
Jim Keenan

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 24, 2013

From @jkeenan

116479-cwd-insufficient.patch
diff --git a/dist/Cwd/Cwd.pm b/dist/Cwd/Cwd.pm
index d9de63c..19d72b6 100644
--- a/dist/Cwd/Cwd.pm
+++ b/dist/Cwd/Cwd.pm
@@ -171,7 +171,7 @@ use strict;
 use Exporter;
 use vars qw(@ISA @EXPORT @EXPORT_OK $VERSION);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 my $xs_version = $VERSION;
 $VERSION =~ tr/_//;
 
diff --git a/dist/Cwd/Makefile.PL b/dist/Cwd/Makefile.PL
index 1add839..999d5e2 100644
--- a/dist/Cwd/Makefile.PL
+++ b/dist/Cwd/Makefile.PL
@@ -18,7 +18,7 @@ WriteMakefile
                            'Scalar::Util' => '0',
                            'Test' => '0'
                          },
-          'INSTALLDIRS' => 'perl',
+          'INSTALLDIRS' => ($] < 5.011 ? 'perl' : 'site'),
           'EXE_FILES' => [],
           'PL_FILES' => {}
         )
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 27, 2013

From @Leont

On Mon, Jan 21, 2013 at 5​:05 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

It used to be that the search order of `@​INC` was perl, then site. If you
installed a new version of something from CPAN, it would be installed into
site, by default. That meant that if it was in core, you'd never see the
new
version, because the core code was in the "perl" lib.

To work around this, libraries found in both core and CPAN overrode their
install location to go into perl lib. This was a lousy situation. Now
it's
over! Perl now looks in site first.

...but lots of dual-life libraries have not been updated to install to
site as
usual! They need fixing.

Libraries that are released primarily on CPAN (i.e., "upstream CPAN"
libraries)
need to have support requests filed against them. Include a patch! For
most
libraries, this means using rt.cpan.org, but don't use it blindly. Do
what the
author wants. Also, check to see that it hasn't already been filed or
resolved
first.

Libraries that are released primarily via the core perl distribution need
to
have patches sent to the core. Use perlbug or contact
perl5-porters@​perl.org

In most cases, libraries will be using ExtUtils​::MakeMaker. They should
end up
with a line something like this​:

INSTALLDIRS => \($\] \< 5\.011 ? 'perl' : 'site'\)\,

They're broken if they currently just say `INSTALLDIRS => 'perl'`

## Partial List

Here are some seemingly broken ones​:

* CPANPLUS
* Compress-Raw-Bzip2
* Compress-Raw-Zlib
* DB_File
* Devel-PPPort
* Digest-SHA
* Encode
* ExtUtils-CBuilder
* ExtUtils-MakeMaker
* IO
* IO-Compress
* IPC-SysV
* List-Util
* Math-Complex
* Mime-Base64
* Module-Build
* Module-CoreList
* Module-Plugable
* Storable
* Time-HiRes
* Time-Piece
* Unicode-Collate
* Unicode-Normalize
* Win32
* Win32API-File
* lib

cpan-grep shows 39 distributions still having this issues[1]. A few of them
are false positives, and some others are non-core dists that never should
have installed to 'perl' in the first place.

Leon

1​: http​://grep.cpan.me/?q=INSTALLDIRS\s*%3D%3E\s*[%27%22]perl%20file%3A<http​://grep.cpan.me/?q=INSTALLDIRS%5Cs*%3D%3E%5Cs*[%27%22]perl%20file%3A>
^Makefile.PL%24&page=2

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 27, 2013

From @rjbs

* James E Keenan via RT <perlbug-followup@​perl.org> [2013-11-23T23​:23​:43]

1. Two of these distros use 5.012 as the cut-off rather than 5.011. Is that
acceptable?

Yes.

2. I have prepared a patch for dist/Cwd/Makefile.PL.
The tests in dist/Cwd/t/ pass, but I get a failure in t/porting/utils.t​:

Hm. Is it possible that we need to check $ENV{PERL_CORE} also? Anyone?

--
rjbs

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 27, 2013

From @nwc10

On Wed, Nov 27, 2013 at 09​:33​:01AM -0500, Ricardo Signes wrote​:

* James E Keenan via RT <perlbug-followup@​perl.org> [2013-11-23T23​:23​:43]

1. Two of these distros use 5.012 as the cut-off rather than 5.011. Is that
acceptable?

Yes.

For any meaningful purpose, odd numbered blead development releases
effectively "don't exist" once the next stable version has shipped.

No-one should have them installed, and for actually figuring out what caused
regressions or other bugs, it's far more important to know the specific commit.

Nicholas Clark

@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.