Re: problem using jgit
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Jul 27, 2008, 03:21 UTC
- Message-ID
- <20080727032107.GC17425@spearce.org>
- In-Reply-To
- <4889E88E.8000701@gmail.com>
Marek Zawirski <marek.zawirski@gmail.com> wrote:
Show 14 quoted lines
> Shawn O. Pearce wrote: >> Marek Zawirski <marek.zawirski@gmail.com> wrote: >>> >>> 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. [...] > > 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:
Well, I think your temporary fix is correct. I don't see another way to implement around it.
> 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?
Yes. If we read something and it is different from what we have cached we need to signal that the cached data is invalid (which is setModified). Doing so allows listeners to (eventually) find out that there are changes made on disk which their subscribers don't know about, but may need to be informed of. This way we can identify changes made by command line tools to a repository that egit has open in the workbench.
Show 5 quoted lines
> 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); >
In this case (and the other like it) we don't call setModified because there hasn't been a change on disk. The file wasn't previously in our cache and the file is empty now. Either way the ref has no data so there is no need to notify listeners.
Now there may be other cases that are missing, but this one is fine as is.
So I'm thinking of applying this:
--8<-- From: Marek Zawirski <marek.zawirski@gmail.com> Subject: [PATCH] Fix Repository.getAllRefs to skip HEAD if it points to an unborn branch
If HEAD is a symbolic reference pointing to an unborn branch (branch not yet created) we get a Ref object back for it supplying the name of the branch, but its ObjectId is null as the branch file itself is not present in the repository. In such cases we should not include the HEAD reference in the returned output.
Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/RefDatabase.java | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java index 82b995e..b9c8c8c 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java @@ -209,7 +209,9 @@ class RefDatabase { readPackedRefs(avail); readLooseRefs(avail, REFS_SLASH, refsDir); try { - avail.put(Constants.HEAD, readRefBasic(Constants.HEAD, 0)); + final Ref r = readRefBasic(Constants.HEAD, 0); + if (r != null && r.getObjectId() != null) + avail.put(Constants.HEAD, r); } catch (IOException e) { // ignore here }
-- 1.6.0.rc0.182.gb96c7 -- Shawn.