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

BUG require filename on windows broken #6166

Closed
p5pRT opened this issue Dec 17, 2002 · 11 comments
Closed

BUG require filename on windows broken #6166

p5pRT opened this issue Dec 17, 2002 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 17, 2002

Migrated from rt.perl.org#19213 (status was 'rejected')

Searchable as RT19213$

@p5pRT
Copy link
Author

p5pRT commented Dec 17, 2002

From kane@ns1.biocede.com

I found the following bug when using require​:

use File​::Spec;
my $file = File​::Spec->catfile("GetOpt", "Long.pm");
require $file;

now, the entry in %INC will be​:
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

usually, that's not so bad, as it merely means that GetOpt​::Long would be
required for a second time, if it were called as such​:

require GetOpt​::Long;

ie, %INC would hold​:
'GetOpt/Long.pm' => 'C​:/Perl/lib/GetOpt/Long.pm',
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

But, when you do this with IPC​::Run (where one of the submodules goes and
require's IPC​::Run itself again), a core dump occurs.

Since ActiveState didn't provide me with a debugging perl, i can't give
more information right now, except my perl -V.
I'm also not sure if this may have been fixed for 5.8, sicne i dont have a
win32 machine with 5.8 right now =/

-jos

===========================================================================

T​:\tools>perl -V
Summary of my perl5 (revision 5 version 6 subversion 1) configuration​:
  Platform​:
  osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
  uname=''
  config_args='undef'
  hint=recommended, useposix=true, d_sigaction=undef
  usethreads=undef use5005threads=undef useithreads=define
usemultiplicity=def
ine
  useperlio=undef d_sfio=undef uselargefiles=undef usesocks=undef
  use64bitint=undef use64bitall=undef uselongdouble=undef
  Compiler​:
  cc='cl', ccflags ='-nologo -O1 -MD -DNDEBUG -DWIN32 -D_CONSOLE
-DNO_STRICT -
DHAVE_DES_FCRYPT -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
-DPERL_MSVCRT_READ
FIX',
  optimize='-O1 -MD -DNDEBUG',
  cppflags='-DWIN32'
  ccversion='', gccversion='', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize
=4
  alignbytes=8, usemymalloc=n, prototype=define
  Linker and Libraries​:
  ld='link', ldflags ='-nologo -nodefaultlib -release
-libpath​:"C​:\Perl\lib\C
ORE" -machine​:x86'
  libpth="C​:\Perl\lib\CORE"
  libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32
.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib
uuid.lib wsoc
k32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib
  perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comd
lg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib
uuid.lib
wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib
msvcrt.lib
  libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl56.lib
  Dynamic Linking​:
  dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -release
-libpath​:"C​:
\Perl\lib\CORE" -machine​:x86'

Characteristics of this binary (from libperl)​:
  Compile-time options​: MULTIPLICITY USE_ITHREADS PERL_IMPLICIT_CONTEXT
PERL_IMP
LICIT_SYS
  Locally applied patches​:
  ActivePerl Build 631
  Built under MSWin32
  Compiled at Jan 2 2002 17​:16​:22
  %ENV​:
  PERL5LIB="t​:/cpanplus/devel/lib"
  @​INC​:
  t​:/cpanplus/devel/lib
  C​:/Perl/lib
  C​:/Perl/site/lib
  .

@p5pRT
Copy link
Author

p5pRT commented Dec 24, 2002

From @chipdude

Jos recently recruited my help in solving the given problem.
A reminder of the bug​: Code that does the equivalent of this on Win32​:

  require Foo​::Bar;
  require catfile('Foo','Bar.pm');

loads and evaluates Foo/Bar.pm twice -- once as "Foo/Bar.pm" and once
as "Foo\\Bar.pm". This is a Bad Thing.

Normally, the slash/backslash confusion of Win32 paths is a user
problem. However, since File​::Spec​::catfile and the path translation
from C<require Foo​::Bar> to "Foo/Bar.pm" are both standard features,
the conflict between them needs to be addressed.

The below patch canonicalizes the name of required files to only use
forward slashes. Jos tested it under Win32 and found it to work. The
patch is appropriate for 5.9 but it may also be good for backporting
to stable versions, at the pumpkings' discretion.

Share & Enjoy!

==== //depot/perl/pp_ctl.c#331 - /u/src/perl/current/pp_ctl.c ====
@​@​ -2940,4 +2940,19 @​@​
  DIE(aTHX_ "Null filename used");
  TAINT_PROPER("require");
+
+#ifdef WIN32
+ /* canonicalize the name to avoid multiple requires */
+ if (strchr(name, '\\')) {
+ char *newname, *p;
+ New(0, newname, len+1, char);
+ Copy(name, newname, len+1, char);
+ p = newname;
+ while ((p = strchr(p, '\\')) != Nullch)
+ *p++ = '/';
+ SAVEFREEPV(newname);
+ name = newname;
+ }
+#endif /* WIN32 */
+
  if (PL_op->op_type == OP_REQUIRE &&
  (svp = hv_fetch(GvHVn(PL_incgv), name, len, 0)) &&

--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
  "It furthers one to have somewhere to go."

@p5pRT
Copy link
Author

p5pRT commented Dec 24, 2002

From @gsar

On Tue, 24 Dec 2002 13​:09​:33 EST, Chip Salzenberg wrote​:

Jos recently recruited my help in solving the given problem.
A reminder of the bug​: Code that does the equivalent of this on Win32​:

require Foo​::Bar;
require catfile('Foo','Bar.pm');

loads and evaluates Foo/Bar.pm twice -- once as "Foo/Bar.pm" and once
as "Foo\\Bar.pm". This is a Bad Thing.

Normally, the slash/backslash confusion of Win32 paths is a user
problem. However, since File​::Spec​::catfile and the path translation
from C<require Foo​::Bar> to "Foo/Bar.pm" are both standard features,
the conflict between them needs to be addressed.

The below patch canonicalizes the name of required files to only use
forward slashes. Jos tested it under Win32 and found it to work. The
patch is appropriate for 5.9 but it may also be good for backporting
to stable versions, at the pumpkings' discretion.

Thanks! Given that this is the uncommon case, it would be nice to
make this only happen when the common case ("Foo/Bar.pm" already
loaded) fails. So moving it down a few lines (after the RETPUSHYES
code) would be good. (The RETPUSHYES code will need to be repeated
after the normalization of the path, though.)

There is a also another issue here​:

  require Foo​::Bar;
  require 'foo/bar.pm';

has the same problem. So how about fixing that one too? :)

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2002

From @chipdude

According to Gurusamy Sarathy​:

Moving it down a few lines (after the RETPUSHYES code) would be
good. (The RETPUSHYES code will need to be repeated after the
normalization of the path, though.)

I mildly disagree. I'd say that the cost of one malloc per require
(and first-time requires are very expensive by comparison) isn't
enough to justify the evil of duplicated code.

require Foo&#8203;::Bar;
require 'foo/bar\.pm';

has the same problem. So how about fixing that one too? :)

Mmm, I'm cautious. Is it _never_ true that Win32 filesystem access is
case-sensitive, even when a Win32 system is accessing a case-sensitive
filesystem via e.g. NFS?
--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
  "It furthers one to have somewhere to go."

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2002

From @gsar

On Fri, 27 Dec 2002 10​:38​:01 EST, Chip Salzenberg wrote​:

According to Gurusamy Sarathy​:

Moving it down a few lines (after the RETPUSHYES code) would be
good. (The RETPUSHYES code will need to be repeated after the
normalization of the path, though.)

I mildly disagree. I'd say that the cost of one malloc per require
(and first-time requires are very expensive by comparison) isn't
enough to justify the evil of duplicated code.

Why not make a function/macro to deal with that?

The performance case I'm worried about is​:

  sub doit {
  require Foo​::Bar; # now hits malloc path every time
  Foo​::Bar​::doit2(...);
  }

require Foo&#8203;::Bar;
require 'foo/bar\.pm';

has the same problem. So how about fixing that one too? :)

Mmm, I'm cautious. Is it _never_ true that Win32 filesystem access is
case-sensitive, even when a Win32 system is accessing a case-sensitive
filesystem via e.g. NFS?

AFAIK, yes.

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2002

From @chipdude

According to Gurusamy Sarathy​:

The performance case I'm worried about is​:
sub doit {
require Foo​::Bar; # now hits malloc path every time
Foo​::Bar​::doit2(...);
}

But in this case, my patch won't hit the malloc, because that path is
conditioned on a search for '\\'. Therefore, require STRING with
user-constructed paths will hit the (slightly-more-expensive) malloc,
and those are vanishingly rare compared to C< require BAREWORD >.

On the other hand, if case-flattening is introduced, all requires with
upper-case letters will have to be canonicalized, and that *is*
expensive. I start to think that %INC will end up with multiple keys
inserted for a single require statement; and I wonder what that would
do to user code that walks %INC?

On the gripping hand, Win32 filenames are Unicode, and I really don't
want to go there; you'll have to find another victim. Sorry. :-,
--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
  "It furthers one to have somewhere to go."

