[PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Ævar Arnfjörð Bjarmason]

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.

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Ævar Arnfjörð Bjarmason]

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.

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Ævar Arnfjörð Bjarmason]

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.

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Junio C Hamano]

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.

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Junio C Hamano]

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.

Re: [PATCHv2/RFC] gitweb: Prepare for splitting gitweb

[Jakub Narebski]

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

[PATCHv3/RFC] gitweb: Prepare for splitting gitweb

[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>
---
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