Show quoted text>>>>> On Fri, 17 Feb 2006 13:15:37 -0500 (EST), "David Wheeler via RT" <bug-SVN-Notify@rt.cpan.org> said:
> On Feb 17, 2006, at 01:42, Andreas Koenig via RT wrote:
>>> This is hardcoded in SVN::Notify here:
>>
>>> # Set up default values.
>>> $params{svnlook} ||= $ENV{SVNLOOK} || '/usr/local/bin/
>>> svnlook';
>>
>> I think I would call it a bug if the PATH environment is not honoured
>> when trying to execute an external command.
> Well, I made it default to /usr/local/bin/svnlook because, when
> SVN::Notify is run from Apache (as is common with subversion), it
> could not find anything in the path, because there is no PATH
> environment variable there. I don't know about when it's running
> outside of Apache, though.
I read your explanation later in the docs somewhere. I'm sorry that I
did not find it earlier.
Show quoted text >> When I come to think if it, I'm also surprised that SVN::Notify passed
>> its test suite. Is the test suite too forgiving or too intelligent?
> I guess it's too intelligent. The tests all point to t/bin/
> testsvnlook, which is a Perl script that mimics svnlook. Same with
> sendmail.
Ok, for sendmail this makes a lot of sense, not sure for svnlook. If
people have no svnlook, your tests should probably fail.
Show quoted text > On Feb 17, 2006, at 03:10, John Peacock via RT wrote:
Show quoted text >> Oh, I see. SVNLOOK is an overrideable option in SVN::Notify, but
>> SVN is not.
>> That I can fix; give me a moment...
> Yes, SVN::Notify doesn't use svn, just svnlook.
Show quoted text >> WFM
>> $ perl -Mblib t/001_basic.t
>> ok 1 - Directories are consistent at rev: 1
>> ok 2 - Updated project1/trunk to correct revision: 2
>> ok 3 - Correct files updated in project1/trunk at rev: 2
>> ok 4 - Directories are consistent at rev: 2
>> ok 5 - No changes in project1/tags/TRUNK-1135534439 at revision: 3
>> ...
>>
>> FWIW, running single tests with Module::Build is usually done
>>
>> $ ./Build test --test_files t/001_basic.t
>>
>> but the other way should work too... :(
> As should prove t/001_basic.t.
John sent me a prerelease of the next version and with that I had
neither this or the other problem, so from my point of view all bugs
appear fixed.
Show quoted text > Anyway, what is the upshot here with regard to SVN::Notify? Should I
> just make it default to 'svnlook' instead of '/usr/local/bin/
> svnlook'? That may well break installations for folks who rely on the
> default path, but it'll only take a second for them to fix it.
Show quoted text > Opinions welcome.
CPAN::FirstTime has a find_exe routine:
sub find_exe {
my($exe,$path) = @_;
my($dir);
#warn "in find_exe exe[$exe] path[@$path]";
for $dir (@$path) {
my $abs = File::Spec->catfile($dir,$exe);
if (($abs = MM->maybe_command($abs))) {
return $abs;
}
}
}
which is typically used like
my(@path) = split /$Config{'path_sep'}/, $ENV{'PATH'};
$exe = find_exe("svnlook",\@path);
I think something like that should work quite well and you could even
append "usr/local/bin" to he path or help the user to do so. I do not
know of a module that contains this functionality.
--
andreas
