All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
@ 2010-04-01  5:36 Mark Rada
  2010-04-01  8:51 ` Jakub Narebski
  2010-04-13 20:28 ` Charles Bailey
  0 siblings, 2 replies; 15+ messages in thread
From: Mark Rada @ 2010-04-01  5:36 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jakub Narebski

The build system added support minifying gitweb.js through a
JavaScript minifier, but most minifiers come with support to
minify CSS files as well, so we should use it if we can.

This patch will add the same facilities to gitweb.css that
gitweb.js has for minification. That does not mean that they
will use the same minifier though, as it is not safe to assume
that all JavaScript minifiers will also minify CSS files.

Though the bandwidth savings will not be as dramatic as with
the JavaScript minifier, every byte saved is important.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>

---

Changes since v4:
	- Reworded some of the comments and documentation


 Makefile        |   21 +++++++++++++++------
 gitweb/INSTALL  |    5 +++++
 gitweb/Makefile |   28 +++++++++++++++++++++-------
 gitweb/README   |    3 ++-
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index 5384d33..450e4df 100644
--- a/Makefile
+++ b/Makefile
@@ -203,6 +203,9 @@ all::
 # Define JSMIN to point to JavaScript minifier that functions as
 # a filter to have gitweb.js minified.
 #
+# Define CSSMIN to point to a CSS minifier in order to generate a minified
+# version of gitweb.css
+#
 # Define DEFAULT_PAGER to a sensible pager command (defaults to "less") if
 # you want to use something different.  The value will be interpreted by the
 # shell at runtime when it is used.
@@ -279,8 +282,9 @@ lib = lib
 # DESTDIR=
 pathsep = :
 
-# JavaScript minifier invocation that can function as filter
+# JavaScript/CSS minifier invocation that can function as filter
 JSMIN =
+CSSMIN =
 
 export prefix bindir sharedir sysconfdir
 
@@ -1564,18 +1568,23 @@ gitweb:
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
 
 ifdef JSMIN
-OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
-gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
-else
-OTHER_PROGRAMS += gitweb/gitweb.cgi
-gitweb/gitweb.cgi: gitweb/gitweb.perl
+GITWEB_PROGRAMS += gitweb/gitweb.min.js
 endif
+ifdef CSSMIN
+GITWEB_PROGRAMS += gitweb/gitweb.min.css
+endif
+OTHER_PROGRAMS +=  gitweb/gitweb.cgi  $(GITWEB_PROGRAMS)
+gitweb/gitweb.cgi: gitweb/gitweb.perl $(GITWEB_PROGRAMS)
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
 
 ifdef JSMIN
 gitweb/gitweb.min.js: gitweb/gitweb.js
 	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
 endif # JSMIN
+ifdef CSSMIN
+gitweb/gitweb.min.css: gitweb/gitweb.css
+	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)
+endif # CSSMIN
 
 
 git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/gitweb.js
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index b76a0cf..b75a90b 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -66,6 +66,11 @@ file for gitweb (in gitweb/README).
   build configuration variables. By default gitweb tries to find them
   in the same directory as gitweb.cgi script.
 
+- You can optionally generate a minified version of gitweb.css by defining
+  the CSSMIN build configuration variable. By default the non-minified
+  version of gitweb.css will be used. NOTE: if you enable this option,
+  substitute gitweb.min.css for all uses of gitweb.css in the help files.
+
 Build example
 ~~~~~~~~~~~~~
 
diff --git a/gitweb/Makefile b/gitweb/Makefile
index c9eb1ee..fffe700 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -6,13 +6,17 @@ all::
 # Define JSMIN to point to JavaScript minifier that functions as
 # a filter to have gitweb.js minified.
 #
+# Define CSSMIN to point to a CSS minifier in order to generate a minified
+# version of gitweb.css
+#
 
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
 RM ?= rm -f
 
-# JavaScript minifier invocation that can function as filter
+# JavaScript/CSS minifier invocation that can function as filter
 JSMIN ?=
+CSSMIN ?=
 
 # default configuration for gitweb
 GITWEB_CONFIG = gitweb_config.perl
@@ -26,7 +30,11 @@ 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
@@ -84,13 +92,14 @@ endif
 
 all:: gitweb.cgi
 
+FILES = gitweb.cgi
 ifdef JSMIN
-FILES=gitweb.cgi gitweb.min.js
-gitweb.cgi: gitweb.perl gitweb.min.js
-else # !JSMIN
-FILES=gitweb.cgi
-gitweb.cgi: gitweb.perl
-endif # JSMIN
+FILES += gitweb.min.js
+endif
+ifdef CSSMIN
+FILES += gitweb.min.css
+endif
+gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
 
 gitweb.cgi:
 	$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -123,6 +132,11 @@ gitweb.min.js: gitweb.js
 	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif # JSMIN
 
