From 0db047654263144234a66fe3806811801a61da4f Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 30 Sep 2011 00:00:22 +0200 Subject: [PATCH] Fire IndexChangedEvent on DirCache.commit() Since we replaced GitIndex by DirCache JGit didn't fire IndexChangedEvents anymore. For EGit this still worked with a high latency since its RepositoryChangeScanner which is scheduled to run each 10 seconds fires the event in case the index changes. This scanner is meant to detect index changes induced by a different process e.g. by calling "git add" from native git. When the index is changed from within the same process we should fire the event synchronously. Compare the index checksum on write to index checksum when index was read earlier to determine if index really changed. Use IndexChangedListener interface to keep DirCache decoupled from Repository. Change-Id: Id4311f7a7859ffe8738863b3d86c83c8b5f513af Signed-off-by: Matthias Sohn --- .../jgit/storage/dht/DhtRepository.java | 5 ++ .../jgit/dircache/DirCacheBuilderTest.java | 72 +++++++++++++++++++ .../org/eclipse/jgit/dircache/DirCache.java | 63 ++++++++++++++-- .../src/org/eclipse/jgit/lib/Repository.java | 17 ++++- .../jgit/storage/file/FileRepository.java | 12 ++-- 5 files changed, 160 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit.storage.dht/src/org/eclipse/jgit/storage/dht/DhtRepository.java b/org.eclipse.jgit.storage.dht/src/org/eclipse/jgit/storage/dht/DhtRepository.java index 9f60ef575..faff469e9 100644 --- a/org.eclipse.jgit.storage.dht/src/org/eclipse/jgit/storage/dht/DhtRepository.java +++ b/org.eclipse.jgit.storage.dht/src/org/eclipse/jgit/storage/dht/DhtRepository.java @@ -153,6 +153,11 @@ public class DhtRepository extends Repository { refdb.clearCache(); } + @Override + public void notifyIndexChanged() { + // we do not support non-bare repositories yet + } + @Override public String toString() { return "DhtRepostitory[" + key + " / " + name + "]"; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheBuilderTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheBuilderTest.java index 1c1cb0fcb..c64ca88f0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheBuilderTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheBuilderTest.java @@ -1,5 +1,6 @@ /* * Copyright (C) 2008-2009, Google Inc. + * Copyright (C) 2011, Matthias Sohn * and other copyright owners as documented in the project's IP log. * * This program and the accompanying materials are made available @@ -49,9 +50,13 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; +import org.eclipse.jgit.events.IndexChangedEvent; +import org.eclipse.jgit.events.IndexChangedListener; +import org.eclipse.jgit.events.ListenerList; import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.RepositoryTestCase; @@ -187,6 +192,73 @@ public class DirCacheBuilderTest extends RepositoryTestCase { } } + @Test + public void testBuildOneFile_Commit_IndexChangedEvent() + throws Exception { + final class ReceivedEventMarkerException extends RuntimeException { + private static final long serialVersionUID = 1L; + // empty + } + + final String path = "a-file-path"; + final FileMode mode = FileMode.REGULAR_FILE; + // "old" date in 2008 + final long lastModified = 1218123387057L; + final int length = 1342; + DirCacheEntry entOrig; + boolean receivedEvent = false; + + DirCache dc = db.lockDirCache(); + IndexChangedListener listener = new IndexChangedListener() { + + public void onIndexChanged(IndexChangedEvent event) { + throw new ReceivedEventMarkerException(); + } + }; + + ListenerList l = db.getListenerList(); + l.addIndexChangedListener(listener); + DirCacheBuilder b = dc.builder(); + + entOrig = new DirCacheEntry(path); + entOrig.setFileMode(mode); + entOrig.setLastModified(lastModified); + entOrig.setLength(length); + b.add(entOrig); + try { + b.commit(); + } catch (ReceivedEventMarkerException e) { + receivedEvent = true; + } + if (!receivedEvent) + fail("did not receive IndexChangedEvent"); + + // do the same again, as this doesn't change index compared to first + // round we should get no event this time + dc = db.lockDirCache(); + listener = new IndexChangedListener() { + + public void onIndexChanged(IndexChangedEvent event) { + throw new ReceivedEventMarkerException(); + } + }; + + l = db.getListenerList(); + l.addIndexChangedListener(listener); + b = dc.builder(); + + entOrig = new DirCacheEntry(path); + entOrig.setFileMode(mode); + entOrig.setLastModified(lastModified); + entOrig.setLength(length); + b.add(entOrig); + try { + b.commit(); + } catch (ReceivedEventMarkerException e) { + fail("unexpected IndexChangedEvent"); + } + } + @Test public void testFindSingleFile() throws Exception { final String path = "a-file-path"; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java index af98c3315..55256d638 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java @@ -1,6 +1,7 @@ /* * Copyright (C) 2008-2010, Google Inc. * Copyright (C) 2008, Shawn O. Pearce + * Copyright (C) 2011, Matthias Sohn * and other copyright owners as documented in the project's IP log. * * This program and the accompanying materials are made available @@ -63,6 +64,8 @@ import java.util.Comparator; import org.eclipse.jgit.JGitText; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.UnmergedPathException; +import org.eclipse.jgit.events.IndexChangedEvent; +import org.eclipse.jgit.events.IndexChangedListener; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -94,6 +97,8 @@ public class DirCache { private static final DirCacheEntry[] NO_ENTRIES = {}; + private static final byte[] NO_CHECKSUM = {}; + static final Comparator ENT_CMP = new Comparator() { public int compare(final DirCacheEntry o1, final DirCacheEntry o2) { final int cr = cmp(o1, o2); @@ -203,6 +208,39 @@ public class DirCache { return c; } + /** + * Create a new in-core index representation, lock it, and read from disk. + *

+ * The new index will be locked and then read before it is returned to the + * caller. Read failures are reported as exceptions and therefore prevent + * the method from returning a partially populated index. On read failure, + * the lock is released. + * + * @param indexLocation + * location of the index file on disk. + * @param fs + * the file system abstraction which will be necessary to perform + * certain file system operations. + * @param indexChangedListener + * listener to be informed when DirCache is committed + * @return a cache representing the contents of the specified index file (if + * it exists) or an empty cache if the file does not exist. + * @throws IOException + * the index file is present but could not be read, or the lock + * could not be obtained. + * @throws CorruptObjectException + * the index file is using a format or extension that this + * library does not support. + */ + public static DirCache lock(final File indexLocation, final FS fs, + IndexChangedListener indexChangedListener) + throws CorruptObjectException, + IOException { + DirCache c = lock(indexLocation, fs); + c.registerIndexChangedListener(indexChangedListener); + return c; + } + /** Location of the current version of the index file. */ private final File liveFile; @@ -224,6 +262,15 @@ public class DirCache { /** Keep track of whether the index has changed or not */ private FileSnapshot snapshot; + /** index checksum when index was read from disk */ + private byte[] readIndexChecksum; + + /** index checksum when index was written to disk */ + private byte[] writeIndexChecksum; + + /** listener to be informed on commit */ + private IndexChangedListener indexChangedListener; + /** * Create a new in-core index representation. *

@@ -330,6 +377,7 @@ public class DirCache { sortedEntries = NO_ENTRIES; entryCnt = 0; tree = null; + readIndexChecksum = NO_CHECKSUM; } private void readFrom(final InputStream inStream) throws IOException, @@ -412,8 +460,8 @@ public class DirCache { } } - final byte[] exp = md.digest(); - if (!Arrays.equals(exp, hdr)) { + readIndexChecksum = md.digest(); + if (!Arrays.equals(readIndexChecksum, hdr)) { throw new CorruptObjectException(JGitText.get().DIRCChecksumMismatch); } } @@ -544,8 +592,8 @@ public class DirCache { dos.write(tmp, 0, 8); bb.writeTo(dos, null); } - - os.write(foot.digest()); + writeIndexChecksum = foot.digest(); + os.write(writeIndexChecksum); os.close(); } @@ -567,6 +615,9 @@ public class DirCache { if (!tmp.commit()) return false; snapshot = tmp.getCommitSnapshot(); + if (indexChangedListener != null + && !Arrays.equals(readIndexChecksum, writeIndexChecksum)) + indexChangedListener.onIndexChanged(new IndexChangedEvent()); return true; } @@ -794,4 +845,8 @@ public class DirCache { } return false; } + + private void registerIndexChangedListener(IndexChangedListener listener) { + this.indexChangedListener = listener; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index 3a3e1c558..f31217a96 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -69,6 +69,8 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.errors.RevisionSyntaxException; +import org.eclipse.jgit.events.IndexChangedEvent; +import org.eclipse.jgit.events.IndexChangedListener; import org.eclipse.jgit.events.ListenerList; import org.eclipse.jgit.events.RepositoryEvent; import org.eclipse.jgit.revwalk.RevBlob; @@ -883,7 +885,15 @@ public abstract class Repository { */ public DirCache lockDirCache() throws NoWorkTreeException, CorruptObjectException, IOException { - return DirCache.lock(getIndexFile(), getFS()); + // we want DirCache to inform us so that we can inform registered + // listeners about index changes + IndexChangedListener l = new IndexChangedListener() { + + public void onIndexChanged(IndexChangedEvent event) { + notifyIndexChanged(); + } + }; + return DirCache.lock(getIndexFile(), getFS(), l); } static byte[] gitInternalSlash(byte[] bytes) { @@ -1065,6 +1075,11 @@ public abstract class Repository { */ public abstract void scanForRepoChanges() throws IOException; + /** + * Notify that the index changed + */ + public abstract void notifyIndexChanged(); + /** * @param refName * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java index cd661e2d2..13776140d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java @@ -393,10 +393,14 @@ public class FileRepository extends Repository { File indexFile = getIndexFile(); if (snapshot == null) snapshot = FileSnapshot.save(indexFile); - else if (snapshot.isModified(indexFile)) { - snapshot = FileSnapshot.save(indexFile); - fireEvent(new IndexChangedEvent()); - } + else if (snapshot.isModified(indexFile)) + notifyIndexChanged(); + } + + @Override + public void notifyIndexChanged() { + snapshot = FileSnapshot.save(getIndexFile()); + fireEvent(new IndexChangedEvent()); } /**