Re: [PATCH] gitweb: Silence stderr in parse_commit*() subroutines

[Jakub Narebski <jnareb@gmail.com>]

On Mon, 13 Feb 2012, rajesh boyapati wrote:
> 2012/2/13 Jakub Narebski <jnareb@gmail.com>
>> On Mon, 13 Feb 2012, rajesh boyapati wrote:


>>> This is the patch I applied
>>>>>>>>>>>>>>>
>>> sub parse_commit {
>>>     my ($commit_id) = @_;
>>>     my %co;
>>>
>>>         return unless defined $commit_id;
>>>
>>>     local $/ = "\0";
>>>
>>>     *open my $fd, "-|", quote_command(
>>>              git_cmd(), "rev-list",*
>>>         "--parents",
>>>         "--header",
>>>         "--max-count=1",
>>>         $commit_id,
>>>        * "--") . '2>/dev/null',*
>>                   ^^^^^^^^^^^^^
>>
>> It should be ' 2>/dev/null', with space before redirection, and not
>> '2>/dev/null'.  This space is here necessary.
> 
> 
> Sorry for the typo error. I changed that and I am now seeing this error in
> Gerrit's error_log
> <<<<<<<<<<<<<
> [2012-02-13 13:45:35,201] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: Not a valid
> object name HEAD
> >>>>>>>>>>>>>

WTF, redirecting stderr (file descriptor number 2) to /dev/null should have
silenced git-rev-list in case of errors.

Compare

  $ git rev-list foo --
  fatal: bad revision 'foo'

with

  $ git rev-list foo -- 2>/dev/null

which gives no output.

I wonder what shell does gitweb use if run from com.google.gerrit.httpd
as servlet...

[...]
>> Many views in gitweb do default to HEAD.  If HEAD does not point to a valid
>> commit, they would fail, in better or worse way.
>>
>> Except for the first one those patches are more of band-aid and workaround
>> than fixing underlying issue that gitweb assumes that HEAD is valid in
>> non-empty repository.  But fixing this will require more work.
>>
> Yes, I agree with you.
>
> If the HEAD's in a git project are pointed to master branch, those are fine
> with Gerrits gitweb. Everything is working fine. (See the images in attached
> file "Master.zip")
> 
> For the HEAD's in the git projects that are pointed to a branch other than
> master (I mean if master branch is empty), I have the problem when I click
> on tabs (log, shortlog, commit, commitdiff, tree) after clicking "summary"
> tab.  (See the images in attched file "Unborn-branch.zip").

That is because those views defaults to HEAD, which doesn't point to
a valid commit because 'master' branch it does point to doesn't have any
commits on it.  They should probably be disabled (grayed-out, and made
ordinary text and not hyperlink) if HEAD is invalid.

But this would require more work than those patches.
>
> If I click other tabs before clicking "summary" tab, they are working fine.

That is because if you select some branch, then all those views use
currently selected branch (passed via URL, e.g. 'h' or 'hb' parameter
in case of query-params URL).

-- 
Jakub Narebski
Poland