threads / discuss / 23881

[PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

Subject: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

## tl;dr

7 messages between May 23, 2010 and May 25, 2010.

replies: 6people: 4as markdown or json

Pavan Kumar Sunkara· May 23, 2010, 07:32 UTC · lore

git-instaweb in its current form (re)creates gitweb.cgi and (some of) required static files in $GIT_DIR/gitweb/ directory. Splitting gitweb would make it difficult for git-instaweb to continue with this method.

Use the instaweb.gitwebdir config variable to point git-instaweb script to a global directory which contains gitweb files as server root and the httpd.conf along with server logs and pid go into '$(GIT_DIR)/gitweb' directory.

While at it, change apache2 configuration to use the same access log and error log files as the rest of web servers supported by git-instaweb.

Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
---
This patch is based on 'jn/gitweb-install' in the next branch by Jakub Nareski
and my previous 2 patches:
* gitweb: Move static files into seperate subdirectory
 http://article.gmane.org/gmane.comp.version-control.git/147321
* gitweb: Set default destination directory for installing gitweb in Makefile
 http://article.gmane.org/gmane.comp.version-control.git/147160
 Makefile        |   11 ++------
 git-instaweb.sh |   71 ++++++++++++++++++++----------------------------------
 2 files changed, 29 insertions(+), 53 deletions(-)
diff --git a/Makefile b/Makefile
index caf2f64..9161172 100644
--- a/Makefile
+++ b/Makefile
@@ -1426,6 +1426,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_SQ = $(subst ','\'',$(htmldir))
 prefix_SQ = $(subst ','\'',$(prefix))
+gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
@@ -1592,15 +1593,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/static/gitweb.css gitweb/
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-	    -e '/@@GITWEB_CGI@@/r gitweb/gitweb.cgi' \
-	    -e '/@@GITWEB_CGI@@/d' \
-	    -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
-	    -e '/@@GITWEB_CSS@@/d' \
-	    -e '/@@GITWEB_JS@@/r $(GITWEB_JS)' \
-	    -e '/@@GITWEB_JS@@/d' \
+	    -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
-            -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS)|' \
-            -e 's|@@GITWEB_JS_NAME@@|$(GITWEB_JS)|' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
@@ -1972,6 +1966,7 @@ install: all
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
+	$(MAKE) -C gitweb gitwebdir=$(gitwebdir_SQ) install
 endif
 ifndef NO_PYTHON
 	$(MAKE) -C git_remote_helpers prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
diff --git a/git-instaweb.sh b/git-instaweb.sh
index f608014..b3e9192 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -24,6 +24,7 @@ restart        restart the web server
 fqgitdir="$GIT_DIR"
 local="$(git config --bool --get instaweb.local)"
 httpd="$(git config --get instaweb.httpd)"
+root="$(git config --get instaweb.gitwebdir)"
 port=$(git config --get instaweb.port)
 module_path="$(git config --get instaweb.modulepath)"
 
@@ -34,6 +35,9 @@ conf="$GIT_DIR/gitweb/httpd.conf"
 # if installed, it doesn't need further configuration (module_path)
 test -z "$httpd" && httpd='lighttpd -f'
 
+# Default is @@GITWEBDIR@@
+test -z "$root" && root='@@GITWEBDIR@@'
+
 # any untaken local port will do...
 test -z "$port" && port=1234
 
