Item906: why are we using '.' to find setlib.cfg when FindBin is in Perl?

replace

    $ENV{FOSWIKI_ACTION} = 'view';
    @INC = ('.', grep { $_ ne '.' } @INC);
    require 'setlib.cfg';

with

    $ENV{FOSWIKI_ACTION} = 'view';
 use FindBin;
    @INC = ($FindBin::Bin, grep { $_ ne $FindBin::Bin } @INC);
    require 'setlib.cfg';

http://search.cpan.org/~tty/kurila-1.14_0/lib/FindBin.pm

I have abused the '.' setting previously to enable multiple t*'s to run from the same script, lib and templates dir, but its probably rare enough to disregard (and I can use other tricks to achieve the same thing)

-- SvenDowideit - 29 Jan 2009

I fully agree with Sven's point. The use of '.' in @INC assumes the script is called from the bin/ directory, what is not always true. RFC 3875 states that the server SHOULD call the script from the directory containing it, but SHOULD is not MUST and there are servers that doesn't follow the recommendation (IIS 6.0 is an example).

-- GilmarSantosJr - 30 Jan 2009

Calling FindBin::again() is required in a persistent Perl environment (e.g. mod_perl) to ensure that $FindBin::Bin is initialized correctly.

-- IsaacLin - 30 Jan 2009

Just inertia, I guess; I could never see a reason to change it. I tend to use FindBin in more modern code myself.

-- CrawfordCurrie - 30 Jan 2009

I just tried this change and got a Insecurity dependency error cause $FindBin::Bin is tainted. I fixed that, but setlib.cfg doesn't work, unless the current working directory is bin. I thought about dropping setlib.cfg but it also adjusts lib/CPAN things. So I came to this change:

--- bin/view    (revision 18527)
+++ bin/view    (local)
@@ -27,7 +27,8 @@
 BEGIN {
     if ( defined $ENV{GATEWAY_INTERFACE} ) {
         $Foswiki::cfg{Engine} = 'Foswiki::Engine::CGI';
-        use CGI::Carp qw(fatalsToBrowser);
+        require CGI::Carp;
+        CGI::Carp->import('fatalsToBrowser');
         $SIG{__DIE__} = \&CGI::Carp::confess;
     }
     else {
@@ -36,7 +37,11 @@
         $SIG{__DIE__} = \&Carp::confess;
     }
     $ENV{FOSWIKI_ACTION} = 'view';
-    @INC = ('.', grep { $_ ne '.' } @INC);
+
+    require FindBin;
+    my ($bin) = $FindBin::Bin =~ /^(.*)$/;
+    @INC = ($bin, grep { $_ ne $bin } @INC);
+    chdir $bin or die "Could not change to directory $bin ($!)";
     require 'setlib.cfg';
 }

What do you think?

-- GilmarSantosJr - 30 Jan 2009

I think that chdir-ing is exactly what we do not want. And it suggests that the $bin isn't in the @INC in the way we want it to be.

-- SvenDowideit - 31 Jan 2009

If we don't want to chdir, then we also need to update setlib.cfg. Just for reference, configure script adjusts its @INC using FindBin and chdir.

-- GilmarSantosJr - 31 Jan 2009

I agree that we don't want to chdir. I think we need to fix setlib.cfg. configure uses chdir historically; it shouldn't.

-- CrawfordCurrie - 31 Jan 2009

the other point is that the scripts are intended to work not just from the hallowed halls of the web server, but also from the command line. it should be possible to

cd /tmp
/var/lib/foswiki/bin/view My.SomeTopic > banana.html

and doing Findbin properly should make this work.

macpro:bin svend$ svn diff
Index: setlib.cfg
===================================================================
--- setlib.cfg  (revision 2408)
+++ setlib.cfg  (working copy)
@@ -32,14 +32,20 @@
 
 use vars qw( $foswikiLibPath @localPerlLibPath );
 
-eval 'require "LocalLib.cfg"';
+require FindBin;
+my ($bin) = $FindBin::Bin =~ /^(.*)$/;
+use Cwd qw( abs_path );
+my ($LocalLib) = Cwd::abs_path( "$bin/LocalLib.cfg" ) =~ /(.*)/;
+
+eval 'require "'.$LocalLib.'";';
+
 # if foswikiLibPath isn't defined, then see if $twikiLibPath is
 # for compatibility
 $foswikiLibPath = $twikiLibPath unless defined( $foswikiLibPath );
 
 unless (( defined ($foswikiLibPath) ) and (-e $foswikiLibPath)) {
        use Cwd qw( abs_path );
-       ( $foswikiLibPath ) = ($foswikiLibPath = Cwd::abs_path( "../lib" )) =~ /(.*)/;
+       ( $foswikiLibPath ) = ($foswikiLibPath = Cwd::abs_path( "$bin/../lib" )) =~ /(.*)/;
 }
 if ($foswikiLibPath eq "") {
     $foswikiLibPath = "../lib";
@@ -65,4 +71,3 @@
 }
 
 1;                             # Return success for module loading
-
Index: view
===================================================================
--- view        (revision 2408)
+++ view        (working copy)
@@ -36,8 +36,13 @@
         $SIG{__DIE__} = \&Carp::confess;
     }
     $ENV{FOSWIKI_ACTION} = 'view';
-    @INC = ('.', grep { $_ ne '.' } @INC);
-    require 'setlib.cfg';
+    #@INC = ('.', grep { $_ ne '.' } @INC);
+    require FindBin;
+    my ($bin) = $FindBin::Bin =~ /^(.*)$/;
+    use Cwd qw( abs_path );
+    my ($setlibCfg) = Cwd::abs_path( "$bin/setlib.cfg" ) =~ /(.*)/;
+
+    eval 'require "'.$setlibCfg.'";';
 }
 
 use Foswiki;

cleans things up, AND removes the bin dir from @INC where it should never have been :/

-- SvenDowideit - 08 Feb 2009

I just applied and tested Sven's patch. It worked nice.

-- GilmarSantosJr - 16 Feb 2009

Note that this "fix" that fixed something most of us did not know was broken caused release 1.0.2 to go down the drain.

So I reverted the change since noone showed any sign of wanting to repair what was damaged.

-- KennethLavrsen - 27 Feb 2009

Somone reopend this

I am reclosing because

• Once a bug report has been released in a version we do not reopen them because then we loose track of which bugs were fixed in which releases. We open a new bug report instead
• The original bugs question : "why are we using '.' to find setlib.cfg when FindBin is in Perl?" has been answered. We do not use FindBin because it does not work in mod_perl. This is clearly documented and warned against on the CPAN pages for FindBin. So please do not reintroduce this problem again. If someone has a bug to fix in current code describe the real problem in a new bug report and fix it another way than with FindBin. And not in 1.0.X scope because obviously any changes to this area requires testing in many different environments.

-- KennethLavrsen - 27 Feb 2009

sorry, i didn't realize 1.0.1 was considered released. it will be interesting to see what the current release notes will look like. :/

-- WillNorris - 28 Feb 2009