changeset 621:99ad1e3a4e4d

RevlogStream: be aware of existence (not HgDataFile), facilitate use of an added HgDataFile over a commit; Rollback: be more sensitive about file changes (file size is not enough: write/rollback leaves it intact); tests
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Sat, 18 May 2013 22:23:57 +0200
parents 272ecffccc8a
children 4e6179bde4fc
files src/org/tmatesoft/hg/internal/COWTransaction.java src/org/tmatesoft/hg/internal/CommitFacility.java src/org/tmatesoft/hg/internal/Internals.java src/org/tmatesoft/hg/internal/RevlogChangeMonitor.java src/org/tmatesoft/hg/internal/RevlogStream.java src/org/tmatesoft/hg/internal/RevlogStreamFactory.java src/org/tmatesoft/hg/internal/Transaction.java src/org/tmatesoft/hg/repo/HgDataFile.java src/org/tmatesoft/hg/repo/HgRepository.java test/org/tmatesoft/hg/test/TestCommit.java
diffstat 10 files changed, 131 insertions(+), 52 deletions(-) [+]
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/internal/COWTransaction.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/COWTransaction.java	Sat May 18 22:23:57 2013 +0200
@@ -48,7 +48,13 @@
 	public File prepare(File f) throws HgIOException {
 		if (!f.exists()) {
 			record(f, null);
-			return f;
+			try {
+				f.getParentFile().mkdirs();
+				f.createNewFile();
+				return f;
+			} catch (IOException ex) {
+				throw new HgIOException("Failed to create new file", ex, f);
+			}
 		}
 		if (known(f)) {
 			return f;
@@ -123,6 +129,10 @@
 					String msg = String.format("Transaction rollback failed, could not rename backup %s back to %s", e.backup.getName(), e.origin.getName());
 					throw new HgIOException(msg, e.origin);
 				}
+				// renameTo() doesn't update timestamp, while the rest of the code relies
+				// on file timestamp to detect revlog changes. Rollback *is* a change,
+				// even if it brings the old state.
+				e.origin.setLastModified(System.currentTimeMillis());
 			}
 			success.add(e);
 			it.remove();
--- a/src/org/tmatesoft/hg/internal/CommitFacility.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/CommitFacility.java	Sat May 18 22:23:57 2013 +0200
@@ -146,15 +146,9 @@
 				// NEW FILE
 				fp = new Pair<Integer, Integer>(NO_REVISION, NO_REVISION);
 			}
-			RevlogStream contentStream;
-			if (df.exists()) {
-				contentStream = repo.getImplAccess().getStream(df);
-			} else {
-				contentStream = repo.createStoreFile(df.getPath());
+			RevlogStream contentStream = repo.getImplAccess().getStream(df);
+			if (!df.exists()) {
 				newlyAddedFiles.put(df.getPath(), contentStream);
-				// FIXME df doesn't get df.content updated, and clients
-				// that would attempt to access newly added file after commit would fail
-				// (despite the fact the file is in there)
 			}
 			RevlogStreamWriter fileWriter = new RevlogStreamWriter(repo, contentStream, transaction);
 			Nodeid fileRev = fileWriter.addRevision(bds, clogRevisionIndex, fp.first(), fp.second());
--- a/src/org/tmatesoft/hg/internal/Internals.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/Internals.java	Sat May 18 22:23:57 2013 +0200
@@ -32,7 +32,6 @@
 import org.tmatesoft.hg.core.SessionContext;
 import org.tmatesoft.hg.repo.HgDataFile;
 import org.tmatesoft.hg.repo.HgInternals;
-import org.tmatesoft.hg.repo.HgInvalidControlFileException;
 import org.tmatesoft.hg.repo.HgRepoConfig.ExtensionsSection;
 import org.tmatesoft.hg.repo.HgRepository;
 import org.tmatesoft.hg.repo.HgRepositoryFiles;
@@ -493,11 +492,7 @@
 	}
 
 	public RevlogStream resolveStoreFile(Path path) {
-		return streamProvider.resolveStoreFile(path);
-	}
-	
-	/*package-local*/ RevlogStream createStoreFile(Path path) throws HgInvalidControlFileException {
-		return streamProvider.createStoreFile(path);
+		return streamProvider.getStoreFile(path, false);
 	}
 
 	// marker method
