gitweb: simplify gitweb.min.* generation and clean-up rules

PATCH In review Mark Rada 2 participants 4 messages Jun 15, 2016

Share & tools

Mark Rada

author

2016-06-15 22:48 UTC #0 lore raw

PATCH 1/1 gitweb: simplify gitweb.min.* generation and clean-up rules

GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed cgi script use to refer to the stylesheet and JavaScript", never "this is the name of the file we are building".

Lose incorrect assignment to them.

While we are at it, lose FILES that is used only for "clean" target in a misguided way. "make clean" should try to remove all the potential build artifacts regardless of a minor configuration change. Instead of trying to remove only the build product "make clean" would have created if it were run without "clean", explicitly list the three potential build products for removal.

In addition, this patch tries to make sure that the scripts are regenerated whenever the replacement variables are modified. For a good measure, if you used different JSMIN/CSSMIN since the last time you produced minified version of these files, they are regenerated.

Signed-off-by Junio C Hamano <[email protected]>

Tested-by Mark Rada <[email protected]>

---

I gave this a test run:
With just jsmin enabled
With just cssmin enabled
With neither enabled
With both enabled
Overriding GITWEB_JS
Overriding GITWEB_JS and jsmin enabled

Instaweb will still generate what it needs to the first time around, but if you change GITWEB_JS or the JSMIN (or css equivalents) then you have to regenerate gitweb first manually before instaweb. I'm not sure if it would be best to swallow up instaweb into this same patch or to fix it separately (also, I still don't quite understand how this patch works).

 config.mak.in   |    2 +
 gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------
 2 files changed, 40 insertions(+), 37 deletions(-)

config.mak.in +2 -0

index 6008ac9..bb828fe 100644
@@ -57,3 +57,5 @@ FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
 SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
 NO_PTHREADS=@NO_PTHREADS@
 PTHREAD_LIBS=@PTHREAD_LIBS@
+GITWEB_JS=/home/ferrous/gitweb.js
+

gitweb/Makefile +38 ~~-37~~

92 lines not on this page · raw message

Signature · 1 line

1.7.1.rc1.237.ge1730

Jakub Narebski

2016-06-15 22:48 UTC #1 lore raw

Quoted 24 lines · Mark Rada

GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed cgi script use to refer to the stylesheet and JavaScript", never "this is the name of the file we are building".
Lose incorrect assignment to them.
While we are at it, lose FILES that is used only for "clean" target in a misguided way. "make clean" should try to remove all the potential build artifacts regardless of a minor configuration change. Instead of trying to remove only the build product "make clean" would have created if it were run without "clean", explicitly list the three potential build products for removal.
In addition, this patch tries to make sure that the scripts are regenerated whenever the replacement variables are modified. For a good measure, if you used different JSMIN/CSSMIN since the last time you produced minified version of these files, they are regenerated.
Signed-off-by Junio C Hamano <[email protected]>
Tested-by Mark Rada <[email protected]>
---

Quoted 7 lines

I gave this a test run:
With just jsmin enabled
With just cssmin enabled
With neither enabled
With both enabled
Overriding GITWEB_JS
Overriding GITWEB_JS and jsmin enabled

Thank you very much.

Quoted 6 lines

Instaweb will still generate what it needs to the first time around, but if you change GITWEB_JS or the JSMIN (or css equivalents) then you have to regenerate gitweb first manually before instaweb. I'm not sure if it would be best to swallow up instaweb into this same patch or to fix it separately (also, I still don't quite understand how this patch works).

Instaweb would need to check if gitweb was run with its GITWEB_JS and with curent JSMIN... and this should be I think a separate patch.

Quoted 9 lines

 config.mak.in   |    2 +
 gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------
 2 files changed, 40 insertions(+), 37 deletions(-)

config.mak.in +0 -0

index 6008ac9..bb828fe 100644

config.mak.in FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@ · line 61

Quoted 5 lines

config.mak.in +2 -0

@@ -57,3 +57,5 @@ FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
 SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
 NO_PTHREADS=@NO_PTHREADS@
 PTHREAD_LIBS=@PTHREAD_LIBS@
+GITWEB_JS=/home/ferrous/gitweb.js
+

I think that you have committed this by accident...

Signature · 2 lines

Jakub Narebski
Poland

Mark Rada

author

2016-06-15 22:48 UTC #2 lore raw

PATCH v2 1/1 gitweb: simplify gitweb.min.* generation and clean-up rules

GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed cgi script use to refer to the stylesheet and JavaScript", never "this is the name of the file we are building".

Lose incorrect assignment to them.

Signed-off-by Junio C Hamano <[email protected]>

Tested-by Mark Rada <[email protected]>

---

Changes since v1:
- Removed extra crap that was not supposed to be part of the patch

 gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 38 insertions(+), 37 deletions(-)

gitweb/Makefile +38 ~~-37~~

index ffee4bd..f2e1d92 100644
@@ -80,54 +80,55 @@ endif
 
 all:: gitweb.cgi
 
-FILES = gitweb.cgi
 ifdef JSMIN
-FILES += gitweb.min.js
 GITWEB_JS = gitweb.min.js
+all:: gitweb.min.js
+gitweb.min.js: gitweb.js GITWEB-BUILD-OPTIONS
+	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif
+
 ifdef CSSMIN
-FILES += gitweb.min.css
 GITWEB_CSS = gitweb.min.css
+all:: gitweb.min.css
+gitweb.min.css: gitweb.css GITWEB-BUILD-OPTIONS
+	$(QUIET_GEN)$(CSSMIN) <$ >$@
 endif
-gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
 
-gitweb.cgi:
+GITWEB_REPLACE = \
+	-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
+	-e 's|++GIT_BINDIR++|$(bindir)|g' \
+	-e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \
+	-e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \
+	-e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \
+	-e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \
+	-e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \
+	-e 's|"++GITWEB_PROJECT_MAXDEPTH++"|$(GITWEB_PROJECT_MAXDEPTH)|g' \
+	-e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \
+	-e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
+	-e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
+	-e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
+	-e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
+	-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
+	-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
+	-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+	-e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
+	-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
+	-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g'
+
+GITWEB-BUILD-OPTIONS: FORCE
+	@rm -f $@+
+	@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
+	@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@
+
+gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-	    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
-	    -e 's|++GIT_BINDIR++|$(bindir)|g' \
-	    -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \
-	    -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \
-	    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \
-	    -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \
-	    -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \
-	    -e 's|"++GITWEB_PROJECT_MAXDEPTH++"|$(GITWEB_PROJECT_MAXDEPTH)|g' \
-	    -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \
-	    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
-	    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
-	    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
-	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
-	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
-	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
-	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
-	    -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
-	    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-	    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
-	    $< >$@+ && \
+		$(GITWEB_REPLACE) $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
-ifdef JSMIN
-gitweb.min.js: gitweb.js
-	$(QUIET_GEN)$(JSMIN) <$< >$@
-endif # JSMIN
-
-ifdef CSSMIN
-gitweb.min.css: gitweb.css
-	$(QUIET_GEN)$(CSSMIN) <$ >$@
-endif
-
 clean:
-	$(RM) $(FILES)
+	$(RM) gitweb.cgi gitweb.min.js gitweb.min.css GITWEB-BUILD-OPTIONS
+
+.PHONY: all clean .FORCE-GIT-VERSION-FILE FORCE
 
-.PHONY: all clean .FORCE-GIT-VERSION-FILE

Signature · 1 line

1.7.1.rc1.237.ge1730

Mark Rada

author

2016-06-15 22:48 UTC #3 lore raw

In reply to Mark Rada

Quoted 24 lines · Mark Rada

GITWEB_CSS and GITWEB_JS are meant to be "what URI should the installed cgi script use to refer to the stylesheet and JavaScript", never "this is the name of the file we are building".
Lose incorrect assignment to them.
While we are at it, lose FILES that is used only for "clean" target in a misguided way. "make clean" should try to remove all the potential build artifacts regardless of a minor configuration change. Instead of trying to remove only the build product "make clean" would have created if it were run without "clean", explicitly list the three potential build products for removal.
In addition, this patch tries to make sure that the scripts are regenerated whenever the replacement variables are modified. For a good measure, if you used different JSMIN/CSSMIN since the last time you produced minified version of these files, they are regenerated.
Signed-off-by Junio C Hamano <[email protected]>
Tested-by Mark Rada <[email protected]>
---

Quoted 7 lines

I gave this a test run:
With just jsmin enabled
With just cssmin enabled
With neither enabled
With both enabled
Overriding GITWEB_JS
Overriding GITWEB_JS and jsmin enabled

I should have mentioned that this these were done successively, without running `make clean', and using the autoconfigure script to set JSMIN/CSSMIN.

Signature · 1 line

Mark Rada