On Sat, 6 Sep 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> > On Thu, 4 Sep 2003, Stas Bekman wrote:
> >
> >
> >>In the effort to remove some of the Win32 noise, I was
> >>thinking that we can write a generic function which gets a
> >>path as an argument and figures out internally if it needs
> >>to keep the argument as passed or mangle it. So it'll do
> >>something like:
> >>
> >> my $cwd = Apache::TestUtil::path(cwd);
> >>
> >>probably need a more intuitive name for this function.
> >
> >
> > That'd be nice - a version that does this appears below.
> > I named it win32_long_path - it'll just return what was
> > passed into it if not on Win32.
>
> But my idea was to eliminate any os-specific words win32. I think it should
> just be long_path... Think of it as File::Spec function.
OK, I'll do that. I put in the win32_ to indicate that it'll
do something for Win32, and otherwise just return what was
passed in.
> I'm still not quite happy about the naming of the function, what exactly
> GetLongPathName($file) does?
> can this be done using some File::Spec function?
I haven't found an equivalent one in File::Spec ... The
problem here is that cwd (or any file/directory name) on
Win32 has two representations - a long path name (that can
include spaces) and a short path name, limited to 6
characters (this is a holdout from early days), plus 3 if an
extension is present. Here we want to match cwd to
/Apache-Test/, but cwd may return the short path name (eg,
Apache~1.0), depending on if, for example, you have
different Apache-Test* directories at the same level. So
GetLongPathName can be used to get the full long path name.
The thing with naming it, eg, just long_path, is that
Unix users (the majority) wouldn't know what it does.
> > +require File::Spec;
>
> > + my $candidate = File::Spec->catfile($_, 'Apache', $file);
>
> we import catfile in all other Apache::Test files, can do
> that here as well... will make code simpler
Good point - thanks.
>
> > + if (-e $candidate) {
> > + $sys_config = $candidate;
> > + last;
> > + }
> > + }
> > + if ($sys_config) {
> > + eval {require $sys_config};
> > + return $sys_config if (not $@ and IN_APACHE_TEST);
> > + $sys_config = undef if $@;
> > + }
>
> Hmm, I thought we agreed that eval {require $sys_config} will always succeed,
> since that file is always available. so this code should be needed:
>
> + die 'Could not find a suitable Apache::TestConfigData'
> + unless defined CONFIG_DATA;
I was thinking here if someone, after installation, had
mis-edited Apache::TestConfigData, either the system one or
one found in some path in @INC being added thru, eg, use
lib). But this might be overkill - not worrying about this
at this time will simplify things here.
> One more problem is TestRun.pm's layout: subs should go
> after the declarations (uses/constants/etc). use 'use
> subs' if you need to predeclare them.
OK - thanks.
> Another issue, is in_apache_test. Since a user may get it
> with modperl-2.0 or separately it should return true in
> either case.
I forgot about that - thanks.
> > +sub write_config {
> > + my $self = shift;
> > + my $fh = Symbol::gensym();
> > + my $vars = $self->{test_config}->{vars};
> > + my $conf_opts = $self->{conf_opts};
> > + my $file = IN_APACHE_TEST ?
> > + catfile($vars->{top_dir}, CONFIG_DATA) :
> > + CONFIG_DATA;
>
> it's easier to parse when written as:
>
> my $file = IN_APACHE_TEST
> ? catfile($vars->{top_dir}, CONFIG_DATA)
> : CONFIG_DATA;
>
> > + my $pkg = << "EOC";
> > +package Apache::TestConfigData;
>
> better have it strict, to avoid misterious errors (same in the pod below)
>
> use strict;
> use warnings;
>
> use vars (\$Apache::TestConfigData);
Thanks - I'll add that.
I was also thinking about the problem of $ENV{HOME} not
being available in mod_perl. As Apache::TestConfigData is
being loaded in order to configure things, shouldn't
this not be a problem, because at this point one isn't
yet in a mod_perl environment?
--
best regards,
randy