+ifdef CSSMIN
+gitweb.min.css: gitweb.css
+	$(QUIET_GEN)$(CSSMIN) <$ >$@
+endif
+
 clean:
 	$(RM) $(FILES)
 
diff --git a/gitweb/README b/gitweb/README
index ad6a04c..71742b3 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -80,7 +80,8 @@ You can specify the following configuration variables when building GIT:
    Points to the location where you put gitweb.css on your web server
    (or to be more generic, the URI of gitweb stylesheet).  Relative to the
    base URI of gitweb.  Note that you can setup multiple stylesheets from
-   the gitweb config file.  [Default: gitweb.css]
+   the gitweb config file.  [Default: gitweb.css (or gitweb.min.css if the
+   CSSMIN variable is defined / CSS minifier is used)]
  * GITWEB_LOGO
    Points to the location where you put git-logo.png on your web server
    (or to be more generic URI of logo, 72x27 size, displayed in top right
-- 
1.7.0.3.519.g7e0613

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-01  5:36 [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css Mark Rada
@ 2010-04-01  8:51 ` Jakub Narebski
  2010-04-13 20:28 ` Charles Bailey
  1 sibling, 0 replies; 15+ messages in thread
From: Jakub Narebski @ 2010-04-01  8:51 UTC (permalink / raw)
  To: Mark Rada; +Cc: git, Junio C Hamano

On Thu, 1 April 2010, Mark Rada wrote:

> The build system added support minifying gitweb.js through a
> JavaScript minifier, but most minifiers come with support to
> minify CSS files as well, so we should use it if we can.
> 
> This patch will add the same facilities to gitweb.css that
> gitweb.js has for minification. That does not mean that they
> will use the same minifier though, as it is not safe to assume
> that all JavaScript minifiers will also minify CSS files.
> 
> Though the bandwidth savings will not be as dramatic as with
> the JavaScript minifier, every byte saved is important.

That's not everything this patch does, isn't it?  I think you should
have added to your commit message the following paragraph:

  While at it, introduce GITWEB_PROGRAMS variable to Makefile and
  use it for gitweb instead of adding to OTHER_PROGRAMS, to keep
  (possible) gitweb dependencies separate.

Or something like that.

> 
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>

I think it is good idea.

[...]
>  ifdef JSMIN
> -OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
> -gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
> -else
> -OTHER_PROGRAMS += gitweb/gitweb.cgi
> -gitweb/gitweb.cgi: gitweb/gitweb.perl
> +GITWEB_PROGRAMS += gitweb/gitweb.min.js
>  endif
> +ifdef CSSMIN
> +GITWEB_PROGRAMS += gitweb/gitweb.min.css
> +endif
> +OTHER_PROGRAMS +=  gitweb/gitweb.cgi  $(GITWEB_PROGRAMS)
> +gitweb/gitweb.cgi: gitweb/gitweb.perl $(GITWEB_PROGRAMS)
>  	$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) $(patsubst gitweb/%,%,$@)

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-01  5:36 [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css Mark Rada
  2010-04-01  8:51 ` Jakub Narebski
@ 2010-04-13 20:28 ` Charles Bailey
  2010-04-13 22:30   ` Jakub Narebski
  1 sibling, 1 reply; 15+ messages in thread
From: Charles Bailey @ 2010-04-13 20:28 UTC (permalink / raw)
  To: Mark Rada; +Cc: git, Junio C Hamano, Jakub Narebski

On 01/04/2010 06:36, Mark Rada wrote:
> @@ -84,13 +92,14 @@ endif
>
>   all:: gitweb.cgi
>
> +FILES = gitweb.cgi
>   ifdef JSMIN
> -FILES=gitweb.cgi gitweb.min.js
> -gitweb.cgi: gitweb.perl gitweb.min.js
> -else # !JSMIN
> -FILES=gitweb.cgi
> -gitweb.cgi: gitweb.perl
> -endif # JSMIN
> +FILES += gitweb.min.js
> +endif
> +ifdef CSSMIN
> +FILES += gitweb.min.css
> +endif
> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>

I have a question about this last line of the patch. Are GITWEB_JS and 
GITWEB_CSS supposed to be a source path or a URI?

The documentation for install (and my previous assumption) was that they 
represented the path on the target web server. I'm used to overriding 
them so that gitweb.cgi can live in my /cgi-bin directory, but the 
static files are served from /gitweb which is readable but not executable.

After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
the list of dependencies for gitweb.cgi otherwise make failed.

Have I got the wrong end of the stick?

Thanks,

Charles.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-13 20:28 ` Charles Bailey
@ 2010-04-13 22:30   ` Jakub Narebski
  2010-04-14  5:40     ` Mark Rada
  2010-04-14 23:58     ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Narebski @ 2010-04-13 22:30 UTC (permalink / raw)
  To: Charles Bailey; +Cc: Mark Rada, git, Junio C Hamano

On Tue, 13 April 2010, Charles Bailey wrote:
> On 01/04/2010 06:36, Mark Rada wrote:
> > @@ -84,13 +92,14 @@ endif
> >
> >   all:: gitweb.cgi
> >
> > +FILES = gitweb.cgi
> >   ifdef JSMIN
> > -FILES=gitweb.cgi gitweb.min.js
> > -gitweb.cgi: gitweb.perl gitweb.min.js
> > -else # !JSMIN
> > -FILES=gitweb.cgi
> > -gitweb.cgi: gitweb.perl
> > -endif # JSMIN
> > +FILES += gitweb.min.js
> > +endif
> > +ifdef CSSMIN
> > +FILES += gitweb.min.css
> > +endif
> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >
> 
> I have a question about this last line of the patch. Are GITWEB_JS and 
> GITWEB_CSS supposed to be a source path or a URI?
> 
> The documentation for install (and my previous assumption) was that they 
> represented the path on the target web server. I'm used to overriding 
> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
> static files are served from /gitweb which is readable but not executable.
> 
> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
> the list of dependencies for gitweb.cgi otherwise make failed.
> 
> Have I got the wrong end of the stick?

Thanks a lot for noticing this bug.


GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
gitweb JavaScript code and default gitweb stylesheet,... but during work
on minification of JavaScript code and CSS file it somehow got confused
to mean source path.

If I remember correctly the original patch, before adding required
support for minified gitweb.js and gitweb.css to git-instaweb script,
and before support for CSS minification had

   ifdef JSMIN
   gitweb.cgi: gitweb.perl gitweb.min.js
   else
   gitweb.cgi: gitweb.perl
   endif

which should probably be replaced in current situation by

   ifdef JSMIN
   gitweb.cgi : gitweb.min.js
   endif
   ifdef CSSMIN
   gitweb.cgi : gitweb.min.css
   endif

just adding prerequisites to gitweb.css target in gitweb/Makefile


I guess that support for adding minifiction support to git-instaweb
would need to be more complicated.  Perhaps

  $(notdir $(GITWEB_JS))   # Makefile function

or

  $(basename $GITWEB_JS)   # shell command

But I guess that it wouldn't work for all cases...

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-13 22:30   ` Jakub Narebski
@ 2010-04-14  5:40     ` Mark Rada
  2010-04-14 17:22       ` Jakub Narebski
  2010-04-14 23:58     ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Mark Rada @ 2010-04-14  5:40 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Charles Bailey, Mark Rada, git, Junio C Hamano

On 10-04-13 6:30 PM, Jakub Narebski wrote:
> On Tue, 13 April 2010, Charles Bailey wrote:
>> On 01/04/2010 06:36, Mark Rada wrote:
>>> @@ -84,13 +92,14 @@ endif
>>>
>>>   all:: gitweb.cgi
>>>
>>> +FILES = gitweb.cgi
>>>   ifdef JSMIN
>>> -FILES=gitweb.cgi gitweb.min.js
>>> -gitweb.cgi: gitweb.perl gitweb.min.js
>>> -else # !JSMIN
>>> -FILES=gitweb.cgi
>>> -gitweb.cgi: gitweb.perl
>>> -endif # JSMIN
>>> +FILES += gitweb.min.js
>>> +endif
>>> +ifdef CSSMIN
>>> +FILES += gitweb.min.css
>>> +endif
>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>
>>
>> I have a question about this last line of the patch. Are GITWEB_JS and 
>> GITWEB_CSS supposed to be a source path or a URI?
>>
>> The documentation for install (and my previous assumption) was that they 
>> represented the path on the target web server. I'm used to overriding 
>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>> static files are served from /gitweb which is readable but not executable.
>>
>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>> the list of dependencies for gitweb.cgi otherwise make failed.
>>
>> Have I got the wrong end of the stick?
> 
> Thanks a lot for noticing this bug.
> 
> 
> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> gitweb JavaScript code and default gitweb stylesheet,... but during work
> on minification of JavaScript code and CSS file it somehow got confused
> to mean source path.
> 
> If I remember correctly the original patch, before adding required
> support for minified gitweb.js and gitweb.css to git-instaweb script,
> and before support for CSS minification had
> 
>    ifdef JSMIN
>    gitweb.cgi: gitweb.perl gitweb.min.js
>    else
>    gitweb.cgi: gitweb.perl
>    endif
> 
> which should probably be replaced in current situation by
> 
>    ifdef JSMIN
>    gitweb.cgi : gitweb.min.js
>    endif
>    ifdef CSSMIN
>    gitweb.cgi : gitweb.min.css
>    endif
> 
> just adding prerequisites to gitweb.css target in gitweb/Makefile
> 
> 
> I guess that support for adding minifiction support to git-instaweb
> would need to be more complicated.  Perhaps
> 
>   $(notdir $(GITWEB_JS))   # Makefile function
> 
> or
> 
>   $(basename $GITWEB_JS)   # shell command
> 
> But I guess that it wouldn't work for all cases...
>

Aw, frig, never thought of using gitweb like that so I made some
assumptions to make things cleaner looking.

I think this can be fixed by just using different variable names? Or
perhaps some nested ifdef's? I'm not sure which will be better.

I wasn't at the computer today so I'm just getting to it now, I'll try
to have something when in the next day, going to bed now. Good night.

-- 
Mark Rada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-14  5:40     ` Mark Rada
@ 2010-04-14 17:22       ` Jakub Narebski
  2010-04-14 19:17         ` Mark Rada
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-04-14 17:22 UTC (permalink / raw)
  To: Mark Rada; +Cc: Charles Bailey, git, Junio C Hamano, Eric Wong

On Wed, 14 Apr 2010, Mark Rada wrote:
> On 10-04-13 6:30 PM, Jakub Narebski wrote:
>> On Tue, 13 April 2010, Charles Bailey wrote:
>>> On 01/04/2010 06:36, Mark Rada wrote:

[...]
>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>>
>>>
>>> I have a question about this last line of the patch. Are GITWEB_JS and 
>>> GITWEB_CSS supposed to be a source path or a URI?
>>>
>>> The documentation for install (and my previous assumption) was that they 
>>> represented the path on the target web server. I'm used to overriding 
>>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>>> static files are served from /gitweb which is readable but not executable.
>>>
>>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>>> the list of dependencies for gitweb.cgi otherwise make failed.
>>>
>>> Have I got the wrong end of the stick?
>> 
>> Thanks a lot for noticing this bug.
>> 
>> 
>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>> on minification of JavaScript code and CSS file it somehow got confused
>> to mean source path.
>> 
>> If I remember correctly the original patch, before adding required
>> support for minified gitweb.js and gitweb.css to git-instaweb script,
>> and before support for CSS minification had
>> 
>>    ifdef JSMIN
>>    gitweb.cgi: gitweb.perl gitweb.min.js
>>    else
>>    gitweb.cgi: gitweb.perl
>>    endif
>> 
>> which should probably be replaced in current situation by
>> 
>>    ifdef JSMIN
>>    gitweb.cgi : gitweb.min.js
>>    endif
>>    ifdef CSSMIN
>>    gitweb.cgi : gitweb.min.css
>>    endif
>> 
>> just adding prerequisites to gitweb.css target in gitweb/Makefile
>> 
>> 
>> I guess that support for adding minifiction support to git-instaweb
>> would need to be more complicated. [...]
> 
> Aw, frig, never thought of using gitweb like that so I made some
> assumptions to make things cleaner looking.
> 
> I think this can be fixed by just using different variable names? Or
> perhaps some nested ifdef's? I'm not sure which will be better.
> 
> I wasn't at the computer today so I'm just getting to it now, I'll try
> to have something when in the next day, going to bed now. Good night.

I think the best solution for prerequisites would be to have multiple 
target-only (without commands) rules, which according to make 
documentation would get concatenated.  This means the following code
in gitweb/Makefile:

    ifdef JSMIN
    gitweb.cgi : gitweb.min.js
    endif
    ifdef CSSMIN
    gitweb.cgi : gitweb.min.css
    endif

in place of

   gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)


For git-instaweb I think that best solution would be to introduce new
variables holding _source_ of gitweb JavaScript code and CSS, e.g.

            -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \

in place of

            -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \

...although GITWEB_CSS might mean something different for Makefile
and git-instaweb than for gitweb/Makefile and gitweb itself.
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-14 17:22       ` Jakub Narebski
@ 2010-04-14 19:17         ` Mark Rada
  2010-04-14 20:04           ` Jakub Narebski
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Rada @ 2010-04-14 19:17 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Mark Rada, Charles Bailey, git, Junio C Hamano, Eric Wong

On 10-04-14 1:22 PM, Jakub Narebski wrote:
> On Wed, 14 Apr 2010, Mark Rada wrote:
>> On 10-04-13 6:30 PM, Jakub Narebski wrote:
>>> On Tue, 13 April 2010, Charles Bailey wrote:
>>>> On 01/04/2010 06:36, Mark Rada wrote:
> 
> [...]
>>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>>>
>>>>
>>>> I have a question about this last line of the patch. Are GITWEB_JS and 
>>>> GITWEB_CSS supposed to be a source path or a URI?
>>>>
>>>> The documentation for install (and my previous assumption) was that they 
>>>> represented the path on the target web server. I'm used to overriding 
>>>> them so that gitweb.cgi can live in my /cgi-bin directory, but the 
>>>> static files are served from /gitweb which is readable but not executable.
>>>>
>>>> After this patch I had to removed $(GITWEB_JS) and $(GITWEB_CSS) from 
>>>> the list of dependencies for gitweb.cgi otherwise make failed.
>>>>
>>>> Have I got the wrong end of the stick?
>>>
>>> Thanks a lot for noticing this bug.
>>>
>>>
>>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>>> on minification of JavaScript code and CSS file it somehow got confused
>>> to mean source path.
>>>
>>> If I remember correctly the original patch, before adding required
>>> support for minified gitweb.js and gitweb.css to git-instaweb script,
>>> and before support for CSS minification had
>>>
>>>    ifdef JSMIN
>>>    gitweb.cgi: gitweb.perl gitweb.min.js
>>>    else
>>>    gitweb.cgi: gitweb.perl
>>>    endif
>>>
>>> which should probably be replaced in current situation by
>>>
>>>    ifdef JSMIN
>>>    gitweb.cgi : gitweb.min.js
>>>    endif
>>>    ifdef CSSMIN
>>>    gitweb.cgi : gitweb.min.css
>>>    endif
>>>
>>> just adding prerequisites to gitweb.css target in gitweb/Makefile
>>>
>>>
>>> I guess that support for adding minifiction support to git-instaweb
>>> would need to be more complicated. [...]
>>
>> Aw, frig, never thought of using gitweb like that so I made some
>> assumptions to make things cleaner looking.
>>
>> I think this can be fixed by just using different variable names? Or
>> perhaps some nested ifdef's? I'm not sure which will be better.
>>
>> I wasn't at the computer today so I'm just getting to it now, I'll try
>> to have something when in the next day, going to bed now. Good night.
> 
> I think the best solution for prerequisites would be to have multiple 
> target-only (without commands) rules, which according to make 
> documentation would get concatenated.  This means the following code
> in gitweb/Makefile:
> 
>     ifdef JSMIN
>     gitweb.cgi : gitweb.min.js
>     endif
>     ifdef CSSMIN
>     gitweb.cgi : gitweb.min.css
>     endif
> 
> in place of
> 
>    gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> 

Hmm, I like this because it is clear (if you know that dependancies
can be joined like that), I was thinking of trying to make a smaller fix
using pathsubst or subst, but that seems to not be as simple as I wanted it
to be.

> For git-instaweb I think that best solution would be to introduce new
> variables holding _source_ of gitweb JavaScript code and CSS, e.g.
> 
>             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
> 
> in place of
> 
>             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
> 
> ...although GITWEB_CSS might mean something different for Makefile
> and git-instaweb than for gitweb/Makefile and gitweb itself.

Did you get those lines mixed up? I might be not understanding something
here.

I was actually planning something along the lines of 

             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_NAME)' \
             -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS_NAME)|' \

where I introduce the GITWEB_CSS_NAME variable, to be consistent with the
token in instaweb. This way we don't touch GITWEB_JS in the top level
makefile.

Also, I should update dependancies for instaweb, since those were
forgotten last time around. Just creating a short list of what the fix will
need for when I get home tonight.

-- 
Mark Rada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-14 19:17         ` Mark Rada
@ 2010-04-14 20:04           ` Jakub Narebski
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Narebski @ 2010-04-14 20:04 UTC (permalink / raw)
  To: Mark Rada; +Cc: Charles Bailey, git, Junio C Hamano, Eric Wong

On Wed, 14 April 2010, Mark Rada wrote:
> On 10-04-14 1:22 PM, Jakub Narebski wrote:

> > For git-instaweb I think that best solution would be to introduce new
> > variables holding _source_ of gitweb JavaScript code and CSS, e.g.
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
> > 
> > in place of
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
> > 
> > ...although GITWEB_CSS might mean something different for Makefile
> > and git-instaweb than for gitweb/Makefile and gitweb itself.
> 
> Did you get those lines mixed up? I might be not understanding something
> here.

Ah, I'm sorry, I mixed up those two lines.  They should be in reverse
direction:


             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
 
  in place of
 
             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \


Actually I'd like to rename @@GITWEB_CSS@@ placeholder etc. in 
git-instaweb.sh, as  @@GITWEB_CSS@@ in git-instaweb.sh means something
quite different from ++GITWEB_CSS++ in gitweb/gitweb.perl...

> 
> I was actually planning something along the lines of 
> 
>              -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_NAME)' \
>              -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS_NAME)|' \
> 
> where I introduce the GITWEB_CSS_NAME variable, to be consistent with the
> token in instaweb. This way we don't touch GITWEB_JS in the top level
> makefile.

Why not:

            -e '/@@GITWEB_CSS_SOURCE@@/r $(GITWEB_CSS_SOURCE)' \
            -e '/@@GITWEB_CSS_SOURCE@@/d' \
            ...
            -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS)|' \

(assuming that $(GITWEB_CSS) does not include '|' in it, I guess...
but see below).

> 
> Also, I should update dependancies for instaweb, since those were
> forgotten last time around. Just creating a short list of what the fix will
> need for when I get home tonight.

Something like

   git-instaweb: git-instaweb.sh gitweb/gitweb.cgi $(GITWEB_CSS_SOURCE) $(GITWEB_JS_SOURCE)


P.S. I have noticed additional complication: git-instaweb really needs
gitweb compiled with *specific* values of GITWEB_CSS and GITWEB_JS,
so that they point to git-instaweb's installed files.

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-13 22:30   ` Jakub Narebski
  2010-04-14  5:40     ` Mark Rada
@ 2010-04-14 23:58     ` Junio C Hamano
  2010-04-15  0:18       ` Charles Bailey
  2010-04-15  0:25       ` Jakub Narebski
  1 sibling, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2010-04-14 23:58 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Charles Bailey, Mark Rada, git

Jakub Narebski <jnareb@gmail.com> writes:

> On Tue, 13 April 2010, Charles Bailey wrote:
>> On 01/04/2010 06:36, Mark Rada wrote:
>> > @@ -84,13 +92,14 @@ endif
>> >
>> >   all:: gitweb.cgi
>> >
>> > +FILES = gitweb.cgi
>> >   ifdef JSMIN
>> > +FILES += gitweb.min.js
>> > +endif
>> > +ifdef CSSMIN
>> > +FILES += gitweb.min.css
>> > +endif
>> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>
> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> gitweb JavaScript code and default gitweb stylesheet,... but during work
> on minification of JavaScript code and CSS file it somehow got confused
> to mean source path.

I am not touching instaweb part, but this would fix the build/clean side
of the things, no?

-- >8 --
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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 gitweb/Makefile |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/gitweb/Makefile b/gitweb/Makefile
index ffee4bd..1787633 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -80,16 +80,7 @@ endif
 
 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)
