changeset 690:b286222158be

Fix file.isCopy() use for status and cat commands
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Thu, 01 Aug 2013 21:45:47 +0200
parents 5050ee565bd1
children 72fc7774b87e
files src/org/tmatesoft/hg/core/HgCatCommand.java src/org/tmatesoft/hg/core/HgChangesetFileSneaker.java src/org/tmatesoft/hg/core/HgLibraryFailureException.java src/org/tmatesoft/hg/repo/HgStatusCollector.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java test/org/tmatesoft/hg/test/TestCatCommand.java test/org/tmatesoft/hg/test/TestStatus.java
diffstat 7 files changed, 163 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/core/HgCatCommand.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/src/org/tmatesoft/hg/core/HgCatCommand.java	Thu Aug 01 21:45:47 2013 +0200
@@ -31,6 +31,7 @@
 import org.tmatesoft.hg.util.ByteChannel;
 import org.tmatesoft.hg.util.CancelSupport;
 import org.tmatesoft.hg.util.CancelledException;
+import org.tmatesoft.hg.util.Outcome;
 import org.tmatesoft.hg.util.Path;
 
 /**
@@ -171,23 +172,22 @@
 			}
 			int revToExtract;
 			if (cset != null) {
-				int csetRev = repo.getChangelog().getRevisionIndex(cset);
-				Nodeid toExtract = null;
-				do {
-					// TODO post-1.0 perhaps, HgChangesetFileSneaker may come handy?
-					toExtract = repo.getManifest().getFileRevision(csetRev, file);
-					if (toExtract == null) {
-						if (dataFile.isCopy()) {
-							file = dataFile.getCopySourceName();
-							dataFile = repo.getFileNode(file);
-						} else {
-							break;
-						}
+				// TODO HgChangesetFileSneaker is handy, but bit too much here, shall extract follow rename code into separate utility
+				HgChangesetFileSneaker fsneaker = new HgChangesetFileSneaker(repo);
+				fsneaker.changeset(cset).followRenames(true);
+				Outcome o = fsneaker.check(file);
+				if (!o.isOk()) {
+					if (o.getException() instanceof HgRuntimeException) {
+						throw new HgLibraryFailureException(o.getMessage(), (HgRuntimeException) o.getException());
 					}
-				} while (toExtract == null);
-				if (toExtract == null) {
-					String m = String.format("File %s nor its origins were known at repository's %s revision", file, cset.shortNotation());
-					throw new HgPathNotFoundException(m, file).setRevision(cset);
+					throw new HgBadArgumentException(o.getMessage(), o.getException()).setFileName(file).setRevision(cset);
+				}
+				if (!fsneaker.exists()) {
+					throw new HgPathNotFoundException(o.getMessage(), file).setRevision(cset);
+				}
+				Nodeid toExtract = fsneaker.revision();
+				if (fsneaker.hasAnotherName()) {
+					dataFile = repo.getFileNode(fsneaker.filename());
 				}
 				revToExtract = dataFile.getRevisionIndex(toExtract);
 			} else if (revision != null) {
--- a/src/org/tmatesoft/hg/core/HgChangesetFileSneaker.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/src/org/tmatesoft/hg/core/HgChangesetFileSneaker.java	Thu Aug 01 21:45:47 2013 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2012 TMate Software Ltd
+ * Copyright (c) 2011-2013 TMate Software Ltd
  *  
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,14 +16,18 @@
  */
 package org.tmatesoft.hg.core;
 
+import java.util.ArrayDeque;
+
 import org.tmatesoft.hg.internal.ManifestRevision;
 import org.tmatesoft.hg.repo.HgDataFile;
 import org.tmatesoft.hg.repo.HgInvalidStateException;
 import org.tmatesoft.hg.repo.HgManifest;
+import org.tmatesoft.hg.repo.HgManifest.Flags;
 import org.tmatesoft.hg.repo.HgRepository;
 import org.tmatesoft.hg.repo.HgRuntimeException;
+import org.tmatesoft.hg.util.Outcome;
+import org.tmatesoft.hg.util.Pair;
 import org.tmatesoft.hg.util.Path;