--- a/src/org/tmatesoft/hg/internal/RevlogChangeMonitor.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/RevlogChangeMonitor.java	Sat May 18 22:23:57 2013 +0200
@@ -29,19 +29,22 @@
 public class RevlogChangeMonitor {
 	
 	private final WeakHashMap<File, Long> lastKnownSize;
+	private final WeakHashMap<File, Long> lastKnownTime;
 	private final File soleFile;
-	private long soleFileLength = -1;
+	private long soleFileSize = -1;
+	private long soleFileTime = -1;
 	
-	// use single for multiple files. TODO repository/session context shall provide
+	// use single for multiple files. TODO [1.2] repository/session context shall provide
 	// alternative (configurable) implementations, so that Java7 users may supply better one
 	public RevlogChangeMonitor() {
 		lastKnownSize = new WeakHashMap<File, Long>();
+		lastKnownTime= new WeakHashMap<File, Long>();
 		soleFile = null;
 	}
 	
 	public RevlogChangeMonitor(File f) {
 		assert f != null;
-		lastKnownSize = null;
+		lastKnownSize = lastKnownTime = null;
 		soleFile = f;
 	}
 	
@@ -49,9 +52,11 @@
 		assert f != null;
 		if (lastKnownSize == null) {
 			assert f == soleFile;
-			soleFileLength = f.length();
+			soleFileSize = f.length();
+			soleFileTime = f.lastModified();
 		} else {
 			lastKnownSize.put(f, f.length());
+			lastKnownTime.put(f, f.lastModified());
 		}
 	}
 	
@@ -59,13 +64,14 @@
 		assert f != null;
 		if (lastKnownSize == null) {
 			assert f == soleFile;
-			return soleFileLength != f.length();
+			return soleFileSize != f.length() || soleFileTime != f.lastModified();
 		} else {
 			Long lastSize = lastKnownSize.get(f);
-			if (lastSize == null) {
+			Long lastTime = lastKnownTime.get(f);
+			if (lastSize == null || lastTime == null) {
 				return true;
 			}
-			return f.length() != lastSize;
+			return f.length() != lastSize || f.lastModified() != lastTime;
 		}
 	}
 }
--- a/src/org/tmatesoft/hg/internal/RevlogStream.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/RevlogStream.java	Sat May 18 22:23:57 2013 +0200
@@ -85,6 +85,10 @@
 		this.indexFile = indexFile;
 		changeTracker = repo.getRevlogTracker(indexFile);
 	}
+	
+	public boolean exists() {
+		return indexFile.exists();
+	}
 
 	/**
 	 * @param shortRead pass <code>true</code> to indicate intention to read few revisions only (as opposed to reading most of/complete revlog)
@@ -408,6 +412,10 @@
 		}
 		assert revision != null;
 		assert !revision.isNull();
+		// next effort doesn't seem to be of any value at least in case of regular commit
+		// as the next call to #initOutline would recognize the file change and reload complete revlog anyway
+		// OTOH, there might be transaction strategy that doesn't update the file until its completion,
+		// while it's handy to know new revisions meanwhile.
 		int[] baseRevisionsCopy = new int[baseRevisions.length + 1];
 		System.arraycopy(baseRevisions, 0, baseRevisionsCopy, 0, baseRevisions.length);
 		baseRevisionsCopy[baseRevisions.length] = baseRevisionIndex;
--- a/src/org/tmatesoft/hg/internal/RevlogStreamFactory.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/RevlogStreamFactory.java	Sat May 18 22:23:57 2013 +0200
@@ -17,11 +17,9 @@
 package org.tmatesoft.hg.internal;
 
 import java.io.File;
-import java.io.IOException;
 import java.lang.ref.SoftReference;
 import java.util.HashMap;
 
-import org.tmatesoft.hg.repo.HgInvalidControlFileException;
 import org.tmatesoft.hg.util.Path;
 
 /**
@@ -57,14 +55,14 @@
 	 * @param path - normalized file name
 	 * @return <code>null</code> if path doesn't resolve to a existing file
 	 */
