Re: problem using jgit

From: Marek Zawirski <marek.zawirski@gmail.com>
Date: Jul 25, 2008, 14:51 UTC
Message-ID: <4889E88E.8000701@gmail.com>
In-Reply-To: <20080722165831.GA11173@spearce.org>

Shawn O. Pearce wrote:

Show 39 quoted lines

> Marek Zawirski <marek.zawirski@gmail.com> wrote:
>   
>> Marek Zawirski wrote:
>>     
>>> Stephen Bannasch wrote:
>>>       
>>>> I've setup a simple test class that integrates jgit to clone a git  
>>>> repository. However I'm getting a NullPointerError when  
>>>> RevWalk.parseAny ends up producing a null object id.
>>>>         
> ...
>   
>> It's caused by 14a630c3: Cached modification times for symbolic refs too
>> Changes introduced by this patch made Repository#getAllRefs() including  
>> Ref objects with null ObjectId in case of unresolvable (invalid?) HEAD  
>> symbolic ref, and null Ref for HEAD  when it doesn't exist. Previous  
>> behavior was just not including such refs in result.
>>     
>
> My intention here was that if a ref cannot be resolved, it should
> not be reported.  So Ref.getObjectId should never return null, and
> it should also never return an ObjectId for which the object does
> not exist in the Repository's object database(s).  (Though that can
> happen in the face of repository corruption, but lets not go there
> just yet).
>
> So IMHO the RefDatabase code is _wrong_ for returning HEAD with a
> null objectId.
>
> Now this case can happen if HEAD points at a stillborn branch.  This
> is easily reproduced in any repository, e.g. just do:
>
> 	git symbolic-ref HEAD refs/heads/`date`
>
> You'll wind up on a branch which doesn't exist.  In this case HEAD
> shouldn't be reported back from RefDatabase, it doesn't exist, as
> branch `date` does not exist either.
>
>

Beside of my temporary fix for that that filters null Ref and Ref with null objectId, I think that 2 more issues may need to be resolved:

1) readRefBasic() method is used for reading arbitrary refs, potentially 
not only those from well-known prefixes as readRefs() does. Is calling 
setModified()  appropriate for those other refs?

2) Am I wrong that setModified() is not called in all cases? Consider 
empty ref file and just...
if (line == null || line.length() == 0)
            return new Ref(Ref.Storage.LOOSE, name, null);