git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* gitweb: local configuration not found
       [not found] <154401401074.29584.11169979442731329694.reportbug@gimli.ms.mff.cuni.cz>
@ 2018-12-05 18:44 ` Jonathan Nieder
  2018-12-05 19:11   ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Nieder @ 2018-12-05 18:44 UTC (permalink / raw)
  To: Martin Mares; +Cc: git, Ævar Arnfjörð Bjarmason

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).

Thanks for reporting and hope that helps,
Jonathan

[1] https://bugs.debian.org/915632

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gitweb: local configuration not found
  2018-12-05 18:44 ` gitweb: local configuration not found Jonathan Nieder
@ 2018-12-05 19:11   ` Ævar Arnfjörð Bjarmason
  2018-12-06  0:56     ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-12-05 19:11 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Martin Mares, git


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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gitweb: local configuration not found
  2018-12-05 19:11   ` Ævar Arnfjörð Bjarmason
@ 2018-12-06  0:56     ` Junio C Hamano
  2018-12-06 15:02       ` Martin Mareš
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2018-12-06  0:56 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Jonathan Nieder, Martin Mares, git

Æ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;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: gitweb: local configuration not found
  2018-12-06  0:56     ` Junio C Hamano
@ 2018-12-06 15:02       ` Martin Mareš
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Mareš @ 2018-12-06 15:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder, git

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <154401401074.29584.11169979442731329694.reportbug@gimli.ms.mff.cuni.cz>
2018-12-05 18:44 ` gitweb: local configuration not found Jonathan Nieder
2018-12-05 19:11   ` Ævar Arnfjörð Bjarmason
2018-12-06  0:56     ` Junio C Hamano
2018-12-06 15:02       ` Martin Mareš

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox