* [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
@ 2010-07-07 9:57 Jakub Narebski
2010-07-07 10:30 ` Ævar Arnfjörð Bjarmason
2010-07-07 16:25 ` Junio C Hamano
0 siblings, 2 replies; 14+ messages in thread
From: Jakub Narebski @ 2010-07-07 9:57 UTC (permalink / raw)
To: git; +Cc: Pavan Kumar Sunkara, Petr Baudis, Christian Couder, Jakub Narebski
Prepare gitweb for having been split into modules that are to be
installed alongside gitweb in 'lib/' subdirectory, by adding
use lib __DIR__.'/lib';
to gitweb.perl (to main gitweb script), and preparing for putting
modules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version uses shell 'for' loop, instead of make's $(foreach)
function, to avoid possible problem with generating a command line
that exceeded the maximum argument list length. This solution has
also advantage of being more readable than original $(foreach ...)
version, with its overly long lines.
This idea is based on discussion surrounding Brandon Casey's patch.
In particular Bruce Stephens's suggestion was implemented to solve
issue of empty $(GITWEB_MODULES) variable (which is in this patch),
as implemented in
[PATCH 2/2 v2] Makefile: work around ksh's failure to handle missing list argument to for loop
http://thread.gmane.org/gmane.comp.version-control.git/147796/focus=150413
This version also moves installing modules to separate install-modules
target, as compared to previous version.
It also contains improvement from Pavan Kumar Sunkara, namely that
target is a directory, rather than destibation filename. This might
be not necessary (and not much of an improvement).
gitweb/Makefile | 14 +++++++++++++-
gitweb/gitweb.perl | 9 +++++++++
2 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 2fb7c2d..84a1d71 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -55,6 +55,7 @@ PERL_PATH ?= /usr/bin/perl
bindir_SQ = $(subst ','\'',$(bindir))#'
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
+gitweblibdir_SQ = $(subst ','\'',$(gitwebdir)/lib)#'
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
@@ -145,12 +146,23 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
### Installation rules
-install: all
+install: all install-modules
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+install-modules:
+ install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
+ for dir in $$install_dirs; do \
+ test -d '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir' || \
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir'; \
+ done
+ gitweb_modules=$(GITWEB_MODULES) && \
+ for mod in $$gitweb_modules; do \
+ $(INSTALL) -m 644 $$mod '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$(dirname $$mod)'; \
+ done
+
### Cleaning rules
clean:
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1f611d2..bf1485c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -9,6 +9,14 @@
use strict;
use warnings;
+
+use File::Spec;
+# __DIR__ is taken from Dir::Self __DIR__ fragment
+sub __DIR__ () {
+ File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
+}
+use lib __DIR__ . '/lib';
+
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser set_message);
@@ -16,6 +24,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+
binmode STDOUT, ':utf8';
our $t0;
--
1.7.0.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 9:57 [PATCHv2/RFC] gitweb: Prepare for splitting gitweb Jakub Narebski
@ 2010-07-07 10:30 ` Ævar Arnfjörð Bjarmason
2010-07-07 20:05 ` Jakub Narebski
2010-07-07 16:25 ` Junio C Hamano
1 sibling, 1 reply; 14+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-07 10:30 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, Jul 7, 2010 at 09:57, Jakub Narebski <jnareb@gmail.com> wrote:
> [...]
> use strict;
> use warnings;
> +
> +use File::Spec;
> +# __DIR__ is taken from Dir::Self __DIR__ fragment
> +sub __DIR__ () {
> + File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
> +}
> +use lib __DIR__ . '/lib';
> +
What was the result of the discussion about using __DIR__ again? You
only need to use that method when you call a perl program whith *then*
includes a module. If you just need to add the lib/ directory relative
to the script you're invoking you can use FindBin:
use FindBin qw($Bin);
use lib "$Bin/lib";
> use CGI qw(:standard :escapeHTML -nosticky);
> use CGI::Util qw(unescape);
> use CGI::Carp qw(fatalsToBrowser set_message);
> @@ -16,6 +24,7 @@ use Encode;
> use Fcntl ':mode';
> use File::Find qw();
> use File::Basename qw(basename);
> +
> binmode STDOUT, ':utf8';
The whitespace change distracts from the real patch a bit.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 9:57 [PATCHv2/RFC] gitweb: Prepare for splitting gitweb Jakub Narebski
2010-07-07 10:30 ` Ævar Arnfjörð Bjarmason
@ 2010-07-07 16:25 ` Junio C Hamano
2010-07-07 20:20 ` Jakub Narebski
1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2010-07-07 16:25 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
Jakub Narebski <jnareb@gmail.com> writes:
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index 2fb7c2d..84a1d71 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -145,12 +146,23 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
>
> ### Installation rules
>
> -install: all
> +install: all install-modules
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> $(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
> $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
>
> +install-modules:
> + install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
> + for dir in $$install_dirs; do \
> + test -d '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir' || \
> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir'; \
> + done
> + gitweb_modules=$(GITWEB_MODULES) && \
> + for mod in $$gitweb_modules; do \
> + $(INSTALL) -m 644 $$mod '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$(dirname $$mod)'; \
> + done
> +
Sorry, but you lost me here. Where is GITWEB_MODULES defined (iow, what
commit is this patch supposed to be applied)?
I also suspect that your assignment to "install_dirs" is completely bogus
when the files listed in GITWEB_MODULES span multiple directories.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 10:30 ` Ævar Arnfjörð Bjarmason
@ 2010-07-07 20:05 ` Jakub Narebski
2010-07-08 7:01 ` Jakub Narebski
2010-07-13 22:24 ` Jakub Narebski
0 siblings, 2 replies; 14+ messages in thread
From: Jakub Narebski @ 2010-07-07 20:05 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, 7 Jul 2010, Ævar Arnfjörð Bjarmason napisał:
> On Wed, Jul 7, 2010 at 09:57, Jakub Narebski <jnareb@gmail.com> wrote:
> > [...]
> > use strict;
> > use warnings;
> > +
> > +use File::Spec;
> > +# __DIR__ is taken from Dir::Self __DIR__ fragment
> > +sub __DIR__ () {
> > + File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
> > +}
> > +use lib __DIR__ . '/lib';
> > +
>
> What was the result of the discussion about using __DIR__ again? You
> only need to use that method when you call a perl program whith *then*
> includes a module. If you just need to add the lib/ directory relative
> to the script you're invoking you can use FindBin:
>
> use FindBin qw($Bin);
> use lib "$Bin/lib";
It's
use lib "$Bin/../lib";
(I don't like this ../lib here, but that's my personal preference).
But you might be right that I am overly cautious in avoiding FindBin.
It is nowadays not recommended solution (on #perl, for example), but
it is perhaps true that the conditions where it fails are not fulfilled
for the case of gitweb.
Or perhaps not:
KNOWN ISSUES
If there are two modules using FindBin from different directories
under the same interpreter, this won't work. Since FindBin uses a
BEGIN block, it'll be executed only once, and only the first caller
will get it right. This is a problem under **mod_perl** and other
persistent Perl environments, where you shouldn't use this module.
Gitweb can be used under mod_perl (to be more exact ModPerl::Registry).
> > use CGI qw(:standard :escapeHTML -nosticky);
> > use CGI::Util qw(unescape);
> > use CGI::Carp qw(fatalsToBrowser set_message);
> > @@ -16,6 +24,7 @@ use Encode;
> > use Fcntl ':mode';
> > use File::Find qw();
> > use File::Basename qw(basename);
> > +
> > binmode STDOUT, ':utf8';
>
> The whitespace change distracts from the real patch a bit.
Ooops, I'm sorry.
I'll fix it in resend, if required.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 16:25 ` Junio C Hamano
@ 2010-07-07 20:20 ` Jakub Narebski
2010-07-08 0:30 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Narebski @ 2010-07-07 20:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, 7 Jul 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > diff --git a/gitweb/Makefile b/gitweb/Makefile
> > index 2fb7c2d..84a1d71 100644
> > --- a/gitweb/Makefile
> > +++ b/gitweb/Makefile
> > @@ -145,12 +146,23 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
> >
> > ### Installation rules
> >
> > -install: all
> > +install: all install-modules
> > $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> > $(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> > $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
> > $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
> >
> > +install-modules:
> > + install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
> > + for dir in $$install_dirs; do \
> > + test -d '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir' || \
> > + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir'; \
> > + done
> > + gitweb_modules=$(GITWEB_MODULES) && \
> > + for mod in $$gitweb_modules; do \
> > + $(INSTALL) -m 644 $$mod '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$(dirname $$mod)'; \
> > + done
> > +
>
> Sorry, but you lost me here. Where is GITWEB_MODULES defined (iow, what
> commit is this patch supposed to be applied)?
This is preparatory patch either for Pavan GSoC2010 work on splitting
gitweb and adding committool / admin write capabilities, or for mine
gitweb caching series split into modules (based on J.H. patch for
git.kernel.org).
So currently $(GITWEB_MODULES) is not defined / empty.
> I also suspect that your assignment to "install_dirs" is completely bogus
> when the files listed in GITWEB_MODULES span multiple directories.
install_dirs contains _list_ of directories gitweb modules are to be
installed into. `$(dir NAMES...)' extracts the directory-part of each
file name in NAMES ('./' if there is no '/' in a file name), and
`$(sort LIST)' is used to remove duplicate words in LIST.
It was actually tested that it works for modularized gitweb caching,
in the older form using `$(foreach ...)' rather than current shell
'for' loop.
For example with the following Makefile:
GITWEB_MODULES += GitwebCache/SimpleFileCache.pm
GITWEB_MODULES += GitwebCache/Capture.pm
GITWEB_MODULES += GitwebCache/Capture/SelectFH.pm
GITWEB_MODULES += Gitweb/Config.pm
all:
@echo $(sort $(dir $(GITWEB_MODULES)))
running 'make' results in:
Gitweb/ GitwebCache/ GitwebCache/Capture/
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 20:20 ` Jakub Narebski
@ 2010-07-08 0:30 ` Junio C Hamano
2010-07-08 6:59 ` Jakub Narebski
0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2010-07-08 0:30 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
Jakub Narebski <jnareb@gmail.com> writes:
>> > +install-modules:
>> > + install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
>> > ...
>
> For example with the following Makefile:
>
> GITWEB_MODULES += GitwebCache/SimpleFileCache.pm
> GITWEB_MODULES += GitwebCache/Capture.pm
> GITWEB_MODULES += GitwebCache/Capture/SelectFH.pm
> GITWEB_MODULES += Gitweb/Config.pm
>
> all:
> @echo $(sort $(dir $(GITWEB_MODULES)))
>
> running 'make' results in:
>
> Gitweb/ GitwebCache/ GitwebCache/Capture/
Try replacing that with:
all:
install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
echo $$install_dirs
perhaps?
Hint: dq.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-08 0:30 ` Junio C Hamano
@ 2010-07-08 6:59 ` Jakub Narebski
2010-07-08 7:20 ` [PATCHv3/RFC] " Jakub Narebski
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Narebski @ 2010-07-08 6:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
Dnia czwartek 8. lipca 2010 02:30, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> >> > +install-modules:
> >> > + install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
> >> > ...
> >
> > For example with the following Makefile:
> >
> > GITWEB_MODULES += GitwebCache/SimpleFileCache.pm
> > GITWEB_MODULES += GitwebCache/Capture.pm
> > GITWEB_MODULES += GitwebCache/Capture/SelectFH.pm
> > GITWEB_MODULES += Gitweb/Config.pm
> >
> > all:
> > @echo $(sort $(dir $(GITWEB_MODULES)))
> >
> > running 'make' results in:
> >
> > Gitweb/ GitwebCache/ GitwebCache/Capture/
>
>
> Try replacing that with:
>
> all:
> install_dirs=$(sort $(dir $(GITWEB_MODULES))) && \
> echo $$install_dirs
>
> perhaps?
>
> Hint: dq.
You are completely right. I'm sorry. I should have copied the
structure from "[PATCH 2/2 v2] Makefile: work around ksh's failure to
handle missing list argument to for loop" more carefully.
I'll resend it.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 20:05 ` Jakub Narebski
@ 2010-07-08 7:01 ` Jakub Narebski
2010-07-13 22:24 ` Jakub Narebski
1 sibling, 0 replies; 14+ messages in thread
From: Jakub Narebski @ 2010-07-08 7:01 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
Jakub Narebski wrote:
> On Wed, 7 Jul 2010, Ævar Arnfjörð Bjarmason napisał:
> > On Wed, Jul 7, 2010 at 09:57, Jakub Narebski <jnareb@gmail.com> wrote:
> > > [...]
> > > use strict;
> > > use warnings;
> > > +
> > > +use File::Spec;
> > > +# __DIR__ is taken from Dir::Self __DIR__ fragment
> > > +sub __DIR__ () {
> > > + File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
> > > +}
> > > +use lib __DIR__ . '/lib';
> > > +
> >
> > What was the result of the discussion about using __DIR__ again? You
> > only need to use that method when you call a perl program whith *then*
> > includes a module. If you just need to add the lib/ directory relative
> > to the script you're invoking you can use FindBin:
> >
> > use FindBin qw($Bin);
> > use lib "$Bin/lib";
>
> It's
>
> use lib "$Bin/../lib";
>
> (I don't like this ../lib here, but that's my personal preference).
>
> But you might be right that I am overly cautious in avoiding FindBin.
> It is nowadays not recommended solution (on #perl, for example), but
> it is perhaps true that the conditions where it fails are not fulfilled
> for the case of gitweb.
>
> Or perhaps not:
>
> KNOWN ISSUES
>
> If there are two modules using FindBin from different directories
> under the same interpreter, this won't work. Since FindBin uses a
> BEGIN block, it'll be executed only once, and only the first caller
> will get it right. This is a problem under **mod_perl** and other
> persistent Perl environments, where you shouldn't use this module.
>
> Gitweb can be used under mod_perl (to be more exact ModPerl::Registry).
And there is also nowadays 'plackup' option for git-instaweb, which
also would have, I think, this problem with FindBin.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv3/RFC] gitweb: Prepare for splitting gitweb
2010-07-08 6:59 ` Jakub Narebski
@ 2010-07-08 7:20 ` Jakub Narebski
0 siblings, 0 replies; 14+ messages in thread
From: Jakub Narebski @ 2010-07-08 7:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
Prepare gitweb for having been split into modules that are to be
installed alongside gitweb in 'lib/' subdirectory, by adding
use lib __DIR__.'/lib';
to gitweb.perl (to main gitweb script), and preparing for putting
modules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Differences from v2:
* Fixed issue with quoting value of shell variable, pointed by Junio
* Removed spurious unrelated addition of an empty line, pointed by
Ævar
gitweb/Makefile | 14 +++++++++++++-
gitweb/gitweb.perl | 8 ++++++++
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 2fb7c2d..b39d716 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -55,6 +55,7 @@ PERL_PATH ?= /usr/bin/perl
bindir_SQ = $(subst ','\'',$(bindir))#'
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
+gitweblibdir_SQ = $(subst ','\'',$(gitwebdir)/lib)#'
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
@@ -145,12 +146,23 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
### Installation rules
-install: all
+install: all install-modules
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+install-modules:
+ install_dirs="$(sort $(dir $(GITWEB_MODULES)))" && \
+ for dir in $$install_dirs; do \
+ test -d '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir' || \
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$dir'; \
+ done
+ gitweb_modules="$(GITWEB_MODULES)" && \
+ for mod in $$gitweb_modules; do \
+ $(INSTALL) -m 644 $$mod '$(DESTDIR_SQ)$(gitwebdir_SQ)/$$(dirname $$mod)'; \
+ done
+
### Cleaning rules
clean:
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1f611d2..ec60a4d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -9,6 +9,14 @@
use strict;
use warnings;
+
+use File::Spec;
+# __DIR__ is taken from Dir::Self __DIR__ fragment
+sub __DIR__ () {
+ File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
+}
+use lib __DIR__ . '/lib';
+
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser set_message);
--
1.7.0.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-07 20:05 ` Jakub Narebski
2010-07-08 7:01 ` Jakub Narebski
@ 2010-07-13 22:24 ` Jakub Narebski
2010-07-13 22:30 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 14+ messages in thread
From: Jakub Narebski @ 2010-07-13 22:24 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, 7 Jul 2010, Jakub Narebski wrote:
> On Wed, 7 Jul 2010, Ævar Arnfjörð Bjarmason napisał:
> > On Wed, Jul 7, 2010 at 09:57, Jakub Narebski <jnareb@gmail.com> wrote:
> > > [...]
> > > use strict;
> > > use warnings;
> > > +
> > > +use File::Spec;
> > > +# __DIR__ is taken from Dir::Self __DIR__ fragment
> > > +sub __DIR__ () {
> > > + File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
> > > +}
> > > +use lib __DIR__ . '/lib';
> > > +
> >
> > What was the result of the discussion about using __DIR__ again? You
> > only need to use that method when you call a perl program whith *then*
> > includes a module. If you just need to add the lib/ directory relative
> > to the script you're invoking you can use FindBin:
> >
> > use FindBin qw($Bin);
> > use lib "$Bin/lib";
>
> It's
>
> use lib "$Bin/../lib";
>
> (I don't like this ../lib here, but that's my personal preference).
Actually for gitweb, where gitweb.cgi is put in "top" directory, and
lib/ directory is in the same directory that the script is, it would
be
use FindBin qw($Bin);
use lib "$Bin/lib";
The documentation says that $FindBin::Bin is
$Bin - path to bin directory from where script was invoked
so it is path to directory, not to script itself.
If it worked...
> But you might be right that I am overly cautious in avoiding FindBin.
> It is nowadays not recommended solution (on #perl, for example), but
> it is perhaps true that the conditions where it fails are not fulfilled
> for the case of gitweb.
>
> Or perhaps not:
>
> KNOWN ISSUES
>
> If there are two modules using FindBin from different directories
> under the same interpreter, this won't work. Since FindBin uses a
> BEGIN block, it'll be executed only once, and only the first caller
> will get it right. This is a problem under **mod_perl** and other
> persistent Perl environments, where you shouldn't use this module.
>
> Gitweb can be used under mod_perl (to be more exact ModPerl::Registry).
I wrote simple script that tests result of __DIR__ and $FindBin::Bin.
For cgi-bin / mod_cgi it was:
__DIR__ = /var/www/cgi-bin/gitweb (symlink to /home/local/gitweb)
$FindBin::Bin = /home/local/gitweb
For mod_perl (ModPerl::Registry handler) it was
__DIR__ = /var/www/perl/gitweb (symlink to /home/local/gitweb)
$FindBin::Bin = /
As you can see it's useless. I have't checked the FastCGI case...
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-13 22:24 ` Jakub Narebski
@ 2010-07-13 22:30 ` Ævar Arnfjörð Bjarmason
2010-07-14 9:24 ` Jakub Narebski
0 siblings, 1 reply; 14+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-13 22:30 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Tue, Jul 13, 2010 at 22:24, Jakub Narebski <jnareb@gmail.com> wrote:
> I wrote simple script that tests result of __DIR__ and $FindBin::Bin.
> For cgi-bin / mod_cgi it was:
>
> __DIR__ = /var/www/cgi-bin/gitweb (symlink to /home/local/gitweb)
> $FindBin::Bin = /home/local/gitweb
>
> For mod_perl (ModPerl::Registry handler) it was
>
> __DIR__ = /var/www/perl/gitweb (symlink to /home/local/gitweb)
> $FindBin::Bin = /
>
> As you can see it's useless. I have't checked the FastCGI case...
Thanks for spending time researching what was an offhand ignorant "hey
wasn't .." comment. Also, sorry :)
But that's very informative. Good to know that FindBin is broken like
that under mod_perl.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-13 22:30 ` Ævar Arnfjörð Bjarmason
@ 2010-07-14 9:24 ` Jakub Narebski
2010-07-14 10:05 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Narebski @ 2010-07-14 9:24 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, 14 Jul 2010, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Jul 13, 2010 at 22:24, Jakub Narebski <jnareb@gmail.com> wrote:
>
> > I wrote simple script that tests result of __DIR__ and $FindBin::Bin.
> > For cgi-bin / mod_cgi it was:
> >
> > __DIR__ = /var/www/cgi-bin/gitweb (symlink to /home/local/gitweb)
> > $FindBin::Bin = /home/local/gitweb
> >
> > For mod_perl (ModPerl::Registry handler) it was
> >
> > __DIR__ = /var/www/perl/gitweb (symlink to /home/local/gitweb)
> > $FindBin::Bin = /
> >
> > As you can see it's useless. I have't checked the FastCGI case...
>
> Thanks for spending time researching what was an offhand ignorant "hey
> wasn't .." comment. Also, sorry :)
Nothing to it. I wanted to check if there really is a problem with
FindBin on mod_perl, as I was not sure with description in "Known Issues"
section in FindBin manpage.
Note that using 'FindBin->again();' after 'use FindBin;' fixes this
issue. So perhaps it would be beter to use FindBin than borrow code
for __DIR__ from Dir::Self.
> But that's very informative. Good to know that FindBin is broken like
> that under mod_perl.
It might be only under ModPerl::Registry, but it might be not.
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-14 9:24 ` Jakub Narebski
@ 2010-07-14 10:05 ` Ævar Arnfjörð Bjarmason
2010-07-14 21:21 ` Jakub Narebski
0 siblings, 1 reply; 14+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-07-14 10:05 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, Jul 14, 2010 at 09:24, Jakub Narebski <jnareb@gmail.com> wrote:
> On Wed, 14 Jul 2010, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, Jul 13, 2010 at 22:24, Jakub Narebski <jnareb@gmail.com> wrote:
>>
>> > I wrote simple script that tests result of __DIR__ and $FindBin::Bin.
>> > For cgi-bin / mod_cgi it was:
>> >
>> > __DIR__ = /var/www/cgi-bin/gitweb (symlink to /home/local/gitweb)
>> > $FindBin::Bin = /home/local/gitweb
>> >
>> > For mod_perl (ModPerl::Registry handler) it was
>> >
>> > __DIR__ = /var/www/perl/gitweb (symlink to /home/local/gitweb)
>> > $FindBin::Bin = /
>> >
>> > As you can see it's useless. I have't checked the FastCGI case...
>>
>> Thanks for spending time researching what was an offhand ignorant "hey
>> wasn't .." comment. Also, sorry :)
>
> Nothing to it. I wanted to check if there really is a problem with
> FindBin on mod_perl, as I was not sure with description in "Known Issues"
> section in FindBin manpage.
>
> Note that using 'FindBin->again();' after 'use FindBin;' fixes this
> issue. So perhaps it would be beter to use FindBin than borrow code
> for __DIR__ from Dir::Self.
That should work, though note that FindBin->again requires at least
perl 5.8.3. This should work on older versions (if Gitweb cares):
if ($] <= 5.008003) {
delete $INC{'FindBin.pm'};
require FindBin;
} else {
FindBin->again;
}
See https://rt.cpan.org/Public/Bug/Display.html?id=57988 and
http://search.cpan.org/diff?from=Module-Install-0.98&to=Module-Install-0.99
for reference.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb
2010-07-14 10:05 ` Ævar Arnfjörð Bjarmason
@ 2010-07-14 21:21 ` Jakub Narebski
0 siblings, 0 replies; 14+ messages in thread
From: Jakub Narebski @ 2010-07-14 21:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Pavan Kumar Sunkara, Petr Baudis, Christian Couder
On Wed, 14 Jul 2010, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Jul 14, 2010 at 09:24, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Wed, 14 Jul 2010, Ævar Arnfjörð Bjarmason wrote:
>>> On Tue, Jul 13, 2010 at 22:24, Jakub Narebski <jnareb@gmail.com> wrote:
>>>
>>>> I wrote simple script that tests result of __DIR__ and $FindBin::Bin.
>>>> For cgi-bin / mod_cgi it was:
>>>>
>>>> __DIR__ = /var/www/cgi-bin/gitweb (symlink to /home/local/gitweb)
>>>> $FindBin::Bin = /home/local/gitweb
>>>>
>>>> For mod_perl (ModPerl::Registry handler) it was
>>>>
>>>> __DIR__ = /var/www/perl/gitweb (symlink to /home/local/gitweb)
>>>> $FindBin::Bin = /
>>>>
>>>> As you can see it's useless. I have't checked the FastCGI case...
>>>
>>> Thanks for spending time researching what was an offhand ignorant "hey
>>> wasn't .." comment. Also, sorry :)
>>
>> Nothing to it. I wanted to check if there really is a problem with
>> FindBin on mod_perl, as I was not sure with description in "Known Issues"
>> section in FindBin manpage.
>>
>> Note that using 'FindBin->again();' after 'use FindBin;' fixes this
>> issue. So perhaps it would be beter to use FindBin than borrow code
>> for __DIR__ from Dir::Self.
>
> That should work, though note that FindBin->again requires at least
> perl 5.8.3. This should work on older versions (if Gitweb cares):
>
> if ($] <= 5.008003) {
> delete $INC{'FindBin.pm'};
> require FindBin;
> } else {
> FindBin->again;
> }
>
> See https://rt.cpan.org/Public/Bug/Display.html?id=57988 and
> http://search.cpan.org/diff?from=Module-Install-0.98&to=Module-Install-0.99
> for reference.
Thanks for research.
I don't think gitweb cares, as it requires Perl with sane Unicode support,
which means at least Perl 5.8.0 (for Encode module), but I think it really
means at least 5.8.3
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-07-14 21:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-07 9:57 [PATCHv2/RFC] gitweb: Prepare for splitting gitweb Jakub Narebski
2010-07-07 10:30 ` Ævar Arnfjörð Bjarmason
2010-07-07 20:05 ` Jakub Narebski
2010-07-08 7:01 ` Jakub Narebski
2010-07-13 22:24 ` Jakub Narebski
2010-07-13 22:30 ` Ævar Arnfjörð Bjarmason
2010-07-14 9:24 ` Jakub Narebski
2010-07-14 10:05 ` Ævar Arnfjörð Bjarmason
2010-07-14 21:21 ` Jakub Narebski
2010-07-07 16:25 ` Junio C Hamano
2010-07-07 20:20 ` Jakub Narebski
2010-07-08 0:30 ` Junio C Hamano
2010-07-08 6:59 ` Jakub Narebski
2010-07-08 7:20 ` [PATCHv3/RFC] " Jakub Narebski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.