@p5pRT
Copy link
Author

p5pRT commented Dec 28, 2002

From Philip.Newton@gmx.net

On Fri, 27 Dec 2002 10​:38​:01 -0500, chip@​pobox.com (Chip Salzenberg)
wrote​:

According to Gurusamy Sarathy​:

require Foo&#8203;::Bar;
require 'foo/bar\.pm';

has the same problem. So how about fixing that one too? :)

Mmm, I'm cautious. Is it _never_ true that Win32 filesystem access is
case-sensitive, even when a Win32 system is accessing a case-sensitive
filesystem via e.g. NFS?

Data point​: we had a bunch of files on a Unix system in pairs called
foobar.sql and foobar.SQL; this Unix FS was mounted via Samba.

At first, the two could not be distinguished (opening either file always
got the same one, don't remember which -- I worked directly on the Unix
side but a co-worker preferred to use a Windows editor), but because
this was inconvenient, the sysadmin fiddled with a Samba setting. As I
understand it, thereafter Win32 apps could distinguish the two files (I
think they both showed up in directory listings previously, but now one
could load one file and actually get *that* file rather than its
other-cased brother).

Which indicates to me that filesystem accesses can be case-sensitive
when they access "foreign" filesystems. Don't ask me about the technical
details of how this was made to happen, though.

Cheers,
Philip

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2012

From @jkeenan

On Tue Dec 17 06​:34​:13 2002, kane@​ns1.biocede.com wrote​:

I found the following bug when using require​:

use File​::Spec;
my $file = File​::Spec->catfile("GetOpt", "Long.pm");
require $file;

now, the entry in %INC will be​:
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

usually, that's not so bad, as it merely means that GetOpt​::Long would be
required for a second time, if it were called as such​:

require GetOpt​::Long;

ie, %INC would hold​:
'GetOpt/Long.pm' => 'C​:/Perl/lib/GetOpt/Long.pm',
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

But, when you do this with IPC​::Run (where one of the submodules goes and
require's IPC​::Run itself again), a core dump occurs.

Since ActiveState didn't provide me with a debugging perl, i can't give
more information right now, except my perl -V.
I'm also not sure if this may have been fixed for 5.8, sicne i dont have a
win32 machine with 5.8 right now =/

-jos

===========================================================================

T​:\tools>perl -V
Summary of my perl5 (revision 5 version 6 subversion 1) configuration​:
Platform​:
osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
uname=''

This RT attracted comments from Chip and Gurusamy but has been
languishing for nearly ten years. Is there anyone who would like to
review the issues discussed and make a recommendation?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented May 10, 2013

From @jkeenan

On Thu Sep 27 18​:47​:39 2012, jkeenan wrote​:

On Tue Dec 17 06​:34​:13 2002, kane@​ns1.biocede.com wrote​:

I found the following bug when using require​:

use File​::Spec;
my $file = File​::Spec->catfile("GetOpt", "Long.pm");
require $file;

now, the entry in %INC will be​:
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

usually, that's not so bad, as it merely means that GetOpt​::Long
would be
required for a second time, if it were called as such​:

require GetOpt​::Long;

ie, %INC would hold​:
'GetOpt/Long.pm' => 'C​:/Perl/lib/GetOpt/Long.pm',
'GetOpt\\Long.pm' => 'C​:/Perl/lib/GetOpt\\Long.pm',

But, when you do this with IPC​::Run (where one of the submodules
goes and
require's IPC​::Run itself again), a core dump occurs.

Since ActiveState didn't provide me with a debugging perl, i can't give
more information right now, except my perl -V.
I'm also not sure if this may have been fixed for 5.8, sicne i dont
have a
win32 machine with 5.8 right now =/

-jos

===========================================================================

T​:\tools>perl -V
Summary of my perl5 (revision 5 version 6 subversion 1) configuration​:
Platform​:
osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
uname=''

This RT attracted comments from Chip and Gurusamy but has been
languishing for nearly ten years. Is there anyone who would like to
review the issues discussed and make a recommendation?

Thank you very much.
Jim Keenan

No comments received in over 7 months; closing.

@p5pRT
Copy link
Author

p5pRT commented May 10, 2013

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

@p5pRT p5pRT closed this as completed May 10, 2013
@p5pRT
Copy link
Author

p5pRT commented May 10, 2013

From @ikegami

Technically, the same problem exists on non-Windows platforms too.

$ perl -le'require "abc.pm"; print for keys %INC'
abc.pm

$ perl -le'require "./abc.pm"; print for keys %INC'
./abc.pm

$ perl -le'require "././abc.pm"; print for keys %INC'
././abc.pm

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