-	/*package-local*/ RevlogStream resolveStoreFile(Path path) {
+	/*package-local*/ RevlogStream getStoreFile(Path path, boolean onlyIfExists) {
 		final SoftReference<RevlogStream> ref = shallCacheRevlogs() ? streamsCache.get(path) : null;
 		RevlogStream cached = ref == null ? null : ref.get();
 		if (cached != null) {
 			return cached;
 		}
 		File f = repo.getFileFromDataDir(path);
-		if (f.exists()) {
+		if (!onlyIfExists || f.exists()) {
 			RevlogStream s = create(f);
 			if (shallCacheRevlogs()) {
 				streamsCache.put(path, new SoftReference<RevlogStream>(s));
@@ -74,23 +72,6 @@
 		return null;
 	}
 	
-	/*package-local*/ RevlogStream createStoreFile(Path path) throws HgInvalidControlFileException {
-		File f = repo.getFileFromDataDir(path);
-		try {
-			if (!f.exists()) {
-				f.getParentFile().mkdirs();
-				f.createNewFile();
-			}
-			RevlogStream s = create(f);
-			if (shallCacheRevlogs()) {
-				streamsCache.put(path, new SoftReference<RevlogStream>(s));
-			}
-			return s;
-		} catch (IOException ex) {
-			throw new HgInvalidControlFileException("Can't create a file in the storage", ex, f);
-		}
-	}
-
 	private boolean shallCacheRevlogs() {
 		return streamsCache != null;
 	}
--- a/src/org/tmatesoft/hg/internal/Transaction.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/Transaction.java	Sat May 18 22:23:57 2013 +0200
@@ -44,6 +44,7 @@
 	/**
 	 * Record the file is going to be modified during this transaction, obtain actual
 	 * destination to write to.
+	 * The file to be modified not necessarily exists, might be just a name of an added file  
 	 */
 	public abstract File prepare(File f) throws HgIOException;
 	/**
--- a/src/org/tmatesoft/hg/repo/HgDataFile.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgDataFile.java	Sat May 18 22:23:57 2013 +0200
@@ -75,16 +75,11 @@
 		path = filePath;
 	}
 
-	/*package-local*/HgDataFile(HgRepository hgRepo, Path filePath) {
-		super(hgRepo);
-		path = filePath;
-	}
-
 	// exists is not the best name possible. now it means no file with such name was ever known to the repo.
 	// it might be confused with files existed before but lately removed. TODO HgFileNode.exists makes more sense.
 	// or HgDataFile.known()
 	public boolean exists() {
-		return content != null; // XXX need better impl
+		return content.exists();
 	}
 
 	/**
--- a/src/org/tmatesoft/hg/repo/HgRepository.java	Sat May 18 21:55:31 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgRepository.java	Sat May 18 22:23:57 2013 +0200
@@ -242,9 +242,7 @@
 
 	public HgDataFile getFileNode(Path path) {
 		RevlogStream content = impl.resolveStoreFile(path);
-		if (content == null) {
-			return new HgDataFile(this, path);
-		}
+		assert content != null;
 		return new HgDataFile(this, path, content);
 	}
 
--- a/test/org/tmatesoft/hg/test/TestCommit.java	Sat May 18 21:55:31 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestCommit.java	Sat May 18 22:23:57 2013 +0200
@@ -37,6 +37,7 @@
 import org.tmatesoft.hg.internal.ByteArrayChannel;
 import org.tmatesoft.hg.internal.COWTransaction;
 import org.tmatesoft.hg.internal.CommitFacility;
+import org.tmatesoft.hg.internal.DirstateReader;
 import org.tmatesoft.hg.internal.DataSerializer.ByteArrayDataSource;
 import org.tmatesoft.hg.internal.FileContentSupplier;
 import org.tmatesoft.hg.internal.Internals;
@@ -138,6 +139,7 @@
 		final int parentCsetRevIndex = hgRepo.getChangelog().getLastRevision();
 		HgChangeset parentCset = new HgLogCommand(hgRepo).range(parentCsetRevIndex, parentCsetRevIndex).execute().get(0);
 		assertEquals("[sanity]", DEFAULT_BRANCH_NAME, parentCset.getBranch());
+		assertEquals("[sanity]", DEFAULT_BRANCH_NAME, hgRepo.getWorkingCopyBranchName());
 		//
 		RepoUtils.modifyFileAppend(fileD, "A CHANGE\n");
 		CommitFacility cf = new CommitFacility(Internals.getInstance(hgRepo), parentCsetRevIndex);
@@ -155,6 +157,9 @@
 		errorCollector.assertEquals("branch1", c1.getBranch());
 		errorCollector.assertEquals("FIRST", c1.getComment());
 		//
+		// check if cached value in hgRepo got updated
+		errorCollector.assertEquals("branch1", hgRepo.getWorkingCopyBranchName());
+		//
 		assertHgVerifyOk(repoLoc);
 	}
 
@@ -343,6 +348,92 @@
 		errorCollector.assertTrue(hgRepo.getTags().tags(commit).contains(tag));
 	}
 	
+	@Test
+	public void testAddedFilesGetStream() throws Exception {
+		File repoLoc = RepoUtils.cloneRepoToTempLocation("log-1", "test-commit-addfile-stream", false);
+		final File newFile = new File(repoLoc, "xx");
+		final byte[] newFileContent = "xyz".getBytes();
+		RepoUtils.createFile(newFile, newFileContent);
+		HgRepository hgRepo = new HgLookup().detect(repoLoc);
+		new HgAddRemoveCommand(hgRepo).add(Path.create("xx")).execute();
+		// save the reference to HgDataFile without valid RevlogStream (entry in the dirstate
+		// doesn't make it valid)
+		final HgDataFile newFileNode = hgRepo.getFileNode("xx");
+		assertFalse(newFileNode.exists());
+		HgCommitCommand cmd = new HgCommitCommand(hgRepo).message("FIRST");
+		Outcome r = cmd.execute();
+		errorCollector.assertTrue(r.isOk());
+		TestStatus.StatusCollector status = new TestStatus.StatusCollector();
+		new HgStatusCommand(hgRepo).all().execute(status);
+		errorCollector.assertTrue(status.getErrors().isEmpty());
+		errorCollector.assertTrue(status.get(Kind.Added).isEmpty());
+		errorCollector.assertTrue(status.get(newFileNode.getPath()).contains(Kind.Clean));
+		//
+		errorCollector.assertTrue(newFileNode.exists());
+		final ByteArrayChannel read1 = new ByteArrayChannel();
+		newFileNode.content(0, read1);
+		errorCollector.assertEquals("Read from existing HgDataFile instance", newFileContent, read1.toArray());
+		final ByteArrayChannel read2 = new ByteArrayChannel();
+		hgRepo.getFileNode(newFileNode.getPath()).content(0, read2);
+		errorCollector.assertEquals("Read from fresh HgDataFile instance", newFileContent, read2.toArray());
+	}
+	
+	@Test
+	public void testRollback() throws Exception {
+		File repoLoc = RepoUtils.cloneRepoToTempLocation("log-1", "test-commit-rollback", false);
+		final Path newFilePath = Path.create("xx");
+		final File newFile = new File(repoLoc, newFilePath.toString());
+		RepoUtils.createFile(newFile, "xyz");
+		HgRepository hgRepo = new HgLookup().detect(repoLoc);
+		HgDataFile dfB = hgRepo.getFileNode("b");
+		HgDataFile dfD = hgRepo.getFileNode("d");
+		assertTrue("[sanity]", dfB.exists());
+		assertTrue("[sanity]", dfD.exists());
+		final File modifiedFile = new File(repoLoc, "b");
+		RepoUtils.modifyFileAppend(modifiedFile, " 1 \n");
+		//
+		new HgAddRemoveCommand(hgRepo).add(newFilePath).remove(dfD.getPath()).execute();
+		//
+		TestStatus.StatusCollector status = new TestStatus.StatusCollector();
+		new HgStatusCommand(hgRepo).all().execute(status);
+		assertTrue(status.getErrors().isEmpty());
+		assertTrue(status.get(Kind.Added).contains(newFilePath));
+		assertTrue(status.get(Kind.Modified).contains(dfB.getPath()));
+		assertTrue(status.get(Kind.Removed).contains(dfD.getPath()));
+		assertEquals(DEFAULT_BRANCH_NAME, hgRepo.getWorkingCopyBranchName());
+		//
+		final int lastClogRevision = hgRepo.getChangelog().getLastRevision();
+		final int lastManifestRev = hgRepo.getManifest().getLastRevision();
+		CommitFacility cf = new CommitFacility(Internals.getInstance(hgRepo), lastClogRevision);
+		cf.add(hgRepo.getFileNode("xx"), new FileContentSupplier(hgRepo, newFile));
+		cf.add(dfB, new FileContentSupplier(hgRepo, modifiedFile));
+		cf.forget(dfD);
+		cf.branch("another-branch");
+		Transaction tr = newTransaction(hgRepo);
+		Nodeid commitRev = cf.commit("Commit to fail",  tr);
+		// with 1 second timestamp granularity, HgChangelog doesn't 
+		// recognize the fact the underlying file got changed twice within
+		// a second, doesn't discard new revision obtained via revisionAdded,
+		// and eventually fails trying to read more revisions than there're in the file
+		Thread.sleep(1000); // FIXME this is a hack to make test pass
+		tr.rollback();
+		//
+		errorCollector.assertEquals(lastClogRevision, hgRepo.getChangelog().getLastRevision());
+		errorCollector.assertEquals(lastManifestRev, hgRepo.getManifest().getLastRevision());
+		errorCollector.assertEquals(DEFAULT_BRANCH_NAME, DirstateReader.readBranch(Internals.getInstance(hgRepo)));
+		errorCollector.assertFalse(hgRepo.getChangelog().isKnown(commitRev));
+		errorCollector.assertFalse(hgRepo.getFileNode("xx").exists());
+		// check dirstate
+		status = new TestStatus.StatusCollector();
+		new HgStatusCommand(hgRepo).all().execute(status);
+		errorCollector.assertTrue(status.getErrors().isEmpty());
+		errorCollector.assertTrue(status.get(Kind.Added).contains(newFilePath));
+		errorCollector.assertTrue(status.get(Kind.Modified).contains(dfB.getPath()));
+		errorCollector.assertTrue(status.get(Kind.Removed).contains(dfD.getPath()));
+		
+		assertHgVerifyOk(repoLoc);
+	}
+	
 	private void assertHgVerifyOk(File repoLoc) throws InterruptedException, IOException {
 		ExecHelper verifyRun = new ExecHelper(new OutputParser.Stub(), repoLoc);
 		verifyRun.run("hg", "verify");