Re: [PATCH/RFC] gitweb: Create install target for gitweb in Makefile

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 1 May 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Installing gitweb is now as easy as
> >   # make gitwebdir=/var/www/cgi-bin gitweb-install  ;# as root
> > The gitweb/INSTALL file was updated accordingly.
> 
> Just a style, but I prefer a blank line on both sides of an example
> command line like this.

O.K.

> > There is a question whether to rely on correctly set file permissions
> > during build phase, i.e.
> >
> > 	$(INSTALL) $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> >
> > or whether to ensure correct file permissions during installl phase
> >
> > 	$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> > 	$(INSTALL) -m 644 $(GITWEB_FILES)    '$(DESTDIR_SQ)$(gitwebdir_SQ)'
> >
> > Currently the first option is used.
> 
> Hmm, the reason being?  I do not have a strong preference either way, but
> the primary Makefile seems to use "-m mode".

No reason, just that this was simpler way (gitweb.cgi is created
executable by its rule).

Changed patch below.
 
> > Note that install-* targets, including new install-gitweb, are not
> > marked as .PHONY
> 
> The reason being?
> 
> My preference for the standard targets like "all", "clean", and "install"
> is to make them double-colon rules and mark them as phoneys.

The reason being that only 'all::' is double-colon rule, and in main
Makefile while 'all' and 'install' targets are marked .PHONY, other
install targets: install-doc, install-info, install-man etc. are not.

I was following example of 'install-doc' target with new 'install-gitweb'
target in main Makefile.  This can be fixed, but I think it is a matter
for a separate commit.

-- >8 --
Subject: [PATCHv2] gitweb: Create install target for gitweb in Makefile

Installing gitweb is now as easy as

  # make gitwebdir=/var/www/cgi-bin gitweb-install  ;# as root

The gitweb/INSTALL file was updated accordingly, to make use of this
new target.

Fix shell quoting, i.e. setting bindir_SQ etc., in gitweb/Makefile.
Those variables were not used previously.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Here is the interdiff:

 diff --git a/gitweb/Makefile b/gitweb/Makefile
 index 729cf50..935d2d2 100644
 --- a/gitweb/Makefile
 +++ b/gitweb/Makefile
 @@ -85,7 +85,7 @@ endif
  
  all:: gitweb.cgi
  
 -GITWEB_FILES = gitweb.cgi
 +GITWEB_PROGRAMS = gitweb.cgi
  
  ifdef JSMIN
  GITWEB_FILES += gitweb.min.js
 @@ -146,7 +146,8 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
  
  install: all
  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
 -	$(INSTALL) $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
 +	$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
 +	$(INSTALL) -m 644 $(GITWEB_FILES)    '$(DESTDIR_SQ)$(gitwebdir_SQ)'
  
  ### Cleaning rules
  

 

 Makefile        |    3 +++
 gitweb/INSTALL  |   11 ++++-------
 gitweb/Makefile |   32 ++++++++++++++++++++++++++++----
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 910f471..dab5a14 100644
--- a/Makefile
+++ b/Makefile
@@ -2004,6 +2004,9 @@ endif
 	  done; } && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
+install-gitweb:
+	$(MAKE) -C gitweb install
+
 install-doc:
 	$(MAKE) -C Documentation install
 
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 1bfd9aa..4447e26 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -6,10 +6,8 @@ First you have to generate gitweb.cgi from gitweb.perl using
 gitweb.css, git-logo.png and git-favicon.png) to their destination.
 For example if git was (or is) installed with /usr prefix, you can do
 
-	$ make prefix=/usr gitweb             ;# as yourself
-	# cp gitweb/*.cgi gitweb/*.css \
-	     gitweb/*.js  gitweb/*.png \
-	     /var/www/cgi-bin/                ;# as root
+	$ make prefix=/usr gitweb                            ;# as yourself
+	# make gitwebdir=/var/www/cgi-bin install-gitweb     ;# as root
 
 Alternatively you can use autoconf generated ./configure script to
 set up path to git binaries (via config.mak.autogen), so you can write
@@ -18,9 +16,8 @@ instead
 	$ make configure                     ;# as yourself
 	$ ./configure --prefix=/usr          ;# as yourself
 	$ make gitweb                        ;# as yourself
-	# cp gitweb/*.cgi gitweb/*.css \
-	     gitweb/*.js  gitweb/*.png \
-	     /var/www/cgi-bin/               ;# as root
+	# make gitwebdir=/var/www/cgi-bin \
+	       install-gitweb                ;# as root
 
 The above example assumes that your web server is configured to run
 [executable] files in /var/www/cgi-bin/ as server scripts (as CGI
diff --git a/gitweb/Makefile b/gitweb/Makefile
index f2e1d92..935d2d2 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -12,7 +12,10 @@ all::
 
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
+gitwebdir ?= /var/www/cgi-bin
+
 RM ?= rm -f
+INSTALL ?= install
 
 # default configuration for gitweb
 GITWEB_CONFIG = gitweb_config.perl
@@ -49,9 +52,11 @@ SHELL_PATH ?= $(SHELL)
 PERL_PATH  ?= /usr/bin/perl
 
 # Shell quote;
-bindir_SQ = $(subst ','\'',$(bindir))         #'
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) #'
-PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))  #'
+bindir_SQ = $(subst ','\'',$(bindir))#'
+gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
+PERL_PATH_SQ  = $(subst ','\'',$(PERL_PATH))#'
+DESTDIR_SQ    = $(subst ','\'',$(DESTDIR))#'
 
 # Quiet generation (unless V=1)
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
@@ -80,20 +85,30 @@ endif
 
 all:: gitweb.cgi
 
+GITWEB_PROGRAMS = gitweb.cgi
+
 ifdef JSMIN
+GITWEB_FILES += gitweb.min.js
 GITWEB_JS = gitweb.min.js
 all:: gitweb.min.js
 gitweb.min.js: gitweb.js GITWEB-BUILD-OPTIONS
 	$(QUIET_GEN)$(JSMIN) <$< >$@
+else
+GITWEB_FILES += gitweb.js
 endif
 
 ifdef CSSMIN
+GITWEB_FILES += gitweb.min.css
 GITWEB_CSS = gitweb.min.css
 all:: gitweb.min.css
 gitweb.min.css: gitweb.css GITWEB-BUILD-OPTIONS
 	$(QUIET_GEN)$(CSSMIN) <$ >$@
+else
+GITWEB_FILES += gitweb.css
 endif
 
+GITWEB_FILES += git-logo.png git-favicon.png
+
 GITWEB_REPLACE = \
 	-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
 	-e 's|++GIT_BINDIR++|$(bindir)|g' \
@@ -127,8 +142,17 @@ gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
 	chmod +x $@+ && \
 	mv $@+ $@
 
+### Installation rules
+
+install: all
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+	$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+	$(INSTALL) -m 644 $(GITWEB_FILES)    '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+
+### Cleaning rules
+
 clean:
 	$(RM) gitweb.cgi gitweb.min.js gitweb.min.css GITWEB-BUILD-OPTIONS
 
-.PHONY: all clean .FORCE-GIT-VERSION-FILE FORCE
+.PHONY: all clean install .FORCE-GIT-VERSION-FILE FORCE
 
-- 
1.7.0.1