2010/5/22 Jakub Narebski <jnareb@xxxxxxxxx>: > 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@xxxxxxxxx> > > 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 <jnareb@xxxxxxxxx> > > _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... 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. >> 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. Nope. I didn't check it. But you are right. > >> 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? Yes. But I included it in this commit because, it is not a big change to be included in another commit. >> @@ -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 > Thanks -Pavan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html