You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Randy Kobes <ra...@theoryx5.uwinnipeg.ca> on 2003/12/01 04:20:16 UTC
Re: [mp2] ModPerl-Registry redirect test
On Sun, 30 Nov 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> > In one of the redirect tests of ModPerl-Registry, catfile
> > is used to construct the location of a script, which for
> > non-unix doesn't use '/' as the directory separator. This
> > ===========================================================
> > Index: ModPerl-Registry/t/redirect.t
> > ===================================================================
> > RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/redirect.t,v
> > retrieving revision 1.6
> > diff -u -r1.6 redirect.t
> > --- ModPerl-Registry/t/redirect.t 23 Nov 2003 21:01:50 -0000 1.6
> > +++ ModPerl-Registry/t/redirect.t 29 Nov 2003 14:28:38 -0000
> > @@ -16,7 +16,7 @@
> > my $redirect_path = "/registry/basic.pl";
> > my $url = "$base_url?$redirect_path";
> > my $vars = Apache::Test::config()->{vars};
> > - my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> > + my $script_file = $vars->{serverroot} . '/cgi-bin/basic.pl';
> >
> > ok t_cmp(
> > "ok $script_file",
> > ====================================================================
> > fixes it.
> >
> > Another option might be to
> >
> > use File::Spec::Unix;
> > my $script = File::Spec::Unix->catfile($var1, $var2);
>
> Ah, what do you get as a response? a path like /tmp/foo.pl on win32? In which
> case the problem would be coming from:
>
> $_[0]->{FILENAME} = $_[1]->filename;
>
> in RegistryCooker.pm, which sets $0, eventually printed by cgi-bin/basic.pl.
> Does $r->filename return a unix path on winFU?
I guess one could look at it from both ends ... I was
thinking here that the problem was in the way that
$script_file was constructed with catfile, which on
Win32 came out as SERVER_ROOT\cgi-bin\basic.pl. So the
patch above changes this to SERVER_ROOT/cgi-bin/basic.pl,
which agrees with the response.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Wed, 3 Dec 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> [...]
> > I think if one wants to grow old fast, offer to help
> > maintain File::Spec :)
>
> There is a better candidate, ExtUtils::MakeMaker. I wonder
> how old was Michael 2 years ago. ;)
:)
[ .. ]
> So if you have:
> C:\\MODPERL~1 => C:\\mod_perl-foo-1
> D:\\MODPERL~1 => D:\\mod_perl-bar-1
>
> and pass MODPERL~1 to Win32::GetLongPathName() without the drive, it will
> expand it as if C:\\mod_perl-foo-1 (if cwd is somewhere in C:\\) even if
> D:\\mod_perl-bar-1 was intended. I guess that's expected, and you better not
> forget to pass that drive letter... ;)
Drive letters are such stupid things ...
> +1 plus probably add an inlined comment for both explaining why there are
> needed, and the pod sections. i'd also add a comment that GetLongPathName
> requires an absolute path to work.
>
> > -my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> > +my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
>
> no need for () as it's imported.
>
> > - my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> > + my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
>
> ditto
I just committed that - thanks, Stas!
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
[...]
> I think if one wants to grow old fast, offer to help
> maintain File::Spec :)
There is a better candidate, ExtUtils::MakeMaker. I wonder how old was Michael
2 years ago. ;)
>>So may be just go with:
>>
>> my $f = File::Spec::Unix->catfile(@_);
>>
>>One more thing, I'm not sure about. File::Spec has a clear
>>separation between path and file/dir, the former
>>optionally includes the drive/volume information, the
>>latter do not. Does Win32::GetLongPathName($f) return a
>>path (including drive/volume) or just dir/file? i.e.
>>t_catfile* should deal only with dir/file and not path?
>
>
> Win32::GetLongPathName(), if given a drive/volume, will use
> that, and include that in what it gives back. So, for
> example, I could get the long path name for something on
> drive D:\ by running a script on drive C:\. If no drive is
> specified, then the input is assumed to be on the drive the
> script is run on, and no drive appears in the output.
So if you have:
C:\\MODPERL~1 => C:\\mod_perl-foo-1
D:\\MODPERL~1 => D:\\mod_perl-bar-1
and pass MODPERL~1 to Win32::GetLongPathName() without the drive, it will
expand it as if C:\\mod_perl-foo-1 (if cwd is somewhere in C:\\) even if
D:\\mod_perl-bar-1 was intended. I guess that's expected, and you better not
forget to pass that drive letter... ;)
>>Or am I wrong and it's file/path vs. dir? It just makes no
>>difference on Unix, so I'm a bit confused here.
> +sub t_catfile {
> + my $f = catfile(@_);
> + return $f unless file_name_is_absolute($f);
> + return Apache::TestConfig::WIN32 ?
> + Win32::GetLongPathName($f) : $f;
> +}
> +
> +sub t_catfile_apache {
> + my $f = File::Spec::Unix->catfile(@_);
> + return $f unless file_name_is_absolute($f);
> + return Apache::TestConfig::WIN32 ?
> + Win32::GetLongPathName($f) : $f;
> }
+1 plus probably add an inlined comment for both explaining why there are
needed, and the pod sections. i'd also add a comment that GetLongPathName
requires an absolute path to work.
> -my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> +my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
no need for () as it's imported.
> - my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> + my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
ditto
> ok t_cmp(
> "ok $script_file",
> ====================================================================
> You raised a good point before about potentially
> File::Spec::Unix at sometime in the future not being usable
> on non-Unix; I guess this is something we'll have to watch
> for. It does happen in principle - for example,
> File::Spec::VMS requires VMS::Filespec, which presumably is
> a VMS-specific module. However, at this point all of the
> non-Unix File::Spec::* require File::Spec::Unix, so it
> appears by design that File::Spec::Unix is the "base" class.
Sounds good.
So go ahead and commit the stuff... thanks Randy!
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Tue, 2 Dec 2003, Stas Bekman wrote:
> Sorry, I didn't know that. Does this work?
>
> sub t_catfile_unix {
> my $f = File::Spec::Unix->canonpath(join "/", @_);
> return $f unless File::Spec->file_name_is_absolute($f);
> return Apache::TestConfig::WIN32 ?
> Win32::GetLongPathName($f) : $f;
> }
Yes ...
> I guess if we do that than your very original patch is
> just right. We came a full cirle back to where we started
> (thanks to me ;).
I think if one wants to grow old fast, offer to help
maintain File::Spec :)
> So may be just go with:
>
> my $f = File::Spec::Unix->catfile(@_);
>
> One more thing, I'm not sure about. File::Spec has a clear
> separation between path and file/dir, the former
> optionally includes the drive/volume information, the
> latter do not. Does Win32::GetLongPathName($f) return a
> path (including drive/volume) or just dir/file? i.e.
> t_catfile* should deal only with dir/file and not path?
Win32::GetLongPathName(), if given a drive/volume, will use
that, and include that in what it gives back. So, for
example, I could get the long path name for something on
drive D:\ by running a script on drive C:\. If no drive is
specified, then the input is assumed to be on the drive the
script is run on, and no drive appears in the output.
> Or am I wrong and it's file/path vs. dir? It just makes no
> difference on Unix, so I'm a bit confused here.
It is confusing, even on Windows :)
Is the following then OK?
===========================================================
Index: Apache-Test/lib/Apache/TestUtil.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestUtil.pm,v
retrieving revision 1.31
diff -u -r1.31 TestUtil.pm
--- Apache-Test/lib/Apache/TestUtil.pm 29 Apr 2003 08:04:04 -0000 1.31
+++ Apache-Test/lib/Apache/TestUtil.pm 4 Dec 2003 05:36:52 -0000
@@ -9,7 +9,7 @@
use Carp ();
use Config;
use File::Basename qw(dirname);
-use File::Spec::Functions qw(catfile);
+use File::Spec::Functions qw(catfile file_name_is_absolute);
use Symbol ();
use Apache::Test ();
@@ -26,7 +26,8 @@
t_client_log_error_is_expected t_client_log_warn_is_expected
);
-@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown);
+@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown
+ t_catfile_apache t_catfile);
%CLEAN = ();
@@ -302,6 +303,20 @@
t_debug("removing dir tree: $_");
t_rmtree($_);
}
+}
+
+sub t_catfile {
+ my $f = catfile(@_);
+ return $f unless file_name_is_absolute($f);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
+}
+
+sub t_catfile_apache {
+ my $f = File::Spec::Unix->catfile(@_);
+ return $f unless file_name_is_absolute($f);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
}
1;
Index: ModPerl-Registry/t/basic.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/basic.t,v
retrieving revision 1.15
diff -u -r1.15 basic.t
--- ModPerl-Registry/t/basic.t 23 Nov 2003 21:01:50 -0000 1.15
+++ ModPerl-Registry/t/basic.t 4 Dec 2003 05:36:52 -0000
@@ -6,7 +6,7 @@
use Apache::TestRequest qw(GET GET_BODY HEAD);
use Apache::TestConfig ();
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(t_catfile_apache);
my %modules = (
registry => 'ModPerl::Registry',
@@ -19,7 +19,7 @@
plan tests => @aliases * 4 + 3;
my $vars = Apache::Test::config()->{vars};
-my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
# very basic compilation/response test
for my $alias (@aliases) {
Index: ModPerl-Registry/t/redirect.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/redirect.t,v
retrieving revision 1.6
diff -u -r1.6 redirect.t
--- ModPerl-Registry/t/redirect.t 23 Nov 2003 21:01:50 -0000 1.6
+++ ModPerl-Registry/t/redirect.t 4 Dec 2003 05:36:52 -0000
@@ -5,7 +5,7 @@
use Apache::TestUtil;
use Apache::TestRequest qw(GET_BODY HEAD);
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(t_catfile_apache);
plan tests => 4, have_lwp;
@@ -16,7 +16,7 @@
my $redirect_path = "/registry/basic.pl";
my $url = "$base_url?$redirect_path";
my $vars = Apache::Test::config()->{vars};
- my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+ my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
ok t_cmp(
"ok $script_file",
====================================================================
You raised a good point before about potentially
File::Spec::Unix at sometime in the future not being usable
on non-Unix; I guess this is something we'll have to watch
for. It does happen in principle - for example,
File::Spec::VMS requires VMS::Filespec, which presumably is
a VMS-specific module. However, at this point all of the
non-Unix File::Spec::* require File::Spec::Unix, so it
appears by design that File::Spec::Unix is the "base" class.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
[...]
>>I won't try to use File::Spec::Unix instead of join '/',
>>because one day it may stop loading on non-Unix...
>
>
> That's a good point ... However, using
> sub t_catfile_apache {
> my $f = canonpath join "/", @_;
> return $f unless File::Spec->file_name_is_absolute($f);
> return Apache::TestConfig::WIN32 ?
> Win32::GetLongPathName($f) : $f;
> }
> doesn't quite work on Win32 :( (and probably not on Macs,
> either), as canonpath flips the '/' back to the native
> directory separator ('\' on Win32). What one could do is
> then s{[\\:]}{/}g afterwards, but on Win32 one has to take
> care of the case that the drive is specified, as
> D:/whatever, and not change the ':'. The following is OK:
Sorry, I didn't know that. Does this work?
sub t_catfile_unix {
my $f = File::Spec::Unix->canonpath(join "/", @_);
return $f unless File::Spec->file_name_is_absolute($f);
return Apache::TestConfig::WIN32 ?
Win32::GetLongPathName($f) : $f;
}
I guess if we do that than your very original patch is just right. We came a
full cirle back to where we started (thanks to me ;).
So may be just go with:
my $f = File::Spec::Unix->catfile(@_);
One more thing, I'm not sure about. File::Spec has a clear separation between
path and file/dir, the former optionally includes the drive/volume
information, the latter do not. Does Win32::GetLongPathName($f) return a path
(including drive/volume) or just dir/file? i.e. t_catfile* should deal only
with dir/file and not path?
Or am I wrong and it's file/path vs. dir? It just makes no difference on Unix,
so I'm a bit confused here.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Mon, 1 Dec 2003, Stas Bekman wrote:
> [...]
>
> I'm still unhappy about whatever_url() working with fs
> paths. I guess I'm taking my words on using Unix-> back.
> At least we know that we work with paths and not urls. How
> about this:
>
> # concat a dir/file using unix path separators
> # no platform specific path cleanups are run unless the filepath is absolute
> sub t_catfile_unix {
> my $f = canonpath join "/", @_;
> return $f unless File::Spec->file_name_is_absolute($f);
> return Apache::TestConfig::WIN32 ?
> Win32::GetLongPathName($f) : $f;
> }
>
> # concat a dir/file ala catfile
> # and run platform specific path cleanups if the filepath is absolute
> sub t_catfile {
> my $f = File::Spec->catfile(@_);
> return $f unless File::Spec->file_name_is_absolute($f);
> return Apache::TestConfig::WIN32 ?
> Win32::GetLongPathName($f) : $f;
> }
>
> or may be even better:
> s/t_catfile_unix/t_catfile_apache/
> ? to denote that we catfile the apache way?
>
> I won't try to use File::Spec::Unix instead of join '/',
> because one day it may stop loading on non-Unix...
That's a good point ... However, using
sub t_catfile_apache {
my $f = canonpath join "/", @_;
return $f unless File::Spec->file_name_is_absolute($f);
return Apache::TestConfig::WIN32 ?
Win32::GetLongPathName($f) : $f;
}
doesn't quite work on Win32 :( (and probably not on Macs,
either), as canonpath flips the '/' back to the native
directory separator ('\' on Win32). What one could do is
then s{[\\:]}{/}g afterwards, but on Win32 one has to take
care of the case that the drive is specified, as
D:/whatever, and not change the ':'. The following is OK:
=============================================================
Index: Apache-Test/lib/Apache/TestUtil.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestUtil.pm,v
retrieving revision 1.31
diff -u -r1.31 TestUtil.pm
--- Apache-Test/lib/Apache/TestUtil.pm 29 Apr 2003 08:04:04 -0000 1.31
+++ Apache-Test/lib/Apache/TestUtil.pm 3 Dec 2003 05:10:59 -0000
@@ -9,7 +9,7 @@
use Carp ();
use Config;
use File::Basename qw(dirname);
-use File::Spec::Functions qw(catfile);
+use File::Spec::Functions qw(catfile canonpath file_name_is_absolute);
use Symbol ();
use Apache::Test ();
@@ -26,7 +26,8 @@
t_client_log_error_is_expected t_client_log_warn_is_expected
);
-@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown);
+@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown
+ t_catfile_apache t_catfile);
%CLEAN = ();
@@ -302,6 +303,21 @@
t_debug("removing dir tree: $_");
t_rmtree($_);
}
+}
+
+sub t_catfile {
+ my $f = catfile(@_);
+ return $f unless file_name_is_absolute($f);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
+}
+
+sub t_catfile_apache {
+ my $f = canonpath join '/', @_;
+ $f =~ s{[\\:](?!\\)}{/}g;
+ return $f unless file_name_is_absolute($f);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
}
1;
Index: ModPerl-Registry/t/basic.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/basic.t,v
retrieving revision 1.15
diff -u -r1.15 basic.t
--- ModPerl-Registry/t/basic.t 23 Nov 2003 21:01:50 -0000 1.15
+++ ModPerl-Registry/t/basic.t 3 Dec 2003 05:10:59 -0000
@@ -6,7 +6,7 @@
use Apache::TestRequest qw(GET GET_BODY HEAD);
use Apache::TestConfig ();
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(t_catfile_apache);
my %modules = (
registry => 'ModPerl::Registry',
@@ -19,7 +19,7 @@
plan tests => @aliases * 4 + 3;
my $vars = Apache::Test::config()->{vars};
-my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
# very basic compilation/response test
for my $alias (@aliases) {
Index: ModPerl-Registry/t/redirect.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/redirect.t,v
retrieving revision 1.6
diff -u -r1.6 redirect.t
--- ModPerl-Registry/t/redirect.t 23 Nov 2003 21:01:50 -0000 1.6
+++ ModPerl-Registry/t/redirect.t 3 Dec 2003 05:10:59 -0000
@@ -5,7 +5,7 @@
use Apache::TestUtil;
use Apache::TestRequest qw(GET_BODY HEAD);
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(t_catfile_apache);
plan tests => 4, have_lwp;
@@ -16,7 +16,7 @@
my $redirect_path = "/registry/basic.pl";
my $url = "$base_url?$redirect_path";
my $vars = Apache::Test::config()->{vars};
- my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+ my $script_file = t_catfile_apache($vars->{serverroot}, 'cgi-bin', 'basic.pl');
ok t_cmp(
"ok $script_file",
=========================================================================
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
[...]
> Those names sound good ... However, there already is a
> makepath function in Apache::TestUtil, for (physically)
> making a path. How about t_caturl and t_catpath?
I don't think it's correct to use 'path', since in File::Spec lingo path
includes volume, whereas dir/file doesn't?
>>BTW, regarding Win32::GetLongPathName($f), shouldn't
>>File::Spec be fixed to do the right thing regarding the
>>short path? what about canonpath()?
>
>
> In this context I'm not sure what the right thing is ...
> If one passed in a short path name into File::Spec, then
> for it to do a conversion to a long path name would probably
> be incorrect. And similarly for passing in a long path
> name and getting a short path name. I think the philosophy
> of File::Spec is just to work with what's given, and leave
> it to the user to do any conversions needed.
The fact that we even have to workaround File::Spec already shows that
something is wrong. (i.e. r->filename doesn't return the platform specific path).
> If I'm reading File::Spec correctly, canonpath is called
> within catfile.
cool
>>>+ return Apache::TestConfig::WIN32 ?
>>>+ Win32::GetLongPathName($f) : $f;
>>>+}
>>>+
>>>+sub caturl_normalized {
>>>+ my $f = File::Spec::Unix->catfile(@_);
>>>+ return Apache::TestConfig::WIN32 ?
>>>+ Win32::GetLongPathName($f) : $f;
BTW, I think this code won't create the long path if you give it a relative
path, since it'll never find it and won't know how to expand it. Am I correct?
>>> }
>>
>>I'd use an explicit: join '/', @_ here. It doesn't sound
>>right to use File::Spec::Unix->catfile for constructing
>>urls, even if it happens to do that,
>
>
> Shouldn't one also do some cleanup and checks, like is done
> in catpath:
>
> sub t_catpath {
> my $file = canonpath(pop @_);
> return $file unless @_;
> my $dir = catdir(@_);
> $dir .= '/' unless substr($dir, -1) eq '/';
> return $dir.$file;
> }
Doesn't canonpath taking care of cleaning the path (/ dups and such)? Can
canonpath be called on a relative path?
I'm still unhappy about whatever_url() working with fs paths. I guess I'm
taking my words on using Unix-> back. At least we know that we work with paths
and not urls. How about this:
# concat a dir/file using unix path separators
# no platform specific path cleanups are run unless the filepath is absolute
sub t_catfile_unix {
my $f = canonpath join "/", @_;
return $f unless File::Spec->file_name_is_absolute($f);
return Apache::TestConfig::WIN32 ?
Win32::GetLongPathName($f) : $f;
}
# concat a dir/file ala catfile
# and run platform specific path cleanups if the filepath is absolute
sub t_catfile {
my $f = File::Spec->catfile(@_);
return $f unless File::Spec->file_name_is_absolute($f);
return Apache::TestConfig::WIN32 ?
Win32::GetLongPathName($f) : $f;
}
or may be even better:
s/t_catfile_unix/t_catfile_apache/
? to denote that we catfile the apache way?
I won't try to use File::Spec::Unix instead of join '/', because one day it
may stop loading on non-Unix...
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Mon, 1 Dec 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> [...]
> >>Yes, it's ugly and those of us coding on unix will never
> >>remember to do that, leaving you with the boring cleanup
> >>job. Therefore please think of extending Apache::TestXXX
> >>API to have this Win32::GetLongPathName part hidden
> >>within. Both join "/", .. and Win32::GetLongPathName can
> >>go inside. e.g. catfile_normalized() and
> >>caturl_normalized()? where any post-processings like
> >>Win32::GetLongPathName will come from the _normalized
> >>part? I'm not sure if the name selection is good. Does it
> >>sound good?
> >
> > Yes, that sounds much better - I just included the diff
> > as an indication of what needed to be done. How about
> > the following:
>
> > =========================================================
> > Index: Apache-Test/lib/Apache/TestUtil.pm
> > ===================================================================
> > RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestUtil.pm,v
> > retrieving revision 1.31
> > diff -u -r1.31 TestUtil.pm
> > --- Apache-Test/lib/Apache/TestUtil.pm 29 Apr 2003 08:04:04 -0000 1.31
> > +++ Apache-Test/lib/Apache/TestUtil.pm 1 Dec 2003 07:03:50 -0000
> > @@ -26,7 +26,8 @@
> > t_client_log_error_is_expected t_client_log_warn_is_expected
> > );
> >
> > -@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown);
> > +@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown
> > + catfile_normalized caturl_normalized);
>
> Right, but I don't quite like the names I've suggested, they are too long and
> if they go to Util, they probably need to follow the convention and be /^t_/.
> Do you have any suggestions? I'd prefer a short name
>
> t_makeurl, t_makepath
Those names sound good ... However, there already is a
makepath function in Apache::TestUtil, for (physically)
making a path. How about t_caturl and t_catpath?
> BTW, regarding Win32::GetLongPathName($f), shouldn't
> File::Spec be fixed to do the right thing regarding the
> short path? what about canonpath()?
In this context I'm not sure what the right thing is ...
If one passed in a short path name into File::Spec, then
for it to do a conversion to a long path name would probably
be incorrect. And similarly for passing in a long path
name and getting a short path name. I think the philosophy
of File::Spec is just to work with what's given, and leave
it to the user to do any conversions needed.
> > +sub catfile_normalized {
> > + my $f = catfile(@_);
>
> may be run through canonpath as well?
If I'm reading File::Spec correctly, canonpath is called
within catfile.
> > + return Apache::TestConfig::WIN32 ?
> > + Win32::GetLongPathName($f) : $f;
> > +}
> > +
> > +sub caturl_normalized {
> > + my $f = File::Spec::Unix->catfile(@_);
> > + return Apache::TestConfig::WIN32 ?
> > + Win32::GetLongPathName($f) : $f;
> > }
>
> I'd use an explicit: join '/', @_ here. It doesn't sound
> right to use File::Spec::Unix->catfile for constructing
> urls, even if it happens to do that,
Shouldn't one also do some cleanup and checks, like is done
in catpath:
sub t_catpath {
my $file = canonpath(pop @_);
return $file unless @_;
my $dir = catdir(@_);
$dir .= '/' unless substr($dir, -1) eq '/';
return $dir.$file;
}
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
[...]
> I couldn't see within Apache a function that would return a
> Win32 path in this context, and from some of the places that
> use r->filename and the like, it appears that a Unix-like
> path is assumed. Might this be for convenience, in that a
> filename can be used to construct a URL, and they didn't
> want to get into a lot of conversions when doing so?
I suppose so.
[...]
>>Yes, it's ugly and those of us coding on unix will never
>>remember to do that, leaving you with the boring cleanup
>>job. Therefore please think of extending Apache::TestXXX
>>API to have this Win32::GetLongPathName part hidden
>>within. Both join "/", .. and Win32::GetLongPathName can
>>go inside. e.g. catfile_normalized() and
>>caturl_normalized()? where any post-processings like
>>Win32::GetLongPathName will come from the _normalized
>>part? I'm not sure if the name selection is good. Does it
>>sound good?
>
>
> Yes, that sounds much better - I just included the diff
> as an indication of what needed to be done. How about
> the following:
> =========================================================
> Index: Apache-Test/lib/Apache/TestUtil.pm
> ===================================================================
> RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestUtil.pm,v
> retrieving revision 1.31
> diff -u -r1.31 TestUtil.pm
> --- Apache-Test/lib/Apache/TestUtil.pm 29 Apr 2003 08:04:04 -0000 1.31
> +++ Apache-Test/lib/Apache/TestUtil.pm 1 Dec 2003 07:03:50 -0000
> @@ -26,7 +26,8 @@
> t_client_log_error_is_expected t_client_log_warn_is_expected
> );
>
> -@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown);
> +@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown
> + catfile_normalized caturl_normalized);
Right, but I don't quite like the names I've suggested, they are too long and
if they go to Util, they probably need to follow the convention and be /^t_/.
Do you have any suggestions? I'd prefer a short name
t_makeurl, t_makepath
BTW, regarding Win32::GetLongPathName($f), shouldn't File::Spec be fixed to do
the right thing regarding the short path? what about canonpath()?
> +sub catfile_normalized {
> + my $f = catfile(@_);
may be run through canonpath as well?
> + return Apache::TestConfig::WIN32 ?
> + Win32::GetLongPathName($f) : $f;
> +}
> +
> +sub caturl_normalized {
> + my $f = File::Spec::Unix->catfile(@_);
> + return Apache::TestConfig::WIN32 ?
> + Win32::GetLongPathName($f) : $f;
> }
I'd use an explicit: join '/', @_ here. It doesn't sound right to use
File::Spec::Unix->catfile for constructing urls, even if it happens to do that,
> If OK, I'll add some comments, and a short pod description
> in Apache::TestUtil, of what caturl_normalized and
> catfile_normalized are doing.
sure, but let's polish the names first. There is no rush to patch things
before we are happy with the API.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 30 Nov 2003, Stas Bekman wrote:
> Randy Kobes wrote:
>
> >>>Ah, what do you get as a response? a path like
> >>>/tmp/foo.pl on win32? In which case the problem would be
> >>>coming from:
> >>>
> >>> $_[0]->{FILENAME} = $_[1]->filename;
> >>>
> >>>in RegistryCooker.pm, which sets $0, eventually printed
> >>>by cgi-bin/basic.pl. Does $r->filename return a unix
> >>>path on winFU?
>
> So does it do that? I suppose that it does...
Sorry, I forgot - yes, $r->filename does return a unix path
on Win32.
> >>I guess one could look at it from both ends ... I was
> >>thinking here that the problem was in the way that
> >>$script_file was constructed with catfile, which on
> >>Win32 came out as SERVER_ROOT\cgi-bin\basic.pl. So the
> >>patch above changes this to SERVER_ROOT/cgi-bin/basic.pl,
> >>which agrees with the response.
>
> Yes, but $script_file is a fs path, not URL. I'm not
> questioning the correctness of your patch, Randy, it just
> looks like you are workaround a problem that's coming from
> $r->filename. Is it possible that it may return the \
> paths as well?
I couldn't see within Apache a function that would return a
Win32 path in this context, and from some of the places that
use r->filename and the like, it appears that a Unix-like
path is assumed. Might this be for convenience, in that a
filename can be used to construct a URL, and they didn't
want to get into a lot of conversions when doing so?
> > Further to this, I think most people on Win32 (and probably
> > all non-Unices) are used to using '/' as a directory
> > separator when inside a web environment (eg, in constructing
> > urls). So I would think that SERVER_ROOT/cgi-bin/basic.pl
> > is the correct cross-platform value ...
> >
> > However (I hate bringing this up :(, on Win32 there's
> > problems with sometimes the dos short path name being
> > used and then compared with the long path name
> > (eg, MODPER~1.0 vs modperl-2.0). This in principle
> > is what needs to be done to get t/basic.t and t/redirect.t
> > to pass under ModPerl-Registry:
> [...]
> > -my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> > +my $script_file = File::Spec::Unix->catfile($vars->{serverroot}, 'cgi-bin', 'basic.pl');
>
> This looks absolutely unintuitive and requires a comment in every place you
> use it. Therefore you are probably better off with your original patch to do
> an explicit: join "/n", ...
>
> > +$script_file = Win32::GetLongPathName($script_file)
> > + if Apache::TestConfig::WIN32;
>
> Yes, it's ugly and those of us coding on unix will never
> remember to do that, leaving you with the boring cleanup
> job. Therefore please think of extending Apache::TestXXX
> API to have this Win32::GetLongPathName part hidden
> within. Both join "/", .. and Win32::GetLongPathName can
> go inside. e.g. catfile_normalized() and
> caturl_normalized()? where any post-processings like
> Win32::GetLongPathName will come from the _normalized
> part? I'm not sure if the name selection is good. Does it
> sound good?
Yes, that sounds much better - I just included the diff
as an indication of what needed to be done. How about
the following:
=========================================================
Index: Apache-Test/lib/Apache/TestUtil.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestUtil.pm,v
retrieving revision 1.31
diff -u -r1.31 TestUtil.pm
--- Apache-Test/lib/Apache/TestUtil.pm 29 Apr 2003 08:04:04 -0000 1.31
+++ Apache-Test/lib/Apache/TestUtil.pm 1 Dec 2003 07:03:50 -0000
@@ -26,7 +26,8 @@
t_client_log_error_is_expected t_client_log_warn_is_expected
);
-@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown);
+@EXPORT_OK = qw(t_write_perl_script t_write_shell_script t_chown
+ catfile_normalized caturl_normalized);
%CLEAN = ();
@@ -302,6 +303,18 @@
t_debug("removing dir tree: $_");
t_rmtree($_);
}
+}
+
+sub catfile_normalized {
+ my $f = catfile(@_);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
+}
+
+sub caturl_normalized {
+ my $f = File::Spec::Unix->catfile(@_);
+ return Apache::TestConfig::WIN32 ?
+ Win32::GetLongPathName($f) : $f;
}
1;
Index: ModPerl-Registry/t/basic.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/basic.t,v
retrieving revision 1.15
diff -u -r1.15 basic.t
--- ModPerl-Registry/t/basic.t 23 Nov 2003 21:01:50 -0000 1.15
+++ ModPerl-Registry/t/basic.t 1 Dec 2003 07:03:50 -0000
@@ -6,7 +6,7 @@
use Apache::TestRequest qw(GET GET_BODY HEAD);
use Apache::TestConfig ();
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(caturl_normalized);
my %modules = (
registry => 'ModPerl::Registry',
@@ -19,7 +19,7 @@
plan tests => @aliases * 4 + 3;
my $vars = Apache::Test::config()->{vars};
-my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+my $script_file = caturl_normalized($vars->{serverroot}, 'cgi-bin', 'basic.pl');
# very basic compilation/response test
for my $alias (@aliases) {
Index: ModPerl-Registry/t/redirect.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/redirect.t,v
retrieving revision 1.6
diff -u -r1.6 redirect.t
--- ModPerl-Registry/t/redirect.t 23 Nov 2003 21:01:50 -0000 1.6
+++ ModPerl-Registry/t/redirect.t 1 Dec 2003 07:03:50 -0000
@@ -5,7 +5,7 @@
use Apache::TestUtil;
use Apache::TestRequest qw(GET_BODY HEAD);
-use File::Spec::Functions qw(catfile);
+use Apache::TestUtil qw(caturl_normalized);
plan tests => 4, have_lwp;
@@ -16,7 +16,7 @@
my $redirect_path = "/registry/basic.pl";
my $url = "$base_url?$redirect_path";
my $vars = Apache::Test::config()->{vars};
- my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+ my $script_file = caturl_normalized($vars->{serverroot}, 'cgi-bin', 'basic.pl');
ok t_cmp(
"ok $script_file",
=================================================================
If OK, I'll add some comments, and a short pod description
in Apache::TestUtil, of what caturl_normalized and
catfile_normalized are doing.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
>>>Ah, what do you get as a response? a path like
>>>/tmp/foo.pl on win32? In which case the problem would be
>>>coming from:
>>>
>>> $_[0]->{FILENAME} = $_[1]->filename;
>>>
>>>in RegistryCooker.pm, which sets $0, eventually printed by cgi-bin/basic.pl.
>>>Does $r->filename return a unix path on winFU?
So does it do that? I suppose that it does...
>>I guess one could look at it from both ends ... I was
>>thinking here that the problem was in the way that
>>$script_file was constructed with catfile, which on
>>Win32 came out as SERVER_ROOT\cgi-bin\basic.pl. So the
>>patch above changes this to SERVER_ROOT/cgi-bin/basic.pl,
>>which agrees with the response.
Yes, but $script_file is a fs path, not URL. I'm not questioning the
correctness of your patch, Randy, it just looks like you are workaround a
problem that's coming from $r->filename. Is it possible that it may return the
\ paths as well?
> Further to this, I think most people on Win32 (and probably
> all non-Unices) are used to using '/' as a directory
> separator when inside a web environment (eg, in constructing
> urls). So I would think that SERVER_ROOT/cgi-bin/basic.pl
> is the correct cross-platform value ...
>
> However (I hate bringing this up :(, on Win32 there's
> problems with sometimes the dos short path name being
> used and then compared with the long path name
> (eg, MODPER~1.0 vs modperl-2.0). This in principle
> is what needs to be done to get t/basic.t and t/redirect.t
> to pass under ModPerl-Registry:
[...]
> -my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
> +my $script_file = File::Spec::Unix->catfile($vars->{serverroot}, 'cgi-bin', 'basic.pl');
This looks absolutely unintuitive and requires a comment in every place you
use it. Therefore you are probably better off with your original patch to do
an explicit: join "/n", ...
> +$script_file = Win32::GetLongPathName($script_file)
> + if Apache::TestConfig::WIN32;
Yes, it's ugly and those of us coding on unix will never remember to do that,
leaving you with the boring cleanup job. Therefore please think of extending
Apache::TestXXX API to have this Win32::GetLongPathName part hidden within.
Both join "/", .. and Win32::GetLongPathName can go inside. e.g.
catfile_normalized() and caturl_normalized()? where any post-processings like
Win32::GetLongPathName will come from the _normalized part? I'm not sure if
the name selection is good. Does it sound good?
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] ModPerl-Registry redirect test
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 30 Nov 2003, Randy Kobes wrote:
> On Sun, 30 Nov 2003, Stas Bekman wrote:
[ .. ]
> > Ah, what do you get as a response? a path like
> > /tmp/foo.pl on win32? In which case the problem would be
> > coming from:
> >
> > $_[0]->{FILENAME} = $_[1]->filename;
> >
> > in RegistryCooker.pm, which sets $0, eventually printed by cgi-bin/basic.pl.
> > Does $r->filename return a unix path on winFU?
>
> I guess one could look at it from both ends ... I was
> thinking here that the problem was in the way that
> $script_file was constructed with catfile, which on
> Win32 came out as SERVER_ROOT\cgi-bin\basic.pl. So the
> patch above changes this to SERVER_ROOT/cgi-bin/basic.pl,
> which agrees with the response.
Further to this, I think most people on Win32 (and probably
all non-Unices) are used to using '/' as a directory
separator when inside a web environment (eg, in constructing
urls). So I would think that SERVER_ROOT/cgi-bin/basic.pl
is the correct cross-platform value ...
However (I hate bringing this up :(, on Win32 there's
problems with sometimes the dos short path name being
used and then compared with the long path name
(eg, MODPER~1.0 vs modperl-2.0). This in principle
is what needs to be done to get t/basic.t and t/redirect.t
to pass under ModPerl-Registry:
============================================================
Index: t/basic.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/basic.t,v
retrieving revision 1.15
diff -u -r1.15 basic.t
--- t/basic.t 23 Nov 2003 21:01:50 -0000 1.15
+++ t/basic.t 1 Dec 2003 04:23:37 -0000
@@ -6,7 +6,7 @@
use Apache::TestRequest qw(GET GET_BODY HEAD);
use Apache::TestConfig ();
-use File::Spec::Functions qw(catfile);
+use File::Spec::Unix;
my %modules = (
registry => 'ModPerl::Registry',
@@ -19,7 +19,9 @@
plan tests => @aliases * 4 + 3;
my $vars = Apache::Test::config()->{vars};
-my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+my $script_file = File::Spec::Unix->catfile($vars->{serverroot}, 'cgi-bin', 'basic.pl');
+$script_file = Win32::GetLongPathName($script_file)
+ if Apache::TestConfig::WIN32;
# very basic compilation/response test
for my $alias (@aliases) {
Index: t/redirect.t
===================================================================
RCS file: /home/cvs/modperl-2.0/ModPerl-Registry/t/redirect.t,v
retrieving revision 1.6
diff -u -r1.6 redirect.t
--- t/redirect.t 23 Nov 2003 21:01:50 -0000 1.6
+++ t/redirect.t 1 Dec 2003 04:23:37 -0000
@@ -5,7 +5,7 @@
use Apache::TestUtil;
use Apache::TestRequest qw(GET_BODY HEAD);
-use File::Spec::Functions qw(catfile);
+use File::Spec::Unix;
plan tests => 4, have_lwp;
@@ -16,7 +16,9 @@
my $redirect_path = "/registry/basic.pl";
my $url = "$base_url?$redirect_path";
my $vars = Apache::Test::config()->{vars};
- my $script_file = catfile $vars->{serverroot}, 'cgi-bin', 'basic.pl';
+ my $script_file = File::Spec::Unix->catfile($vars->{serverroot}, 'cgi-bin', 'basic.pl');
+ $script_file = Win32::GetLongPathName($script_file)
+ if Apache::TestConfig::WIN32;
ok t_cmp(
"ok $script_file",
==================================================================
but it's not pretty ...
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org