+gitweb.cgi: gitweb.perl
 
 gitweb.cgi:
 	$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -118,16 +109,18 @@ gitweb.cgi:
 	mv $@+ $@
 
 ifdef JSMIN
+all:: gitweb.min.js
 gitweb.min.js: gitweb.js
 	$(QUIET_GEN)$(JSMIN) <$< >$@
 endif # JSMIN
 
 ifdef CSSMIN
+all:: gitweb.min.css
 gitweb.min.css: gitweb.css
 	$(QUIET_GEN)$(CSSMIN) <$ >$@
 endif
 
 clean:
-	$(RM) $(FILES)
+	$(RM) gitweb.cgi gitweb.min.css gitweb.min.js
 
 .PHONY: all clean .FORCE-GIT-VERSION-FILE

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-14 23:58     ` Junio C Hamano
@ 2010-04-15  0:18       ` Charles Bailey
  2010-04-15  0:25       ` Jakub Narebski
  1 sibling, 0 replies; 15+ messages in thread
From: Charles Bailey @ 2010-04-15  0:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, Mark Rada, git

On Wed, Apr 14, 2010 at 04:58:12PM -0700, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > On Tue, 13 April 2010, Charles Bailey wrote:
> >> On 01/04/2010 06:36, Mark Rada wrote:
> >> > @@ -84,13 +92,14 @@ endif
> >> >
> >> >   all:: gitweb.cgi
> >> >
> >> > +FILES = gitweb.cgi
> >> >   ifdef JSMIN
> >> > +FILES += gitweb.min.js
> >> > +endif
> >> > +ifdef CSSMIN
> >> > +FILES += gitweb.min.css
> >> > +endif
> >> > +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >
> > GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
> > gitweb JavaScript code and default gitweb stylesheet,... but during work
> > on minification of JavaScript code and CSS file it somehow got confused
> > to mean source path.
> 
> I am not touching instaweb part, but this would fix the build/clean side
> of the things, no?
> 

Yes, this certainly fixes make and make clean for me with overridden
GITWEB_JS and GITWEB_CSS paths.

Charles.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-14 23:58     ` Junio C Hamano
  2010-04-15  0:18       ` Charles Bailey
@ 2010-04-15  0:25       ` Jakub Narebski
  2010-04-15  0:46         ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-04-15  0:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Charles Bailey, Mark Rada, git