@@ -57,7 +61,7 @@ resolve_full_httpd () {
 		# these days and those are not in most users $PATHs
 		# in addition, we may have generated a server script
 		# in $fqgitdir/gitweb.
-		for i in /usr/local/sbin /usr/sbin "$fqgitdir/gitweb"
+		for i in /usr/local/sbin /usr/sbin "$root" "$fqgitdir/gitweb"
 		do
 			if test -x "$i/$httpd_only"
 			then
@@ -159,8 +163,8 @@ done
 mkdir -p "$GIT_DIR/gitweb/tmp"
 GIT_EXEC_PATH="$(git --exec-path)"
 GIT_DIR="$fqgitdir"
-export GIT_EXEC_PATH GIT_DIR
-
+GITWEB_CONFIG="$fqgitdir/gitweb/gitweb_config.perl"
+export GIT_EXEC_PATH GIT_DIR GITWEB_CONFIG
 
 webrick_conf () {
 	# generate a standalone server script in $fqgitdir/gitweb.
@@ -192,7 +196,7 @@ EOF
 
 	cat >"$conf" <<EOF
 :Port: $port
-:DocumentRoot: "$fqgitdir/gitweb"
+:DocumentRoot: "$root"
 :DirectoryIndex: ["gitweb.cgi"]
 :PidFile: "$fqgitdir/pid"
 EOF
@@ -201,7 +205,7 @@ EOF
 
 lighttpd_conf () {
 	cat > "$conf" <<EOF
-server.document-root = "$fqgitdir/gitweb"
+server.document-root = "$root"
 server.port = $port
 server.modules = ( "mod_setenv", "mod_cgi" )
 server.indexfiles = ( "gitweb.cgi" )
@@ -212,7 +216,7 @@ server.errorlog = "$fqgitdir/gitweb/error.log"
 # variable above and uncomment this
 #accesslog.filename = "$fqgitdir/gitweb/access.log"
 
-setenv.add-environment = ( "PATH" => env.PATH )
+setenv.add-environment = ( "PATH" => env.PATH, "GITWEB_CONFIG" => env.GITWEB_CONFIG )
 
 cgi.assign = ( ".cgi" => "" )
 
@@ -277,14 +281,15 @@ EOF
 
 apache2_conf () {
 	test -z "$module_path" && module_path=/usr/lib/apache2/modules
-	mkdir -p "$GIT_DIR/gitweb/logs"
 	bind=
 	test x"$local" = xtrue && bind='127.0.0.1:'
 	echo 'text/css css' > "$fqgitdir/mime.types"
 	cat > "$conf" <<EOF
 ServerName "git-instaweb"
-ServerRoot "$fqgitdir/gitweb"
-DocumentRoot "$fqgitdir/gitweb"
+ServerRoot "$root"
+DocumentRoot "$root"
+ErrorLog "$fqgitdir/gitweb/error.log"
+CustomLog "$fqgitdir/gitweb/access.log" combined
 PidFile "$fqgitdir/pid"
 Listen $bind$port
 EOF
@@ -303,13 +308,14 @@ EOF
 	# check to see if Dennis Stosberg's mod_perl compatibility patch
 	# (<20060621130708.Gcbc6e5c@leonov.stosberg.net>) has been applied
 	if test -f "$module_path/mod_perl.so" &&
-	   sane_grep 'MOD_PERL' "$GIT_DIR/gitweb/gitweb.cgi" >/dev/null
+	   sane_grep 'MOD_PERL' "$root/gitweb.cgi" >/dev/null
 	then
 		# favor mod_perl if available
 		cat >> "$conf" <<EOF
 LoadModule perl_module $module_path/mod_perl.so
 PerlPassEnv GIT_DIR
 PerlPassEnv GIT_EXEC_DIR
+PerlPassEnv GITWEB_CONFIG
 <Location /gitweb.cgi>
 	SetHandler perl-script
 	PerlResponseHandler ModPerl::Registry
@@ -353,7 +359,7 @@ mongoose_conf() {
 # For detailed description of every option, visit
 # http://code.google.com/p/mongoose/wiki/MongooseManual
 
-root		$fqgitdir/gitweb
+root		$root
 ports		$port
 index_files	gitweb.cgi
 #ssl_cert	$fqgitdir/gitweb/ssl_cert.pem
@@ -361,7 +367,7 @@ error_log	$fqgitdir/gitweb/error.log
 access_log	$fqgitdir/gitweb/access.log
 
 #cgi setup
-cgi_env		PATH=$PATH,GIT_DIR=$GIT_DIR,GIT_EXEC_PATH=$GIT_EXEC_PATH
+cgi_env		PATH=$PATH,GIT_DIR=$GIT_DIR,GIT_EXEC_PATH=$GIT_EXEC_PATH,GITWEB_CONFIG=$GITWEB_CONFIG
 cgi_interp	$PERL
 cgi_ext		cgi,pl
 
@@ -370,41 +376,16 @@ mime_types	.gz=application/x-gzip,.tar.gz=application/x-tgz,.tgz=application/x-t
 EOF
 }
 
-
-script='
-s#^(my|our) \$projectroot =.*#$1 \$projectroot = "'$(dirname "$fqgitdir")'";#;
-s#(my|our) \$gitbin =.*#$1 \$gitbin = "'$GIT_EXEC_PATH'";#;
-s#(my|our) \$projects_list =.*#$1 \$projects_list = \$projectroot;#;
-s#(my|our) \$git_temp =.*#$1 \$git_temp = "'$fqgitdir/gitweb/tmp'";#;'
-
-gitweb_cgi () {
-	cat > "$1.tmp" <<\EOFGITWEB
-@@GITWEB_CGI@@
-EOFGITWEB
-	# Use the configured full path to perl to match the generated
-	# scripts' 'hashpling' line
-	"$PERL" -p -e "$script" "$1.tmp"  > "$1"
-	chmod +x "$1"
-	rm -f "$1.tmp"
-}
-
-gitweb_css () {
-	cat > "$1" <<\EOFGITWEB
-@@GITWEB_CSS@@
-
-EOFGITWEB
-}
-
-gitweb_js () {
-	cat > "$1" <<\EOFGITWEB
-@@GITWEB_JS@@
-
-EOFGITWEB
+gitweb_conf() {
+	cat > "$fqgitdir/gitweb/gitweb_config.perl" <<EOF
+#!/usr/bin/perl
+our \$projectroot = "$(dirname "$fqgitdir")";
+our \$git_temp = "$fqgitdir/gitweb/tmp";
+our \$projects_list = \$projectroot;
+EOF
 }
 
-gitweb_cgi "$GIT_DIR/gitweb/gitweb.cgi"
-gitweb_css "$GIT_DIR/@@GITWEB_CSS_NAME@@"
-gitweb_js  "$GIT_DIR/@@GITWEB_JS_NAME@@"
+gitweb_conf
 
 case "$httpd" in
 *lighttpd*)
-- 
1.7.1.18.g74211d.dirty
Jakub Narebski· May 23, 2010, 09:54 UTC · re: Pavan Kumar Sunkara · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

On Sun, 23 May 2010, Pavan Kumar Sunkara wrote:
Show 15 quoted lines
> git-instaweb in its current form (re)creates gitweb.cgi and
> (some of) required static files in $GIT_DIR/gitweb/ directory.
> Splitting gitweb would make it difficult for git-instaweb to
> continue with this method.
> 
> Use the instaweb.gitwebdir config variable to point git-instaweb script
> to a global directory which contains gitweb files as server root
> and the httpd.conf along with server logs and pid go into
> '$(GIT_DIR)/gitweb' directory.
> 
> While at it, change apache2 configuration to use the same access log
> and error log files as the rest of web servers supported by
> git-instaweb.
> 
> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
For what it is worth:
Acked-by: Jakub Narebski <jnareb@gmail.com>

But I think more important would be ACK from Eric Wong, author and maintainer of git-instaweb. I guess that because it is preparatory work, only related to the main stated goal of your GSoC 2010, you don't need "signoff" from mentor(s) (in the form of Mentored-by:)...

-- 
Jakub Narebski
Poland
Eric Wong· May 23, 2010, 09:56 UTC · re: Pavan Kumar Sunkara · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

Pavan Kumar Sunkara <pavan.sss1991@gmail.com> wrote:
> While at it, change apache2 configuration to use the same access log
> and error log files as the rest of web servers supported by
> git-instaweb.

Hi Pavan, I'm late to this thread, but I'm leaning against forcing access logs onto people. I don't believe access logs are necessary for instaweb. Error logs are important for debugging breakage, yes, but access logs aren't worth the trouble IMHO.

Also, using the same error log paths might make it confusing to people switching between web servers. In fact, it may be the most confusing for those folks if they're trying different web servers and forgetting to shut down the previous server before starting the next. They could end up seeing confusing log messages from the wrong server if they look in the error log to debug the problem.

I'm not sure why Mongoose has access logs enabled by default, but I know I conciously left it out by default for lighttpd/apache/webrick.

-- 
Eric Wong
Pavan Kumar Sunkara· May 23, 2010, 10:18 UTC · re: Eric Wong · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

On Sun, May 23, 2010 at 3:26 PM, Eric Wong <normalperson@yhbt.net> wrote:
Show 22 quoted lines
> Pavan Kumar Sunkara <pavan.sss1991@gmail.com> wrote:
>> While at it, change apache2 configuration to use the same access log
>> and error log files as the rest of web servers supported by
>> git-instaweb.
>
> Hi Pavan, I'm late to this thread, but I'm leaning against forcing
> access logs onto people.  I don't believe access logs are necessary for
> instaweb.  Error logs are important for debugging breakage, yes, but
> access logs aren't worth the trouble IMHO.
>
> Also, using the same error log paths might make it confusing to people
> switching between web servers.  In fact, it may be the most confusing
> for those folks if they're trying different web servers and forgetting
> to shut down the previous server before starting the next.  They could
> end up seeing confusing log messages from the wrong server if they
> look in the error log to debug the problem.
>
> I'm not sure why Mongoose has access logs enabled by default, but I know
> I conciously left it out by default for lighttpd/apache/webrick.
>
> --
> Eric Wong

Well, Until before this patch, lighttpd and mongoose have been using the same error log and access log. So, I enabled it for apache2 too in the process of debugging git-instaweb while using apache2.

IMVHO If there is no problem using the same logs for lighttpd and mongoose until now, I don't think it will be a problem when apache2 is included into that group.

- Pavan
>
Christian Couder· May 24, 2010, 09:55 UTC · re: Pavan Kumar Sunkara · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

Hi Pavan,
On Sunday 23 May 2010 09:32:48 Pavan Kumar Sunkara wrote:
Show 7 quoted lines
> 
> This patch is based on 'jn/gitweb-install' in the next branch by Jakub
>  Nareski and my previous 2 patches:
> * gitweb: Move static files into seperate subdirectory
>  http://article.gmane.org/gmane.comp.version-control.git/147321
> * gitweb: Set default destination directory for installing gitweb in
>  Makefile http://article.gmane.org/gmane.comp.version-control.git/147160

I tried to apply your 2 patches on top of next I got some errors with the "Move static files into seperate subdirectory" patch.

Could you resend the 3 patches in a patch series after making sure that they apply on top of next?

And I think that it would be better if the web server logs would be in a directory with the web server name. For example "$fqgitdir/gitweb/$httpd/access.log" instead of "$fqgitdir/gitweb/access.log". But it would be nice if you make these changes (as well as the apache2 confguration changes) in another patch.

Thanks in advance, Christian.

Pavan Kumar Sunkara· May 24, 2010, 15:14 UTC · re: Christian Couder · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

On Mon, May 24, 2010 at 3:25 PM, Christian Couder <chriscool@tuxfamily.org> wrote:

Show 13 quoted lines
> Hi Pavan,
>
> On Sunday 23 May 2010 09:32:48 Pavan Kumar Sunkara wrote:
>>
>> This patch is based on 'jn/gitweb-install' in the next branch by Jakub
>>  Nareski and my previous 2 patches:
>> * gitweb: Move static files into seperate subdirectory
>>  http://article.gmane.org/gmane.comp.version-control.git/147321
>> * gitweb: Set default destination directory for installing gitweb in
>>  Makefile http://article.gmane.org/gmane.comp.version-control.git/147160
>
> I tried to apply your 2 patches on top of next I got some errors with the
> "Move static files into seperate subdirectory" patch.

It is a small error in Makefile only with this patch. The others are ok. I teseted it with alt-git.git repository's next branch. So, I will be sending only that patch once again.

Show 9 quoted lines
> Could you resend the 3 patches in a patch series after making sure that they
> apply on top of next?
>
> And I think that it would be better if the web server logs would be in a
> directory with the web server name. For example
> "$fqgitdir/gitweb/$httpd/access.log" instead of "$fqgitdir/gitweb/access.log".
> But it would be nice if you make these changes (as well as the apache2
> confguration changes) in another patch.
>
Yeah. Sure.
- Pavan
Jakub Narebski· May 25, 2010, 01:34 UTC · re: Christian Couder · lore

Re: [PATCHv3 GSoC] git-instaweb: Configure it to work with new gitweb structure

On Mon, 24 May 2010, Christian Couder wrote:
Show 5 quoted lines
> And I think that it would be better if the web server logs would be in a 
> directory with the web server name. For example 
> "$fqgitdir/gitweb/$httpd/access.log" instead of "$fqgitdir/gitweb/access.log". 
> But it would be nice if you make these changes (as well as the apache2 
> configuration changes) in another patch.

You probably meant "$fqgitdir/gitweb/$httpd_only/access.log", and not "$fqgitdir/gitweb/$httpd/access.log" ($httpd_only in place of $httpd), because $httpd might be 'apache2 -f' in the case of 'apache2', or in the future "$fqgitdir/gitweb/gitweb.psgi" in the case of 'plackup'.

Just FYI
-- 
Jakub Narebski
Poland

← back to recent threads