-import org.tmatesoft.hg.util.Outcome;
 
 /**
  * Primary purpose is to provide information about file revisions at specific changeset. Multiple {@link #check(Path)} calls 
@@ -131,7 +135,6 @@
 			return checkResult;
 		}
 		Nodeid toExtract = null;
-		HgManifest.Flags extractRevFlags = null;
 		String phaseMsg = "Extract manifest revision failed";
 		try {
 			if (cachedManifest == null) {
@@ -141,27 +144,59 @@
 				// cachedManifest shall be meaningful - changelog.getRevisionIndex() above ensures we've got version that exists.
 			}
 			toExtract = cachedManifest.nodeid(file);
-			extractRevFlags = cachedManifest.flags(file);
 			phaseMsg = "Follow copy/rename failed";
 			if (toExtract == null && followRenames) {
-				while (toExtract == null && dataFile.isCopy()) {
-					renamed = true;
-					file = dataFile.getCopySourceName();
-					dataFile = repo.getFileNode(file);
-					toExtract = cachedManifest.nodeid(file);
-					extractRevFlags = cachedManifest.flags(file);
+				int csetIndex = repo.getChangelog().getRevisionIndex(cset);
+				int ccFileRevIndex = dataFile.getLastRevision(); // copy candidate
+				int csetFileEnds = dataFile.getChangesetRevisionIndex(ccFileRevIndex);
+				if (csetIndex > csetFileEnds) {
+					return new Outcome(Outcome.Kind.Success, String.format("%s: last known changeset for the file %s is %d. Follow renames is possible towards older changesets only", phaseMsg, file, csetFileEnds));
 				}
+				// XXX code is similar to that in HgStatusCollector#getOriginIfCopy. Why it's different in lastOrigin processing then?
+				// traceback stack keeps record of all files with isCopy(fileRev) == true we've tried to follow, so that we can try earlier file
+				// revisions in case followed fileRev didn't succeed
+				ArrayDeque<Pair<HgDataFile, Integer>> traceback = new ArrayDeque<Pair<HgDataFile, Integer>>();
+				do {
+					int ccCsetIndex = dataFile.getChangesetRevisionIndex(ccFileRevIndex);
+					if (ccCsetIndex <= csetIndex) {
+						// present dataFile is our (distant) origin
+						toExtract = dataFile.getRevision(ccFileRevIndex);
+						renamed = true;
+						break;
+					}
+					if (!dataFile.isCopy(ccFileRevIndex)) {
+						// nothing left to return to when traceback.isEmpty()
+						while (ccFileRevIndex == 0 && !traceback.isEmpty()) {
+							Pair<HgDataFile, Integer> lastTurnPoint = traceback.pop();
+							dataFile = lastTurnPoint.first();
+							ccFileRevIndex = lastTurnPoint.second(); // generally ccFileRevIndex != 0 here, but doesn't hurt to check, hence while
+							// fall through to shift down from the file revision we've already looked at
+						}
+						ccFileRevIndex--;
+						continue;
+					}
+					if (ccFileRevIndex > 0) {
+						// there's no reason to memorize turn point if it's the very first revision
+						// of the file and we won't be able to try any other earlier revision
+						traceback.push(new Pair<HgDataFile, Integer>(dataFile, ccFileRevIndex));
+					}
+					HgFileRevision origin = dataFile.getCopySource(ccFileRevIndex);
+					dataFile = repo.getFileNode(origin.getPath());
+					ccFileRevIndex = dataFile.getRevisionIndex(origin.getRevision());
+				} while (ccFileRevIndex >= 0);
+				// didn't get to csetIndex, no ancestor in file rename history found.
 			}
 		} catch (HgRuntimeException ex) {
 			checkResult = new Outcome(Outcome.Kind.Failure, phaseMsg, ex);
 			return checkResult;
 		}
 		if (toExtract != null) {
+			Flags extractRevFlags = cachedManifest.flags(dataFile.getPath());
 			fileRevision = new HgFileRevision(repo, toExtract, extractRevFlags, dataFile.getPath());
 			checkResult = new Outcome(Outcome.Kind.Success, String.format("File %s, revision %s found at changeset %s", dataFile.getPath(), toExtract.shortNotation(), cset.shortNotation()));
 			return checkResult;
 		} 
-		checkResult = new Outcome(Outcome.Kind.Success, String.format("File %s nor its origins were known at repository %s revision", file, cset.shortNotation()));
+		checkResult = new Outcome(Outcome.Kind.Success, String.format("File %s nor its origins were known at revision %s", file, cset.shortNotation()));
 		return checkResult;
 	}
 	
--- a/src/org/tmatesoft/hg/core/HgLibraryFailureException.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/src/org/tmatesoft/hg/core/HgLibraryFailureException.java	Thu Aug 01 21:45:47 2013 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012 TMate Software Ltd
+ * Copyright (c) 2012-2013 TMate Software Ltd
  *  
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -32,7 +32,12 @@
 		super(cause);
 		assert cause != null;
 	}
-	
+
+	public HgLibraryFailureException(String extraDetails, HgRuntimeException cause) {
+		super(extraDetails, cause);
+		assert cause != null;
+	}
+
 	@Override
 	public HgRuntimeException getCause() {
 		return (HgRuntimeException) super.getCause();
--- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Thu Aug 01 21:45:47 2013 +0200
@@ -298,7 +298,9 @@
 
 		final CancelSupport cs = CancelSupport.Factory.get(inspector);
 
-		TreeSet<Path> r1Files = new TreeSet<Path>(r1.files());
+		
+		Collection<Path> allBaseFiles = r1.files();
+		TreeSet<Path> r1Files = new TreeSet<Path>(allBaseFiles);
 		for (Path r2fname : r2.files()) {
 			if (!scope.accept(r2fname)) {
 				continue;
@@ -317,7 +319,7 @@
 			} else {
 				try {
 					Path copyTarget = r2fname;
-					Path copyOrigin = detectCopies ? getOriginIfCopy(repo, copyTarget, r2.nodeid(copyTarget), r1Files, rev1) : null;
+					Path copyOrigin = detectCopies ? getOriginIfCopy(repo, copyTarget, r2.nodeid(copyTarget), allBaseFiles, rev1) : null;
 					if (copyOrigin != null) {
 						inspector.copied(getPathPool().mangle(copyOrigin) /*pipe through pool, just in case*/, copyTarget);
 					} else {
@@ -362,6 +364,9 @@
 		return rv;
 	}
 	
