From 3922e026e006ad738bf66b8b3b1afc59f9b004ae Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 13 Dec 2010 14:19:02 -0800 Subject: [PATCH] FileBasedConfig: Use FileSnapshot for isOutdated() Relying only on the last modified time for a file can be tricky. The "racy git" problem may cause some modifications to be missed. Use the new FileSnapshot code to track when a configuration file has been modified, and needs to be reloaded in memory. Change-Id: Ib6312fdd3b2403eee5af3f8ae711294b0e5f9035 Signed-off-by: Shawn O. Pearce --- .../jgit/storage/file/FileBasedConfig.java | 40 ++++++++++++++++--- .../jgit/storage/file/FileSnapshot.java | 4 ++ .../eclipse/jgit/storage/file/LockFile.java | 29 +++++++++++--- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java index da1f3af60..212646b11 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java @@ -58,6 +58,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.IO; @@ -68,8 +69,9 @@ */ public class FileBasedConfig extends StoredConfig { private final File configFile; - private volatile long lastModified; private final FS fs; + private volatile FileSnapshot snapshot; + private volatile ObjectId hash; /** * Create a configuration with no default fallback. @@ -99,6 +101,8 @@ public FileBasedConfig(Config base, File cfgLocation, FS fs) { super(base); configFile = cfgLocation; this.fs = fs; + this.snapshot = FileSnapshot.DIRTY; + this.hash = ObjectId.zeroId(); } @Override @@ -125,11 +129,24 @@ public final File getFile() { */ @Override public void load() throws IOException, ConfigInvalidException { - lastModified = getFile().lastModified(); + final FileSnapshot oldSnapshot = snapshot; + final FileSnapshot newSnapshot = FileSnapshot.save(getFile()); try { - fromText(RawParseUtils.decode(IO.readFully(getFile()))); + final byte[] in = IO.readFully(getFile()); + final ObjectId newHash = hash(in); + if (hash.equals(newHash)) { + if (oldSnapshot.equals(newSnapshot)) + oldSnapshot.setClean(newSnapshot); + else + snapshot = newSnapshot; + } else { + fromText(RawParseUtils.decode(in)); + snapshot = newSnapshot; + hash = newHash; + } } catch (FileNotFoundException noFile) { clear(); + snapshot = newSnapshot; } catch (IOException e) { final IOException e2 = new IOException(MessageFormat.format(JGitText.get().cannotReadFile, getFile())); e2.initCause(e); @@ -157,18 +174,29 @@ public void save() throws IOException { if (!lf.lock()) throw new IOException(MessageFormat.format(JGitText.get().cannotLockFile, getFile())); try { - lf.setNeedStatInformation(true); + lf.setNeedSnapshot(true); lf.write(out); if (!lf.commit()) throw new IOException(MessageFormat.format(JGitText.get().cannotCommitWriteTo, getFile())); } finally { lf.unlock(); } - lastModified = lf.getCommitLastModified(); + snapshot = lf.getCommitSnapshot(); + hash = hash(out); // notify the listeners fireConfigChangedEvent(); } + @Override + public void clear() { + hash = hash(new byte[0]); + super.clear(); + } + + private static ObjectId hash(final byte[] rawText) { + return ObjectId.fromRaw(Constants.newMessageDigest().digest(rawText)); + } + @Override public String toString() { return getClass().getSimpleName() + "[" + getFile().getPath() + "]"; @@ -179,6 +207,6 @@ public String toString() { * than the file on disk */ public boolean isOutdated() { - return getFile().lastModified() != lastModified; + return snapshot.isModified(getFile()); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java index c1ce449dc..87ae57b82 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java @@ -104,6 +104,10 @@ private FileSnapshot(long read, long modified) { this.cannotBeRacilyClean = notRacyClean(read); } + long lastModified() { + return lastModified; + } + /** * Check if the path has been modified since the snapshot was saved. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java index c1fb704a5..52aa7eeb9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java @@ -89,11 +89,11 @@ public boolean accept(File dir, String name) { private FileOutputStream os; - private boolean needStatInformation; + private boolean needSnapshot; private boolean fsync; - private long commitLastModified; + private FileSnapshot commitSnapshot; private final FS fs; @@ -334,12 +334,24 @@ private void requireLock() { /** * Request that {@link #commit()} remember modification time. + *

+ * This is an alias for {@code setNeedSnapshot(true)}. * * @param on * true if the commit method must remember the modification time. */ public void setNeedStatInformation(final boolean on) { - needStatInformation = on; + setNeedSnapshot(on); + } + + /** + * Request that {@link #commit()} remember the {@link FileSnapshot}. + * + * @param on + * true if the commit method must remember the FileSnapshot. + */ + public void setNeedSnapshot(final boolean on) { + needSnapshot = on; } /** @@ -442,8 +454,8 @@ private boolean renameLock() { } private void saveStatInformation() { - if (needStatInformation) - commitLastModified = lck.lastModified(); + if (needSnapshot) + commitSnapshot = FileSnapshot.save(lck); } /** @@ -452,7 +464,12 @@ private void saveStatInformation() { * @return modification time of the lock file right before we committed it. */ public long getCommitLastModified() { - return commitLastModified; + return commitSnapshot.lastModified(); + } + + /** @return get the {@link FileSnapshot} just before commit. */ + public FileSnapshot getCommitSnapshot() { + return commitSnapshot; } /**