On Thu, 1 Apr 2010, Mark Rada wrote: > This will allow users to set a JavaScript/CSS minifier when/if they run > the autoconfigure script while building git. This is a good idea, IMHO. > This is much more > convenient than editing Makefile and gitweb/Makefile manually. Actually recommended way of customizing the build if one doesn't want to use ./configure script (besides providing values in command line when invoking make) is not to edit Makefile, but to add appropriate definitions to [untracked] config.mak file. > > Signed-off-by: Mark Rada <marada@xxxxxxxxxxxx> > > --- > > No changes since the previous version. > > Makefile | 4 ---- > configure.ac | 20 ++++++++++++++++++++ > gitweb/Makefile | 14 ++------------ > 3 files changed, 22 insertions(+), 16 deletions(-) Why there is no change to config.mak.in? I would thought that it would contain JSMIN=@JSMIN@ etc. But see also below: perhaps current version is a better version. > > diff --git a/Makefile b/Makefile > index 450e4df..ef1a232 100644 > --- a/Makefile > +++ b/Makefile > @@ -282,10 +282,6 @@ lib = lib > # DESTDIR= > pathsep = : > > -# JavaScript/CSS minifier invocation that can function as filter > -JSMIN = > -CSSMIN = > - I think it is a good change anyway, as we unset variables in Makefile only in "Guard against environment variables" for variables such as LIB_OBJS. So even without support for JSMIN/CSSMIN in ./configure it would be a good change. > export prefix bindir sharedir sysconfdir > > CC = gcc > diff --git a/configure.ac b/configure.ac > index 914ae57..bf36c72 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -179,6 +179,26 @@ fi], > AC_MSG_NOTICE([Will try -pthread then -lpthread to enable POSIX Threads.]) > ]) > > +# Define option to enable JavaScript minification > +AC_ARG_ENABLE([jsmin], > + [AS_HELP_STRING([--enable-jsmin=ARG], > + [ARG is the value to pass to make to enable JavaScript minification.])], > + [ > + JSMIN=$enableval; > + AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying]) > + GIT_CONF_APPEND_LINE(JSMIN=$enableval); > + ]) > + > +# Define option to enable CSS minification > +AC_ARG_ENABLE([cssmin], > + [AS_HELP_STRING([--enable-cssmin=ARG], > + [ARG is the value to pass to make to enable CSS minification.])], > + [ > + CSSMIN=$enableval; > + AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying]) > + GIT_CONF_APPEND_LINE(CSSMIN=$enableval); > + ]) > + Why not follow the code as it was done e.g. for iconv (--without-iconv and --with-iconv=path); this would require JSMIN=@JSMIN@ in config.mak.in (and respectively for CSSMIN). Alternatively, if you decide on appending to config.mak.autogen (by the way of config.mak.append) instead of filling config.mak.in, why not use ready macro GIT_PARSE_WITH_SET_MAKE_VAR? [...] > diff --git a/gitweb/Makefile b/gitweb/Makefile > index fffe700..ffee4bd 100644 > --- a/gitweb/Makefile > +++ b/gitweb/Makefile > @@ -14,10 +14,6 @@ prefix ?= $(HOME) > bindir ?= $(prefix)/bin > RM ?= rm -f > > -# JavaScript/CSS minifier invocation that can function as filter > -JSMIN ?= > -CSSMIN ?= > - I can understand that this was not needed anyway. > # default configuration for gitweb > GITWEB_CONFIG = gitweb_config.perl > GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf > @@ -30,18 +26,10 @@ GITWEB_STRICT_EXPORT = > GITWEB_BASE_URL = > GITWEB_LIST = > GITWEB_HOMETEXT = indextext.html > -ifdef CSSMIN > -GITWEB_CSS = gitweb.min.css > -else > GITWEB_CSS = gitweb.css > -endif > GITWEB_LOGO = git-logo.png > GITWEB_FAVICON = git-favicon.png > -ifdef JSMIN > -GITWEB_JS = gitweb.min.js > -else > GITWEB_JS = gitweb.js > -endif > GITWEB_SITE_HEADER = > GITWEB_SITE_FOOTER = > > @@ -95,9 +83,11 @@ all:: gitweb.cgi > FILES = gitweb.cgi > ifdef JSMIN > FILES += gitweb.min.js > +GITWEB_JS = gitweb.min.js > endif > ifdef CSSMIN > FILES += gitweb.min.css > +GITWEB_CSS = gitweb.min.css > endif > gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS) O.K., this change means that it is two conditionals less... -- 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