+	// originals shall include all names known in base revision, not those not yet consumed
+	// see TestStatus#testDetectRenamesInNonFirstRev and log-renames test repository
+	// where a and d in r5 are compared to a and b in r1, while d is in fact descendant of original a, and a is original b (through c)
 	/*package-local*/static Path getOriginIfCopy(HgRepository hgRepo, Path fname, Nodeid fnameRev, Collection<Path> originals, int originalChangesetIndex) throws HgRuntimeException {
 		HgDataFile df = hgRepo.getFileNode(fname);
 		if (!df.exists()) {
@@ -380,8 +385,9 @@
 			int csetRevIndex = df.getChangesetRevisionIndex(fileRevIndex);
 			if (csetRevIndex <= originalChangesetIndex) {
 				// we've walked past originalChangelogRevIndex and no chances we'll find origin
-				// if we get here, it means fname's origin is not from the base revision 
-				return null;
+				// if we get here, it means either fname's origin is not from the base revision
+				// or the last found rename is still valid
+				return lastOriginFound;
 			}
 			HgFileRevision origin = df.getCopySource(fileRevIndex);
 			// prepare for the next step, df(copyFromFileRev) would point to copy origin and its revision
@@ -400,6 +406,9 @@
 				// origin wasn't renamed once more in [originalChangesetIndex..copyFromCsetIndex]
 				lastOriginFound = origin.getPath();
 				// FALL-THROUGH
+			} else {
+				// clear last known origin if the file was renamed once again to something we don't have in base
+				lastOriginFound = null;
 			}
 			// try more steps away
 			// copyFromFileRev or one of its predecessors might be copies as well
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Thu Aug 01 21:45:47 2013 +0200
@@ -433,7 +433,7 @@
 							continue;
 						}
 						// see if file revision known in this parent got copied from one of baseRevNames
