Closed p5pRT closed 10 years ago
If the @INC directory where a .pm and .pmc are located has a trailing slash\, require will fail to load the .pmc. I suspect this bug was added in eb70bb4a400e88a66c7e10414a2d52b5da4cfd1f
$ pwd /Users/schwern/tmp
$ perl -I ~/tmp -wle 'use Foo; print Foo::test()' pmc
$ perl -I ~/tmp/ -wle 'use Foo; print Foo::test()' pm
$ cat Foo.pm package Foo;
sub test { "pm" }
=pod
=head1 NAME
Foo - this is the pm
=cut
1;
$ cat Foo.pmc package Foo;
sub test { "pmc" }
=pod
=head1 NAME
Foo - this is the pmc
=cut
1;
This bug appears in... 5.18.1 5.18.2 5.19.10
This bug does not appear in... 5.10.1 5.12.5 5.14.4 5.16.3
On Wed\, Mar 26\, 2014 at 03:23:06PM -0700\, Michael G Schwern wrote:
If the @INC directory where a .pm and .pmc are located has a trailing slash\, require will fail to load the .pmc. I suspect this bug was added in eb70bb4a400e88a66c7e10414a2d52b5da4cfd1f
A bisect comes up with this:
commit 6b0bdd7f2041803dc3ec72b53d28052705861967 Author: Matthew Horsfall (alh) \wolfsage@​gmail\.com AuthorDate: Thu Dec 27 10:38:08 2012 -0500 Commit: Father Chrysostomos \sprout@​cpan\.org CommitDate: Sun Feb 10 12:19:15 2013 -0800
RT-116192 - If a directory in @INC already has a trailing '/'\, don't add another.
-- Nothing ventured\, nothing lost.
The RT System itself - Status changed from 'new' to 'open'
On Sat\, Mar 29\, 2014 at 3:28 PM\, Dave Mitchell \davem@​iabyn\.com wrote:
On Wed\, Mar 26\, 2014 at 03:23:06PM -0700\, Michael G Schwern wrote:
If the @INC directory where a .pm and .pmc are located has a trailing slash\, require will fail to load the .pmc. I suspect this bug was added in eb70bb4a400e88a66c7e10414a2d52b5da4cfd1f
A bisect comes up with this:
commit 6b0bdd7f2041803dc3ec72b53d28052705861967 Author: Matthew Horsfall (alh) \wolfsage@​gmail\.com AuthorDate: Thu Dec 27 10:38:08 2012 -0500 Commit: Father Chrysostomos \sprout@​cpan\.org CommitDate: Sun Feb 10 12:19:15 2013 -0800
RT\-116192 \- If a directory in @​INC already has a trailing '/'\, don't add another\.
My apologies! Patch attached.
The original code always assumed a '/' would be appended\, so it set the length of the resulting SV to be +1.
With my earlier patch\, this wasn't the case if the SV already ended in a '/'\, so later checks on the SV that were based off the length of the SV would be wrong.
New tests included.
-- Matthew Horsfall (alh)
On Mon Mar 31 05:32:55 2014\, alh wrote:
RT-116192 - If a directory in @INC already has a trailing '/'\, don't add another.
My apologies! Patch attached.
The original code always assumed a '/' would be appended\, so it set the length of the resulting SV to be +1.
With my earlier patch\, this wasn't the case if the SV already ended in a '/'\, so later checks on the SV that were based off the length of the SV would be wrong.
New tests included.
I think this patch or something like it should be applied to blead and backported.
The only issue I have with the patch itself is it could be simpler\, instead of adding extra variable\, the if could simply ++dirlen when it bumps tmp.
Nothing else within the scope depends on dirlen being the length of the original @INC entry.
Tony
On Mon\, Mar 31\, 2014 at 7:28 PM\, Tony Cook via RT \perlbug\-followup@​perl\.org wrote:
I think this patch or something like it should be applied to blead and backported.
The only issue I have with the patch itself is it could be simpler\, instead of adding extra variable\, the if could simply ++dirlen when it bumps tmp.
Nothing else within the scope depends on dirlen being the length of the original @INC entry.
Thanks for the review Tony. Attached a version with your suggested changes.
Side question (related to both of these patches) - is it bad that in the case of dirs that already end with '/'\, we're calling SvGROW with one more byte than we end up using? Or is there nothing wrong with SvGROW to an arbitrary length and SvCUR_set to a smaller size?
Thanks\,
-- Matthew Horsfall (alh)
On Tue Apr 01 09:42:26 2014\, alh wrote:
Side question (related to both of these patches) - is it bad that in the case of dirs that already end with '/'\, we're calling SvGROW with one more byte than we end up using? Or is there nothing wrong with SvGROW to an arbitrary length and SvCUR_set to a smaller size?
That isn't a problem\, SvGROW() will often allocate a bit more space anyway.
pp_sysread() provides one example where we grow to a length where perhaps not all that space is used.
Tony
On Tue Apr 01 21:12:13 2014\, tonyc wrote:
On Tue Apr 01 09:42:26 2014\, alh wrote:
Side question (related to both of these patches) - is it bad that in the case of dirs that already end with '/'\, we're calling SvGROW with one more byte than we end up using? Or is there nothing wrong with SvGROW to an arbitrary length and SvCUR_set to a smaller size?
That isn't a problem\, SvGROW() will often allocate a bit more space anyway.
pp_sysread() provides one example where we grow to a length where perhaps not all that space is used.
Tony
Alright\, thanks. I've pushed this to blead.
Closing.
-- Matthew Horsfall (alh)
-- -- Matthew Horsfall (alh)
@wolfsage - Status changed from 'open' to 'resolved'
Relevant commit: 9fdd5a7ac74817cfaab65f7284f98ea66faca324
-- Matthew Horsfall (alh)
Relevant commit: 9fdd5a7ac74817cfaab65f7284f98ea66faca324
-- Matthew Horsfall (alh)
Migrated from rt.perl.org#121512 (status was 'resolved')
Searchable as RT121512$