Mercurial > jhg
changeset 282:e51dd9a14b6f
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)
author | Artem Tikhomirov <tikhomirov.artem@gmail.com> |
---|---|
date | Sat, 03 Sep 2011 01:21:03 +0200 |
parents | 81e9a3c9bafe |
children | 7a8e1a305a78 |
files | src/org/tmatesoft/hg/repo/HgStatusCollector.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java test/org/tmatesoft/hg/test/TestHistory.java test/org/tmatesoft/hg/test/TestIncoming.java test/org/tmatesoft/hg/test/TestManifest.java test/org/tmatesoft/hg/test/TestStatus.java |
diffstat | 6 files changed, 131 insertions(+), 61 deletions(-) [+] |
line wrap: on
line diff
--- 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 ;
--- 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);
--- 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()); }
--- 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();
--- 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
--- 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) {