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

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.

Signed-off-by: Pavan Kumar Sunkara <redacted>
---

This patch is based on commit 'jn/gitweb-install' in the next branch.

 Makefile        |   10 +------
 git-instaweb.sh |   71 ++++++++++++++++++++----------------------------------
 2 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/Makefile b/Makefile
index caf2f64..91cd726 100644
--- a/Makefile
+++ b/Makefile

@@ -1592,15 +1592,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)|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 +1965,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) 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

On Sat, 22 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.

That's not all this patch changes, isn't it?

  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.

While it would be better to have it as a separate commit, I think it
doesn't matter much, and having it in this patch is acceptable as
"while at it" change.

Signed-off-by: Pavan Kumar Sunkara <redacted>

I like this change, because it simplifies greatly git-instaweb
generation; besides the fact that it is necessary for future splitting
gitweb for better maintability, and for write functionality for gitweb.


Acked-by: Jakub Narebski <redacted>

_If_ there is no problem with $(gitwebdir) and not $(gitwebdir_SQ),
see below for details.

---

This patch is based on commit 'jn/gitweb-install' in the next branch.

I think it is based on your earlier 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

Those are necessary for this patch to be applied.  Well, the second is
necessary for it to make sense...


BTW. which web servers supported by git-instaweb: lighttpd, apache2,
webrick, mongoose you have tested your change with?

[...]

Have you checked that you can use $(gitwebdir) and don't need
$(gitwebdir_SQ) here?  Does git-instaweb installs and works correctly
if 'gitwebdir' contains spaces and single or double quote characters?

But perhaps that doesn't matter in practice, and this is good enough.

Nice.

You probably want to update comment above this loop.  But this is not
something very important.

