changeset 275:6d1804fe0ed7

Issue 10: Report file content length with respect of metadata. Respect dirstate parents for WC's status. Exceptions to keep useful attributes of the location
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Thu, 25 Aug 2011 21:35:03 +0200
parents 9fb50c04f03c
children 6355ecda1f08
files cmdline/org/tmatesoft/hg/console/Main.java src/org/tmatesoft/hg/core/HgCallbackTargetException.java src/org/tmatesoft/hg/core/HgDataStreamException.java src/org/tmatesoft/hg/core/HgException.java src/org/tmatesoft/hg/core/HgIncomingCommand.java src/org/tmatesoft/hg/core/HgLogCommand.java src/org/tmatesoft/hg/core/HgOutgoingCommand.java src/org/tmatesoft/hg/core/HgStatusCommand.java src/org/tmatesoft/hg/repo/HgDataFile.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java
diffstat 10 files changed, 166 insertions(+), 98 deletions(-) [+]
line wrap: on
line diff
--- a/cmdline/org/tmatesoft/hg/console/Main.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/cmdline/org/tmatesoft/hg/console/Main.java	Thu Aug 25 21:35:03 2011 +0200
@@ -25,6 +25,7 @@
 import java.util.Map;
 
 import org.junit.Assert;
+import org.tmatesoft.hg.core.HgDataStreamException;
 import org.tmatesoft.hg.core.HgLogCommand;
 import org.tmatesoft.hg.core.HgCatCommand;
 import org.tmatesoft.hg.core.HgFileInformer;
@@ -445,7 +446,7 @@
 	}
 
 
