# HG changeset patch # User Artem Tikhomirov # Date 1358447032 -3600 # Node ID f7fbf48b938386d7df6c4a8303f6873a71ef4fdc # Parent 47b7bedf0569d849920703e67deabbda68e70daf Report rename when walking file history regardless of followRenames parameter, solely based on HgFileRenameHandlerMixin presence diff -r 47b7bedf0569 -r f7fbf48b9383 src/org/tmatesoft/hg/core/HgLogCommand.java --- a/src/org/tmatesoft/hg/core/HgLogCommand.java Tue Jan 15 19:46:19 2013 +0100 +++ b/src/org/tmatesoft/hg/core/HgLogCommand.java Thu Jan 17 19:23:52 2013 +0100 @@ -343,9 +343,9 @@ } else { filterInsp.delegateTo(csetTransform); final HgFileRenameHandlerMixin withCopyHandler = Adaptable.Factory.getAdapter(handler, HgFileRenameHandlerMixin.class, null); - List> fileRenames = buildFileRenamesQueue(); - progressHelper.start(-1/*XXX enum const, or a dedicated method startUnspecified(). How about startAtLeast(int)?*/); - + FileRenameQueueBuilder frqBuilder = new FileRenameQueueBuilder(); + List> fileRenames = frqBuilder.buildFileRenamesQueue(); + progressHelper.start(fileRenames.size()); for (int nameIndex = 0, fileRenamesSize = fileRenames.size(); nameIndex < fileRenamesSize; nameIndex++) { Pair curRename = fileRenames.get(nameIndex); HgDataFile fileNode = curRename.first(); @@ -388,7 +388,7 @@ } } } - if (followRenames && withCopyHandler != null && nameIndex + 1 < fileRenamesSize) { + if (withCopyHandler != null && nameIndex + 1 < fileRenamesSize) { Pair nextRename = fileRenames.get(nameIndex+1); HgFileRevision src, dst; // A -> B @@ -404,7 +404,9 @@ } withCopyHandler.copy(src, dst); } + progressHelper.worked(1); } // for renames + frqBuilder.reportRenameIfNotInQueue(fileRenames, withCopyHandler); } // file != null } catch (HgRuntimeException ex) { throw new HgLibraryFailureException(ex); @@ -531,7 +533,8 @@ // renamed files in the queue are placed with respect to #iterateDirection // i.e. if we iterate from new to old, recent filenames come first - List> fileRenamesQueue = buildFileRenamesQueue(); + FileRenameQueueBuilder frqBuilder = new FileRenameQueueBuilder(); + List> fileRenamesQueue = frqBuilder.buildFileRenamesQueue(); // XXX perhaps, makes sense to look at selected file's revision when followAncestry is true // to ensure file we attempt to trace is in the WC's parent. Native hg aborts if not. progressHelper.start(4 * fileRenamesQueue.size()); @@ -541,16 +544,20 @@ dispatcher.prepare(progressHelper, renameInfo); cancelHelper.checkCancelled(); if (namesIndex > 0) { - dispatcher.connectWithLastJunctionPoint(renameInfo, fileRenamesQueue.get(namesIndex - 1), renameHandler); + dispatcher.connectWithLastJunctionPoint(renameInfo, fileRenamesQueue.get(namesIndex - 1)); } if (namesIndex + 1 < renamesQueueSize) { // there's at least one more name we are going to look at - dispatcher.updateJunctionPoint(renameInfo, fileRenamesQueue.get(namesIndex+1)); + dispatcher.updateJunctionPoint(renameInfo, fileRenamesQueue.get(namesIndex+1), renameHandler != null); } else { dispatcher.clearJunctionPoint(); } dispatcher.dispatchAllChanges(); + if (renameHandler != null && namesIndex + 1 < renamesQueueSize) { + dispatcher.reportRenames(renameHandler); + } } // for fileRenamesQueue; + frqBuilder.reportRenameIfNotInQueue(fileRenamesQueue, renameHandler); progressHelper.done(); } @@ -573,54 +580,95 @@ } /** - * Follows file renames and build a list of all corresponding file nodes and revisions they were - * copied/renamed/branched at (IOW, their latest revision to look at). - * - * If {@link #followRenames} is false, the list contains one element only, - * file node with the name of the file as it was specified by the user. - * - * For the most recent file revision depends on {@link #followAncestry}, and is file revision from working copy parent - * in it's true. null indicates file's TIP revision shall be used. - * - * TODO may use HgFileRevision (after some refactoring to accept HgDataFile and Nodeid) instead of Pair - * and possibly reuse this functionality - * - * @return list of file renames, ordered with respect to {@link #iterateDirection} + * Utility to build sequence of file renames */ - private List> buildFileRenamesQueue() throws HgPathNotFoundException { - LinkedList> rv = new LinkedList>(); - Nodeid startRev = null; - HgDataFile fileNode = repo.getFileNode(file); - if (!fileNode.exists()) { - throw new HgPathNotFoundException(String.format("File %s not found in the repository", file), file); - } - if (followAncestry) { - // TODO subject to dedicated method either in HgRepository (getWorkingCopyParentRevisionIndex) - // or in the HgDataFile (getWorkingCopyOriginRevision) - Nodeid wdParentChangeset = repo.getWorkingCopyParents().first(); - if (!wdParentChangeset.isNull()) { - int wdParentRevIndex = repo.getChangelog().getRevisionIndex(wdParentChangeset); - startRev = repo.getManifest().getFileRevision(wdParentRevIndex, fileNode.getPath()); + private class FileRenameQueueBuilder { + + /** + * Follows file renames and build a list of all corresponding file nodes and revisions they were + * copied/renamed/branched at (IOW, their latest revision to look at). + * + * @param followRename when false, the list contains one element only, + * file node with the name of the file as it was specified by the user. + * + * @param followAncestry the most recent file revision reported depends on this parameter, + * and it is file revision from working copy parent in there when it's true. + * null as Pair's second indicates file's TIP revision shall be used. + * + * TODO may use HgFileRevision (after some refactoring to accept HgDataFile and Nodeid) instead of Pair + * and possibly reuse this functionality + * + * @return list of file renames, ordered with respect to {@link #iterateDirection} + */ + public List> buildFileRenamesQueue() throws HgPathNotFoundException { + LinkedList> rv = new LinkedList>(); + Nodeid startRev = null; + HgDataFile fileNode = repo.getFileNode(file); + if (!fileNode.exists()) { + throw new HgPathNotFoundException(String.format("File %s not found in the repository", file), file); } - // else fall-through, assume null (eventually, lastRevision()) is ok here - } - rv.add(new Pair(fileNode, startRev)); - if (!followRenames) { + if (followAncestry) { + // TODO subject to dedicated method either in HgRepository (getWorkingCopyParentRevisionIndex) + // or in the HgDataFile (getWorkingCopyOriginRevision) + Nodeid wdParentChangeset = repo.getWorkingCopyParents().first(); + if (!wdParentChangeset.isNull()) { + int wdParentRevIndex = repo.getChangelog().getRevisionIndex(wdParentChangeset); + startRev = repo.getManifest().getFileRevision(wdParentRevIndex, fileNode.getPath()); + } + // else fall-through, assume null (eventually, lastRevision()) is ok here + } + Pair p = new Pair(fileNode, startRev); + rv.add(p); + if (!followRenames) { + return rv; + } + while (hasOrigin(p)) { + p = origin(p); + if (iterateDirection == HgIterateDirection.OldToNew) { + rv.addFirst(p); + } else { + assert iterateDirection == HgIterateDirection.NewToOld; + rv.addLast(p); + } + }; return rv; } - while (fileNode.isCopy()) { + + public boolean hasOrigin(Pair p) { + return p.first().isCopy(); + } + + public Pair origin(Pair p) { + HgDataFile fileNode = p.first(); + assert fileNode.isCopy(); Path fp = fileNode.getCopySourceName(); Nodeid copyRev = fileNode.getCopySourceRevision(); fileNode = repo.getFileNode(fp); - Pair p = new Pair(fileNode, copyRev); - if (iterateDirection == HgIterateDirection.OldToNew) { - rv.addFirst(p); - } else { - assert iterateDirection == HgIterateDirection.NewToOld; - rv.addLast(p); + return new Pair(fileNode, copyRev); + } + + /** + * Shall report renames based solely on HgFileRenameHandlerMixin presence, + * even if queue didn't get rename information due to followRenames == false + * + * @param queue value from {@link #buildFileRenamesQueue()} + * @param renameHandler may be null + */ + public void reportRenameIfNotInQueue(List> queue, HgFileRenameHandlerMixin renameHandler) throws HgCallbackTargetException { + if (renameHandler != null && !followRenames) { + // If followRenames is true, all the historical names were in the queue and are processed already. + // Hence, shall process origin explicitly only when renameHandler is present but followRenames is not requested. + assert queue.size() == 1; // see the way queue is constructed above + Pair curRename = queue.get(0); + if (hasOrigin(curRename)) { + Pair origin = origin(curRename); + HgFileRevision src, dst; + src = new HgFileRevision(origin.first(), origin.second(), null); + dst = new HgFileRevision(curRename.first(), curRename.first().getRevision(0), src.getPath()); + renameHandler.copy(src, dst); + } } - }; - return rv; + } } private static class TreeBuildInspector implements HgChangelog.ParentInspector, HgChangelog.RevisionInspector { @@ -774,6 +822,8 @@ // node where current file history chunk intersects with same file under other name history // either mock of B(0) or A(k), depending on iteration order private HistoryNode junctionNode; + // initialized when there's HgFileRenameHandlerMixin + private HgFileRevision copiedFrom, copiedTo; // parentProgress shall be initialized with 4 XXX refactor all this stuff with parentProgress public void prepare(ProgressSupport parentProgress, Pair renameInfo) { @@ -804,8 +854,10 @@ // switch to present chunk's file node switchTo(renameInfo.first()); } - - public void updateJunctionPoint(Pair curRename, Pair nextRename) { + + public void updateJunctionPoint(Pair curRename, Pair nextRename, boolean needCopyFromTo) { + copiedFrom = copiedTo = null; + // // A (old) renamed to B(new). A(0..k..n) -> B(0..m). If followAncestry, k == n // curRename.second() points to A(k) if (iterateDirection == HgIterateDirection.OldToNew) { @@ -819,6 +871,10 @@ // Save mock A(k) 1) not to keep whole A history in memory 2) Don't need it's parent and children once get to B // moreover, children of original A(k) (junctionSrc) would list mock B(0) which is undesired once we iterate over real B junctionNode = new HistoryNode(junctionSrc.changeset, junctionSrc.fileRevision, null, null); + if (needCopyFromTo) { + copiedFrom = new HgFileRevision(curRename.first(), junctionNode.fileRevision, null); // "A", A(k) + copiedTo = new HgFileRevision(nextRename.first(), junctionDestMock.fileRevision, copiedFrom.getPath()); // "B", B(0) + } } else { assert iterateDirection == HgIterateDirection.NewToOld; // looking at B chunk (curRename), nextRename points at A @@ -834,14 +890,30 @@ junctionSrcMock.bindChild(junctionDest); // Save mock B(0), for reasons see above for opposite direction junctionNode = new HistoryNode(junctionDest.changeset, junctionDest.fileRevision, null, null); + if (needCopyFromTo) { + copiedFrom = new HgFileRevision(nextRename.first(), junctionSrcMock.fileRevision, null); // "A", A(k) + copiedTo = new HgFileRevision(curRename.first(), junctionNode.fileRevision, copiedFrom.getPath()); // "B", B(0) + } + } + } + + public void reportRenames(HgFileRenameHandlerMixin renameHandler) throws HgCallbackTargetException { + if (renameHandler != null) { // shall report renames + assert copiedFrom != null; + assert copiedTo != null; + renameHandler.copy(copiedFrom, copiedTo); } } public void clearJunctionPoint() { junctionNode = null; + copiedFrom = copiedTo = null; } - public void connectWithLastJunctionPoint(Pair curRename, Pair prevRename, HgFileRenameHandlerMixin renameHandler) throws HgCallbackTargetException { + /** + * Replace mock src/dest HistoryNode connected to junctionNode with a real one + */ + public void connectWithLastJunctionPoint(Pair curRename, Pair prevRename) { assert junctionNode != null; // A renamed to B. A(0..k..n) -> B(0..m). If followAncestry: k == n if (iterateDirection == HgIterateDirection.OldToNew) { @@ -852,11 +924,6 @@ HistoryNode junctionDest = changeHistory.get(0); // B(0) // junctionNode is A(k) junctionNode.bindChild(junctionDest); - if (renameHandler != null) { // shall report renames - HgFileRevision copiedFrom = new HgFileRevision(prevRename.first(), junctionNode.fileRevision, null); // "A", A(k) - HgFileRevision copiedTo = new HgFileRevision(curRename.first(), junctionDest.fileRevision, copiedFrom.getPath()); // "B", B(0) - renameHandler.copy(copiedFrom, copiedTo); - } } else { assert iterateDirection == HgIterateDirection.NewToOld; // changeHistory points to A @@ -865,11 +932,6 @@ // if followAncestry: A(k) is latest in changeHistory (k == n) HistoryNode junctionSrc = findJunctionPointInCurrentChunk(curRename.second()); // A(k) junctionSrc.bindChild(junctionNode); - if (renameHandler != null) { - HgFileRevision copiedFrom = new HgFileRevision(curRename.first(), junctionSrc.fileRevision, null); // "A", A(k) - HgFileRevision copiedTo = new HgFileRevision(prevRename.first(), junctionNode.fileRevision, copiedFrom.getPath()); // "B", B(0) - renameHandler.copy(copiedFrom, copiedTo); - } } } diff -r 47b7bedf0569 -r f7fbf48b9383 test/org/tmatesoft/hg/test/TestHistory.java --- a/test/org/tmatesoft/hg/test/TestHistory.java Tue Jan 15 19:46:19 2013 +0100 +++ b/test/org/tmatesoft/hg/test/TestHistory.java Thu Jan 17 19:23:52 2013 +0100 @@ -38,6 +38,7 @@ import org.tmatesoft.hg.core.HgChangesetTreeHandler; import org.tmatesoft.hg.core.HgFileRenameHandlerMixin; import org.tmatesoft.hg.core.HgFileRevision; +import org.tmatesoft.hg.core.HgIterateDirection; import org.tmatesoft.hg.core.HgLogCommand; import org.tmatesoft.hg.core.HgLogCommand.CollectHandler; import org.tmatesoft.hg.core.Nodeid; @@ -234,7 +235,9 @@ CollectWithRenameHandler h = new CollectWithRenameHandler(); new HgLogCommand(repo).file(fname2, false, true).execute(h); - errorCollector.assertEquals(0, h.rh.renames.size()); + // renames are reported regardless of followRenames parameter, but + // solely based on HgFileRenameHandlerMixin + errorCollector.assertEquals(1, h.rh.renames.size()); report("HgChangesetHandler(renames: false, ancestry:true)", h.getChanges(), fname2Follow, true, errorCollector); // // Direction @@ -252,15 +255,12 @@ final List fname2Follow = getAncestryWithoutRenamesFromCmdline(fname2); TreeCollectHandler h = new TreeCollectHandler(false); - RenameCollector rh = new RenameCollector(h); h.checkPrevInParents = true; new HgLogCommand(repo).file(fname2, false, true).execute(h); - errorCollector.assertEquals(0, rh.renames.size()); report("HgChangesetTreeHandler(renames: false, ancestry:true)", h.getResult(), fname2Follow, true, errorCollector); // Direction h = new TreeCollectHandler(false); - rh = new RenameCollector(h); h.checkPrevInChildren = true; new HgLogCommand(repo).file(fname2, false, true).order(NewToOld).execute(h); report("HgChangesetTreeHandler(renames: false, ancestry:true)", h.getResult(), fname2Follow, false, errorCollector); @@ -351,6 +351,68 @@ assertEquals(1, rh.renames.size()); assertEquals(Path.create(fname), rh.renames.get(0).second().getPath()); } + + /** + * Ensure {@link HgFileRenameHandlerMixin} is always notified, even + * if followRename is false. + * Shall check: + * both {@link HgLogCommand#execute(HgChangesetHandler)} and {@link HgLogCommand#execute(HgChangesetTreeHandler)} + * and for both iteration directions in each case + */ + @Test + public void testRenameHandlerNotifiedEvenIfNotFollowRename() throws Exception { + repo = Configuration.get().find("log-follow"); + final String fname1 = "file1_a"; + final String fname2 = "file1_b"; + final String fnameNoRename = "file2"; + assertTrue("[sanity]", repo.getFileNode(fnameNoRename).exists()); + + // first, check that file without renames doesn't report any accidentally + CollectWithRenameHandler h1 = new CollectWithRenameHandler(); + HgLogCommand cmd = new HgLogCommand(repo).file(fnameNoRename, false, false); + cmd.execute(h1); + errorCollector.assertEquals(0, h1.rh.renames.size()); + TreeCollectHandler h2 = new TreeCollectHandler(false); + RenameCollector rh = new RenameCollector(h2); + cmd.execute(h2); + errorCollector.assertEquals(0, rh.renames.size()); + + // check default iterate direction + cmd = new HgLogCommand(repo).file(fname2, false, false); + cmd.execute(h1 = new CollectWithRenameHandler()); + errorCollector.assertEquals(1, h1.rh.renames.size()); + assertRename(fname1, fname2, h1.rh.renames.get(0)); + + h2 = new TreeCollectHandler(false); + rh = new RenameCollector(h2); + cmd.execute(h2); + errorCollector.assertEquals(1, rh.renames.size()); + assertRename(fname1, fname2, rh.renames.get(0)); + + eh.run("hg", "log", "--debug", fname2, "--cwd", repo.getLocation()); + report("HgChangesetHandler+RenameHandler with followRenames = false, default iteration order", h1.getChanges(), true); + report("HgChangesetTreeHandler+RenameHandler with followRenames = false, default iteration order", h2.getResult(), true); + + // + // Now, check that iteration in opposite direction (new to old) + // still reports renames (and correct revisions, too) + cmd.order(HgIterateDirection.NewToOld); + cmd.execute(h1 = new CollectWithRenameHandler()); + errorCollector.assertEquals(1, h1.rh.renames.size()); + assertRename(fname1, fname2, h1.rh.renames.get(0)); + h2 = new TreeCollectHandler(false); + rh = new RenameCollector(h2); + cmd.execute(h2); + errorCollector.assertEquals(1, rh.renames.size()); + assertRename(fname1, fname2, rh.renames.get(0)); + report("HgChangesetHandler+RenameHandler with followRenames = false, new2old iteration order", h1.getChanges(), false); + report("HgChangesetTreeHandler+RenameHandler with followRenames = false, new2old iteration order", h2.getResult(), false); + } + + private void assertRename(String fnameFrom, String fnameTo, Pair rename) { + errorCollector.assertEquals(fnameFrom, rename.first().getPath().toString()); + errorCollector.assertEquals(fnameTo, rename.second().getPath().toString()); + } /** * @see TestAuxUtilities#testChangelogCancelSupport() @@ -623,7 +685,7 @@ public RenameCollector(AdapterPlug ap) { ap.attachAdapter(HgFileRenameHandlerMixin.class, this); } - + public void copy(HgFileRevision from, HgFileRevision to) { copyReported = true; renames.add(new Pair(from, to));