changeset 284:7232b94f2ae3

HgDirstate shall operate with Path instead of String for file names. Use of Pair instead of array of unspecified length for parents.
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Sat, 03 Sep 2011 13:12:13 +0200
parents 7a8e1a305a78
children 6dbbc53fc46d
files src/org/tmatesoft/hg/repo/HgDirstate.java src/org/tmatesoft/hg/repo/HgInternals.java src/org/tmatesoft/hg/repo/HgMergeState.java src/org/tmatesoft/hg/repo/HgRepository.java src/org/tmatesoft/hg/repo/HgStatusCollector.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java
diffstat 6 files changed, 124 insertions(+), 100 deletions(-) [+]
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/repo/HgDirstate.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgDirstate.java	Sat Sep 03 13:12:13 2011 +0200
@@ -16,6 +16,8 @@
  */
 package org.tmatesoft.hg.repo;
 
+import static org.tmatesoft.hg.core.Nodeid.NULL;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
@@ -28,7 +30,9 @@
 import org.tmatesoft.hg.core.HgBadStateException;
 import org.tmatesoft.hg.core.Nodeid;
 import org.tmatesoft.hg.internal.DataAccess;
+import org.tmatesoft.hg.util.Pair;
 import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.PathPool;
 
 
 /**
@@ -42,21 +46,22 @@
 
 	private final HgRepository repo;
 	private final File dirstateFile;
-	// deliberate String, not Path as it seems useless to keep Path here
-	private Map<String, Record> normal;
-	private Map<String, Record> added;
-	private Map<String, Record> removed;
-	private Map<String, Record> merged;
-	private Nodeid p1, p2;
+	private final PathPool pathPool;
+	private Map<Path, Record> normal;
+	private Map<Path, Record> added;
+	private Map<Path, Record> removed;
+	private Map<Path, Record> merged;
+	private Pair<Nodeid, Nodeid> parents;
 	private String currentBranch;
 	
-	public HgDirstate(HgRepository hgRepo, File dirstate) {
+	public HgDirstate(HgRepository hgRepo, File dirstate, PathPool pathPool) {
 		repo = hgRepo;
-		dirstateFile = dirstate; // XXX decide whether file names shall be kept local to reader (see #branches()) or passed from outside 
+		dirstateFile = dirstate; // XXX decide whether file names shall be kept local to reader (see #branches()) or passed from outside
+		this.pathPool = pathPool;
 	}
 
 	private void read() {
-		normal = added = removed = merged = Collections.<String, Record>emptyMap();
+		normal = added = removed = merged = Collections.<Path, Record>emptyMap();
 		if (dirstateFile == null || !dirstateFile.exists()) {
 			return;
 		}
@@ -65,16 +70,12 @@
 			return;
 		}
 		// not sure linked is really needed here, just for ease of debug
-		normal = new LinkedHashMap<String, Record>();
-		added = new LinkedHashMap<String, Record>();
-		removed = new LinkedHashMap<String, Record>();
-		merged = new LinkedHashMap<String, Record>();
+		normal = new LinkedHashMap<Path, Record>();
+		added = new LinkedHashMap<Path, Record>();
+		removed = new LinkedHashMap<Path, Record>();
+		merged = new LinkedHashMap<Path, Record>();
 		try {
-			byte[] parents = new byte[40];
-			da.readBytes(parents, 0, 40);
-			p1 = Nodeid.fromBinary(parents, 0);
-			p2 = Nodeid.fromBinary(parents, 20);
-			parents = null;
+			parents = internalReadParents(da);
 			// hg init; hg up produces an empty repository where dirstate has parents (40 bytes) only
 			while (!da.isEmpty()) {
 				final byte state = da.readByte();
@@ -95,7 +96,7 @@
 				if (fn1 == null) {
 					fn1 = new String(name);
 				}
-				Record r = new Record(fmode, size, time, fn1, fn2);
+				Record r = new Record(fmode, size, time, pathPool.path(fn1), fn2 == null ? null : pathPool.path(fn2));
 				if (state == 'n') {
 					normal.put(r.name1, r);
 				} else if (state == 'a') {
@@ -115,21 +116,39 @@
 		}
 	}
 
-	// do not read whole dirstate if all we need is WC parent information
-	private void readParents() {
+	private static Pair<Nodeid, Nodeid> internalReadParents(DataAccess da) throws IOException {
+		byte[] parents = new byte[40];
+		da.readBytes(parents, 0, 40);
+		Nodeid n1 = Nodeid.fromBinary(parents, 0);
+		Nodeid n2 = Nodeid.fromBinary(parents, 20);
+		parents = null;
+		return new Pair<Nodeid, Nodeid>(n1, n2);
+	}
+	
+	/**
+	 * @return array of length 2 with working copy parents, non null.
+	 */
+	public Pair<Nodeid,Nodeid> parents() {
+		if (parents == null) {
+			parents = readParents(repo, dirstateFile);
+		}
+		return parents;
+	}
+	
+	/**
+	 * @return pair of parents, both {@link Nodeid#NULL} if dirstate is not available
+	 */
+	public static Pair<Nodeid, Nodeid> readParents(HgRepository repo, File dirstateFile) {
+		// do not read whole dirstate if all we need is WC parent information
 		if (dirstateFile == null || !dirstateFile.exists()) {
-			return;
+			return new Pair<Nodeid,Nodeid>(NULL, NULL);
 		}
 		DataAccess da = repo.getDataAccess().create(dirstateFile);
 		if (da.isEmpty()) {
-			return;
+			return new Pair<Nodeid,Nodeid>(NULL, NULL);
 		}
 		try {
-			byte[] parents = new byte[40];
-			da.readBytes(parents, 0, 40);
-			p1 = Nodeid.fromBinary(parents, 0);
-			p2 = Nodeid.fromBinary(parents, 20);
-			parents = null;
+			return internalReadParents(da);
 		} catch (IOException ex) {
 			throw new HgBadStateException(ex); // XXX in fact, our exception is not the best solution here.
 		} finally {
@@ -138,49 +157,46 @@
 	}
 	
 	/**
-	 * @return array of length 2 with working copy parents, non null.
-	 */
-	public Nodeid[] parents() {
-		if (p1 == null) {
-			readParents();
-		}
-		Nodeid[] rv = new Nodeid[2];
-		rv[0] = p1;
-		rv[1] = p2;
-		return rv;
-	}
-	
-	/**
+	 * XXX is it really proper place for the method?
 	 * @return branch associated with the working directory
 	 */
 	public String branch() {
 		if (currentBranch == null) {
-			currentBranch = HgRepository.DEFAULT_BRANCH_NAME;
-			File branchFile = new File(repo.getRepositoryRoot(), "branch");
-			if (branchFile.exists()) {
-				try {
-					BufferedReader r = new BufferedReader(new FileReader(branchFile));
-					String b = r.readLine();
-					if (b != null) {
-						b = b.trim().intern();
-					}
-					currentBranch = b == null || b.length() == 0 ? HgRepository.DEFAULT_BRANCH_NAME : b;
-					r.close();
-				} catch (IOException ex) {
-					ex.printStackTrace(); // XXX log verbose debug, exception might be legal here (i.e. FileNotFound)
-					// IGNORE
-				}
-			}
+			currentBranch = readBranch(repo);
 		}
 		return currentBranch;
 	}
+	
+	/**
+	 * XXX is it really proper place for the method?
+	 * @return branch associated with the working directory
+	 */
+	public static String readBranch(HgRepository repo) {
+		String branch = HgRepository.DEFAULT_BRANCH_NAME;
+		File branchFile = new File(repo.getRepositoryRoot(), "branch");
+		if (branchFile.exists()) {
+			try {
+				BufferedReader r = new BufferedReader(new FileReader(branchFile));
+				String b = r.readLine();
+				if (b != null) {
+					b = b.trim().intern();
+				}
+				branch = b == null || b.length() == 0 ? HgRepository.DEFAULT_BRANCH_NAME : b;
+				r.close();
+			} catch (IOException ex) {
+				ex.printStackTrace(); // XXX log verbose debug, exception might be legal here (i.e. FileNotFound)
+				// IGNORE
+			}
+		}
+		return branch;
+	}
 
 	// new, modifiable collection
-	/*package-local*/ TreeSet<String> all() {
+	/*package-local*/ TreeSet<Path> all() {
 		read();
-		TreeSet<String> rv = new TreeSet<String>();
+		TreeSet<Path> rv = new TreeSet<Path>();
 		@SuppressWarnings("unchecked")
-		Map<String, Record>[] all = new Map[] { normal, added, removed, merged };
+		Map<Path, Record>[] all = new Map[] { normal, added, removed, merged };
 		for (int i = 0; i < all.length; i++) {
 			for (Record r : all[i].values()) {
 				rv.add(r.name1);
@@ -190,20 +206,17 @@
 	}
 	
 	/*package-local*/ Record checkNormal(Path fname) {
-		return normal.get(fname.toString());
+		return normal.get(fname);
 	}
 
 	/*package-local*/ Record checkAdded(Path fname) {
-		return added.get(fname.toString());
+		return added.get(fname);
 	}
 	/*package-local*/ Record checkRemoved(Path fname) {
-		return removed.get(fname.toString());
-	}
-	/*package-local*/ Record checkRemoved(String fname) {
 		return removed.get(fname);
 	}
 	/*package-local*/ Record checkMerged(Path fname) {
-		return merged.get(fname.toString());
+		return merged.get(fname);
 	}
 
 
@@ -212,7 +225,7 @@
 	/*package-local*/ void dump() {
 		read();
 		@SuppressWarnings("unchecked")
-		Map<String, Record>[] all = new Map[] { normal, added, removed, merged };
+		Map<Path, Record>[] all = new Map[] { normal, added, removed, merged };
 		char[] x = new char[] {'n', 'a', 'r', 'm' };
 		for (int i = 0; i < all.length; i++) {
 			for (Record r : all[i].values()) {
@@ -232,10 +245,10 @@
 		// Thus, can't compare directly to HgDataFile.length()
 		final int size; 
 		final int time;
-		final String name1;
-		final String name2;
+		final Path name1;
+		final Path name2;
 
-		public Record(int fmode, int fsize, int ftime, String name1, String name2) {
+		public Record(int fmode, int fsize, int ftime, Path name1, Path name2) {
 			mode = fmode;
 			size = fsize;
 			time = ftime;
--- a/src/org/tmatesoft/hg/repo/HgInternals.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgInternals.java	Sat Sep 03 13:12:13 2011 +0200
@@ -31,6 +31,7 @@
 import org.tmatesoft.hg.util.FileIterator;
 import org.tmatesoft.hg.util.FileWalker;
 import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.PathPool;
 import org.tmatesoft.hg.util.PathRewrite;
 
 
@@ -53,7 +54,7 @@
 	}
 
 	public void dumpDirstate() {
-		repo.loadDirstate().dump();
+		repo.loadDirstate(new PathPool(new PathRewrite.Empty())).dump();
 	}
 
 	public boolean[] checkIgnored(String... toCheck) {
--- a/src/org/tmatesoft/hg/repo/HgMergeState.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgMergeState.java	Sat Sep 03 13:12:13 2011 +0200
@@ -32,6 +32,7 @@
 import org.tmatesoft.hg.core.Nodeid;
 import org.tmatesoft.hg.internal.ManifestRevision;
 import org.tmatesoft.hg.internal.Pool;
+import org.tmatesoft.hg.util.Pair;
 import org.tmatesoft.hg.util.Path;
 import org.tmatesoft.hg.util.PathPool;
 import org.tmatesoft.hg.util.PathRewrite;
@@ -99,8 +100,8 @@
 		}
 		Pool<Nodeid> nodeidPool = new Pool<Nodeid>();
 		Pool<String> fnamePool = new Pool<String>();
-		Nodeid[] wcParents = repo.loadDirstate().parents();
-		wcp1 = nodeidPool.unify(wcParents[0]); wcp2 = nodeidPool.unify(wcParents[1]);
+		Pair<Nodeid, Nodeid> wcParents = repo.getWorkingCopyParents();
+		wcp1 = nodeidPool.unify(wcParents.first()); wcp2 = nodeidPool.unify(wcParents.second());
 		ArrayList<Entry> result = new ArrayList<Entry>();
 		PathPool pathPool = new PathPool(new PathRewrite.Empty());
 		final ManifestRevision m1 = new ManifestRevision(nodeidPool, fnamePool);
--- a/src/org/tmatesoft/hg/repo/HgRepository.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgRepository.java	Sat Sep 03 13:12:13 2011 +0200
@@ -16,8 +16,6 @@
  */
 package org.tmatesoft.hg.repo;
 
-import static org.tmatesoft.hg.core.Nodeid.NULL;
-
 import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
@@ -40,6 +38,7 @@
 import org.tmatesoft.hg.util.CancelledException;
 import org.tmatesoft.hg.util.Pair;
 import org.tmatesoft.hg.util.Path;
+import org.tmatesoft.hg.util.PathPool;
 import org.tmatesoft.hg.util.PathRewrite;
 import org.tmatesoft.hg.util.ProgressSupport;
 
@@ -232,18 +231,19 @@
 	public PathRewrite getToRepoPathHelper() {
 		return normalizePath;
 	}
-	
-	@Experimental(reason="return type and possible values (presently null, perhaps Nodeid.NULL) may get changed")
+
+	/**
+	 * @return pair of values, {@link Pair#first()} and {@link Pair#second()} are respective parents, never <code>null</code>. 
+	 */
 	public Pair<Nodeid,Nodeid> getWorkingCopyParents() {
-		Nodeid[] p = loadDirstate().parents();
-		return new Pair<Nodeid,Nodeid>(NULL == p[0] ? null : p[0], NULL == p[1] ? null : p[1]);
+		return HgDirstate.readParents(this, new File(repoDir, "dirstate"));
 	}
 	
 	/**
 	 * @return name of the branch associated with working directory, never <code>null</code>.
 	 */
 	public String getWorkingCopyBranchName() {
-		return loadDirstate().branch();
+		return HgDirstate.readBranch(this);
 	}
 
 	/**
@@ -272,8 +272,8 @@
 
 	// XXX package-local, unless there are cases when required from outside (guess, working dir/revision walkers may hide dirstate access and no public visibility needed)
 	// XXX consider passing Path pool or factory to produce (shared) Path instead of Strings
-	/*package-local*/ final HgDirstate loadDirstate() {
-		return new HgDirstate(this, new File(repoDir, "dirstate"));
+	/*package-local*/ final HgDirstate loadDirstate(PathPool pathPool) {
+		return new HgDirstate(this, new File(repoDir, "dirstate"), pathPool);
 	}
 
 	// package-local, see comment for loadDirstate
--- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Sat Sep 03 13:12:13 2011 +0200
@@ -65,9 +65,7 @@
 		cacheNodes = new Pool<Nodeid>();
 		cacheFilenames = new Pool<String>();
 
-		emptyFakeState = new ManifestRevision(null, null);
-		emptyFakeState.begin(-1, null, -1);
-		emptyFakeState.end(-1);
+		emptyFakeState = createEmptyManifestRevision();
 	}
 	
 	public HgRepository getRepo() {
@@ -137,6 +135,13 @@
 		});
 	}
 	
+	/*package-local*/ static ManifestRevision createEmptyManifestRevision() {
+		ManifestRevision fakeEmptyRev = new ManifestRevision(null, null);
+		fakeEmptyRev.begin(-1, null, -1);
+		fakeEmptyRev.end(-1);
+		return fakeEmptyRev;
+	}
+	
 	/*package-local*/ ManifestRevision raw(int rev) {
 		return get(rev);
 	}
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Sat Sep 03 13:10:12 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Sat Sep 03 13:12:13 2011 +0200
@@ -97,12 +97,13 @@
 	
 	private HgDirstate getDirstate() {
 		if (dirstate == null) {
-			dirstate = repo.loadDirstate();
+			dirstate = repo.loadDirstate(getPathPool());
 		}
 		return dirstate;
 	}
 	
 	private ManifestRevision getManifest(int changelogLocalRev) {
+		assert changelogLocalRev >= 0;
 		ManifestRevision mr;
 		if (baseRevisionCollector != null) {
 			mr = baseRevisionCollector.raw(changelogLocalRev);
@@ -117,9 +118,13 @@
 		// WC not necessarily points to TIP, but may be result of update to any previous revision.
 		// In such case, we need to compare local files not to their TIP content, but to specific version at the time of selected revision
 		if (dirstateParentManifest == null) {
-			Nodeid dirstateParent = getDirstate().parents()[0];
-			int changelogLocalRev = repo.getChangelog().getLocalRevision(dirstateParent);
-			dirstateParentManifest = getManifest(changelogLocalRev);
+			Nodeid dirstateParent = getDirstate().parents().first();
+			if (dirstateParent.isNull()) {
+				dirstateParentManifest = baseRevisionCollector != null ? baseRevisionCollector.raw(-1) : HgStatusCollector.createEmptyManifestRevision();
+			} else {
+				int changelogLocalRev = repo.getChangelog().getLocalRevision(dirstateParent);
+				dirstateParentManifest = getManifest(changelogLocalRev);
+			}
 		}
 		return dirstateParentManifest;
 	}
@@ -152,16 +157,15 @@
 			((HgStatusCollector.Record) inspector).init(rev1, rev2, sc);
 		}
 		final HgIgnore hgIgnore = repo.getIgnore();
-		TreeSet<String> knownEntries = getDirstate().all();
+		TreeSet<Path> knownEntries = getDirstate().all();
 		repoWalker.reset();
-		final PathPool pp = getPathPool();
 		while (repoWalker.hasNext()) {
 			repoWalker.next();
-			final Path fname = pp.path(repoWalker.name());
+			final Path fname = getPathPool().path(repoWalker.name());
 			File f = repoWalker.file();
 			if (!f.exists()) {
 				// file coming from iterator doesn't exist.
-				if (knownEntries.remove(fname.toString())) {
+				if (knownEntries.remove(fname)) {
 					if (getDirstate().checkRemoved(fname) == null) {
 						inspector.missing(fname);
 					} else {
@@ -190,7 +194,7 @@
 				continue;
 			}
 			assert f.isFile();
-			if (knownEntries.remove(fname.toString())) {
+			if (knownEntries.remove(fname)) {
 				// tracked file.
 				// modified, added, removed, clean
 				if (collect != null) { // need to check against base revision, not FS file
@@ -211,26 +215,26 @@
 		}
 		if (collect != null) {
 			for (String r : baseRevFiles) {
-				final Path fromBase = pp.path(r);
+				final Path fromBase = getPathPool().path(r);
 				if (repoWalker.inScope(fromBase)) {
 					inspector.removed(fromBase);
 				}
 			}
 		}
-		for (String m : knownEntries) {
-			if (!repoWalker.inScope(pp.path(m))) {
+		for (Path m : knownEntries) {
+			if (!repoWalker.inScope(m)) {
 				// do not report as missing/removed those FileIterator doesn't care about.
 				continue;
 			}
 			// missing known file from a working dir  
 			if (getDirstate().checkRemoved(m) == null) {
 				// not removed from the repository = 'deleted'  
-				inspector.missing(pp.path(m));
+				inspector.missing(m);
 			} else {
 				// removed from the repo
 				// if we check against non-tip revision, do not report files that were added past that revision and now removed.
-				if (collect == null || baseRevFiles.contains(m)) {
-					inspector.removed(pp.path(m));
+				if (collect == null || baseRevFiles.contains(m.toString())) {
+					inspector.removed(m);
 				}
 			}
 		}