On Thu, 15 Apr 2010, Junio C Hamano wrote: > Jakub Narebski <jnareb@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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 -- 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