On Thu, 15 Apr 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Tue, 13 April 2010, Charles Bailey wrote:
>>> On 01/04/2010 06:36, Mark Rada wrote:
>>>> @@ -84,13 +92,14 @@ endif
>>>>
>>>>   all:: gitweb.cgi
>>>>
>>>> +FILES = gitweb.cgi
>>>>   ifdef JSMIN
>>>> +FILES += gitweb.min.js
>>>> +endif
>>>> +ifdef CSSMIN
>>>> +FILES += gitweb.min.css
>>>> +endif
>>>> +gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>
>> GITWEB_JS and GITWEB_CSS were originally meant to be URI to file with
>> gitweb JavaScript code and default gitweb stylesheet,... but during work
>> on minification of JavaScript code and CSS file it somehow got confused
>> to mean source path.
> 
> I am not touching instaweb part, but this would fix the build/clean side
> of the things, no?

Close, see below.

> 
> -->8 --
> 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.

Actually the assignment was intended to provide correct *default* values
for GITWEB_CSS and GITWEB_JS, so that if e.g. JSMIN is defined gitweb,
in absence of build time configuration, would link gitweb.min.js instead
of gitweb.js.

> 
> 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.

Good.

> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  gitweb/Makefile |   15 ++++-----------
>  1 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index ffee4bd..1787633 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -80,16 +80,7 @@ endif
>  
>  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

