# HG changeset patch # User Artem Tikhomirov # Date 1369149862 -7200 # Node ID b4948b159ab1ae4491fe1a34ac9162270e8d5573 # Parent 507602cb4fb3044dc4f05ffa4c384c369de8dde9 Refactor internals of blame support, tests diff -r 507602cb4fb3 -r b4948b159ab1 src/org/tmatesoft/hg/internal/BlameHelper.java --- a/src/org/tmatesoft/hg/internal/BlameHelper.java Mon May 20 20:34:33 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/BlameHelper.java Tue May 21 17:24:22 2013 +0200 @@ -16,6 +16,7 @@ */ package org.tmatesoft.hg.internal; +import static org.tmatesoft.hg.core.HgIterateDirection.OldToNew; import static org.tmatesoft.hg.repo.HgRepository.NO_REVISION; import java.util.LinkedList; @@ -48,15 +49,38 @@ private final HgBlameInspector insp; private FileLinesCache linesCache; - // FIXME exposing internals (use of FileLinesCache through cons arg and #useFileUpTo) smells bad, refactor! - - public BlameHelper(HgBlameInspector inspector, int cacheHint) { + public BlameHelper(HgBlameInspector inspector) { insp = inspector; - linesCache = new FileLinesCache(cacheHint); } - - public void useFileUpTo(HgDataFile df, int clogRevIndex) { - linesCache.useFileUpTo(df, clogRevIndex); + + /** + * Build history of the file for the specified range (follow renames if necessary). This history + * is used to access various file revision data during subsequent {@link #diff(int, int, int, int)} and + * {@link #annotateChange(int, int, int[], int[])} calls. Callers can use returned history for own approaches + * to iteration over file history. + + *

