{"thread":{"id":"23467","subject":"[PATCH] gitweb: simplify gitweb.min.* generation and clean-up rules","startedAt":"2010-04-15T03:37:35Z","lastAt":"2010-04-15T12:57:18Z","messageCount":4,"participants":["Mark Rada","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"139546","messageId":"4BC689FF.9080308@mailservices.uwaterloo.ca","threadId":"23467","inReplyTo":null,"subject":"[PATCH] gitweb: simplify gitweb.min.* generation and clean-up rules","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2010-04-15T03:37:35Z","receivedAt":"2010-04-15T03:37:35Z","isPatch":true,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"GITWEB_CSS and GITWEB_JS are meant to be \"what URI should the installed\ncgi script use to refer to the stylesheet and JavaScript\", never \"this\nis the name of the file we are building\".\n\nLose incorrect assignment to them.\n\nWhile we are at it, lose FILES that is used only for \"clean\" target in a\nmisguided way.  \"make clean\" should try to remove all the potential\nbuild artifacts regardless of a minor configuration change.  Instead of\ntrying to remove only the build product \"make clean\" would have created\nif it were run without \"clean\", explicitly list the three potential build\nproducts for removal.\n\nIn addition, this patch tries to make sure that the scripts are\nregenerated whenever the replacement variables are modified.  For a good\nmeasure, if you used different JSMIN/CSSMIN since the last time you\nproduced minified version of these files, they are regenerated.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nTested-by: Mark Rada <marada@uwaterloo.ca>\n\n\n---\n\nI gave this a test run:\n\tWith just jsmin enabled\n\tWith just cssmin enabled\n\tWith neither enabled\n\tWith both enabled\n\tOverriding GITWEB_JS\n\tOverriding GITWEB_JS and jsmin enabled\n\nInstaweb will still generate what it needs to the first time around,\nbut if you change GITWEB_JS or the JSMIN (or css equivalents) then you\nhave to regenerate gitweb first manually before instaweb. I'm not sure\nif it would be best to swallow up instaweb into this same patch or to\nfix it separately (also, I still don't quite understand how this patch\nworks).\n\n\n config.mak.in   |    2 +\n gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------\n 2 files changed, 40 insertions(+), 37 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex 6008ac9..bb828fe 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -57,3 +57,5 @@ FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@\n SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\n NO_PTHREADS=@NO_PTHREADS@\n PTHREAD_LIBS=@PTHREAD_LIBS@\n+GITWEB_JS=/home/ferrous/gitweb.js\n+\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex ffee4bd..f2e1d92 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -80,54 +80,55 @@ endif\n \n all:: gitweb.cgi\n \n-FILES = gitweb.cgi\n ifdef JSMIN\n-FILES += gitweb.min.js\n GITWEB_JS = gitweb.min.js\n+all:: gitweb.min.js\n+gitweb.min.js: gitweb.js GITWEB-BUILD-OPTIONS\n+\t$(QUIET_GEN)$(JSMIN) <$< >$@\n endif\n+\n ifdef CSSMIN\n-FILES += gitweb.min.css\n GITWEB_CSS = gitweb.min.css\n+all:: gitweb.min.css\n+gitweb.min.css: gitweb.css GITWEB-BUILD-OPTIONS\n+\t$(QUIET_GEN)$(CSSMIN) <$ >$@\n endif\n-gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)\n \n-gitweb.cgi:\n+GITWEB_REPLACE = \\\n+\t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n+\t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\n+\t-e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n+\t-e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n+\t-e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n+\t-e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n+\t-e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n+\t-e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n+\t-e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n+\t-e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n+\t-e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n+\t-e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n+\t-e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n+\t-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n+\t-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n+\t-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n+\t-e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n+\t-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n+\t-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g'\n+\n+GITWEB-BUILD-OPTIONS: FORCE\n+\t@rm -f $@+\n+\t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n+\t@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@\n+\n+gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n-\t    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n-\t    -e 's|++GIT_BINDIR++|$(bindir)|g' \\\n-\t    -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n-\t    -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n-\t    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n-\t    -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n-\t    -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n-\t    -e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n-\t    -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n-\t    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n-\t    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n-\t    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n-\t    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n-\t    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n-\t    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n-\t    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n-\t    -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n-\t    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n-\t    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n-\t    $< >$@+ && \\\n+\t\t$(GITWEB_REPLACE) $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-ifdef JSMIN\n-gitweb.min.js: gitweb.js\n-\t$(QUIET_GEN)$(JSMIN) <$< >$@\n-endif # JSMIN\n-\n-ifdef CSSMIN\n-gitweb.min.css: gitweb.css\n-\t$(QUIET_GEN)$(CSSMIN) <$ >$@\n-endif\n-\n clean:\n-\t$(RM) $(FILES)\n+\t$(RM) gitweb.cgi gitweb.min.js gitweb.min.css GITWEB-BUILD-OPTIONS\n+\n+.PHONY: all clean .FORCE-GIT-VERSION-FILE FORCE\n \n-.PHONY: all clean .FORCE-GIT-VERSION-FILE\n-- \n1.7.1.rc1.237.ge1730\n"},{"id":"139549","messageId":"4BC691FC.8040401@mailservices.uwaterloo.ca","threadId":"23467","inReplyTo":"4BC689FF.9080308@mailservices.uwaterloo.ca","subject":"Re: [PATCH] gitweb: simplify gitweb.min.* generation and clean-up rules","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2010-04-15T04:11:40Z","receivedAt":"2010-04-15T04:11:40Z","isPatch":true,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"On 10-04-14 11:37 PM, Mark Rada wrote:\n> GITWEB_CSS and GITWEB_JS are meant to be \"what URI should the installed\n> cgi script use to refer to the stylesheet and JavaScript\", never \"this\n> is the name of the file we are building\".\n> \n> Lose incorrect assignment to them.\n> \n> While we are at it, lose FILES that is used only for \"clean\" target in a\n> misguided way.  \"make clean\" should try to remove all the potential\n> build artifacts regardless of a minor configuration change.  Instead of\n> trying to remove only the build product \"make clean\" would have created\n> if it were run without \"clean\", explicitly list the three potential build\n> products for removal.\n> \n> In addition, this patch tries to make sure that the scripts are\n> regenerated whenever the replacement variables are modified.  For a good\n> measure, if you used different JSMIN/CSSMIN since the last time you\n> produced minified version of these files, they are regenerated.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Tested-by: Mark Rada <marada@uwaterloo.ca>\n> \n> \n> ---\n> \n> I gave this a test run:\n> \tWith just jsmin enabled\n> \tWith just cssmin enabled\n> \tWith neither enabled\n> \tWith both enabled\n> \tOverriding GITWEB_JS\n> \tOverriding GITWEB_JS and jsmin enabled\n\nI should have mentioned that this these were done successively, without running\n`make clean', and using the autoconfigure script to set JSMIN/CSSMIN.\n\n\n-- \nMark Rada\n"},{"id":"139554","messageId":"201004150811.55627.jnareb@gmail.com","threadId":"23467","inReplyTo":"4BC689FF.9080308@mailservices.uwaterloo.ca","subject":"Re: [PATCH] gitweb: simplify gitweb.min.* generation and clean-up rules","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-15T06:11:53Z","receivedAt":"2010-04-15T06:11:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Mark Rada wrote:\n\n> GITWEB_CSS and GITWEB_JS are meant to be \"what URI should the installed\n> cgi script use to refer to the stylesheet and JavaScript\", never \"this\n> is the name of the file we are building\".\n> \n> Lose incorrect assignment to them.\n> \n> While we are at it, lose FILES that is used only for \"clean\" target in a\n> misguided way.  \"make clean\" should try to remove all the potential\n> build artifacts regardless of a minor configuration change.  Instead of\n> trying to remove only the build product \"make clean\" would have created\n> if it were run without \"clean\", explicitly list the three potential build\n> products for removal.\n> \n> In addition, this patch tries to make sure that the scripts are\n> regenerated whenever the replacement variables are modified.  For a good\n> measure, if you used different JSMIN/CSSMIN since the last time you\n> produced minified version of these files, they are regenerated.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Tested-by: Mark Rada <marada@uwaterloo.ca>\n> \n> \n> ---\n> \n> I gave this a test run:\n> \tWith just jsmin enabled\n> \tWith just cssmin enabled\n> \tWith neither enabled\n> \tWith both enabled\n> \tOverriding GITWEB_JS\n> \tOverriding GITWEB_JS and jsmin enabled\n\nThank you very much.\n \n> Instaweb will still generate what it needs to the first time around,\n> but if you change GITWEB_JS or the JSMIN (or css equivalents) then you\n> have to regenerate gitweb first manually before instaweb. I'm not sure\n> if it would be best to swallow up instaweb into this same patch or to\n> fix it separately (also, I still don't quite understand how this patch\n> works).\n\nInstaweb would need to check if gitweb was run with its GITWEB_JS and\nwith curent JSMIN... and this should be I think a separate patch.\n\n>  config.mak.in   |    2 +\n>  gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------\n>  2 files changed, 40 insertions(+), 37 deletions(-)\n> \n> diff --git a/config.mak.in b/config.mak.in\n> index 6008ac9..bb828fe 100644\n> --- a/config.mak.in\n> +++ b/config.mak.in\n> @@ -57,3 +57,5 @@ FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@\n>  SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\n>  NO_PTHREADS=@NO_PTHREADS@\n>  PTHREAD_LIBS=@PTHREAD_LIBS@\n> +GITWEB_JS=/home/ferrous/gitweb.js\n> +\n\nI think that you have committed this by accident...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"139583","messageId":"4BC70D2E.6040508@mailservices.uwaterloo.ca","threadId":"23467","inReplyTo":"201004150811.55627.jnareb@gmail.com","subject":"[PATCHv2] gitweb: simplify gitweb.min.* generation and clean-up rules","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2010-04-15T12:57:18Z","receivedAt":"2010-04-15T12:57:18Z","isPatch":false,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"GITWEB_CSS and GITWEB_JS are meant to be \"what URI should the installed\ncgi script use to refer to the stylesheet and JavaScript\", never \"this\nis the name of the file we are building\".\n\nLose incorrect assignment to them.\n\nWhile we are at it, lose FILES that is used only for \"clean\" target in a\nmisguided way.  \"make clean\" should try to remove all the potential\nbuild artifacts regardless of a minor configuration change. Instead of\ntrying to remove only the build product \"make clean\" would have created\nif it were run without \"clean\", explicitly list the three potential build\nproducts for removal.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nTested-by: Mark Rada <marada@uwaterloo.co>\n\n---\n\n\tChanges since v1:\n\t\t- Removed extra crap that was not supposed to be part of\n\t\t  the patch\n\n\n gitweb/Makefile |   75 ++++++++++++++++++++++++++++---------------------------\n 1 files changed, 38 insertions(+), 37 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex ffee4bd..f2e1d92 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -80,54 +80,55 @@ endif\n \n all:: gitweb.cgi\n \n-FILES = gitweb.cgi\n ifdef JSMIN\n-FILES += gitweb.min.js\n GITWEB_JS = gitweb.min.js\n+all:: gitweb.min.js\n+gitweb.min.js: gitweb.js GITWEB-BUILD-OPTIONS\n+\t$(QUIET_GEN)$(JSMIN) <$< >$@\n endif\n+\n ifdef CSSMIN\n-FILES += gitweb.min.css\n GITWEB_CSS = gitweb.min.css\n+all:: gitweb.min.css\n+gitweb.min.css: gitweb.css GITWEB-BUILD-OPTIONS\n+\t$(QUIET_GEN)$(CSSMIN) <$ >$@\n endif\n-gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)\n \n-gitweb.cgi:\n+GITWEB_REPLACE = \\\n+\t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n+\t-e 's|++GIT_BINDIR++|$(bindir)|g' \\\n+\t-e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n+\t-e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n+\t-e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n+\t-e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n+\t-e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n+\t-e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n+\t-e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n+\t-e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n+\t-e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n+\t-e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n+\t-e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n+\t-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n+\t-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n+\t-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n+\t-e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n+\t-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n+\t-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g'\n+\n+GITWEB-BUILD-OPTIONS: FORCE\n+\t@rm -f $@+\n+\t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n+\t@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@\n+\n+gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n-\t    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n-\t    -e 's|++GIT_BINDIR++|$(bindir)|g' \\\n-\t    -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \\\n-\t    -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \\\n-\t    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n-\t    -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n-\t    -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n-\t    -e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n-\t    -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n-\t    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n-\t    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\n-\t    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \\\n-\t    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \\\n-\t    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \\\n-\t    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \\\n-\t    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \\\n-\t    -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \\\n-\t    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \\\n-\t    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n-\t    $< >$@+ && \\\n+\t\t$(GITWEB_REPLACE) $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-ifdef JSMIN\n-gitweb.min.js: gitweb.js\n-\t$(QUIET_GEN)$(JSMIN) <$< >$@\n-endif # JSMIN\n-\n-ifdef CSSMIN\n-gitweb.min.css: gitweb.css\n-\t$(QUIET_GEN)$(CSSMIN) <$ >$@\n-endif\n-\n clean:\n-\t$(RM) $(FILES)\n+\t$(RM) gitweb.cgi gitweb.min.js gitweb.min.css GITWEB-BUILD-OPTIONS\n+\n+.PHONY: all clean .FORCE-GIT-VERSION-FILE FORCE\n \n-.PHONY: all clean .FORCE-GIT-VERSION-FILE\n-- \n1.7.1.rc1.237.ge1730\n"}]}