git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 11, 2009, 16:41 UTC
Message-ID
<200912111741.51760.jnareb@gmail.com>
In-Reply-To
<4B2271B4.2010301@kernel.org>
On Fri, 11 Dec 2009, J.H. wrote:
Show 13 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.
In my opinion it would be better solution because it would reduce size
of main (master) Makefile, and not be much larger than this solution.
 
git-gui/, Documentation/, perl/ all have their own makefiles, which do
the work, and are called from main (master) Makefile.
Show 17 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.
I was thinking here about this large block of .PHONY declarations,
the one which is not inside conditional.
Show 15 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.
Please at least describe this change in commit message.
But I think it could be moved to other patch, or put in separate patch.
This change has nothing to do with easier gitweb generation.
Show 12 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?
Why do you need to define SHELL?
Show 10 quoted lines
>>> +
>>> +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.
What is the reason of this phony .PHONY?  If gitweb.cgi is newer than
gitweb.perl (and other sources), then without .PHONY it wouldn't be 
regenerated.  With .PHONY it would call master Makefile... which would
notice that gitweb.cgi is newer than gitweb.perl and do not regenerate.
 
So what this .PHONY does is unnecessary call make on master Makefile...

But I guess this issue would be moot if it was the other way around, i.e. master Makefile calling gitweb/Makefile.

-- 
Jakub Narebski
Poland
Previous: J.H.Next: Jakub Narebski
Message 8 of 40 in “Gitweb caching changes v2”
  1. 0/6 Gitweb caching changes v2John 'Warthog9' Hawley, Dec 10, 2009
  2. 1/6 GITWEB - Load CheckingJohn 'Warthog9' Hawley, Dec 10, 2009
  3. 2/6 GITWEB - Missmatching git w/ gitwebJohn 'Warthog9' Hawley, Dec 10, 2009
  4. 3/6 GITWEB - Add git:// link to summary pagesJohn 'Warthog9' Hawley, Dec 10, 2009
  5. 4/6 GITWEB - Makefile changesJohn 'Warthog9' Hawley, Dec 10, 2009
  6. Jakub NarebskiDec 11, 2009
  7. J.H.Dec 11, 2009
  8. Jakub NarebskiDec 11, 2009
  9. 4/6 gitweb: Makefile improvementsJakub Narebski, Dec 19, 2009
  10. Johannes SchindelinDec 11, 2009
  11. Jakub NarebskiDec 11, 2009
  12. 3/6 gitweb: Optionally add "git" links in project list pageJakub Narebski, Dec 18, 2009
  13. Jakub NarebskiDec 11, 2009
  14. 2/6 gitweb: Add option to force version matchJakub Narebski, Dec 18, 2009
  15. Johannes SchindelinDec 11, 2009
  16. Sverre RabbelierDec 10, 2009
  17. Jakub NarebskiDec 11, 2009
  18. Junio C HamanoDec 11, 2009
  19. J.H.Dec 11, 2009
  20. Junio C HamanoDec 11, 2009
  21. J.H.Dec 11, 2009
  22. J.H.Dec 11, 2009
  23. Junio C HamanoDec 11, 2009
  24. Jakub NarebskiDec 11, 2009
  25. 1/6 gitweb: Load checkingJakub Narebski, Dec 18, 2009
  26. Mihamina RakotomandimbyDec 11, 2009
  27. Sverre RabbelierDec 10, 2009
  28. Jakub NarebskiDec 11, 2009
  29. 6/6 GITWEB - Separate defaults from main fileJohn 'Warthog9' Hawley, Dec 10, 2009
  30. Jakub NarebskiDec 11, 2009
  31. J.H.Dec 11, 2009
  32. Jakub NarebskiDec 11, 2009
  33. Junio C HamanoDec 16, 2009
  34. J.H.Dec 16, 2009
  35. Jakub NarebskiDec 16, 2009
  36. J.H.Dec 16, 2009
  37. Jakub NarebskiDec 16, 2009
  38. Jakub NarebskiDec 11, 2009
  39. J.H.Dec 11, 2009
  40. Jakub NarebskiDec 12, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.