I wonder about removing assigmnet to GITWEB_JS and GITWEB_CSS.  Without
it "make gitweb" (in top dir) would create gitweb/gitweb.cgi and 
gitweb/gitweb.min.js etc.... but generated gitweb/gitweb.cgi would
refer to gitweb.js, not gitweb.min.js.  Unless of course one provides
values for GITWEB_JS during build time.

> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> +gitweb.cgi: gitweb.perl
>  
>  gitweb.cgi:
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
> @@ -118,16 +109,18 @@ gitweb.cgi:
>  	mv $@+ $@
>  
>  ifdef JSMIN
> +all:: gitweb.min.js
>  gitweb.min.js: gitweb.js
>  	$(QUIET_GEN)$(JSMIN) <$<>$@
>  endif # JSMIN
>  
>  ifdef CSSMIN
> +all:: gitweb.min.css
>  gitweb.min.css: gitweb.css
>  	$(QUIET_GEN)$(CSSMIN) <$>$@
>  endif

That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
It might be right... and I think the rightness or wrongness might be
tied with values of GITWEB>JS and GITWEB_CSS.

>  
>  clean:
> -	$(RM) $(FILES)
> +	$(RM) gitweb.cgi gitweb.min.css gitweb.min.js
>  
>  .PHONY: all clean .FORCE-GIT-VERSION-FILE
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-15  0:25       ` Jakub Narebski
@ 2010-04-15  0:46         ` Junio C Hamano
  2010-04-15  1:02           ` Jakub Narebski
  2010-04-15  1:42           ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2010-04-15  0:46 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Charles Bailey, Mark Rada, git

Jakub Narebski <jnareb@gmail.com> writes:

>> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>> +gitweb.cgi: gitweb.perl
> ...
> That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.

That is what I inteded to do.

Unless you are including gitweb.cgi (iow, the contents of the generated
file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
does _not_ depend on these files.  Of course if you generate gitweb.cgi
out of gitweb.perl with one setting of GITWEB_JS and then change your
mind, then you need to regenerate it, but that is not something you can do
by comparing file timestamp of gitweb.cgi and the file timestamp of
$(GITWEB_JS) anyway.  You would need to imitate something like how
GIT-BUILD-OPTIONS is used by the primary Makefile.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-15  0:46         ` Junio C Hamano
@ 2010-04-15  1:02           ` Jakub Narebski
  2010-04-15  1:21             ` Mark Rada
  2010-04-15  1:42           ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Narebski @ 2010-04-15  1:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Charles Bailey, Mark Rada, git

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> >> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
> >> +gitweb.cgi: gitweb.perl
> > ...
> > That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
> 
> That is what I inteded to do.
> 
> Unless you are including gitweb.cgi (iow, the contents of the generated
> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
> does _not_ depend on these files.  Of course if you generate gitweb.cgi
> out of gitweb.perl with one setting of GITWEB_JS and then change your
> mind, then you need to regenerate it, but that is not something you can do
> by comparing file timestamp of gitweb.cgi and the file timestamp of
> $(GITWEB_JS) anyway.  You would need to imitate something like how
> GIT-BUILD-OPTIONS is used by the primary Makefile.

Right.  I agree.

P.S. This is nt the case with git-instaweb, which literally include 
gitweb.js or gitweb.min.js...

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-15  1:02           ` Jakub Narebski
@ 2010-04-15  1:21             ` Mark Rada
  0 siblings, 0 replies; 15+ messages in thread
From: Mark Rada @ 2010-04-15  1:21 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Charles Bailey, Mark Rada, git

On 10-04-14 9:02 PM, Jakub Narebski wrote:
> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>>> -gitweb.cgi: gitweb.perl $(GITWEB_JS) $(GITWEB_CSS)
>>>> +gitweb.cgi: gitweb.perl
>>> ...
>>> That makes gitweb.cgi not depend on gitweb.min.js, not gitweb.min.css.
>>
>> That is what I inteded to do.
>>
>> Unless you are including gitweb.cgi (iow, the contents of the generated
>> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
>> does _not_ depend on these files.  Of course if you generate gitweb.cgi
>> out of gitweb.perl with one setting of GITWEB_JS and then change your
>> mind, then you need to regenerate it, but that is not something you can do
>> by comparing file timestamp of gitweb.cgi and the file timestamp of
>> $(GITWEB_JS) anyway.  You would need to imitate something like how
>> GIT-BUILD-OPTIONS is used by the primary Makefile.
> 
> Right.  I agree.
> 
> P.S. This is nt the case with git-instaweb, which literally include 
> gitweb.js or gitweb.min.js...
> 

I believe we can just change the variable names as planned before as well
as the dependancies, and that should fix up instaweb as far as will need
to be fixed, the rest has to happen at gitweb.cgi generation time.

My understanding (please correct me if I am wrong) is that comparing the
mtimes of the files is not reliable. So can't we just make gitweb.cgi
depend on source .js and .css files, since any modifications there will
also cause the minified versions to be regenerated? Can Junio's patch just
be updated to subst the suffix of GITWEB_JS and GITWEB_CSS, which makes sure
the correct version is being used?



-- 
Mark Rada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css
  2010-04-15  0:46         ` Junio C Hamano
  2010-04-15  1:02           ` Jakub Narebski
@ 2010-04-15  1:42           ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2010-04-15  1:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, Charles Bailey, Mark Rada, git

Junio C Hamano <gitster@pobox.com> writes:

> Unless you are including gitweb.cgi (iow, the contents of the generated
> file depends on the _contents_ of gitweb.min.js (or gitweb.js), gitweb.cgi
> does _not_ depend on these files.  Of course if you generate gitweb.cgi
> out of gitweb.perl with one setting of GITWEB_JS and then change your
> mind, then you need to regenerate it, but that is not something you can do
> by comparing file timestamp of gitweb.cgi and the file timestamp of
> $(GITWEB_JS) anyway.  You would need to imitate something like how
> GIT-BUILD-OPTIONS is used by the primary Makefile.

Here is a minimally tested patch.

In addition to the points raised by the proposed log message for the
previous one, this 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.

If this seems to work Ok, please send it back to me with a proper commit
log message (concat of the above and the previous) with Tested-by: or
Acked-by: as appropriate, so that we can fix it at the tip of 'master'
before we tag 1.7.1-rc2.

Thanks.

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

diff --git a/gitweb/Makefile b/gitweb/Makefile
index ffee4bd..f2e1d92 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-04-15  1:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-01  5:36 [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css Mark Rada
2010-04-01  8:51 ` Jakub Narebski
2010-04-13 20:28 ` Charles Bailey
2010-04-13 22:30   ` Jakub Narebski
2010-04-14  5:40     ` Mark Rada
2010-04-14 17:22       ` Jakub Narebski
2010-04-14 19:17         ` Mark Rada
2010-04-14 20:04           ` Jakub Narebski
2010-04-14 23:58     ` Junio C Hamano
2010-04-15  0:18       ` Charles Bailey
2010-04-15  0:25       ` Jakub Narebski
2010-04-15  0:46         ` Junio C Hamano
2010-04-15  1:02           ` Jakub Narebski
2010-04-15  1:21             ` Mark Rada
2010-04-15  1:42           ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.