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

> 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 <pavan.sss1991@gmail.com>
>
> 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 <jnareb@gmail.com>
>
> _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...

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

>
>> 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.
>
>> @@ -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".
>
>>  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
>
>> @@ -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
> [...]
>> -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.
>
>> @@ -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?

>> @@ -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 () {
> [...]
>
>> +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
>