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

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

[Pavan Kumar Sunkara]

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

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

[Jakub Narebski]

On Sun, 23 May 2010, Pavan Kumar Sunkara wrote:

> 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

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

[Eric Wong]

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

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

[Pavan Kumar Sunkara]

On Sun, May 23, 2010 at 3:26 PM, Eric Wong <normalperson@yhbt.net> wrote:
> 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
>

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

[Christian Couder]

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.

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.

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

[Pavan Kumar Sunkara]

On Mon, May 24, 2010 at 3:25 PM, Christian Couder
<chriscool@tuxfamily.org> wrote:
> 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.

> 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

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

[Jakub Narebski]

On Mon, 24 May 2010, Christian Couder wrote:

> 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