On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote: > 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@xxxxxxxxx> > Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@xxxxxxxxx> > --- First, it would be better if the comment from the cover letter about this patch (quoted below) was [also] put as the comment for this patch (i.e. in this area). > The second patch is produced by my commit ammend to Jakub Narębski's > initial commit to prepare splitting of gitweb in the message-id: > http://mid.gmane.org/1276531710-22945-4-git-send-email-jnareb@xxxxxxxxx > > There is a small ammendment to that patch. > $(INSTALL) -m 644 $(mod) '$(DESTDIR_SQ)$(gitwebdir_SQ)/$(mod)' > is changed to > $(INSTALL) -m 644 $(mod) '$(DESTDIR_SQ)$(gitwebdir_SQ)/$(dir > $(mod))';) because it is better if we gave 'dir $(mod)' as the target > for install command rather than '$(mod)' because it may cause > problems in the future when gitweblibdir is used and the modules are > installed elsewhere rather than in /use/share/gitweb. Second, I don't quite understand the reason for your amendment. The difference is between two argument install [OPTION]... SOURCE DEST in my original version of this patch, and two argument install [OPTION]... SOURCE DIRECTORY after your change. In both cases we install _single_ file at once. What problems first version may cause if $(gitweblibdir) is used and it points elsewhere than $(gitwebdir)/lib? > gitweb/Makefile | 3 +++ > gitweb/gitweb.perl | 9 +++++++++ > 2 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/gitweb/Makefile b/gitweb/Makefile > index d2584fe..c7610b3 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))#' > @@ -150,6 +151,8 @@ install: all > $(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)' > + $(foreach dir,$(sort $(dir $(GITWEB_MODULES))),test -d '$(DESTDIR_SQ)$(gitwebdir_SQ)/$(dir)' || $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)/$(dir)';) > + $(foreach mod,$(GITWEB_MODULES),$(INSTALL) -m 644 $(mod) '$(DESTDIR_SQ)$(gitwebdir_SQ)/$(dir $(mod))';) Third, if you were folloring git mailing list (or at least patches for gitweb on it), you would notice new version of my "gitweb: Prepare for splitting gitweb" patch: "[PATCHv3/RFC] gitweb: Prepare for splitting gitweb" Message-ID: <201007080920.38724.jnareb@xxxxxxxxx> http://thread.gmane.org/gmane.comp.version-control.git/150463/focus=150544 which uses shell for loop, instead of make's $(foreach ...) function to avoid _possible_ problems with generating a command line that exceeded the maximum argument list length, as explained in comment section of http://thread.gmane.org/gmane.comp.version-control.git/150463/focus=150463 Note that this version uses SOURCE DIRECTORY version rather than SOURCE DEST, like in your patch... probably unnecessary. > > ### Cleaning rules > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index 518328f..bda7da3 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; In "[PATCHv3/RFC] gitweb: Prepare for splitting gitweb" this spurious change was removed from patch. Anyway, eventual replacing of this patch by other version should be fairly easy, so it shouldn't hold this series. -- Jakub Narebski Poland -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html