Alternatively, if e.g. webrick.rb and webrick (shell script) are
installed in "$fqgitdir/gitweb" directory, there is no need to check
"$root".

 webrick_conf () {

[...]

-:DocumentRoot: "$fqgitdir/gitweb"
+:DocumentRoot: "$root"

 lighttpd_conf () {
 	cat > "$conf" <<EOF
-server.document-root = "$fqgitdir/gitweb"
+server.document-root = "$root"

[...]

-setenv.add-environment = ( "PATH" => env.PATH )
+setenv.add-environment = ( "PATH" => env.PATH, "GITWEB_CONFIG" => env.GITWEB_CONFIG )

 apache2_conf () {

[...]

-ServerRoot "$fqgitdir/gitweb"
-DocumentRoot "$fqgitdir/gitweb"
+ServerRoot "$root"
+DocumentRoot "$root"

[...]

 PerlPassEnv GIT_DIR
 PerlPassEnv GIT_EXEC_DIR
+PerlPassEnv GITWEB_CONFIG

[...]

-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

All right, those changes are pretty clear.

This is independent change, modifying configuration of apache2 web
server to use the same files for access log and for error log
("$fqgitdir/gitweb/access.log" and "$fqgitdir/gitweb/error.log",
respectively) as the rest of web servers.

Isn't it?

[...]

+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
 }

Right, $GIT (formerly $gitbin) is set when generating gitweb.cgi from
gitweb.perl.

Actually $git_temp is not needed by modern gitweb (from quite some time,
since using external /usr/bin/diff was replaced by git-diff-tree), so
setting it could be removed from gitweb_config.perl.  Nevertheless it is
not a problem having it.

-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
Poland

2010/5/22 Jakub Narebski [off-list ref]:

On Sat, 22 May 2010, Pavan Kumar Sunkara wrote:

quoted

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.

That's not all this patch changes, isn't it?

 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.

While it would be better to have it as a separate commit, I think it
doesn't matter much, and having it in this patch is acceptable as
"while at it" change.

quoted

Signed-off-by: Pavan Kumar Sunkara <redacted>

I like this change, because it simplifies greatly git-instaweb
generation; besides the fact that it is necessary for future splitting
gitweb for better maintability, and for write functionality for gitweb.

Thanks.

Acked-by: Jakub Narebski <redacted>

_If_ there is no problem with $(gitwebdir) and not $(gitwebdir_SQ),
see below for details.

quoted

---

This patch is based on commit 'jn/gitweb-install' in the next branch.

I think it is based on your earlier 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

Those are necessary for this patch to be applied.  Well, the second is
necessary for it to make sense...

Yes. They are necessary patches before this patch.
I assume, they are going to be merged before this patch is.

BTW. which web servers supported by git-instaweb: lighttpd, apache2,
webrick, mongoose you have tested your change with?

I tested this patch with lighttpd and apache2.
And am sure about mongoose. But don't know the status with webrick.

quoted

 Makefile        |   10 +------
 git-instaweb.sh |   71 ++++++++++++++++++++----------------------------------
 2 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/Makefile b/Makefile
index caf2f64..91cd726 100644
--- a/Makefile
+++ b/Makefile

@@ -1592,15 +1592,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/static/gitweb.css gitweb/

      sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \

[...]

quoted

+         -e 's|@@GITWEBDIR@@|$(gitwebdir)|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 +1965,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) install
 endif

Have you checked that you can use $(gitwebdir) and don't need
$(gitwebdir_SQ) here?  Does git-instaweb installs and works correctly
if 'gitwebdir' contains spaces and single or double quote characters?
But perhaps that doesn't matter in practice, and this is good enough.

Nope. I didn't check it. But you are right.

quoted

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@@'
+

Nice.

quoted

@@ -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

You probably want to update comment above this loop.  But this is not
something very important.

Alternatively, if e.g. webrick.rb and webrick (shell script) are
installed in "$fqgitdir/gitweb" directory, there is no need to check
"$root".

quoted

 webrick_conf () {

[...]

quoted

-:DocumentRoot: "$fqgitdir/gitweb"
+:DocumentRoot: "$root"

quoted

 lighttpd_conf () {
      cat > "$conf" <<EOF
-server.document-root = "$fqgitdir/gitweb"
+server.document-root = "$root"

[...]

quoted

-setenv.add-environment = ( "PATH" => env.PATH )
+setenv.add-environment = ( "PATH" => env.PATH, "GITWEB_CONFIG" => env.GITWEB_CONFIG )

quoted

 apache2_conf () {

[...]

quoted

-ServerRoot "$fqgitdir/gitweb"
-DocumentRoot "$fqgitdir/gitweb"
+ServerRoot "$root"
+DocumentRoot "$root"

[...]

quoted

 PerlPassEnv GIT_DIR
 PerlPassEnv GIT_EXEC_DIR
+PerlPassEnv GITWEB_CONFIG

quoted

@@ -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

[...]

quoted

-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

All right, those changes are pretty clear.

quoted

@@ -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

This is independent change, modifying configuration of apache2 web
server to use the same files for access log and for error log
("$fqgitdir/gitweb/access.log" and "$fqgitdir/gitweb/error.log",
respectively) as the rest of web servers.

Isn't it?

Yes. But I included it in this commit because, it is not a big change
to be included in another commit.

quoted

@@ -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 () {

[...]

quoted

+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
 }

Right, $GIT (formerly $gitbin) is set when generating gitweb.cgi from
gitweb.perl.

Actually $git_temp is not needed by modern gitweb (from quite some time,
since using external /usr/bin/diff was replaced by git-diff-tree), so
setting it could be removed from gitweb_config.perl.  Nevertheless it is
not a problem having it.

quoted

-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
Poland

Thanks
-Pavan

On Sat, 22 May 2010, Pavan Kumar Sunkara wrote:

2010/5/22 Jakub Narebski [off-list ref]:

quoted

On Sat, 22 May 2010, Pavan Kumar Sunkara wrote:

quoted

Acked-by: Jakub Narebski <redacted>

_If_ there is no problem with $(gitwebdir) and not $(gitwebdir_SQ),
see below for details.

[...]

quoted

quoted

diff --git a/Makefile b/Makefile
index caf2f64..91cd726 100644
--- a/Makefile
+++ b/Makefile

@@ -1592,15 +1592,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/static/gitweb.css gitweb/

      sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \

[...]

quoted

+         -e 's|@@GITWEBDIR@@|$(gitwebdir)|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 +1965,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) install
 endif

Have you checked that you can use $(gitwebdir) and don't need
$(gitwebdir_SQ) here?  Does git-instaweb installs and works correctly
if 'gitwebdir' contains spaces and single or double quote characters?
But perhaps that doesn't matter in practice, and this is good enough.

Nope. I didn't check it. But you are right.

Does it mean that after your patch git-instaweb works correctly if
'gitwebdir' is set to something like (for example):

  gitwebdir="/home/local/some strange \"path'"

-- 
Jakub Narebski
Poland

No. It doesn't.

Do you want me to make that change to and send the patch again ?

- Pavan

On Sat, 22 May 2010, Pavan Kumar Sunkara wrote:

Do you want me to make that change to and send the patch again ?

Yes, I would like you to.

And push this change to your repository after it is accepted as a final 
version. TIA.
-- 
Jakub Narebski
Poland