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

Re: [PATCH 6/6] GITWEB - Separate defaults from main file

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 11, 2009, 15:46 UTC
Message-ID
<m3ljh9cy3b.fsf@localhost.localdomain>
In-Reply-To
<1260488743-25855-7-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel.org> writes:
Show 13 quoted lines
> This is an attempt to break out the default values & associated
> documentation from the main gitweb file so that it's easier to
> browse / read and understand without the associated code involved.
> 
> This helps by making defaults self contained with their documentation
> making it easier for someone to read through things and find what
> they want
> 
> This is also a not-so-subtle start of trying to break up gitweb into
> separate files for easier maintainability, having everything in a
> single file is just a mess and makes the whole thing more complicated
> than it needs to be.  This is a bit of a baby step towards breaking it
> up for easier maintenance.

The question is if easier maintenance and development by spliting gitweb for developers offsets ease of install for users.

> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
Signoff mismatch.
Show 19 quoted lines
> ---
>  .gitignore                  |    1 +
>  Makefile                    |   15 +-
>  gitweb/Makefile             |    2 +-
>  gitweb/gitweb.perl          |  515 +++++--------------------------------------
>  gitweb/gitweb_defaults.perl |  468 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 537 insertions(+), 464 deletions(-)
>  create mode 100644 gitweb/gitweb_defaults.perl
> 
> 
> diff --git a/.gitignore b/.gitignore
> index ac02a58..5e48102 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -151,6 +151,7 @@
>  /git-core-*/?*
>  /gitk-git/gitk-wish
>  /gitweb/gitweb.cgi
> +/gitweb/gitweb_defaults.pl

Hmmm... gitweb/gitweb_defaults.perl as source file, and gitweb/gitweb_defaults.pl as generated file? Wouldn't it be better to go with the convention used elsewhere in gitweb and use gitweb/gitweb_defaults.perl.in or gitweb/gitweb_defaults.pl.in as source file?

Show 25 quoted lines
>  /test-chmtime
>  /test-ctype
>  /test-date
> diff --git a/Makefile b/Makefile
> index 8db9d01..2c5f139 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1510,14 +1510,16 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
>  	mv $@+ $@
>  
>  .PHONY: gitweb
> -gitweb: gitweb/gitweb.cgi
> +gitweb: gitweb/gitweb.cgi gitweb/gitweb_defaults.pl
>  ifdef JSMIN
> -OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js
> -gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
> +OTHER_PROGRAMS += gitweb/gitweb.cgi   gitweb/gitweb.min.js gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi gitweb/gitweb_defaults.pl: gitweb/gitweb.perl gitweb/gitweb.min.js gitweb/gitweb_defaults.perl
>  else
> -OTHER_PROGRAMS += gitweb/gitweb.cgi
> -gitweb/gitweb.cgi: gitweb/gitweb.perl
> +OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi: gitweb/gitweb_defaults.pl
> +gitweb/gitweb.cgi gitweb/gitweb_defaults.pl: gitweb/gitweb.perl gitweb/gitweb_defaults.perl
>  endif
> +	#$(QUIET_GEN)$(RM) $@ $@+ &&
What this line is about?
Show 9 quoted lines
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
>  	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>  	    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
> @@ -1539,7 +1541,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,$@) >$@+ && \
> +	    $(patsubst %.cgi,%.perl,$(patsubst %.pl, %.perl, $@)) >$@+ && \
Why the slightly inconsistent style ("%.cgi,%perl" vs "%.pl, %perl")?

Also wouldn't all replacements be in the new gitweb_defaults file, so there would be no need then to do replacements for gitweb.cgi?

Oh, I see there is at least one that stayed in gitweb.perl: $version
Show 31 quoted lines
>  	chmod +x $@+ && \
>  	mv $@+ $@
>  
> @@ -1913,6 +1915,7 @@ clean:
>  	$(MAKE) -C Documentation/ clean
>  ifndef NO_PERL
>  	$(RM) gitweb/gitweb.cgi
> +	$(RM) gitweb/gitweb_defaults.pl
>  	$(MAKE) -C perl clean
>  endif
>  	$(MAKE) -C templates/ clean
> diff --git a/gitweb/Makefile b/gitweb/Makefile
> index 8d318b3..2bd421a 100644
> --- a/gitweb/Makefile
> +++ b/gitweb/Makefile
> @@ -1,6 +1,6 @@
>  SHELL = /bin/bash
>  
> -FILES = gitweb.cgi
> +FILES = gitweb.cgi gitweb_defaults.pl
>  
>  .PHONY: $(FILES)
>  
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3b44371..fd41539 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -36,466 +36,67 @@ our $version = "++GIT_VERSION++";
>  our $my_url = $cgi->url();
>  our $my_uri = $cgi->url(-absolute => 1);
>  
[cut deletion]
Show 58 quoted lines
> +# Define and than setup our configuration 
> +#
> +our(
> +	$VERSION,
> +	$path_info,
> +	$GIT,
> +	$projectroot,
> +	$project_maxdepth,
> +	$home_link,
> +	$home_link_str,
> +	$site_name,
> +	$site_header,
> +	$home_text,
> +	$site_footer,
> +	@stylesheets,
> +	$stylesheet,
> +	$logo,
> +	$favicon,
> +	$javascript,
> +	$logo_url,
> +	$logo_label,
> +	$projects_list,
> +	$projects_list_description_width,
> +	$default_projects_order,
> +	$export_ok,
> +	$export_auth_hook,
> +	$strict_export,
> +	@git_base_url_list,
> +	$default_blob_plain_mimetype,
> +	$default_text_plain_charset,
> +	$mimetypes_file,
> +	$missmatch_git,
> +	$gitlinkurl,
> +	$maxload,
> +	$cache_enable,
> +	$minCacheTime,
> +	$maxCacheTime,
> +	$cachedir,
> +	$backgroundCache,
> +	$nocachedata,
> +	$nocachedatabin,
> +	$fullhashpath,
> +	$fullhashbinpath,
> +	$export_auth_hook,
> +	%known_snapshot_format_aliases,
> +	%known_snapshot_formats,
> +	$path_info,
> +	$fallback_encoding,
> +	%avatar_size,
> +	$project_maxdepth,
> +	$headerRefresh,
> +	$base_url,
> +	$projects_list_description_width,
> +	$default_projects_order,
> +	$prevent_xss,
> +	@diff_opts,
> +	%feature
>  );

Why this block is required? Why not have variables defined (using "our") in gitweb_defaults file?

[cut deletion]
Show 20 quoted lines
> +do 'gitweb_defaults.pl';
>  
>  sub gitweb_get_feature {
>  	my ($name) = @_;
> diff --git a/gitweb/gitweb_defaults.perl b/gitweb/gitweb_defaults.perl
> new file mode 100644
> index 0000000..ede0daf
> --- /dev/null
> +++ b/gitweb/gitweb_defaults.perl
> @@ -0,0 +1,468 @@
> +# gitweb - simple web interface to track changes in git repositories
> +#
> +# (C) 2005-2006, Kay Sievers <kay.sievers@vrfy.org>
> +# (C) 2005, Christian Gierke
> +#
> +# This program is licensed under the GPLv2
> +
> +# Base URL for relative URLs in gitweb ($logo, $favicon, ...),
> +# needed and used only for URLs with nonempty PATH_INFO
> +$base_url = $my_url;
Why not "our $base_url = $my_url;"?
[cut]
> +1;
-- 
Jakub Narebski
Poland
ShadeHawk on #git
Previous: John 'Warthog9' HawleyNext: J.H.
Message 30 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.