# HG changeset patch
# User Artem Tikhomirov <tikhomirov.artem@gmail.com>
# Date 1315005663 -7200
# Node ID e51dd9a14b6fc51809a3471a38577ea93412cdc7
# Parent  81e9a3c9bafee5baed13aba6d352e1cdb26350f6
Yet another WC status fix, where dirstate parent and base revision are treated right (dirstate parent other than tip and explicit baseRevision are not the same)

diff -r 81e9a3c9bafe -r e51dd9a14b6f src/org/tmatesoft/hg/repo/HgStatusCollector.java
--- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Sat Sep 03 01:21:03 2011 +0200
@@ -170,11 +170,10 @@
 		walk(parents[0], rev, inspector);
 	}
 	
-	// I assume revision numbers are the same for changelog and manifest - here 
-	// user would like to pass changelog revision numbers, and I use them directly to walk manifest.
-	// if this assumption is wrong, fix this (lookup manifest revisions from changeset).
-	// rev1 and rev2 may be -1 to indicate comparison to empty repository
-	// argument order matters 
+	// rev1 and rev2 are changelog revision numbers, argument order matters.
+	// Either rev1 or rev2 may be -1 to indicate comparison to empty repository (XXX this is due to use of 
+	// parents in #change(), I believe. Perhaps, need a constant for this? Otherwise this hidden knowledge gets
+	// exposed to e.g. Record
 	public void walk(int rev1, int rev2, HgStatusInspector inspector) {
 		if (rev1 == rev2) {
 			throw new IllegalArgumentException();
@@ -182,9 +181,6 @@
 		if (inspector == null) {
 			throw new IllegalArgumentException();
 		}
-		if (inspector instanceof Record) {
-			((Record) inspector).init(rev1, rev2, this);
-		}
 		final int lastManifestRevision = repo.getChangelog().getLastRevision();
 		if (rev1 == TIP) {
 			rev1 = lastManifestRevision;
@@ -192,6 +188,9 @@
 		if (rev2 == TIP) {
 			rev2 = lastManifestRevision; 
 		}
+		if (inspector instanceof Record) {
+			((Record) inspector).init(rev1, rev2, this);
+		}
 		// in fact, rev1 and rev2 are often next (or close) to each other,
 		// thus, we can optimize Manifest reads here (manifest.walk(rev1, rev2))
 		ManifestRevision r1, r2 ;
diff -r 81e9a3c9bafe -r e51dd9a14b6f src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Sat Sep 03 01:21:03 2011 +0200
@@ -59,6 +59,7 @@
 	private HgDirstate dirstate;
 	private HgStatusCollector baseRevisionCollector;
 	private PathPool pathPool;
+	private ManifestRevision dirstateParentManifest;
 
 	public HgWorkingCopyStatusCollector(HgRepository hgRepo) {
 		this(hgRepo, new HgInternals(hgRepo).createWorkingDirWalker(null));
@@ -101,45 +102,57 @@
 		return dirstate;
 	}
 	
-	// may be invoked few times
+	private ManifestRevision getManifest(int changelogLocalRev) {
+		ManifestRevision mr;
+		if (baseRevisionCollector != null) {
+			mr = baseRevisionCollector.raw(changelogLocalRev);
+		} else {
+			mr = new ManifestRevision(null, null);
+			repo.getManifest().walk(changelogLocalRev, changelogLocalRev, mr);
+		}
+		return mr;
+	}
+	
+	private ManifestRevision getDirstateParentManifest() {
+		// 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);
+		}
+		return dirstateParentManifest;
+	}
+	
+	// may be invoked few times, TIP or WORKING_COPY indicate comparison shall be run against working copy parent
 	// NOTE, use of TIP constant requires certain care. TIP here doesn't mean latest cset, but actual working copy parent.
-	// XXX this shall be changed, though, and use of TIP throughout code shall be revised - 
-	// consider case when repository is updated to one of its previous revisions. TIP points to last change, but a lot of
-	// commands need to work with revision that is in dirstate now.
 	public void walk(int baseRevision, HgStatusInspector inspector) {
-		if (HgInternals.wrongLocalRevision(baseRevision) || baseRevision == BAD_REVISION || baseRevision == WORKING_COPY) {
+		if (HgInternals.wrongLocalRevision(baseRevision) || baseRevision == BAD_REVISION) {
 			throw new IllegalArgumentException(String.valueOf(baseRevision));
 		}
-		final HgIgnore hgIgnore = repo.getIgnore();
-		TreeSet<String> knownEntries = getDirstate().all();
-		if (baseRevision == TIP) {
-			// 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
-			Nodeid dirstateParentRev = getDirstate().parents()[0];
-			Nodeid lastCsetRev = repo.getChangelog().getRevision(HgRepository.TIP);
-			if (lastCsetRev.equals(dirstateParentRev)) {
-				baseRevision = repo.getChangelog().getLastRevision();
-			} else {
-				// can do it right away, but explicit check above might save few cycles (unless getLocalRevision(Nodeid) is effective)
-				baseRevision = repo.getChangelog().getLocalRevision(dirstateParentRev);
-			}
-		}
-		final boolean isTipBase = baseRevision == repo.getChangelog().getLastRevision();
-		ManifestRevision collect = null;
+		ManifestRevision collect = null; // non null indicates we compare against base revision
 		Set<String> baseRevFiles = Collections.emptySet(); // files from base revision not affected by status calculation 
-		if (!isTipBase) {
-			if (baseRevisionCollector != null) {
-				collect = baseRevisionCollector.raw(baseRevision);
-			} else {
-				collect = new ManifestRevision(null, null);
-				repo.getManifest().walk(baseRevision, baseRevision, collect);
-			}
+		if (baseRevision != TIP && baseRevision != WORKING_COPY) {
+			collect = getManifest(baseRevision);
 			baseRevFiles = new TreeSet<String>(collect.files());
 		}
 		if (inspector instanceof HgStatusCollector.Record) {
 			HgStatusCollector sc = baseRevisionCollector == null ? new HgStatusCollector(repo) : baseRevisionCollector;
-			((HgStatusCollector.Record) inspector).init(baseRevision, BAD_REVISION, sc);
+			// nodeidAfterChange(dirstate's parent) doesn't make too much sense,
+			// because the change might be actually in working copy. Nevertheless, 
+			// as long as no nodeids can be provided for WC, seems reasonable to report
+			// latest known nodeid change (although at the moment this is not used and
+			// is done mostly not to leave stale initialization in the Record)
+			int rev1,rev2 = getDirstateParentManifest().changesetLocalRev();
+			if (baseRevision == TIP || baseRevision == WORKING_COPY) {
+				rev1 = rev2 - 1; // just use revision prior to dirstate's parent
+			} else {
+				rev1 = baseRevision;
+			}
+			((HgStatusCollector.Record) inspector).init(rev1, rev2, sc);
 		}
+		final HgIgnore hgIgnore = repo.getIgnore();
+		TreeSet<String> knownEntries = getDirstate().all();
 		repoWalker.reset();
 		final PathPool pp = getPathPool();
 		while (repoWalker.hasNext()) {
@@ -245,7 +258,8 @@
 				// size is the same or unknown, and, perhaps, different timestamp
 				// check actual content to avoid false modified files
 				HgDataFile df = repo.getFileNode(fname);
-				if (!areTheSame(f, df, HgRepository.TIP)) {
+				Nodeid rev = getDirstateParentManifest().nodeid(fname.toString());
+				if (!areTheSame(f, df, rev)) {
 					inspector.modified(df.getPath());
 				} else {
 					inspector.clean(df.getPath());
@@ -304,33 +318,40 @@
 			inspector.added(fname);
 		} else {
 			// was known; check whether clean or modified
-			if ((r = getDirstate().checkNormal(fname)) != null) {
+			Nodeid nidFromDirstate = getDirstateParentManifest().nodeid(fname.toString());
+			if ((r = getDirstate().checkNormal(fname)) != null && nid1.equals(nidFromDirstate)) {
+				// regular file, was the same up to WC initialization. Check if was modified since, and, if not, report right away
+				// same code as in #checkLocalStatusAgainstFile
 				final boolean timestampEqual = getFileModificationTime(f) == r.time, sizeEqual = r.size == f.length();
+				boolean handled = false;
 				if (timestampEqual && sizeEqual) {
 					inspector.clean(fname);
-					baseRevNames.remove(fname.toString()); // consumed, processed, handled.
-					return;
+					handled = true;
 				} else if (!sizeEqual && r.size >= 0) {
 					inspector.modified(fname);
+					handled = true;
+				} else if (!todoCheckFlagsEqual(f, flags)) {
+					// seems like flags have changed, no reason to check content further
+					inspector.modified(fname);
+					handled = true;
+				}
+				if (handled) {
 					baseRevNames.remove(fname.toString()); // consumed, processed, handled.
 					return;
 				}
-				// otherwise, shall check actual content (size not the same, or unknown (-1 or -2), or timestamp is different) 
+				// otherwise, shall check actual content (size not the same, or unknown (-1 or -2), or timestamp is different,
+				// or nodeid in dirstate is different, but local change might have brought it back to baseRevision state)
 				// FALL THROUGH
 			}
-			if (r != null /*Normal dirstate, but needs extra check*/ || (r = getDirstate().checkMerged(fname)) != null || (r = getDirstate().checkAdded(fname)) != null) {
+			if (r != null || (r = getDirstate().checkMerged(fname)) != null || (r = getDirstate().checkAdded(fname)) != null) {
+				// check actual content to see actual changes
 				// when added - seems to be the case of a file added once again, hence need to check if content is different
 				// either clean or modified
-				if (r.size != -1 /*XXX what about ==-2?*/&& r.size != f.length() || !todoCheckFlagsEqual(f, flags)) {
-					inspector.modified(fname);
+				HgDataFile fileNode = repo.getFileNode(fname);
+				if (areTheSame(f, fileNode, nid1)) {
+					inspector.clean(fname);
 				} else {
-					// check actual content to see actual changes
-					HgDataFile fileNode = repo.getFileNode(fname);
-					if (areTheSame(f, fileNode, fileNode.getLocalRevision(nid1))) {
-						inspector.clean(fname);
-					} else {
-						inspector.modified(fname);
-					}
+					inspector.modified(fname);
 				}
 				baseRevNames.remove(fname.toString()); // consumed, processed, handled.
 			} else if (getDirstate().checkRemoved(fname) != null) {
@@ -349,11 +370,12 @@
 		// The question is whether original Hg treats this case (same content, different parents and hence nodeids) as 'modified' or 'clean'
 	}
 
-	private boolean areTheSame(File f, HgDataFile dataFile, int localRevision) {
+	private boolean areTheSame(File f, HgDataFile dataFile, Nodeid revision) {
 		// XXX consider adding HgDataDile.compare(File/byte[]/whatever) operation to optimize comparison
 		ByteArrayChannel bac = new ByteArrayChannel();
 		boolean ioFailed = false;
 		try {
+			int localRevision = dataFile.getLocalRevision(revision);
 			// need content with metadata striped off - although theoretically chances are metadata may be different,
 			// WC doesn't have it anyway 
 			dataFile.content(localRevision, bac);
diff -r 81e9a3c9bafe -r e51dd9a14b6f test/org/tmatesoft/hg/test/TestHistory.java
--- a/test/org/tmatesoft/hg/test/TestHistory.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/test/org/tmatesoft/hg/test/TestHistory.java	Sat Sep 03 01:21:03 2011 +0200
@@ -20,7 +20,6 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertTrue;
 
-import java.io.File;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -75,7 +74,7 @@
 
 	private TestHistory(HgRepository hgRepo) {
 		repo = hgRepo;
-		eh = new ExecHelper(changelogParser = new LogOutputParser(true), new File(repo.getLocation()));
+		eh = new ExecHelper(changelogParser = new LogOutputParser(true), repo.getWorkingDir());
 		
 	}
 
diff -r 81e9a3c9bafe -r e51dd9a14b6f test/org/tmatesoft/hg/test/TestIncoming.java
--- a/test/org/tmatesoft/hg/test/TestIncoming.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/test/org/tmatesoft/hg/test/TestIncoming.java	Sat Sep 03 01:21:03 2011 +0200
@@ -89,7 +89,7 @@
 		cmd.against(hgRemote);
 		HgLogCommand.CollectHandler collector = new HgLogCommand.CollectHandler();
 		LogOutputParser outParser = new LogOutputParser(true);
-		ExecHelper eh = new ExecHelper(outParser, new File(localRepo.getLocation()));
+		ExecHelper eh = new ExecHelper(outParser, localRepo.getWorkingDir());
 		cmd.executeFull(collector);
 		eh.run("hg", "incoming", "--debug", hgRemote.getLocation());
 		List<Nodeid> liteResult = cmd.executeLite();
diff -r 81e9a3c9bafe -r e51dd9a14b6f test/org/tmatesoft/hg/test/TestManifest.java
--- a/test/org/tmatesoft/hg/test/TestManifest.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/test/org/tmatesoft/hg/test/TestManifest.java	Sat Sep 03 01:21:03 2011 +0200
@@ -76,7 +76,7 @@
 	private TestManifest(HgRepository hgRepo) {
 		repo = hgRepo;
 		assertTrue(!repo.isInvalid());
-		eh = new ExecHelper(manifestParser = new ManifestOutputParser(), null);
+		eh = new ExecHelper(manifestParser = new ManifestOutputParser(), repo.getWorkingDir());
 	}
 
 	@Test
diff -r 81e9a3c9bafe -r e51dd9a14b6f test/org/tmatesoft/hg/test/TestStatus.java
--- a/test/org/tmatesoft/hg/test/TestStatus.java	Fri Sep 02 13:59:21 2011 +0200
+++ b/test/org/tmatesoft/hg/test/TestStatus.java	Sat Sep 03 01:21:03 2011 +0200
@@ -22,6 +22,7 @@
 import static org.tmatesoft.hg.core.HgStatus.Kind.*;
 import static org.tmatesoft.hg.repo.HgRepository.TIP;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,6 +32,7 @@
 import java.util.TreeMap;
 
 import org.junit.Assume;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.tmatesoft.hg.core.HgStatus;
@@ -64,6 +66,13 @@
 		test.testStatusCommand();
 		test.testPerformance();
 		test.errorCollector.verify();
+		//
+		TestStatus t2 = new TestStatus(new HgLookup().detect("/temp/hg/hg4j-merging/hg4j"));
+		t2.testDirstateParentOtherThanTip(238);
+		t2.errorCollector.verify();
+		TestStatus t3 = new TestStatus(new HgLookup().detect("/temp/hg/cpython"));
+		t3.testDirstateParentOtherThanTip(-1);
+		t3.errorCollector.verify();
 	}
 	
 	public TestStatus() throws Exception {
@@ -74,7 +83,7 @@
 		repo = hgRepo;
 		Assume.assumeTrue(!repo.isInvalid());
 		statusParser = new StatusOutputParser();
-		eh = new ExecHelper(statusParser, null);
+		eh = new ExecHelper(statusParser, hgRepo.getWorkingDir());
 	}
 	
 	@Test
@@ -104,6 +113,42 @@
 		r = new HgStatusCollector(repo).status(revision, rev2);
 		report("Status -A -rev " + range, r, statusParser);
 	}
+
+	/**
+	 * hg up --rev <earlier rev>; hg status
+	 * 
+	 * To check if HgWorkingCopyStatusCollector respects actual working copy parent (takes from dirstate)
+	 * and if status is calculated correctly 
+	 */
+	@Test
+	@Ignore("modifies test repository, needs careful configuration")
+	public void testDirstateParentOtherThanTip(int revToUpdate) throws Exception {
+		final HgWorkingCopyStatusCollector wcc = new HgWorkingCopyStatusCollector(repo);
+		statusParser.reset();
+		try {
+			if (revToUpdate != -1) {
+				// there are repositories (like cpython) where WC is not tip-based, and no need to
+				// accomplish that artificially 
+				eh.run("hg", "up", "--rev", String.valueOf(revToUpdate));
+			}
+			//
+			eh.run("hg", "status", "-A");
+			HgStatusCollector.Record r = wcc.status(HgRepository.TIP);
+			report("hg status -A", r, statusParser);
+			//
+			statusParser.reset();
+			int revision = 3;
+			eh.run("hg", "status", "-A", "--rev", String.valueOf(revision));
+			r = wcc.status(revision);
+			report("status -A --rev " + revision, r, statusParser);
+		} finally {
+			if (revToUpdate != -1) {
+				// bring the repository to the tip just in case anyone else is using it afterwards
+				eh.run("hg", "up");
+			}
+		}
+	}
+
 	
 	@Test
 	public void testStatusCommand() throws Exception {
@@ -456,9 +501,14 @@
 		errorCollector.checkThat(what + "#COPIED", copyDiff, equalTo(Collections.<Path,String>emptyMap()));
 	}
 	
-	private <T> void reportNotEqual(String what, Collection<T> l1, Collection<T> l2) {
-		List<T> diff = difference(l1, l2);
-		errorCollector.checkThat(what, diff, equalTo(Collections.<T>emptyList()));
+	private <T extends Comparable<? super T>> void reportNotEqual(String what, Collection<T> l1, Collection<T> l2) {
+//		List<T> diff = difference(l1, l2);
+//		errorCollector.checkThat(what, diff, equalTo(Collections.<T>emptyList()));
+		ArrayList<T> sl1 = new ArrayList<T>(l1);
+		Collections.sort(sl1);
+		ArrayList<T> sl2 = new ArrayList<T>(l2);
+		Collections.sort(sl2);
+		errorCollector.checkThat(what, sl1, equalTo(sl2));
 	}
 
 	private static <T> List<T> difference(Collection<T> l1, Collection<T> l2) {