Re: [PATCH 4/6] GITWEB - Makefile changes

From: J.H. <warthog9@kernel.org>
Date: Dec 11, 2009, 16:22 UTC
Message-ID: <4B2271B4.2010301@kernel.org>
In-Reply-To: <m3pr6ld1p2.fsf@localhost.localdomain>

<snip>

Show 8 quoted lines

> IMPORTANT!
> 
> A note about this change: I think it would be better to move creating
> gitweb.cgi (and optionally gitweb.min.js) to gitweb/Makefile, and make
> main Makefile call gitweb/Makefile, and not vice versa like in your
> solution.
> 
> If it is possible.

It's quite possible, and I'm fine with doing that. If no one has any objections I can re-work those with the understanding that the build process for gitweb shift to the gitweb/ directory instead of the main Makefile.

Show 14 quoted lines

>  
>> diff --git a/Makefile b/Makefile
>> index 4a1e5bc..8db9d01 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1509,6 +1509,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
>>  	chmod +x $@+ && \
>>  	mv $@+ $@
>>  
>> +.PHONY: gitweb
> 
> Why it is here, and not with the .PHONY block at line 1924 of
> Makefile?  It would be nice to have comment supporting this choice in
> email with this patch (or in commit message).

There are 6 other instances of .PHONY in the makefile, having the .PHONY localized seemed to make it the most obvious since it was right next to the actual target.

Show 13 quoted lines

> 
>> +gitweb: gitweb/gitweb.cgi
>>  ifdef JSMIN
>>  OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
>>  gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
>> @@ -1537,7 +1539,7 @@ endif
>>  	    -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' \
>> -	    $< >$@+ && \
>> +	    $(patsubst %.cgi,%.perl,$@) >$@+ && \
> 
> Why this change?

Preparation for a later change. The change could happen all at the same time if it makes more logical sense.

Show 20 quoted lines

> 
>>  	chmod +x $@+ && \
>>  	mv $@+ $@
>>  
>> diff --git a/gitweb/Makefile b/gitweb/Makefile
>> new file mode 100644
>> index 0000000..8d318b3
>> --- /dev/null
>> +++ b/gitweb/Makefile
>> @@ -0,0 +1,14 @@
>> +SHELL = /bin/bash
> 
> Why is this needed?
> 
>> +
>> +FILES = gitweb.cgi
>> +
>> +.PHONY: $(FILES)
> 
> Why .PHONY?  $(FILES) are created.

 From this makefile I wanted to explicitly call up to the main makefile 
no matter what, the main makefile doesn't consider the targets .PHONY 
and it has all the dependencies that it would expect.

Show 9 quoted lines

>> +
>> +all: $(FILES)
>> +
>> +$(FILES):
>> +	$(MAKE) $(MFLAGS) -C ../ -f Makefile gitweb/$@
>> +
>> +clean:
>> +	rm -rf $(FILES)
>> +