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