Re: Bug? "git branch" failing to list all branches

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 3, 2010, 06:55 UTC
Message-ID: <20100603065527.GA23439@progeny.tock>
In-Reply-To: <AANLkTinCNgsIf_MsiysfphKm3MCW2Uio_xiemfDZraH7@mail.gmail.com>

Simo Melenius wrote:

Show 6 quoted lines

> My change doesn't change the current behaviour. At least git 1.7.0.4
> didn't give a nonzero exit status either. It would be good if it did.
> 
> I could add that to my patch. I'm, however, unsure of what's the best
> way to communicate the error from append_ref() to cmd_branch(). A
> static variable in branch.c would of course do.

I see.  You can use the cb_data argument to pass a result back:

 struct append_ref_cb {
 	struct ref_list *ref_list;
 	int err;
 };

A static variable might be simpler.

But that is a separate topic (for a separate patch). I can pick it up if you don’t.

Show 11 quoted lines

>> If I have 37 branches and an error is encountered looking up one of
>> them, with this patch the error message will scroll off the screen.
>> Is this worth worrying about?  It depends on what the usual causes for
>> broken branch refs are and whether they require attention or can be
>> safely ignored.
>
> Since this only concerns the printing of branches, often for
> interactive viewing or bash completion, and does not affect any of the
> operations that modify the repository, I think it's sufficient that
> the error message is still readable from stderr for those who are
> interested.

Sorry, I must have been unclear.  Let me illustrate with an example:

 $ git branch
 * (no branch)
   cc/sequencer-rebase-i
   db/svn-fe
 error: branch 'dk/hash' does not point at a commit.
   gp/debian-pu
   gp/sid-patches
   jl/gitk-submodule
   jk/pull-rebase-message
   jn/debian-build-depends
   js/grep-open
   ks/gitk-notes
   nd/gitbox
   nd/setup
   rr/svn-remote
   sb/sequencer-dev
   sb/sequencer-rfc
 ...

If this scrolls on for too long, I will not see the message. Now if the error is due to some kind of corruption or a broken script, I would want to know about it right away, even if I can carry on with my work without. So in that case, it would make sense to add at the end:

 fatal: some refs could not be read.

On the other hand, maybe there is some harmless process that often creates these broken refs and such a message would be a nuisance.

Hoping that is clearer, Jonathan