-						Path origin = HgStatusCollector.getOriginIfCopy(repo, fname, fileRev, baseRevNames, baseRevision);
+						Path origin = HgStatusCollector.getOriginIfCopy(repo, fname, fileRev, collect.files(), baseRevision);
 						if (origin != null) {
 							inspector.copied(getPathPool().mangle(origin), fname);
 							return;
--- a/test/org/tmatesoft/hg/test/TestCatCommand.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestCatCommand.java	Thu Aug 01 21:45:47 2013 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012 TMate Software Ltd
+ * Copyright (c) 2012-2013 TMate Software Ltd
  *  
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -18,6 +18,7 @@
 
 import static org.junit.Assert.*;
 
+import org.junit.Rule;
 import org.junit.Test;
 import org.tmatesoft.hg.core.HgCatCommand;
 import org.tmatesoft.hg.core.HgChangesetFileSneaker;
@@ -34,7 +35,10 @@
  */
 public class TestCatCommand {
 	
-	private final HgRepository repo;
+	@Rule
+	public ErrorCollectorExt errorCollector = new ErrorCollectorExt();
+
+	private HgRepository repo;
 	
 	public TestCatCommand() throws Exception {
 		repo = new HgLookup().detectFromWorkingDir();
@@ -64,4 +68,35 @@
 		assertEquals(result1, result2);
 	}
 
+	// ensure code to follow rename history in the command is correct
+	@Test
+	public void testRenamedFileInCset() throws Exception {
+		repo = Configuration.get().find("log-renames");
+		HgCatCommand cmd1 = new HgCatCommand(repo);
+		HgCatCommand cmd2 = new HgCatCommand(repo);
+		cmd1.file(Path.create("a")); // a is initial b through temporary c
+		cmd2.file(Path.create("c"));
+		ByteArrayChannel sink1, sink2;
+		// a from wc/tip was c in rev 4
+		cmd1.changeset(4).execute(sink1 = new ByteArrayChannel());
+		cmd2.changeset(4).execute(sink2 = new ByteArrayChannel());
+		assertArrayEquals(sink2.toArray(), sink1.toArray());
+		//
+		// d from wc/tip was a in 0..2 and b in rev 3..4. Besides, there's another d in r4 
+		cmd2.file(Path.create("d"));
+		cmd1.changeset(2).execute(sink1 = new ByteArrayChannel());
+		cmd2.changeset(2).execute(sink2 = new ByteArrayChannel());
+		assertArrayEquals(sink1.toArray(), sink2.toArray());
+		// 
+		cmd1.file(Path.create("b"));
+		cmd1.changeset(3).execute(sink1 = new ByteArrayChannel());
+		cmd2.changeset(3).execute(sink2 = new ByteArrayChannel());
+		assertArrayEquals(sink1.toArray(), sink2.toArray());
+		//
+		cmd2.changeset(4).execute(sink2 = new ByteArrayChannel()); // ensure d in r4 is not from a or b
+		assertArrayEquals("d:4\n".getBytes(), sink2.toArray());
+		cmd2.changeset(5).execute(sink2 = new ByteArrayChannel()); // d in r5 is copy of b in r4
+		cmd1.changeset(4).execute(sink1 = new ByteArrayChannel());
+		assertArrayEquals(sink1.toArray(), sink2.toArray());
+	}
 }
--- a/test/org/tmatesoft/hg/test/TestStatus.java	Sat Jul 27 22:06:14 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestStatus.java	Thu Aug 01 21:45:47 2013 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2012 TMate Software Ltd
+ * Copyright (c) 2011-2013 TMate Software Ltd
  *  
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -17,8 +17,7 @@
 package org.tmatesoft.hg.test;
 
 import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 import static org.tmatesoft.hg.core.HgStatus.Kind.*;
 import static org.tmatesoft.hg.repo.HgRepository.TIP;
 import static org.tmatesoft.hg.repo.HgRepository.WORKING_COPY;
@@ -658,6 +657,47 @@
 		assertEquals(Path.create("skip/a"), ignored.get(0));
 		assertTrue(sc.get(Path.create("s1/b")).isEmpty());
 	}
+	
+	@Test
+	public void testDetectRenamesInNonFirstRev() throws Exception {
+		repo = Configuration.get().find("log-renames");
+		eh.cwd(repo.getWorkingDir());
+		final HgStatusCommand cmd = new HgStatusCommand(repo).defaults();
+		StatusCollector sc;
+		for (int r : new int[] {2,3,4}) {
+			statusParser.reset();
+			eh.run("hg", "status", "-C", "--change", String.valueOf(r));
+			cmd.change(r).execute(sc = new StatusCollector());
+			sr.report("hg status -C --change " + r, sc);
+		}
+		// a and d from r5 are missing in r3
+		statusParser.reset();
+		eh.run("hg", "status", "-C", "--rev", "3", "--rev", "5");
+		cmd.base(3).revision(5).execute(sc = new StatusCollector());
+		sr.report("hg status -C 3..5 ", sc);
+		//
+		// a is c which is initially b
+		// d is b which is initially a
+		Path fa = Path.create("a"); 
+		Path fb = Path.create("b");
+		Path fc = Path.create("c");
+		Path fd = Path.create("d");
+		// neither initial a nor b have isCopy(() == true
+		assertFalse("[sanity]", repo.getFileNode(fa).isCopy());
+		// check HgStatusCollector
+		// originals (base revision) doesn't contain first copy origin (there's no b in r2)
+		cmd.base(2).revision(5).execute(sc = new StatusCollector());
+		errorCollector.assertEquals(fa, sc.new2oldName.get(fd));
+		errorCollector.assertEquals(Collections.singletonList(Removed), sc.get(fc));
+		// ensure same result with HgWorkingCopyStatusCollector
+		cmd.base(2).revision(WORKING_COPY).execute(sc = new StatusCollector());
+		errorCollector.assertEquals(fa, sc.new2oldName.get(fd));
+		errorCollector.assertEquals(Collections.singletonList(Removed), sc.get(fc));
+		// originals (base revision) does contain first copy origin (b is in r1)
+		cmd.base(1).revision(5).execute(sc = new StatusCollector());
+		errorCollector.assertEquals(fa, sc.new2oldName.get(fd));
+		errorCollector.assertEquals(Collections.singletonList(Removed), sc.get(fb));
+	}
 
 	/*
 	 * With warm-up of previous tests, 10 runs, time in milliseconds