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