| Message ID | 20181205184404.GC246451@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gitweb: local configuration not found | expand |
On Wed, Dec 05 2018, Jonathan Nieder wrote: > Martin Mares wrote[1]: > >> After upgrade to Stretch, gitweb no longer finds the configuration file >> "gitweb_config.perl" in the current directory. However, "man gitweb" still >> mentions this as one of the possible locations of the config file (and >> indeed a useful one when using multiple instances of gitweb). >> >> It was probably broken by Perl dropping "." from the default search path >> for security reasons. > > Indeed, perldelta(1) tells me that in 5.24.1 (and 5.26, etc), > > Core modules and tools no longer search "." for optional modules > > gitweb.perl contains > > sub read_config_file { > my $filename = shift; > return unless defined $filename; > # die if there are errors parsing config file > if (-e $filename) { > do $filename; > > which implies an @INC search but it is silly --- as the "-e" test > illustrates, this never intended to search @INC. > > Documentation says "If you are absolutely certain that you want your > script to load and execute a file from the current directory, then use > a ./ prefix". We can do that, like so: > > diff --git i/gitweb/Makefile w/gitweb/Makefile > index cd194d057f..3160b6cc5d 100644 > --- i/gitweb/Makefile > +++ w/gitweb/Makefile > @@ -18,7 +18,7 @@ RM ?= rm -f > INSTALL ?= install > > # default configuration for gitweb > -GITWEB_CONFIG = gitweb_config.perl > +GITWEB_CONFIG = ./gitweb_config.perl > GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf > GITWEB_CONFIG_COMMON = /etc/gitweb-common.conf > GITWEB_HOME_LINK_STR = projects > > but that does not help if someone overrides GITWEB_CONFIG, and besides, > it would be nicer to avoid the possibility of an @INC search altogether. > Another alternative would be to use > > local @INC = ('.'); > > Would that be better? > > Advice from someone more versed than I am in perl would be very welcome > (hence the cc to Ævar). It seems most sensible to follow the ./FILE.pl advice per https://metacpan.org/pod/distribution/perl/pod/perl5260delta.pod#Removal-of-the-current-directory-(%22.%22)-from-@INC Just: local @INC = '.'; do 'FILE.pl'; Would do the same thing, but seems like a more indirect way to do it if all we want is ./ anyway. FWIW to be pedantically bug-compatible with the old version (we should not do this) it's: local @INC = (@INC, "."); do 'FILE.pl'; I.e. before our behavior was implicitly to check whether we had a local FILE.pl, then loop through all of @INC to see if we found it there, and finally come back to the file we did the -e check for.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> Documentation says "If you are absolutely certain that you want your >> script to load and execute a file from the current directory, then use >> a ./ prefix". We can do that, like so: >> >> diff --git i/gitweb/Makefile w/gitweb/Makefile >> index cd194d057f..3160b6cc5d 100644 >> --- i/gitweb/Makefile >> +++ w/gitweb/Makefile >> @@ -18,7 +18,7 @@ RM ?= rm -f >> INSTALL ?= install >> >> # default configuration for gitweb >> -GITWEB_CONFIG = gitweb_config.perl >> +GITWEB_CONFIG = ./gitweb_config.perl >> GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf >> GITWEB_CONFIG_COMMON = /etc/gitweb-common.conf >> GITWEB_HOME_LINK_STR = projects >> >> but that does not help if someone overrides GITWEB_CONFIG, and besides, >> it would be nicer to avoid the possibility of an @INC search altogether. >> ... > Just: > > local @INC = '.'; > do 'FILE.pl'; > > Would do the same thing, but seems like a more indirect way to do it if > all we want is ./ anyway. Yeah, it does look indirect. Despite what you said, it also would support users giving an absolute path via GITWEB_CONFIG. With "use File::Spec", perhaps something like this? gitweb/gitweb.perl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 2594a4badb..239e7cbc25 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -719,6 +719,10 @@ sub filter_and_validate_refs { sub read_config_file { my $filename = shift; return unless defined $filename; + + $filename = File::Spec->catfile(".", $filename) + unless File::Spec->file_name_is_absolute($filename); + # die if there are errors parsing config file if (-e $filename) { do $filename;
Hello! > Yeah, it does look indirect. Despite what you said, it also would > support users giving an absolute path via GITWEB_CONFIG. > > With "use File::Spec", perhaps something like this? Yes, this looks right. Martin
diff --git i/gitweb/Makefile w/gitweb/Makefile index cd194d057f..3160b6cc5d 100644 --- i/gitweb/Makefile +++ w/gitweb/Makefile @@ -18,7 +18,7 @@ RM ?= rm -f INSTALL ?= install # default configuration for gitweb -GITWEB_CONFIG = gitweb_config.perl +GITWEB_CONFIG = ./gitweb_config.perl GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf GITWEB_CONFIG_COMMON = /etc/gitweb-common.conf GITWEB_HOME_LINK_STR = projects