{"thread":{"id":"23281","subject":"[PATCHv5 3/6] Gitweb: add autoconfigure support for minifiers","startedAt":"2010-04-01T05:36:25Z","lastAt":"2010-04-02T22:08:42Z","messageCount":4,"participants":["Mark Rada","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"138323","messageId":"4BB430D9.1090900@mailservices.uwaterloo.ca","threadId":"23281","inReplyTo":null,"subject":"[PATCHv5 3/6] Gitweb: add autoconfigure support for minifiers","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2010-04-01T05:36:25Z","receivedAt":"2010-04-01T05:36:25Z","isPatch":false,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":"This will allow users to set a JavaScript/CSS minifier when/if they run\nthe autoconfigure script while building git. This is much more\nconvenient than editing Makefile and gitweb/Makefile manually.\n\nSigned-off-by: Mark Rada <marada@uwaterloo.ca>\n\n---\n\n\tNo changes since the previous version.\n\n Makefile        |    4 ----\n configure.ac    |   20 ++++++++++++++++++++\n gitweb/Makefile |   14 ++------------\n 3 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 450e4df..ef1a232 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -282,10 +282,6 @@ lib = lib\n # DESTDIR=\n pathsep = :\n \n-# JavaScript/CSS minifier invocation that can function as filter\n-JSMIN =\n-CSSMIN =\n-\n export prefix bindir sharedir sysconfdir\n \n CC = gcc\ndiff --git a/configure.ac b/configure.ac\nindex 914ae57..bf36c72 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -179,6 +179,26 @@ fi],\n    AC_MSG_NOTICE([Will try -pthread then -lpthread to enable POSIX Threads.])\n ])\n \n+# Define option to enable JavaScript minification\n+AC_ARG_ENABLE([jsmin],\n+ [AS_HELP_STRING([--enable-jsmin=ARG],\n+   [ARG is the value to pass to make to enable JavaScript minification.])],\n+ [\n+   JSMIN=$enableval;\n+   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])\n+   GIT_CONF_APPEND_LINE(JSMIN=$enableval);\n+ ])\n+\n+# Define option to enable CSS minification\n+AC_ARG_ENABLE([cssmin],\n+ [AS_HELP_STRING([--enable-cssmin=ARG],\n+   [ARG is the value to pass to make to enable CSS minification.])],\n+ [\n+   CSSMIN=$enableval;\n+   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])\n+   GIT_CONF_APPEND_LINE(CSSMIN=$enableval);\n+ ])\n+\n ## Site configuration (override autodetection)\n ## --with-PACKAGE[=ARG] and --without-PACKAGE\n AC_MSG_NOTICE([CHECKS for site configuration])\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex fffe700..ffee4bd 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -14,10 +14,6 @@ prefix ?= $(HOME)\n bindir ?= $(prefix)/bin\n RM ?= rm -f\n \n-# JavaScript/CSS minifier invocation that can function as filter\n-JSMIN ?=\n-CSSMIN ?=\n-\n # default configuration for gitweb\n GITWEB_CONFIG = gitweb_config.perl\n GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n@@ -30,18 +26,10 @@ GITWEB_STRICT_EXPORT =\n GITWEB_BASE_URL =\n GITWEB_LIST =\n GITWEB_HOMETEXT = indextext.html\n-ifdef CSSMIN\n-GITWEB_CSS = gitweb.min.css\n-else\n GITWEB_CSS = gitweb.css\n-endif\n GITWEB_LOGO = git-logo.png\n GITWEB_FAVICON = git-favicon.png\n-ifdef JSMIN\n-GITWEB_JS = gitweb.min.js\n-else\n GITWEB_JS = gitweb.js\n-endif\n GITWEB_SITE_HEADER =\n GITWEB_SITE_FOOTER =\n \n@@ -95,9 +83,11 @@ all:: gitweb.cgi\n FILES = gitweb.cgi\n ifdef JSMIN\n FILES += gitweb.min.js\n+GITWEB_JS = gitweb.min.js\n endif\n ifdef CSSMIN\n FILES += gitweb.min.css\n+GITWEB_CSS = gitweb.min.css\n endif\n gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)\n \n-- \n1.7.0.3.436.g45b2d\n"},{"id":"138368","messageId":"201004020040.26177.jnareb@gmail.com","threadId":"23281","inReplyTo":"4BB430D9.1090900@mailservices.uwaterloo.ca","subject":"Re: [PATCHv5 3/6] Gitweb: add autoconfigure support for minifiers","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-01T22:40:25Z","receivedAt":"2010-04-01T22:40:25Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 1 Apr 2010, Mark Rada wrote:\n\n> This will allow users to set a JavaScript/CSS minifier when/if they run\n> the autoconfigure script while building git.\n\nThis is a good idea, IMHO.\n\n>                                              This is much more \n> convenient than editing Makefile and gitweb/Makefile manually.\n\nActually recommended way of customizing the build if one doesn't\nwant to use ./configure script (besides providing values in\ncommand line when invoking make) is not to edit Makefile, but\nto add appropriate definitions to [untracked] config.mak file.\n\n> \n> Signed-off-by: Mark Rada <marada@uwaterloo.ca>\n> \n> ---\n> \n> \tNo changes since the previous version.\n> \n>  Makefile        |    4 ----\n>  configure.ac    |   20 ++++++++++++++++++++\n>  gitweb/Makefile |   14 ++------------\n>  3 files changed, 22 insertions(+), 16 deletions(-)\n\nWhy there is no change to config.mak.in?  I would thought that it\nwould contain JSMIN=@JSMIN@ etc.\n\nBut see also below: perhaps current version is a better version.\n\n> \n> diff --git a/Makefile b/Makefile\n> index 450e4df..ef1a232 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -282,10 +282,6 @@ lib = lib\n>  # DESTDIR=\n>  pathsep = :\n>  \n> -# JavaScript/CSS minifier invocation that can function as filter\n> -JSMIN =\n> -CSSMIN =\n> -\n\nI think it is a good change anyway, as we unset variables in Makefile\nonly in \"Guard against environment variables\" for variables such as\nLIB_OBJS.  So even without support for JSMIN/CSSMIN in ./configure it\nwould be a good change.\n\n>  export prefix bindir sharedir sysconfdir\n>  \n>  CC = gcc\n> diff --git a/configure.ac b/configure.ac\n> index 914ae57..bf36c72 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -179,6 +179,26 @@ fi],\n>     AC_MSG_NOTICE([Will try -pthread then -lpthread to enable POSIX Threads.])\n>  ])\n>  \n> +# Define option to enable JavaScript minification\n> +AC_ARG_ENABLE([jsmin],\n> + [AS_HELP_STRING([--enable-jsmin=ARG],\n> +   [ARG is the value to pass to make to enable JavaScript minification.])],\n> + [\n> +   JSMIN=$enableval;\n> +   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])\n> +   GIT_CONF_APPEND_LINE(JSMIN=$enableval);\n> + ])\n> +\n> +# Define option to enable CSS minification\n> +AC_ARG_ENABLE([cssmin],\n> + [AS_HELP_STRING([--enable-cssmin=ARG],\n> +   [ARG is the value to pass to make to enable CSS minification.])],\n> + [\n> +   CSSMIN=$enableval;\n> +   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])\n> +   GIT_CONF_APPEND_LINE(CSSMIN=$enableval);\n> + ])\n> +\n\nWhy not follow the code as it was done e.g. for iconv (--without-iconv\nand --with-iconv=path); this would require JSMIN=@JSMIN@ in \nconfig.mak.in (and respectively for CSSMIN).\n\nAlternatively, if you decide on appending to config.mak.autogen (by the\nway of config.mak.append) instead of filling config.mak.in, why not use\nready macro GIT_PARSE_WITH_SET_MAKE_VAR?\n\n[...]\n> diff --git a/gitweb/Makefile b/gitweb/Makefile\n> index fffe700..ffee4bd 100644\n> --- a/gitweb/Makefile\n> +++ b/gitweb/Makefile\n> @@ -14,10 +14,6 @@ prefix ?= $(HOME)\n>  bindir ?= $(prefix)/bin\n>  RM ?= rm -f\n>  \n> -# JavaScript/CSS minifier invocation that can function as filter\n> -JSMIN ?=\n> -CSSMIN ?=\n> -\n\nI can understand that this was not needed anyway.\n\n>  # default configuration for gitweb\n>  GITWEB_CONFIG = gitweb_config.perl\n>  GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n> @@ -30,18 +26,10 @@ GITWEB_STRICT_EXPORT =\n>  GITWEB_BASE_URL =\n>  GITWEB_LIST =\n>  GITWEB_HOMETEXT = indextext.html\n> -ifdef CSSMIN\n> -GITWEB_CSS = gitweb.min.css\n> -else\n>  GITWEB_CSS = gitweb.css\n> -endif\n>  GITWEB_LOGO = git-logo.png\n>  GITWEB_FAVICON = git-favicon.png\n> -ifdef JSMIN\n> -GITWEB_JS = gitweb.min.js\n> -else\n>  GITWEB_JS = gitweb.js\n> -endif\n>  GITWEB_SITE_HEADER =\n>  GITWEB_SITE_FOOTER =\n>  \n> @@ -95,9 +83,11 @@ all:: gitweb.cgi\n>  FILES = gitweb.cgi\n>  ifdef JSMIN\n>  FILES += gitweb.min.js\n> +GITWEB_JS = gitweb.min.js\n>  endif\n>  ifdef CSSMIN\n>  FILES += gitweb.min.css\n> +GITWEB_CSS = gitweb.min.css\n>  endif\n>  gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)\n\nO.K., this change means that it is two conditionals less...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"138441","messageId":"4BB6294A.7020800@mailservices.uwaterloo.ca","threadId":"23281","inReplyTo":"CBD7C6CF-01CB-4525-8AAB-B1E8086CA06E@mailservices.uwaterloo.ca","subject":"Re: [PATCHv5 3/6] Gitweb: add autoconfigure support for minifiers","fromName":"Mark Rada","fromEmail":"marada@uwaterloo.ca","sentAt":"2010-04-02T17:28:42Z","receivedAt":"2010-04-02T17:28:42Z","isPatch":false,"sender":{"key":"marada@uwaterloo.ca","avatar":"https://avatars.githubusercontent.com/u/38430?v=4"},"body":">> Makefile        |    4 ----\n>> configure.ac    |   20 ++++++++++++++++++++\n>> gitweb/Makefile |   14 ++------------\n>> 3 files changed, 22 insertions(+), 16 deletions(-)\n>\n> Why there is no change to config.mak.in?  I would thought that it\n> would contain JSMIN=@JSMIN@ etc.\n>\n> But see also below: perhaps current version is a better version.\n>\n \nI don't understand what you mean by this. Are you saying that the\npatch is good as is?\n \n \n>> export prefix bindir sharedir sysconfdir\n>>\n>> CC = gcc\n>> diff --git a/configure.ac b/configure.ac\n>> index 914ae57..bf36c72 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -179,6 +179,26 @@ fi],\n>>    AC_MSG_NOTICE([Will try -pthread then -lpthread to enable\n>> POSIX Threads.])\n>> ])\n>>\n>> +# Define option to enable JavaScript minification\n>> +AC_ARG_ENABLE([jsmin],\n>> + [AS_HELP_STRING([--enable-jsmin=ARG],\n>> +   [ARG is the value to pass to make to enable\n>> JavaScript minification.])],\n>> + [\n>> +   JSMIN=$enableval;\n>> +   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable\n>> JavaScript minifying])\n>> +   GIT_CONF_APPEND_LINE(JSMIN=$enableval);\n>> + ])\n>> +\n>> +# Define option to enable CSS minification\n>> +AC_ARG_ENABLE([cssmin],\n>> + [AS_HELP_STRING([--enable-cssmin=ARG],\n>> +   [ARG is the value to pass to make to enable CSS minification.])],\n>> + [\n>> +   CSSMIN=$enableval;\n>> +   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])\n>> +   GIT_CONF_APPEND_LINE(CSSMIN=$enableval);\n>> + ])\n>> +\n>\n> Why not follow the code as it was done e.g. for iconv (--without-iconv\n> and --with-iconv=path); this would require JSMIN=@JSMIN@ in \n> config.mak.in (and respectively for CSSMIN).\n>\n> Alternatively, if you decide on appending to config.mak.autogen (by the\n> way of config.mak.append) instead of filling config.mak.in, why not use\n> ready macro GIT_PARSE_WITH_SET_MAKE_VAR?\n \nI think this is what I am really not understanding here. Are you saying\nthat you think it would be better to use —with-OPT=PATH instead of\n—enable-OPT=PATH? \n\nIs this just a preference? I'm not seeing the problem with —enable-OPT..\nThis is confusing me a bit...\n\nThank you for the input.\n\n—\nMark Rada\nmarada@uwaterloo.ca\n"},{"id":"138467","messageId":"201004030008.43513.jnareb@gmail.com","threadId":"23281","inReplyTo":"4BB6294A.7020800@mailservices.uwaterloo.ca","subject":"Re: [PATCHv5 3/6] Gitweb: add autoconfigure support for minifiers","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-04-02T22:08:42Z","receivedAt":"2010-04-02T22:08:42Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Mark Rada wrote:\n\n>>> Makefile        |    4 ----\n>>> configure.ac    |   20 ++++++++++++++++++++\n>>> gitweb/Makefile |   14 ++------------\n>>> 3 files changed, 22 insertions(+), 16 deletions(-)\n>>\n>> Why there is no change to config.mak.in?  I would thought that it\n>> would contain JSMIN=@JSMIN@ etc.\n>>\n>> But see also below: perhaps current version is a better version.\n>  \n> I don't understand what you mean by this. Are you saying that the\n> patch is good as is?\n\nYes, I think that while it could be improved a bit, it is good as\nit is now.\n\nI'm sorry for causing confusion: at first I was wondering why you do \nnot use JSMIN=@JSMIN@ etc. in config.mak.in, but then I noticed that\nother existing configuration uses \"add to config.mak.append\" trick.\n\n[...]\n>>> +# Define option to enable CSS minification\n>>> +AC_ARG_ENABLE([cssmin],\n>>> + [AS_HELP_STRING([--enable-cssmin=ARG],\n>>> +   [ARG is the value to pass to make to enable CSS minification.])],\n>>> + [\n>>> +   CSSMIN=$enableval;\n>>> +   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])\n>>> +   GIT_CONF_APPEND_LINE(CSSMIN=$enableval);\n>>> + ])\n>>> +\n>>\n>> Why not follow the code as it was done e.g. for iconv (--without-iconv\n>> and --with-iconv=path); this would require JSMIN=@JSMIN@ in \n>> config.mak.in (and respectively for CSSMIN).\n\nThis is about alternate solution; please discard this question.\n\n>> Alternatively, if you decide on appending to config.mak.autogen (by the\n>> way of config.mak.append) instead of filling config.mak.in, why not use\n>> ready macro GIT_PARSE_WITH_SET_MAKE_VAR?\n>  \n> I think this is what I am really not understanding here. Are you saying\n> that you think it would be better to use —with-OPT=PATH instead of\n> —enable-OPT=PATH? \n> \n> Is this just a preference? I'm not seeing the problem with —enable-OPT..\n> This is confusing me a bit...\n\nWell, I haven't noticed that GIT_PARSE_WITH_SET_MAKE_VAR uses \n—with-OPT=PATH and not —enable-OPT=PATH.\n\nWhat would be nice to have is GIT_PARSE_ENABLE_SET_MAKE_VAR macro... but\nfor only two callsites it might be overkill.\n\n-- \nJakub Narebski\nPoland\n"}]}