-	private void testStatusInternals() {
+	private void testStatusInternals() throws HgDataStreamException {
 		HgDataFile n = hgRepo.getFileNode(Path.create("design.txt"));
 		for (String s : new String[] {"011dfd44417c72bd9e54cf89b82828f661b700ed", "e5529faa06d53e06a816e56d218115b42782f1ba", "c18e7111f1fc89a80a00f6a39d51288289a382fc"}) {
 			// expected: 359, 2123, 3079
--- a/src/org/tmatesoft/hg/core/HgCallbackTargetException.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgCallbackTargetException.java	Thu Aug 25 21:35:03 2011 +0200
@@ -16,10 +16,9 @@
  */
 package org.tmatesoft.hg.core;
 
-import static org.tmatesoft.hg.repo.HgRepository.BAD_REVISION;
+import org.tmatesoft.hg.util.Path;
 
-import org.tmatesoft.hg.repo.HgRepository;
-import org.tmatesoft.hg.util.Path;
+
 
 /**
  * Checked exception that indicates errors in client code and tries to supply extra information about the context it occured in.
@@ -29,10 +28,6 @@
  */
 @SuppressWarnings("serial")
 public class HgCallbackTargetException extends HgException {
-	private int revNumber = BAD_REVISION;
-	private Nodeid revision;
-	private Path filename;
-
 	/**
 	 * @param cause can't be <code>null</code>
 	 */
@@ -49,42 +44,6 @@
 		}
 	}
 
-	/**
-	 * @return not {@link HgRepository#BAD_REVISION} only when local revision number was supplied at the construction time
-	 */
-	public int getRevisionNumber() {
-		return revNumber;
-	}
-
-	public HgCallbackTargetException setRevisionNumber(int rev) {
-		revNumber = rev;
-		return this;
-	}
-
-	/**
-	 * @return non-null only when revision was supplied at construction time
-	 */
-	public Nodeid getRevision() {
-		return revision;
-	}
-
-	public HgCallbackTargetException setRevision(Nodeid r) {
-		revision = r;
-		return this;
-	}
-
-	/**
-	 * @return non-null only if file name was set at construction time
-	 */
-	public Path getFileName() {
-		return filename;
-	}
-
-	public HgCallbackTargetException setFileName(Path name) {
-		filename = name;
-		return this;
-	}
-
 	@SuppressWarnings("unchecked")
 	public <T extends Exception> T getTargetException() {
 		return (T) getCause();
@@ -97,23 +56,23 @@
 	@Override
 	public String getMessage() {
 		StringBuilder sb = new StringBuilder();
-		if (filename != null) {
-			sb.append(filename);
-			sb.append(':');
-			sb.append(' ');
-		}
-		if (revNumber != BAD_REVISION) {
-			sb.append(revNumber);
-			if (revision != null) {
-				sb.append(':');
-			}
-		}
-		if (revision != null) {
-			sb.append(revision.shortNotation());
-		}
+		appendDetails(sb);
 		return sb.toString();
 	}
 
+	@Override
+	public HgCallbackTargetException setRevision(Nodeid r) {
+		return (HgCallbackTargetException) super.setRevision(r);
+	}
+	@Override
+	public HgCallbackTargetException setRevisionNumber(int rev) {
+		return (HgCallbackTargetException) super.setRevisionNumber(rev);
+	}
+	@Override
+	public HgCallbackTargetException setFileName(Path name) {
+		return (HgCallbackTargetException) super.setFileName(name);
+	}
+
 	/**
 	 * Given the approach high-level handlers throw RuntimeExceptions to indicate errors, and
 	 * a need to throw reasonable checked exception from client code, clients may utilize this class
--- a/src/org/tmatesoft/hg/core/HgDataStreamException.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgDataStreamException.java	Thu Aug 25 21:35:03 2011 +0200
@@ -27,19 +27,28 @@
  */
 @SuppressWarnings("serial")
 public class HgDataStreamException extends HgException {
-	private final Path fname;
 
 	public HgDataStreamException(Path file, String message, Throwable cause) {
 		super(message, cause);
-		fname = file;
+		setFileName(file);
 	}
 	
 	public HgDataStreamException(Path file, Throwable cause) {
 		super(cause);
-		fname = file;
+		setFileName(file);
 	}
 
-	public Path getFileName() {
-		return fname;
+	@Override
+	public HgDataStreamException setRevision(Nodeid r) {
+		return (HgDataStreamException) super.setRevision(r);
+	}
+	
+	@Override
+	public HgDataStreamException setRevisionNumber(int rev) {
+		return (HgDataStreamException) super.setRevisionNumber(rev);
+	}
+	@Override
+	public HgDataStreamException setFileName(Path name) {
+		return (HgDataStreamException) super.setFileName(name);
 	}
 }
--- a/src/org/tmatesoft/hg/core/HgException.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgException.java	Thu Aug 25 21:35:03 2011 +0200
@@ -16,6 +16,11 @@
  */
 package org.tmatesoft.hg.core;
 
+import static org.tmatesoft.hg.repo.HgRepository.BAD_REVISION;
+
+import org.tmatesoft.hg.repo.HgRepository;
+import org.tmatesoft.hg.util.Path;
+
 /**
  * Root class for all hg4j exceptions.
  * 
@@ -25,6 +30,10 @@
 @SuppressWarnings("serial")
 public class HgException extends Exception {
 
+	protected int revNumber = BAD_REVISION;
+	protected Nodeid revision;
+	protected Path filename;
+
 	public HgException(String reason) {
 		super(reason);
 	}
@@ -37,6 +46,59 @@
 		super(cause);
 	}
 
+	/**
+	 * @return not {@link HgRepository#BAD_REVISION} only when local revision number was supplied at the construction time
+	 */
+	public int getRevisionNumber() {
+		return revNumber;
+	}
+
+	public HgException setRevisionNumber(int rev) {
+		revNumber = rev;
+		return this;
+	}
+
+	/**
+	 * @return non-null only when revision was supplied at construction time
+	 */
+	public Nodeid getRevision() {
+		return revision;
+	}
+
+	public HgException setRevision(Nodeid r) {
+		revision = r;
+		return this;
+	}
+
+	/**
+	 * @return non-null only if file name was set at construction time
+	 */
+	public Path getFileName() {
+		return filename;
+	}
+
+	public HgException setFileName(Path name) {
+		filename = name;
+		return this;
+	}
+	
+	protected void appendDetails(StringBuilder sb) {
+		if (filename != null) {
+			sb.append(filename);
+			sb.append(':');
+			sb.append(' ');
+		}
+		if (revNumber != BAD_REVISION) {
+			sb.append(revNumber);
+			if (revision != null) {
+				sb.append(':');
+			}
+		}
+		if (revision != null) {
+			sb.append(revision.shortNotation());
+		}
+	}
+
 //	/* XXX CONSIDER capability to pass extra information about errors */
 //	public static class Status {
 //		public Status(String message, Throwable cause, int errorCode, Object extraData) {
--- a/src/org/tmatesoft/hg/core/HgIncomingCommand.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgIncomingCommand.java	Thu Aug 25 21:35:03 2011 +0200
@@ -124,7 +124,7 @@
 	 * @throws HgException
 	 * @throws CancelledException
 	 */
-	public void executeFull(final HgChangesetHandler handler) throws HgException/*FIXME specific type*/, HgCallbackTargetException, CancelledException {
+	public void executeFull(final HgChangesetHandler handler) throws HgException/*FIXME specific type*/, HgException, CancelledException {
 		if (handler == null) {
 			throw new IllegalArgumentException("Delegate can't be null");
 		}
--- a/src/org/tmatesoft/hg/core/HgLogCommand.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgLogCommand.java	Thu Aug 25 21:35:03 2011 +0200
@@ -179,7 +179,7 @@
 		CollectHandler collector = new CollectHandler();
 		try {
 			execute(collector);
-		} catch (HgCallbackTargetException ex) {
+		} catch (HgException ex) {
 			// can't happen as long as our CollectHandler doesn't throw any exception
 			throw new HgBadStateException(ex.getCause());
 		} catch (CancelledException ex) {
--- a/src/org/tmatesoft/hg/core/HgOutgoingCommand.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgOutgoingCommand.java	Thu Aug 25 21:35:03 2011 +0200
@@ -109,7 +109,7 @@
 	 * 
 	 * @param handler delegate to process changes
 	 */
-	public void executeFull(final HgChangesetHandler handler) throws HgRemoteConnectionException, HgCallbackTargetException, CancelledException {
+	public void executeFull(final HgChangesetHandler handler) throws HgRemoteConnectionException, HgException, CancelledException {
 		if (handler == null) {
 			throw new IllegalArgumentException("Delegate can't be null");
 		}
--- a/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Aug 25 21:35:03 2011 +0200
@@ -162,7 +162,7 @@
 	 * @throws IllegalArgumentException if handler is <code>null</code>
 	 * @throws ConcurrentModificationException if this command already runs (i.e. being used from another thread)
 	 */
-	public void execute(Handler statusHandler) throws CancellationException, HgCallbackTargetException {
+	public void execute(Handler statusHandler) throws CancellationException, HgException {
 		if (statusHandler == null) {
 			throw new IllegalArgumentException();
 		}
--- a/src/org/tmatesoft/hg/repo/HgDataFile.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgDataFile.java	Thu Aug 25 21:35:03 2011 +0200
@@ -79,8 +79,26 @@
 		return path; // hgRepo.backresolve(this) -> name? In this case, what about hashed long names?
 	}
 
-	public int length(Nodeid nodeid) {
-		return content.dataLength(getLocalRevision(nodeid));
+	/**
+	 * @return size of the file content at the given revision
+	 */
+	public int length(Nodeid nodeid) throws HgDataStreamException {
+		return length(getLocalRevision(nodeid));
+		
+	}
+	
+	/**
+	 * @return size of the file content at the revision identified by local revision number.
+	 */
+	public int length(int localRev) throws HgDataStreamException {
+		if (metadata == null || !metadata.checked(localRev)) {
+			checkAndRecordMetadata(localRev);
+		}
+		final int dataLen = content.dataLength(localRev);
+		if (metadata.known(localRev)) {
+			return dataLen - metadata.dataOffset(localRev);
+		}
+		return dataLen;
 	}
 
 	/**
@@ -258,27 +276,7 @@
 
 	public boolean isCopy() throws HgDataStreamException {
 		if (metadata == null || !metadata.checked(0)) {
-			// content() always initializes metadata.
-			// FIXME this is expensive way to find out metadata, distinct RevlogStream.Iterator would be better.
-			// Alternatively, may parameterize MetadataContentPipe to do prepare only.
-			// For reference, when throwing CancelledException, hg status -A --rev 3:80 takes 70 ms
-			// however, if we just consume buffer instead (buffer.position(buffer.limit()), same command takes ~320ms
-			// (compared to command-line counterpart of 190ms)
-			try {
-				content(0, new ByteChannel() { // No-op channel
-					public int write(ByteBuffer buffer) throws IOException, CancelledException {
-						// pretend we consumed whole buffer
-//						int rv = buffer.remaining();
-//						buffer.position(buffer.limit());
-//						return rv;
-						throw new CancelledException();
-					}
-				});
-			} catch (CancelledException ex) {
-				// it's ok, we did that
-			} catch (Exception ex) {
-				throw new HgDataStreamException(getPath(), "Can't initialize metadata", ex);
-			}
+			checkAndRecordMetadata(0);
 		}
 		if (!metadata.known(0)) {
 			return false;
@@ -308,6 +306,26 @@
 		sb.append(')');
 		return sb.toString();
 	}
+	
+	private void checkAndRecordMetadata(int localRev) throws HgDataStreamException {
+		// content() always initializes metadata.
+		// FIXME this is expensive way to find out metadata, distinct RevlogStream.Iterator would be better.
+		// Alternatively, may parameterize MetadataContentPipe to do prepare only.
+		// For reference, when throwing CancelledException, hg status -A --rev 3:80 takes 70 ms
+		// however, if we just consume buffer instead (buffer.position(buffer.limit()), same command takes ~320ms
+		// (compared to command-line counterpart of 190ms)
+		try {
+			content(localRev, new ByteChannel() { // No-op channel
+				public int write(ByteBuffer buffer) throws IOException, CancelledException {
+					throw new CancelledException();
+				}
+			});
+		} catch (CancelledException ex) {
+			// it's ok, we did that
+		} catch (Exception ex) {
+			throw new HgDataStreamException(getPath(), "Can't initialize metadata", ex).setRevisionNumber(localRev);
+		}
+	}
 
 	private static final class MetadataEntry {
 		private final String entry;
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Thu Aug 25 03:57:39 2011 +0200
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Thu Aug 25 21:35: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));
@@ -100,21 +101,31 @@
 		}
 		return dirstate;
 	}
-
+	
 	// may be invoked few times
+	// 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) {
 			throw new IllegalArgumentException(String.valueOf(baseRevision));
 		}
 		final HgIgnore hgIgnore = repo.getIgnore();
 		TreeSet<String> knownEntries = getDirstate().all();
-		final boolean isTipBase;
 		if (baseRevision == TIP) {
-			baseRevision = repo.getChangelog().getLastRevision();
-			isTipBase = true;
-		} else {
-			isTipBase = baseRevision == repo.getChangelog().getLastRevision();
+			// 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;
 		Set<String> baseRevFiles = Collections.emptySet(); // files from base revision not affected by status calculation 
 		if (!isTipBase) {
@@ -289,8 +300,16 @@
 			if ((r = getDirstate().checkNormal(fname)) != null || (r = getDirstate().checkMerged(fname)) != null || (r = getDirstate().checkAdded(fname)) != null) {
 				// either clean or modified
 				HgDataFile fileNode = repo.getFileNode(fname);
-				final int lengthAtRevision = fileNode.length(nid1);
-				if (r.size /* XXX File.length() ?! */ != lengthAtRevision || flags != todoGenerateFlags(fname /*java.io.File*/)) {
+				int lengthAtRevision;
+				try {
+					lengthAtRevision = fileNode.length(nid1);
+				} catch (HgDataStreamException ex) {
+					ex.printStackTrace(); // XXX log error
+					lengthAtRevision = -1; // compare file content then
+				}
+				// XXX is it safe with respect to filters (keyword, eol) to compare lengthAtRevision (unprocessed) with size 
+				// from dirstate, which I assume is size of processed data?
+				if (r.size != -1 && r.size /* XXX File.length() ?! */ != lengthAtRevision || flags != todoGenerateFlags(fname /*java.io.File*/)) {
 					inspector.modified(fname);
 				} else {
 					// check actual content to see actual changes