NOTE, clogRevIndexEnd has to list name of the supplied file in the corresponding manifest, + * as it's not possible to trace rename history otherwise. + */ + public FileHistory prepare(HgDataFile df, int clogRevIndexStart, int clogRevIndexEnd) { + assert clogRevIndexStart <= clogRevIndexEnd; + FileHistory fileHistory = new FileHistory(df, clogRevIndexStart, clogRevIndexEnd); + fileHistory.build(); + int cacheHint = 5; // cache comes useful when we follow merge branches and don't want to + // parse base revision twice. There's no easy way to determine max(distance(all(base,merge))), + // hence the heuristics to use the longest history chunk: + for (FileRevisionHistoryChunk c : fileHistory.iterate(OldToNew)) { + // iteration order is not important here + if (c.revisionCount() > cacheHint) { + cacheHint = c.revisionCount(); + } + } + linesCache = new FileLinesCache(cacheHint); + for (FileRevisionHistoryChunk fhc : fileHistory.iterate(OldToNew)) { + // iteration order is not important here + linesCache.useFileUpTo(fhc.getFile(), fhc.getEndChangeset()); + } + return fileHistory; } // NO_REVISION is not allowed as any argument @@ -116,6 +140,9 @@ private final int limit; private final LinkedList> files; // TODO in fact, need sparse array + /** + * @param lruLimit how many parsed file revisions to keep + */ public FileLinesCache(int lruLimit) { limit = lruLimit; lruCache = new LinkedList>(); diff -r 507602cb4fb3 -r b4948b159ab1 src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java --- a/src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java Mon May 20 20:34:33 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java Tue May 21 17:24:22 2013 +0200 @@ -163,6 +163,13 @@ return rv; } + /** + * @return number of file revisions in this chunk of its history + */ + public int revisionCount() { + return fileRevsToVisit.size(); + } + public int changeset(int fileRevIndex) { return file2changelog[fileRevIndex]; } diff -r 507602cb4fb3 -r b4948b159ab1 src/org/tmatesoft/hg/repo/HgDataFile.java --- a/src/org/tmatesoft/hg/repo/HgDataFile.java Mon May 20 20:34:33 2013 +0200 +++ b/src/org/tmatesoft/hg/repo/HgDataFile.java Tue May 21 17:24:22 2013 +0200 @@ -16,7 +16,6 @@ */ package org.tmatesoft.hg.repo; -import static org.tmatesoft.hg.core.HgIterateDirection.OldToNew; import static org.tmatesoft.hg.repo.HgInternals.wrongRevisionIndex; import static org.tmatesoft.hg.repo.HgRepository.*; import static org.tmatesoft.hg.util.LogFacility.Severity.Info; @@ -430,12 +429,10 @@ * mimic 'hg diff -r clogRevIndex1 -r clogRevIndex2' */ public void diff(int clogRevIndex1, int clogRevIndex2, HgBlameInspector insp) throws HgCallbackTargetException { - // FIXME clogRevIndex1 and clogRevIndex2 may point to different files, need to decide whether to throw an exception - // or to attempt to look up correct file node (tricky) int fileRevIndex1 = fileRevIndex(this, clogRevIndex1); int fileRevIndex2 = fileRevIndex(this, clogRevIndex2); - BlameHelper bh = new BlameHelper(insp, 5); - bh.useFileUpTo(this, clogRevIndex2); + BlameHelper bh = new BlameHelper(insp); + bh.prepare(this, clogRevIndex1, clogRevIndex2); bh.diff(fileRevIndex1, clogRevIndex1, fileRevIndex2, clogRevIndex2); } @@ -463,13 +460,9 @@ if (!exists()) { return; } - FileHistory fileHistory = new FileHistory(this, changelogRevIndexStart, changelogRevIndexEnd); - fileHistory.build(); - BlameHelper bh = new BlameHelper(insp, 10); - for (FileRevisionHistoryChunk fhc : fileHistory.iterate(OldToNew)) { - // iteration order is not important here - bh.useFileUpTo(fhc.getFile(), fhc.getEndChangeset()); - } + BlameHelper bh = new BlameHelper(insp); + FileHistory fileHistory = bh.prepare(this, changelogRevIndexStart, changelogRevIndexEnd); + int[] fileClogParentRevs = new int[2]; int[] fileParentRevs = new int[2]; for (FileRevisionHistoryChunk fhc : fileHistory.iterate(iterateOrder)) { @@ -498,11 +491,12 @@ if (changelogRevisionIndex == TIP) { changelogRevisionIndex = getChangesetRevisionIndex(fileRevIndex); } - BlameHelper bh = new BlameHelper(insp, 5); - bh.useFileUpTo(this, changelogRevisionIndex); int[] fileClogParentRevs = new int[2]; fileClogParentRevs[0] = fileRevParents[0] == NO_REVISION ? NO_REVISION : getChangesetRevisionIndex(fileRevParents[0]); fileClogParentRevs[1] = fileRevParents[1] == NO_REVISION ? NO_REVISION : getChangesetRevisionIndex(fileRevParents[1]); + BlameHelper bh = new BlameHelper(insp); + int clogIndexStart = fileClogParentRevs[0] == NO_REVISION ? (fileClogParentRevs[1] == NO_REVISION ? 0 : fileClogParentRevs[1]) : fileClogParentRevs[0]; + bh.prepare(this, clogIndexStart, changelogRevisionIndex); bh.annotateChange(fileRevIndex, changelogRevisionIndex, fileRevParents, fileClogParentRevs); } diff -r 507602cb4fb3 -r b4948b159ab1 test/org/tmatesoft/hg/test/TestBlame.java --- a/test/org/tmatesoft/hg/test/TestBlame.java Mon May 20 20:34:33 2013 +0200 +++ b/test/org/tmatesoft/hg/test/TestBlame.java Tue May 21 17:24:22 2013 +0200 @@ -45,12 +45,14 @@ import org.tmatesoft.hg.core.HgCallbackTargetException; import org.tmatesoft.hg.core.HgIterateDirection; import org.tmatesoft.hg.core.HgRepoFacade; +import org.tmatesoft.hg.core.Nodeid; import org.tmatesoft.hg.internal.FileAnnotation; import org.tmatesoft.hg.internal.FileAnnotation.LineDescriptor; import org.tmatesoft.hg.internal.FileAnnotation.LineInspector; import org.tmatesoft.hg.internal.IntVector; import org.tmatesoft.hg.repo.HgBlameInspector; import org.tmatesoft.hg.repo.HgBlameInspector.BlockData; +import org.tmatesoft.hg.repo.HgChangelog; import org.tmatesoft.hg.repo.HgDataFile; import org.tmatesoft.hg.repo.HgLookup; import org.tmatesoft.hg.repo.HgRepository; @@ -71,7 +73,7 @@ public void testSingleParentBlame() throws Exception { HgRepository repo = new HgLookup().detectFromWorkingDir(); final String fname = "src/org/tmatesoft/hg/internal/PatchGenerator.java"; - final int checkChangeset = 539; + final int checkChangeset = repo.getChangelog().getRevisionIndex(Nodeid.fromAscii("946b131962521f9199e1fedbdc2487d3aaef5e46")); // 539 HgDataFile df = repo.getFileNode(fname); ByteArrayOutputStream bos = new ByteArrayOutputStream(); df.annotateSingleRevision(checkChangeset, new DiffOutInspector(new PrintStream(bos))); @@ -91,7 +93,12 @@ HgDataFile df = repo.getFileNode(fname); AnnotateRunner ar = new AnnotateRunner(df.getPath(), null); - for (int cs : new int[] { 539, 541 /*, TIP */}) { + final HgChangelog clog = repo.getChangelog(); + final int[] toTest = new int[] { + clog.getRevisionIndex(Nodeid.fromAscii("946b131962521f9199e1fedbdc2487d3aaef5e46")), // 539 + clog.getRevisionIndex(Nodeid.fromAscii("1e95f48d9886abe79b9711ab371bc877ca5e773e")), // 541 + /*, TIP */}; + for (int cs : toTest) { ar.run(cs, false); FileAnnotateInspector fa = new FileAnnotateInspector(); FileAnnotation.annotate(df, cs, fa); @@ -254,6 +261,45 @@ errorCollector.assertEquals(hgAnnotateLine.trim(), apiLine); } } + + + @Test + public void testDiffTwoRevisions() throws Exception { + HgRepository repo = Configuration.get().find("test-annotate"); + HgDataFile df = repo.getFileNode("file1"); + LineGrepOutputParser gp = new LineGrepOutputParser("^@@.+"); + ExecHelper eh = new ExecHelper(gp, repo.getWorkingDir()); + int[] toTest = { 3, 4, 5 }; // p1 ancestry line, p2 ancestry line, not in ancestry line + for (int cs : toTest) { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + df.diff(cs, 8, new DiffOutInspector(new PrintStream(bos))); + eh.run("hg", "diff", "-r", String.valueOf(cs), "-r", "8", "-U", "0", df.getPath().toString()); + // + String[] apiResult = splitLines(bos.toString()); + String[] expected = splitLines(gp.result()); + Assert.assertArrayEquals(expected, apiResult); + gp.reset(); + } + } + + /** + * Make sure boundary values are ok (down to BlameHelper#prepare and FileHistory) + */ + @Test + public void testAnnotateFirstFileRev() throws Exception { + HgRepository repo = Configuration.get().find("test-annotate"); + HgDataFile df = repo.getFileNode("file1"); + LineGrepOutputParser gp = new LineGrepOutputParser("^@@.+"); + ExecHelper eh = new ExecHelper(gp, repo.getWorkingDir()); + eh.run("hg", "diff", "-c", "0", "-U", "0", df.getPath().toString()); + // + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + df.annotateSingleRevision(0, new DiffOutInspector(new PrintStream(bos))); + // + String[] apiResult = splitLines(bos.toString()); + String[] expected = splitLines(gp.result()); + Assert.assertArrayEquals(expected, apiResult); + } // TODO HgWorkingCopyStatusCollector (and HgStatusCollector), with their ancestors (rev 59/69) have examples // of *incorrect* assignment of common lines (like "}") - our impl doesn't process common lines in any special way @@ -285,47 +331,6 @@ } - private void aaa() throws Exception { - HgRepository repo = new HgLookup().detectFromWorkingDir(); - final String fname = "src/org/tmatesoft/hg/internal/PatchGenerator.java"; - final int checkChangeset = 539; - HgDataFile df = repo.getFileNode(fname); - DiffOutInspector dump = new DiffOutInspector(System.out); - System.out.println("541 -> 543"); - df.annotateSingleRevision(543, dump); - System.out.println("539 -> 541"); - df.annotateSingleRevision(541, dump); - System.out.println("536 -> 539"); - df.annotateSingleRevision(checkChangeset, dump); - System.out.println("531 -> 536"); - df.annotateSingleRevision(536, dump); - System.out.println(" -1 -> 531"); - df.annotateSingleRevision(531, dump); - - FileAnnotateInspector fai = new FileAnnotateInspector(); - FileAnnotation.annotate(df, 541, fai); - for (int i = 0; i < fai.lineRevisions.length; i++) { - System.out.printf("%3d: LINE %d\n", fai.lineRevisions[i], i+1); - } - } - - private void bbb() throws Exception { - HgRepository repo = new HgLookup().detectFromWorkingDir(); - final String fname = "src/org/tmatesoft/hg/repo/HgManifest.java"; - final int checkChangeset = 415; - HgDataFile df = repo.getFileNode(fname); - DiffOutInspector dump = new DiffOutInspector(System.out); -// System.out.println("413 -> 415"); -// af.diff(df, 413, 415, dump); -// System.out.println("408 -> 415"); -// af.diff(df, 408, 415, dump); -// System.out.println("Combined (with merge):"); -// dump.needRevisions(true); -// af.annotateChange(df, checkChangeset, dump); - dump.needRevisions(true); - df.annotate(checkChangeset, dump, HgIterateDirection.OldToNew); - } - private void ccc() throws Throwable { HgRepository repo = new HgLookup().detect("/home/artem/hg/hgtest-annotate-merge/"); HgDataFile df = repo.getFileNode("file.txt"); @@ -354,10 +359,6 @@ } public static void main(String[] args) throws Throwable { -// System.out.println(Arrays.equals(new String[0], splitLines(""))); -// System.out.println(Arrays.equals(new String[] { "abc" }, splitLines("abc"))); -// System.out.println(Arrays.equals(new String[] { "a", "bc" }, splitLines("a\nbc"))); -// System.out.println(Arrays.equals(new String[] { "a", "bc" }, splitLines("a\nbc\n"))); TestBlame tt = new TestBlame(); tt.ccc(); }