[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [jgit-dev] InMemoryRepository and HEAD reference
|
On Mon, Mar 18, 2013 at 3:06 PM, Alex Blewitt <alex.blewitt@xxxxxxxxx> wrote:
> On 18 Mar 2013, at 21:13, Shawn Pearce wrote:
>
>> On Mon, Mar 18, 2013 at 1:10 PM, Alex Blewitt <alex.blewitt@xxxxxxxxx> wrote:
>>> If I use the InMemoryRepository to set up a simple DFS based Git repository, I can push and clone over the Git protocol fairly easily with:
>>>
>>> Daemon daemon = new Daemon(new InetSocketAddress(9148));
>>> daemon.getService("git-receive-pack").setEnabled(true);
>>> daemon.setRepositoryResolver(…return new InMemoryRepository());
>>> daemon.start();
>>>
>>> However, when I clone from this repository I get a message:
>>>
>>> warning: remote HEAD refers to nonexistent ref, unable to checkout
>>>
>>> If I try to add HEAD as a symbolic reference to refs/master/head in the 'scanAllRefs' of the InMemoryRefDatabase, when I check out I get an error on the server side where the RefMap class is trying to compare a loose refs/heads/master with a symbolic reference HEAD -> refs/heads/master, and then falling into the WTF comment of RefMap's 'resolveloose' method.
>>>
>>> On the other hand, if I had HEAD as a non-symbolic reference (using new ObjectIdRef.PeeledNonTag with a name of HEAD) then it seems to work as expected.
>>>
>>> Is managing a virtual HEAD ref on the server the right way to go about this, and do I need to hook up a listener such that when the default branch is changed I also update the HEAD, or is there a better way of solving the 'default branch' problem in JGit with DFS?
>>
>> I think this is a bug in MemRefDatabase, the inner class of
>> InMemoryRepository. HEAD works correctly as a symbolic reference in
>> our storage system at Google. So it works correctly in a DFS type
>> repository. Our implementation of DfsRefDatabase is ~1200 lines of
>> non-trivial code, but I can't see anything really wrong with
>> MemRefDatabase just by glancing at it.
>>
>> How did you add the HEAD to the InMemoryRepository?
>
> The code I tried to do was in the 'scanAllRefs()' where I determined if a ref was 'refs/heads/master' and if so added a symbolic one:
>
> • protected RefCache scanAllRefs() throws IOException {
> • RefList.Builder<Ref> ids = new RefList.Builder<Ref>();
> • RefList.Builder<Ref> sym = new RefList.Builder<Ref>();
> • for (Ref ref : refs.values()) {
> if(ref.getName().equals("refs/heads/master")) {
> sym.add(new SymbolicRef("HEAD",ref))
> }
This also has to be in the ids RefList. That is the contract you broke.
> • if (ref.isSymbolic())
> • sym.add(ref);
> • ids.add(ref);
> • }
> • ids.sort();
> • sym.sort();
> • return new RefCache(ids.toRefList(), sym.toRefList());
> • }
>
> Although this returned the value correctly, the RefMap ended up trying to compare the SymbolicRef with the Ref, finding they were different, and then throwing the error message.
Because you broke the contract and didn't also put the symbolic
reference into ids.
> • private Ref resolveLoose(final Ref l) {
> • if (resolvedIdx < resolved.size()) {
> • Ref r = resolved.get(resolvedIdx);
> • int cmp = RefComparator.compareTo(l, r);
> • if (cmp == 0) {
> • resolvedIdx++;
> • return r;
> • } else if (cmp > 0) {
> • // WTF, we have a symbolic entry but no match
> • // in the loose collection. That's an error.
> • throw new IllegalStateException();
> • }
> • }
> • return l;
> • }
>
> By the way, why are we doing 'cmp > 0' here? It's comparing the references by string name in this collection. I'm not sure why it's only throwing this for the >0 case and not the <0 one though.
Because not every entry exists. And you broke the contract.
