git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output

From
Jakub Narebski <jnareb@gmail.com>
Date
Jun 14, 2008, 18:07 UTC
Message-ID
<200806142007.35288.jnareb@gmail.com>
In-Reply-To
<4853D84D.5010303@gmail.com>
Lea Wiemann wrote:
Show 10 quoted lines
> Jakub Narebski wrote:
> >
> > NOTE: Currently test_external_without_stderr fails because when trying
> > to access URL for non-existent commit gitweb writes to STDERR; it is
> > not necessarily a bug because it is not written to web server logs
> 
> Without having looked at the cause of that, I think that gitweb should 
> not be writing stuff to stderr unless an internal or serious error 
> occurs; in particular trying to access invalid commits shouldn't cause 
> messages on stderr, only to log files if at all.
Actually it isn't gitweb (or Perl) writing to stderr, but git itself.
Somehow, at least for gitweb run as CGI script (and under legacy
mod_perl) with Apache 2 as web server this error message:
  fatal: bad revision 'non-existent'
doesn't land in web server logs (/var/log/httpd/error_log).  So it lands
in /dev/null when running gitweb as web script, so it was deemed not
important (also fixing this is not very easy, as you can read below).

t/t9500-gitweb-standalone-no-errors.sh considers as errors only those error message which would make it into web server logs, see gitweb_run() function there. This catches compilation errors.

Fixing this is not that simple. There is no option to git-rev-list to not write any output to stderr (no, '--quiet' is about something else), and I'd rather not lose all advantages of list (shell-less) form of magical "|-" open only for "2>/dev/null" redirection as in git_object subroutine in gitweb.

Perhaps it could be solved in Git.pm, and when gitweb is rewritten to use "use Git" (and global $repo object instead of global $git_dir variable) it would automatically fix it (using Git.pm would have the advantage of making gitweb more portable, I think - to ActiveState broken Perl implementation, with broken magic "|-" open).

Or git-rev-list, or even git wrapper itself, could acquire option to redirect all stderr to dev null... I think adding it in git wrapper would be even better; simply change "warning" and "die" to null functions (I was even thinking about doing that...).

Show 13 quoted lines
> That said, as long as it isn't fixed, here's my workaround to 
> temporarily discard stderr (from my t/t9710/test.pl):
> 
> our $old_stderr;
> sub discard_stderr {
>          open our $old_stderr, ">&", STDERR or die "cannot save STDERR";
>          close STDERR;
> }
> sub restore_stderr {
>          open STDERR, ">&", $old_stderr or die "cannot restore STDERR";
> }
> 
> It works on Unix, but I don't know about other platforms.
Thanks.
Show 6 quoted lines
> > +cat >gitweb_config.perl <<EOF
> > [...]
> > +our \$GIT = "git";
> 
> t9500 seems to be doing the same(?) thing, but this somehow doesn't work 
> with your t9503 test:

It should work. test-lib.sh sets up $PATH to have 'git' binary (just compiled git binary) in it...

Show 8 quoted lines
> $ git     # no git in PATH to make sure it picks up the right git binary
> bash: git: command not found
> $ ./t9500-gitweb-standalone-no-errors.sh | grep passed
> * passed all 75 test(s)
> $ ./t9503-gitweb-Mechanize.sh -v
> [...]
> 	gitweb.perl: Can't exec "git": No such file or directory at 
> /home/lea/source/git/fresh-git/gitweb/gitweb.perl line 380.

...and it would be very strange for t9500 to pass, but t9503 do not pass. (Of course both tests passes at my computer, otherwise I wouldn't send this patch in current form).

Hmmm... perhaps $PATH doesn't get passed down... strange.

But thanks to your report I have found bug in gitweb. I have changed t/t9503-gitweb-Mechanize.sh... 

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
index 5df22c4..a5be275 100755
--- a/t/t9503-gitweb-Mechanize.sh
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -92,7 +92,7 @@ cat >gitweb_config.perl <<EOF
 # gitweb configuration for tests
 
 our \$version = "current";
-our \$GIT = "git";
+our \$GIT = "$safe_pwd/../../git";
 our \$projectroot = "$safe_pwd";
 our \$project_maxdepth = 8;
 our \$home_link_str = "projects";

...and found out that gitweb doesn't like when $GIT contains spaces
in _one_ place: finding git version.  It should be
 
 our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";

with $GIT quoted.  (Patch will be send shortly).


So better solution would be

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
index 5df22c4..a5be275 100755
--- a/t/t9503-gitweb-Mechanize.sh
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -92,7 +92,7 @@ cat >gitweb_config.perl <<EOF
 # gitweb configuration for tests
 
 our \$version = "current";
-our \$GIT = "git";
+our \$GIT = "$GIT_EXEC_PATH/git";
 our \$projectroot = "$safe_pwd";
 our \$project_maxdepth = 8;
 our \$home_link_str = "projects";


Does it works for you?

-- 
Jakub Narebski
Poland
Previous: Lea WiemannNext: Lea Wiemann
Message 3 of 41 in “gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output”
  1. gitweb: Use Test::WWW::Mechanize::CGI to test gitweb outputJakub Narebski, Jun 14, 2008
  2. Lea WiemannJun 14, 2008
  3. Jakub NarebskiJun 14, 2008
  4. Lea WiemannJun 14, 2008
  5. Jakub NarebskiJun 14, 2008
  6. Lea WiemannJun 14, 2008
  7. Jakub NarebskiJun 15, 2008
  8. Lea WiemannJun 14, 2008
  9. Jakub NarebskiJun 14, 2008
  10. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 14, 2008
  11. Jakub NarebskiJun 15, 2008
  12. Lea WiemannJun 15, 2008
  13. Jakub NarebskiJun 16, 2008
  14. Lea WiemannJun 16, 2008
  15. Jakub NarebskiJun 16, 2008
  16. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 20, 2008
  17. Jakub NarebskiJun 20, 2008
  18. Lea WiemannJun 20, 2008
  19. Jakub NarebskiJun 20, 2008
  20. Lea WiemannJun 20, 2008
  21. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 20, 2008
  22. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 23, 2008
  23. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 23, 2008
  24. Junio C HamanoJun 23, 2008
  25. Lea WiemannJun 23, 2008
  26. Jakub NarebskiJun 23, 2008
  27. Lea WiemannJun 23, 2008
  28. Jakub NarebskiJun 23, 2008
  29. Lea WiemannJun 24, 2008
  30. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 24, 2008
  31. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 26, 2008
  32. gitweb: add test suite with Test::WWW::Mechanize::CGILea Wiemann, Jun 26, 2008
  33. Jakub NarebskiJun 29, 2008
  34. Lea WiemannJun 29, 2008
  35. Jakub NarebskiJun 29, 2008
  36. Lea WiemannJun 30, 2008
  37. Jakub NarebskiJun 30, 2008
  38. Junio C HamanoJun 24, 2008
  39. Lea WiemannJun 24, 2008
  40. Jakub NarebskiJun 24, 2008
  41. Jakub NarebskiJun 30, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.