changeset 663:46b56864b483

Pull: phase2 - update phases from remote, fncache with added files. Tests
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Wed, 10 Jul 2013 16:41:49 +0200
parents af5223b86dd3
children ae2d439fbed3
files build.xml src/org/tmatesoft/hg/core/HgPullCommand.java src/org/tmatesoft/hg/core/HgPushCommand.java src/org/tmatesoft/hg/internal/AddRevInspector.java src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java src/org/tmatesoft/hg/internal/FNCacheFile.java src/org/tmatesoft/hg/internal/Internals.java src/org/tmatesoft/hg/internal/PhasesHelper.java src/org/tmatesoft/hg/internal/RevlogStreamWriter.java src/org/tmatesoft/hg/repo/HgBundle.java src/org/tmatesoft/hg/repo/HgParentChildMap.java test/org/tmatesoft/hg/test/TestPull.java test/org/tmatesoft/hg/test/TestPush.java
diffstat 13 files changed, 403 insertions(+), 131 deletions(-) [+]
line wrap: on
line diff
--- a/build.xml	Wed Jul 10 11:53:19 2013 +0200
+++ b/build.xml	Wed Jul 10 16:41:49 2013 +0200
@@ -113,6 +113,7 @@
 			<test name="org.tmatesoft.hg.test.TestRevisionSet" />
 			<test name="org.tmatesoft.hg.test.TestRevisionMaps" />
 			<test name="org.tmatesoft.hg.test.TestPush" />
+			<test name="org.tmatesoft.hg.test.TestPull" />
 			<test name="org.tmatesoft.hg.test.ComplexTest" />
 		</junit>
 	</target>
--- a/src/org/tmatesoft/hg/core/HgPullCommand.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/core/HgPullCommand.java	Wed Jul 10 16:41:49 2013 +0200
@@ -16,6 +16,9 @@
  */
 package org.tmatesoft.hg.core;
 
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.tmatesoft.hg.internal.AddRevInspector;
@@ -44,6 +47,7 @@
 
 	private final HgRepository repo;
 	private HgRemoteRepository remote;
+	private RevisionSet added;
 
 	public HgPullCommand(HgRepository hgRepo) {
 		repo = hgRepo;
@@ -76,30 +80,32 @@
 			Transaction tr = trFactory.create(repo);
 			try {
 				incoming.inspectAll(insp = new AddRevInspector(implRepo, tr));
+				insp.done();
 				tr.commit();
 			} catch (HgRuntimeException ex) {
 				tr.rollback();
 				throw ex;
+			} catch (IOException ex) {
+				tr.rollback();
+				throw new HgIOException(ex.getMessage(), ex, null); // FIXME throw HgIOException right away
 			} catch (RuntimeException ex) {
 				tr.rollback();
 				throw ex;
 			}
 			progress.worked(45);
-			RevisionSet added = insp.addedChangesets();
+			added = insp.addedChangesets();
 			
+			if (!added.isEmpty()) {
+				// FIXME refresh parentHelper with newly added revisions in effective way
+				parentHelper.init(); 
+			}
 			// get remote phases, update local phases to match that of remote
+			// do not update any remote phase (it's pull, after all)
 			final PhasesHelper phaseHelper = new PhasesHelper(implRepo, parentHelper);
 			if (phaseHelper.isCapableOfPhases()) {
 				RevisionSet rsCommon = new RevisionSet(common);
 				HgRemoteRepository.Phases remotePhases = remote.getPhases();
-				if (remotePhases.isPublishingServer()) {
-					final RevisionSet knownPublic = rsCommon.union(added);
-					RevisionSet newDraft = phaseHelper.allDraft().subtract(knownPublic);
-					RevisionSet newSecret = phaseHelper.allSecret().subtract(knownPublic);
-					phaseHelper.updateRoots(newDraft.asList(), newSecret.asList());
-				} else {
-					// FIXME refactor reuse from HgPushCommand
-				}
+				phaseHelper.synchronizeWithRemote(remotePhases, rsCommon.union(added));
 			}
 			progress.worked(5);
 		} catch (HgRuntimeException ex) {
@@ -108,4 +114,8 @@
 			progress.done();
 		}
 	}
+	
+	public Collection<Nodeid> getPulledRevisions() {
+		return added == null ? Collections.<Nodeid>emptyList() : added.asList();
+	}
 }
--- a/src/org/tmatesoft/hg/core/HgPushCommand.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/core/HgPushCommand.java	Wed Jul 10 16:41:49 2013 +0200
@@ -18,7 +18,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -99,60 +98,8 @@
 			//
 			// update phase information
 			if (phaseHelper.isCapableOfPhases()) {
-				RevisionSet presentSecret = phaseHelper.allSecret();
-				RevisionSet presentDraft = phaseHelper.allDraft();
-				RevisionSet secretLeft, draftLeft;
 				HgRemoteRepository.Phases remotePhases = remoteRepo.getPhases();
-				RevisionSet remoteDrafts = knownRemoteDrafts(remotePhases, parentHelper, outgoing, presentSecret);
-				if (remotePhases.isPublishingServer()) {
-					// although it's unlikely outgoing would affect secret changesets,
-					// it doesn't hurt to check secret roots along with draft ones
-					secretLeft = presentSecret.subtract(outgoing);
-					draftLeft = presentDraft.subtract(outgoing);
-				} else {
-					// shall merge local and remote phase states
-					// revisions that cease to be secret (gonna become Public), e.g. someone else pushed them
-					RevisionSet secretGone = presentSecret.intersect(remoteDrafts);
-					// parents of those remote drafts are public, mark them as public locally, too
-					RevisionSet remotePublic = presentSecret.ancestors(secretGone, parentHelper);
-					secretLeft = presentSecret.subtract(secretGone).subtract(remotePublic);
-					/*
-					 * Revisions grow from left to right (parents to the left, children to the right)
-					 * 
-					 * I: Set of local is subset of remote
-					 * 
-					 *               local draft 
-					 * --o---r---o---l---o--
-					 *       remote draft
-					 * 
-					 * Remote draft roots shall be updated
-					 *
-					 *
-					 * II: Set of local is superset of remote
-					 * 
-					 *       local draft 
-					 * --o---l---o---r---o--
-					 *               remote draft 
-					 *               
-					 * Local draft roots shall be updated
-					 */
-					RevisionSet sharedDraft = presentDraft.intersect(remoteDrafts); // (I: ~presentDraft; II: ~remoteDraft
-					// XXX do I really need sharedDrafts here? why not ancestors(remoteDrafts)?
-					RevisionSet localDraftRemotePublic = presentDraft.ancestors(sharedDraft, parentHelper); // I: 0; II: those treated public on remote
-					// remoteDrafts are local revisions known as draft@remote
-					// remoteDraftsLocalPublic - revisions that would cease to be listed as draft on remote
-					RevisionSet remoteDraftsLocalPublic = remoteDrafts.ancestors(sharedDraft, parentHelper);
-					RevisionSet remoteDraftsLeft = remoteDrafts.subtract(remoteDraftsLocalPublic);
-					// forget those deemed public by remote (drafts shared by both remote and local are ok to stay)
-					RevisionSet combinedDraft = presentDraft.union(remoteDraftsLeft);
-					draftLeft = combinedDraft.subtract(localDraftRemotePublic);
-				}
-				final RevisionSet newDraftRoots = draftLeft.roots(parentHelper);
-				final RevisionSet newSecretRoots = secretLeft.roots(parentHelper);
-				phaseHelper.updateRoots(newDraftRoots.asList(), newSecretRoots.asList());
-				//
-				// if there's a remote draft root that points to revision we know is public
-				RevisionSet remoteDraftsLocalPublic = remoteDrafts.subtract(draftLeft).subtract(secretLeft);
+				RevisionSet remoteDraftsLocalPublic = phaseHelper.synchronizeWithRemote(remotePhases, outgoing);
 				if (!remoteDraftsLocalPublic.isEmpty()) {
 					// foreach remoteDraftsLocallyPublic.heads() do push Draft->Public
 					for (Nodeid n : remoteDraftsLocalPublic.heads(parentHelper)) {
@@ -202,27 +149,4 @@
 	public Collection<Nodeid> getPushedRevisions() {
 		return outgoing == null ? Collections.<Nodeid>emptyList() : outgoing.asList();
 	}
-	
-	private RevisionSet knownRemoteDrafts(HgRemoteRepository.Phases remotePhases, HgParentChildMap<HgChangelog> parentHelper, RevisionSet outgoing, RevisionSet localSecret) {
-		ArrayList<Nodeid> knownRemoteDraftRoots = new ArrayList<Nodeid>();
-		for (Nodeid rdr : remotePhases.draftRoots()) {
-			if (parentHelper.knownNode(rdr)) {
-				knownRemoteDraftRoots.add(rdr);
-			}
-		}
-		// knownRemoteDraftRoots + childrenOf(knownRemoteDraftRoots) is everything remote may treat as Draft
-		RevisionSet remoteDrafts = new RevisionSet(knownRemoteDraftRoots);
-		RevisionSet localChildren = remoteDrafts.children(parentHelper);
-		// we didn't send any local secret revision
-		localChildren = localChildren.subtract(localSecret);
-		// draft roots are among remote drafts
-		remoteDrafts = remoteDrafts.union(localChildren);
-		// 1) outgoing.children gives all local revisions accessible from outgoing.
-		// 2) outgoing.roots.children is equivalent with smaller intermediate set, the way we build
-		// childrenOf doesn't really benefits from that.
-		RevisionSet localChildrenNotSent = outgoing.children(parentHelper).subtract(outgoing);
-		// remote shall know only what we've sent, subtract revisions we didn't actually sent
-		remoteDrafts = remoteDrafts.subtract(localChildrenNotSent);
-		return remoteDrafts;
-	}
 }
--- a/src/org/tmatesoft/hg/internal/AddRevInspector.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/AddRevInspector.java	Wed Jul 10 16:41:49 2013 +0200
@@ -16,6 +16,7 @@
  */
 package org.tmatesoft.hg.internal;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Set;
 
@@ -30,24 +31,29 @@
 import org.tmatesoft.hg.util.Pair;
 
 /**
+ * FIXME pretty much alike HgCloneCommand.WriteDownMate, shall converge
+ * 
  * @author Artem Tikhomirov
  * @author TMate Software Ltd.
  */
 public final class AddRevInspector implements HgBundle.Inspector {
 	private final Internals repo;
 	private final Transaction tr;
+	private final FNCacheFile.Mediator fncache;
 	private Set<Nodeid> added;
 	private RevlogStreamWriter revlog;
 	private RevMap clogRevs;
 	private RevMap revlogRevs;
+	private HgDataFile fileNode;
+	private boolean newFile = false;
 
 	public AddRevInspector(Internals implRepo, Transaction transaction) {
 		repo = implRepo;
 		tr = transaction;
+		fncache = new FNCacheFile.Mediator(implRepo);
 	}
 
 	public void changelogStart() throws HgRuntimeException {
-		// TODO Auto-generated method stub
 		RevlogStream rs = repo.getImplAccess().getChangelogStream();
 		revlog = new RevlogStreamWriter(repo, rs, tr);
 		revlogRevs = clogRevs = new RevMap(rs);
@@ -71,14 +77,17 @@
 	}
 
 	public void fileStart(String name) throws HgRuntimeException {
-		HgDataFile df = repo.getRepo().getFileNode(name);
-		RevlogStream rs = repo.getImplAccess().getStream(df);
+		fileNode = repo.getRepo().getFileNode(name);
+		newFile = !fileNode.exists();
+		RevlogStream rs = repo.getImplAccess().getStream(fileNode);
 		revlog = new RevlogStreamWriter(repo, rs, tr);
 		revlogRevs = new RevMap(rs);
-		// FIXME collect new files and update fncache
 	}
 
 	public void fileEnd(String name) throws HgRuntimeException {
+		if (newFile) {
+			fncache.registerNew(fileNode.getPath(), revlog.getRevlogStream());
+		}
 		revlog = null;
 		revlogRevs = null;
 	}
@@ -86,6 +95,12 @@
 	public boolean element(GroupElement ge) throws HgRuntimeException {
 		assert clogRevs != null;
 		assert revlogRevs != null;
+		if (revlog.getRevlogStream().findRevisionIndex(ge.node()) != HgRepository.BAD_REVISION) {
+			// HgRemoteRepository.getChanges(common) builds a bundle that includes these common
+			// revisions. Hence, shall not add these common (i.e. known locally) revisions
+			// once again
+			return true;
+		}
 		try {
 			Pair<Integer, Nodeid> newRev = revlog.addPatchRevision(ge, clogRevs, revlogRevs);
 			revlogRevs.update(newRev.first(), newRev.second());
@@ -98,6 +113,10 @@
 	public RevisionSet addedChangesets() {
 		return new RevisionSet(added);
 	}
+	
+	public void done() throws IOException {
+		fncache.complete();
+	}
 
 	private static class RevMap implements RevlogStreamWriter.RevisionToIndexMap {
 		
--- a/src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java	Wed Jul 10 16:41:49 2013 +0200
@@ -52,7 +52,7 @@
 	@Override
 	public void readBytes(byte[] buf, int off, int len) throws IOException {
 		if (len > (this.length - pos)) {
-			throw new IOException();
+			throw new IOException(String.format("Requested %d bytes at position %d of %d total", len, pos, length));
 		}
 		System.arraycopy(data, offset+pos, buf, off, len);
 		pos += len;
--- a/src/org/tmatesoft/hg/internal/FNCacheFile.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/FNCacheFile.java	Wed Jul 10 16:41:49 2013 +0200
@@ -76,7 +76,7 @@
 	}
 	*/
 	
-	public void write() throws IOException {
+	public void write() throws IOException { // FIXME transaction! and HgIOException
 		if (addedDotI.isEmpty() && addedDotD.isEmpty()) {
 			return;
 		}
@@ -114,4 +114,34 @@
 	public void addData(Path p) {
 		addedDotD.add(p);
 	}
+
+	/**
+	 * Register new files with fncache if one is enabled for the repo, do nothing otherwise
+	 */
+	public static class Mediator {
+		private final Internals repo;
+		private FNCacheFile fncache;
+
+		public Mediator(Internals internalRepo) {
+			repo = internalRepo;
+		}
+		
+		public void registerNew(Path f, RevlogStream rs) {
+			if (fncache != null || repo.fncacheInUse()) {
+				if (fncache == null) {
+					fncache = new FNCacheFile(repo);
+				}
+				fncache.addIndex(f);
+				if (!rs.isInlineData()) {
+					fncache.addData(f);
+				}
+			}
+		}
+		
+		public void complete() throws IOException {
+			if (fncache != null) {
+				fncache.write();
+			}
+		}
+	}
 }
--- a/src/org/tmatesoft/hg/internal/Internals.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/Internals.java	Wed Jul 10 16:41:49 2013 +0200
@@ -104,12 +104,23 @@
 	 * Alternative approach, controlled with this option, first combines these there patches into one,
 	 * and only then applies it to base revision, eliminating 2 intermediate elements.
 	 * <p>
-	 * Present default value for this option is <b>FALSE</b>, and will be changed in future, once
-	 * tests prove support is fully functional (likely in v1.2).
+	 * Since 1.2, default value for this option is <em>TRUE</em>, (was <code>false</code> in <b>Hg4J 1.1</b>)
 	 * 
 	 * @since 1.1
 	 */
 	public static final String CFG_PROPERTY_PATCH_MERGE = "hg4j.repo.merge_revlog_patches";
+	
+	/**
+	 * Phases were introduced in Mercurial 2.1. Unless there's <code>phaseroots</code> file in the 
+	 * repository's storage area, <b>Hg4J</b> pretends phases are not enabled and doesn't update
+	 * phase information on commit/push/pull. If, however, it's desired to keep phase information,
+	 * this option may be set to <code>true</code>, and <code>phaseroots</code> file gets updated
+	 * along with repository changes.
+	 * 
+	 * <p>Default value: <code>false</code>
+	 * @since 1.2
+	 */
+	public static final String CFG_PROPERTY_CREATE_PHASEROOTS = "hg4j.repo.create_phaseroots";
 
 	public static final int REVLOGV1_RECORD_SIZE = 64;
 
@@ -126,6 +137,7 @@
 	private final PathRewrite repoPathHelper; // access to system files (under .hg/store if requires has 'store' flag)
 
 	private final boolean shallMergePatches;
+	private final boolean shallWritePhaseroots;
 	private final RevlogStreamFactory streamProvider;
 
 	public Internals(HgRepository hgRepo, File hgDir, ImplAccess implementationAccess) throws HgRuntimeException {
@@ -143,6 +155,7 @@
 		boolean shallCacheRevlogsInRepo = pm.getBoolean(CFG_PROPERTY_REVLOG_STREAM_CACHE, true);
 		streamProvider = new RevlogStreamFactory(this, shallCacheRevlogsInRepo); 
 		shallMergePatches = pm.getBoolean(Internals.CFG_PROPERTY_PATCH_MERGE, true);
+		shallWritePhaseroots = pm.getBoolean(Internals.CFG_PROPERTY_CREATE_PHASEROOTS, false);
 	}
 	
 	public boolean isInvalid() {
@@ -280,6 +293,10 @@
 	boolean shallMergePatches() {
 		return shallMergePatches;
 	}
+	
+	boolean shallCreatePhaseroots() {
+		return shallWritePhaseroots;
+	}
 
 	RevlogChangeMonitor getRevlogTracker(File f) {
 		// TODO decide whether to use one monitor per multiple files or 
--- a/src/org/tmatesoft/hg/internal/PhasesHelper.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/PhasesHelper.java	Wed Jul 10 16:41:49 2013 +0200
@@ -39,6 +39,7 @@
 import org.tmatesoft.hg.repo.HgInvalidStateException;
 import org.tmatesoft.hg.repo.HgParentChildMap;
 import org.tmatesoft.hg.repo.HgPhase;
+import org.tmatesoft.hg.repo.HgRemoteRepository;
 import org.tmatesoft.hg.repo.HgRepository;
 import org.tmatesoft.hg.repo.HgRuntimeException;
 
@@ -68,7 +69,7 @@
 		repo = internalRepo;
 		parentHelper = pw;
 	}
-	
+
 	public HgRepository getRepo() {
 		return repo.getRepo();
 	}
@@ -79,16 +80,19 @@
 		}
 		return repoSupporsPhases.booleanValue();
 	}
-	
+
 	public boolean withSecretRoots() {
 		return !secretPhaseRoots.isEmpty();
 	}
 
 	/**
-	 * @param cset revision to query
+	 * @param cset
+	 *            revision to query
 	 * @return phase of the changeset, never <code>null</code>
-	 * @throws HgInvalidControlFileException if failed to access revlog index/data entry. <em>Runtime exception</em>
-	 * @throws HgRuntimeException subclass thereof to indicate other issues with the library. <em>Runtime exception</em>
+	 * @throws HgInvalidControlFileException
+	 *             if failed to access revlog index/data entry. <em>Runtime exception</em>
+	 * @throws HgRuntimeException
+	 *             subclass thereof to indicate other issues with the library. <em>Runtime exception</em>
 	 */
 	public HgPhase getPhase(HgChangeset cset) throws HgRuntimeException {
 		final Nodeid csetRev = cset.getNodeid();
@@ -97,11 +101,15 @@
 	}
 
 	/**
-	 * @param csetRevIndex revision index to query
-	 * @param csetRev revision nodeid, optional 
+	 * @param csetRevIndex
+	 *            revision index to query
+	 * @param csetRev
+	 *            revision nodeid, optional
 	 * @return phase of the changeset, never <code>null</code>
-	 * @throws HgInvalidControlFileException if failed to access revlog index/data entry. <em>Runtime exception</em>
-	 * @throws HgRuntimeException subclass thereof to indicate other issues with the library. <em>Runtime exception</em>
+	 * @throws HgInvalidControlFileException
+	 *             if failed to access revlog index/data entry. <em>Runtime exception</em>
+	 * @throws HgRuntimeException
+	 *             subclass thereof to indicate other issues with the library. <em>Runtime exception</em>
 	 */
 	public HgPhase getPhase(final int csetRevIndex, Nodeid csetRev) throws HgRuntimeException {
 		if (!isCapableOfPhases()) {
@@ -111,8 +119,8 @@
 		if (parentHelper != null && (csetRev == null || csetRev.isNull())) {
 			csetRev = getRepo().getChangelog().getRevision(csetRevIndex);
 		}
-					
-		for (HgPhase phase : new HgPhase[] {HgPhase.Secret, HgPhase.Draft }) {
+
+		for (HgPhase phase : new HgPhase[] { HgPhase.Secret, HgPhase.Draft }) {
 			List<Nodeid> roots = getPhaseRoots(phase);
 			if (roots.isEmpty()) {
 				continue;
@@ -138,24 +146,24 @@
 		return HgPhase.Public;
 	}
 
-
 	/**
 	 * @return all revisions with secret phase
 	 */
 	public RevisionSet allSecret() {
 		return allOf(HgPhase.Secret);
 	}
-	
+
 	/**
 	 * @return all revisions with draft phase
 	 */
 	public RevisionSet allDraft() {
 		return allOf(HgPhase.Draft).subtract(allOf(HgPhase.Secret));
 	}
-	
+
+	// XXX throw HgIOException instead?
 	public void updateRoots(Collection<Nodeid> draftRoots, Collection<Nodeid> secretRoots) throws HgInvalidControlFileException {
-		draftPhaseRoots = draftRoots.isEmpty() ? Collections.<Nodeid>emptyList() : new ArrayList<Nodeid>(draftRoots);
-		secretPhaseRoots = secretRoots.isEmpty() ? Collections.<Nodeid>emptyList() : new ArrayList<Nodeid>(secretRoots);
+		draftPhaseRoots = draftRoots.isEmpty() ? Collections.<Nodeid> emptyList() : new ArrayList<Nodeid>(draftRoots);
+		secretPhaseRoots = secretRoots.isEmpty() ? Collections.<Nodeid> emptyList() : new ArrayList<Nodeid>(secretRoots);
 		String fmt = "%d %s\n";
 		File phaseroots = repo.getRepositoryFile(Phaseroots);
 		FileWriter fw = null;
@@ -203,19 +211,114 @@
 	}
 
 	/**
+	 * @return set of revisions that are public locally, but draft on remote.
+	 */
+	public RevisionSet synchronizeWithRemote(HgRemoteRepository.Phases remotePhases, RevisionSet sharedWithRemote) throws HgInvalidControlFileException {
+		assert parentHelper != null;
+		RevisionSet presentSecret = allSecret();
+		RevisionSet presentDraft = allDraft();
+		RevisionSet secretLeft, draftLeft;
+		RevisionSet remoteDrafts = knownRemoteDrafts(remotePhases, sharedWithRemote, presentSecret);
+		if (remotePhases.isPublishingServer()) {
+			// although it's unlikely shared revisions would affect secret changesets,
+			// it doesn't hurt to check secret roots along with draft ones
+			// 
+			// local drafts that are known to be public now
+			RevisionSet draftsBecomePublic = presentDraft.intersect(sharedWithRemote);
+			RevisionSet secretsBecomePublic = presentSecret.intersect(sharedWithRemote);
+			// any ancestor of the public revision is public, too
+			RevisionSet draftsGone = presentDraft.ancestors(draftsBecomePublic, parentHelper);
+			RevisionSet secretsGone = presentSecret.ancestors(secretsBecomePublic, parentHelper);
+			// remove public and their ancestors from drafts
+			draftLeft = presentDraft.subtract(draftsGone).subtract(draftsBecomePublic);
+			secretLeft = presentSecret.subtract(secretsGone).subtract(secretsBecomePublic);
+		} else {
+			// shall merge local and remote phase states
+			// revisions that cease to be secret (gonna become Public), e.g. someone else pushed them
+			RevisionSet secretGone = presentSecret.intersect(remoteDrafts);
+			// parents of those remote drafts are public, mark them as public locally, too
+			RevisionSet remotePublic = presentSecret.ancestors(secretGone, parentHelper);
+			secretLeft = presentSecret.subtract(secretGone).subtract(remotePublic);
+			/*
+			 * Revisions grow from left to right (parents to the left, children to the right)
+			 * 
+			 * I: Set of local is subset of remote
+			 * 
+			 * local draft
+			 * --o---r---o---l---o--
+			 * remote draft
+			 * 
+			 * Remote draft roots shall be updated
+			 * 
+			 * 
+			 * II: Set of local is superset of remote
+			 * 
+			 * local draft
+			 * --o---l---o---r---o--
+			 * remote draft
+			 * 
+			 * Local draft roots shall be updated
+			 */
+			RevisionSet sharedDraft = presentDraft.intersect(remoteDrafts); // (I: ~presentDraft; II: ~remoteDraft
+			// XXX do I really need sharedDrafts here? why not ancestors(remoteDrafts)?
+			RevisionSet localDraftRemotePublic = presentDraft.ancestors(sharedDraft, parentHelper); // I: 0; II: those treated public on remote
+			// remoteDrafts are local revisions known as draft@remote
+			// remoteDraftsLocalPublic - revisions that would cease to be listed as draft on remote
+			RevisionSet remoteDraftsLocalPublic = remoteDrafts.ancestors(sharedDraft, parentHelper);
+			RevisionSet remoteDraftsLeft = remoteDrafts.subtract(remoteDraftsLocalPublic);
+			// forget those deemed public by remote (drafts shared by both remote and local are ok to stay)
+			RevisionSet combinedDraft = presentDraft.union(remoteDraftsLeft);
+			draftLeft = combinedDraft.subtract(localDraftRemotePublic);
+		}
+		final RevisionSet newDraftRoots = draftLeft.roots(parentHelper);
+		final RevisionSet newSecretRoots = secretLeft.roots(parentHelper);
+		updateRoots(newDraftRoots.asList(), newSecretRoots.asList());
+		//
+		// if there's a remote draft root that points to revision we know is public
+		RevisionSet remoteDraftsLocalPublic = remoteDrafts.subtract(draftLeft).subtract(secretLeft);
+		return remoteDraftsLocalPublic;
+	}
+
+	// shared - set of revisions we've shared with remote
+	private RevisionSet knownRemoteDrafts(HgRemoteRepository.Phases remotePhases, RevisionSet shared, RevisionSet localSecret) {
+		ArrayList<Nodeid> knownRemoteDraftRoots = new ArrayList<Nodeid>();
+		for (Nodeid rdr : remotePhases.draftRoots()) {
+			if (parentHelper.knownNode(rdr)) {
+				knownRemoteDraftRoots.add(rdr);
+			}
+		}
+		// knownRemoteDraftRoots + childrenOf(knownRemoteDraftRoots) is everything remote may treat as Draft
+		RevisionSet remoteDrafts = new RevisionSet(knownRemoteDraftRoots);
+		RevisionSet localChildren = remoteDrafts.children(parentHelper);
+		// we didn't send any local secret revision
+		localChildren = localChildren.subtract(localSecret);
+		// draft roots are among remote drafts
+		remoteDrafts = remoteDrafts.union(localChildren);
+		// remoteDrafts is set of local revisions remote may see as Draft. However,
+		// need to remove from this set revisions we didn't share with remote:
+		// 1) shared.children gives all local revisions accessible from shared.
+		// 2) shared.roots.children is equivalent with smaller intermediate set, the way we build
+		// childrenOf doesn't really benefits from that.
+		RevisionSet localChildrenNotSent = shared.children(parentHelper).subtract(shared);
+		// remote shall know only what we've sent, subtract revisions we didn't actually sent
+		remoteDrafts = remoteDrafts.subtract(localChildrenNotSent);
+		return remoteDrafts;
+	}
+
+	/**
 	 * For a given phase, collect all revisions with phase that is the same or more private (i.e. for Draft, returns Draft+Secret)
-	 * The reason is not a nice API intention (which is awful, indeed), but an ease of implementation 
+	 * The reason is not a nice API intention (which is awful, indeed), but an ease of implementation
 	 */
 	private RevisionSet allOf(HgPhase phase) {
 		assert phase != HgPhase.Public;
 		if (!isCapableOfPhases()) {
-			return new RevisionSet(Collections.<Nodeid>emptyList());
+			return new RevisionSet(Collections.<Nodeid> emptyList());
 		}
 		final List<Nodeid> roots = getPhaseRoots(phase);
 		if (parentHelper != null) {
 			return new RevisionSet(roots).union(new RevisionSet(parentHelper.childrenOf(roots)));
 		} else {
-			RevisionSet rv = new RevisionSet(Collections.<Nodeid>emptyList());
+			RevisionSet rv = new RevisionSet(Collections.<Nodeid> emptyList());
 			for (RevisionDescendants rd : getPhaseDescendants(phase)) {
 				rv = rv.union(rd.asRevisionSet());
 			}
@@ -227,6 +330,11 @@
 		File phaseroots = repo.getRepositoryFile(Phaseroots);
 		try {
 			if (!phaseroots.exists()) {
+				if (repo.shallCreatePhaseroots()) {
+					draftPhaseRoots = Collections.<Nodeid>emptyList();
+					secretPhaseRoots = Collections.<Nodeid>emptyList();
+					return Boolean.TRUE;
+				}
 				return Boolean.FALSE;
 			}
 			LineReader lr = new LineReader(phaseroots, repo.getLog());
@@ -254,8 +362,8 @@
 				}
 				roots.add(rootRev);
 			}
-			draftPhaseRoots = phase2roots.containsKey(Draft) ? phase2roots.get(Draft) : Collections.<Nodeid>emptyList();
-			secretPhaseRoots = phase2roots.containsKey(Secret) ? phase2roots.get(Secret) : Collections.<Nodeid>emptyList();
+			draftPhaseRoots = phase2roots.containsKey(Draft) ? phase2roots.get(Draft) : Collections.<Nodeid> emptyList();
+			secretPhaseRoots = phase2roots.containsKey(Secret) ? phase2roots.get(Secret) : Collections.<Nodeid> emptyList();
 		} catch (HgIOException ex) {
 			throw new HgInvalidControlFileException(ex, true);
 		}
@@ -264,13 +372,14 @@
 
 	private List<Nodeid> getPhaseRoots(HgPhase phase) {
 		switch (phase) {
-		case Draft : return draftPhaseRoots;
-		case Secret : return secretPhaseRoots;
+		case Draft:
+			return draftPhaseRoots;
+		case Secret:
+			return secretPhaseRoots;
 		}
 		return Collections.emptyList();
 	}
 
-
 	private RevisionDescendants[] getPhaseDescendants(HgPhase phase) throws HgRuntimeException {
 		int ordinal = phase.ordinal();
 		if (phaseDescendants[ordinal] == null) {
@@ -288,7 +397,7 @@
 		}
 		return rv;
 	}
-	
+
 	private int[] toIndexes(List<Nodeid> roots) throws HgRuntimeException {
 		int[] rv = new int[roots.size()];
 		for (int i = 0; i < rv.length; i++) {
--- a/src/org/tmatesoft/hg/internal/RevlogStreamWriter.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/RevlogStreamWriter.java	Wed Jul 10 16:41:49 2013 +0200
@@ -17,8 +17,10 @@
 package org.tmatesoft.hg.internal;
 
 import static org.tmatesoft.hg.internal.Internals.REVLOGV1_RECORD_SIZE;
+import static org.tmatesoft.hg.repo.HgRepository.BAD_REVISION;
 import static org.tmatesoft.hg.repo.HgRepository.NO_REVISION;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 
@@ -48,7 +50,8 @@
 	private final DigestHelper dh = new DigestHelper();
 	private final RevlogCompressor revlogDataZip;
 	private final Transaction transaction;
-	private int lastEntryBase, lastEntryIndex, lastEntryActualLen;
+	// init with illegal values
+	private int lastEntryBase = BAD_REVISION, lastEntryIndex = BAD_REVISION, lastEntryActualLen = -1;
 	// record revision and its full content
 	// the name might be misleading, it does not necessarily match lastEntryIndex
 	private Pair<Integer, byte[]> lastFullContent;
@@ -66,6 +69,10 @@
 		transaction = tr;
 	}
 	
+	public RevlogStream getRevlogStream() {
+		return revlogStream;
+	}
+	
 	public Pair<Integer,Nodeid> addPatchRevision(GroupElement ge, RevisionToIndexMap clogRevs, RevisionToIndexMap revlogRevs) throws HgIOException, HgRuntimeException {
 		populateLastEntryIndex();
 		//
@@ -110,15 +117,18 @@
 					complete = p.apply(new ByteArrayDataAccess(new byte[0]), -1);
 					baseRev = 0; // it's done above, but doesn't hurt
 				} else {
-					ReadContentInspector insp = new ReadContentInspector().read(revlogStream, baseRev);
+					assert patchBaseRev != NO_REVISION;
+					ReadContentInspector insp = new ReadContentInspector().read(revlogStream, patchBaseRev);
 					complete = p.apply(new ByteArrayDataAccess(insp.content), -1);
 					baseRev = lastEntryIndex + 1;
 				}
 				ds = new ByteArrayDataSource(complete);
 				revLen = complete.length;
 			} catch (IOException ex) {
-				// unlikely to happen, as ByteArrayDataSource doesn't throw IOException
-				throw new HgIOException("Failed to reconstruct revision", ex, null);
+				// unlikely to happen, as ByteArrayDataSource throws IOException only in case of programming errors
+				// FIXME next approach to get indexFile is awful:
+				File indexFile = revlogStream.initWithIndexFile(new HgInvalidControlFileException("", ex, null)).getFile();
+				throw new HgIOException("Failed to reconstruct revision", ex, indexFile);
 			}
 		}
 		doAdd(nodeRev, p1, p2, linkRev, baseRev, revLen, ds);
@@ -236,7 +246,11 @@
 	
 	private int dataLength(int revisionIndex) throws HgInvalidControlFileException, HgInvalidRevisionException {
 		assert revisionIndex >= 0;
-		if (revisionIndex == lastEntryIndex) {
+		if (revisionIndex == lastEntryIndex && lastEntryActualLen >= 0) {
+			// if the last entry is the one we've just written, we know its actual len.
+			// it's possible, however, that revisionIndex == lastEntryIndex just
+			// because revision being added comes right after last locally known one
+			// and lastEntryActualLen is not set
 			return lastEntryActualLen;
 		}
 		if (lastFullContent != null && lastFullContent.first() == revisionIndex) {
--- a/src/org/tmatesoft/hg/repo/HgBundle.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgBundle.java	Wed Jul 10 16:41:49 2013 +0200
@@ -413,7 +413,7 @@
 	private static void readGroup(DataAccess da, Inspector inspector) throws IOException, HgRuntimeException {
 		int len = da.readInt();
 		boolean good2go = true;
-		Nodeid prevNodeid = Nodeid.NULL;
+		Nodeid prevNodeid = null;
 		while (len > 4 && !da.isEmpty() && good2go) {
 			byte[] nb = new byte[80];
 			da.readBytes(nb, 0, 80);
@@ -498,7 +498,7 @@
 		 * @return revision of delta base, never <code>null</code>
 		 */
 		public Nodeid patchBase() {
-			return deltaBase;
+			return deltaBase == null ? firstParent() : deltaBase;
 		}
 		
 		public byte[] rawDataByteArray() throws IOException { // XXX IOException or HgInvalidFileException?
--- a/src/org/tmatesoft/hg/repo/HgParentChildMap.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgParentChildMap.java	Wed Jul 10 16:41:49 2013 +0200
@@ -135,9 +135,9 @@
 		heads = _heads;
 	}
 	
-	private void assertSortedIndex(int x) {
+	private static void assertSortedIndex(int x) {
 		if (x < 0) {
-			throw new HgInvalidStateException(String.format("Bad index", x));
+			throw new HgInvalidStateException(String.format("Bad index %d", x));
 		}
 	}
 	
--- a/test/org/tmatesoft/hg/test/TestPull.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestPull.java	Wed Jul 10 16:41:49 2013 +0200
@@ -16,22 +16,36 @@
  */
 package org.tmatesoft.hg.test;
 
+import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.tmatesoft.hg.repo.HgRepository.TIP;
 
 import java.io.File;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.Rule;
 import org.junit.Test;
+import org.tmatesoft.hg.core.HgAddRemoveCommand;
+import org.tmatesoft.hg.core.HgCheckoutCommand;
+import org.tmatesoft.hg.core.HgCommitCommand;
 import org.tmatesoft.hg.core.HgIncomingCommand;
 import org.tmatesoft.hg.core.HgPullCommand;
 import org.tmatesoft.hg.core.Nodeid;
+import org.tmatesoft.hg.internal.BasicSessionContext;
+import org.tmatesoft.hg.internal.Internals;
+import org.tmatesoft.hg.internal.PhasesHelper;
+import org.tmatesoft.hg.internal.RevisionSet;
+import org.tmatesoft.hg.repo.HgInternals;
 import org.tmatesoft.hg.repo.HgLookup;
+import org.tmatesoft.hg.repo.HgPhase;
 import org.tmatesoft.hg.repo.HgRemoteRepository;
 import org.tmatesoft.hg.repo.HgRepository;
+import org.tmatesoft.hg.util.Path;
 
 /**
- * 
+ * FIXME need TestTransaction to check transaction rolback/commit as it's tricky to test transactions as part of pull/push commands
  * @author Artem Tikhomirov
  * @author TMate Software Ltd.
  */
@@ -48,7 +62,7 @@
 		try {
 			final HgLookup hgLookup = new HgLookup();
 			final HgRemoteRepository srcRemote = hgLookup.detect(server.getURL());
-			HgRepository dstRepo = hgLookup.detect(dstRepoLoc);
+			final HgRepository dstRepo = hgLookup.detect(dstRepoLoc);
 			HgPullCommand cmd = new HgPullCommand(dstRepo).source(srcRemote);
 			cmd.execute();
 			final HgRepository srcRepo = hgLookup.detect(srcRepoLoc);
@@ -60,8 +74,135 @@
 			server.stop();
 		}
 	}
+
+	/**
+	 * pull comes with 2 changes, one of them with new file
+	 */
+	@Test
+	public void testPullChanges() throws Exception {
+ 
+		File srcRepoLoc = RepoUtils.cloneRepoToTempLocation("test-annotate", "test-pull-src", false);
+		File dstRepoLoc = RepoUtils.cloneRepoToTempLocation("test-annotate", "test-pull-dst", false);
+		File f1 = new File(srcRepoLoc, "file1");
+		assertTrue("[sanity]", f1.canWrite());
+		final HgLookup hgLookup = new HgLookup();
+		// add two commits, one with new file at different branch
+		// commit 1
+		final HgRepository srcRepo = hgLookup.detect(srcRepoLoc);
+		assertEquals("[sanity]", "default", srcRepo.getWorkingCopyBranchName());
+		RepoUtils.modifyFileAppend(f1, "change1");
+		HgCommitCommand commitCmd = new HgCommitCommand(srcRepo).message("Commit 1");
+		assertTrue(commitCmd.execute().isOk());
+		final Nodeid cmt1 = commitCmd.getCommittedRevision();
+		// commit 2
+		new HgCheckoutCommand(srcRepo).changeset(7).clean(true).execute();
+		assertEquals("[sanity]", "no-merge", srcRepo.getWorkingCopyBranchName());
+		RepoUtils.createFile(new File(srcRepoLoc, "file-new"), "whatever");
+		new HgAddRemoveCommand(srcRepo).add(Path.create("file-new")).execute();
+		commitCmd = new HgCommitCommand(srcRepo).message("Commit 2");
+		assertTrue(commitCmd.execute().isOk());
+		final Nodeid cmt2 = commitCmd.getCommittedRevision();
+		//
+		// pull
+		HgServer server = new HgServer().start(srcRepoLoc);
+		final HgRepository dstRepo = hgLookup.detect(dstRepoLoc);
+		try {
+			final HgRemoteRepository srcRemote = hgLookup.detect(server.getURL());
+			new HgPullCommand(dstRepo).source(srcRemote).execute();
+		} finally {
+			server.stop();
+		}
+		//
+		errorCollector.assertTrue(dstRepo.getChangelog().isKnown(cmt1));
+		errorCollector.assertTrue(dstRepo.getChangelog().isKnown(cmt2));
+		checkRepositoriesAreSame(srcRepo, dstRepo);
+		RepoUtils.assertHgVerifyOk(errorCollector, dstRepoLoc);
+	}
 	
-	// test when pull comes with new file (if AddRevInspector/RevlogStreamWriter is ok with file that doesn't exist 
+	/**
+	 * Add two draft changesets, one child of r8 (local:draft, remote:public) and another 
+	 * as child of r4 (public), pull and see if 5, 7 and 8 became public, but newly added drafts remained
+	 */
+	@Test
+	public void testPullFromPublishing() throws Exception {
+		// copy, not clone as latter updates phase information
+		File srcRepoLoc = RepoUtils.copyRepoToTempLocation("test-phases", "test-pull-pub-src");
+		File dstRepoLoc = RepoUtils.copyRepoToTempLocation("test-phases", "test-pull-pub-dst");
+		File f1 = new File(dstRepoLoc, "hello.c");
+		assertTrue("[sanity]", f1.canWrite());
+		final HgLookup hgLookup = new HgLookup();
+		HgRepository dstRepo = hgLookup.detect(dstRepoLoc);
+		PhasesHelper phaseHelper = new PhasesHelper(HgInternals.getImplementationRepo(dstRepo));
+		//
+		// new child revision for shared public parent
+		assertEquals(HgPhase.Public, phaseHelper.getPhase(4, null));
+		new HgCheckoutCommand(dstRepo).changeset(4).clean(true).execute();
+		RepoUtils.modifyFileAppend(f1, "// aaa");
+		HgCommitCommand commitCmd = new HgCommitCommand(dstRepo).message("Commit 1");
+		assertTrue(commitCmd.execute().isOk());
+		final Nodeid cmt1 = commitCmd.getCommittedRevision();
+		//
+		// new child rev for parent locally draft, remotely public
+		assertEquals(HgPhase.Draft, phaseHelper.getPhase(5, null));
+		assertEquals(HgPhase.Draft, phaseHelper.getPhase(7, null));
+		assertEquals(HgPhase.Draft, phaseHelper.getPhase(8, null));
+		new HgCheckoutCommand(dstRepo).changeset(8).clean(true).execute();
+		RepoUtils.modifyFileAppend(f1, "// bbb");
+		commitCmd = new HgCommitCommand(dstRepo).message("Commit 2");
+		assertTrue(commitCmd.execute().isOk());
+		final Nodeid cmt2 = commitCmd.getCommittedRevision();
+		// both new revisions shall be draft
+		phaseHelper = new PhasesHelper(HgInternals.getImplementationRepo(dstRepo)); // refresh PhasesHelper
+		assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(cmt1), cmt1));
+		assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(cmt2), cmt2));
+		//
+
+		HgServer server = new HgServer().publishing(true).start(srcRepoLoc);
+		try {
+			final HgRemoteRepository srcRemote = hgLookup.detect(server.getURL());
+			new HgPullCommand(dstRepo).source(srcRemote).execute();
+		} finally {
+			server.stop();
+		}
+		// refresh PhasesHelper
+		phaseHelper = new PhasesHelper(HgInternals.getImplementationRepo(dstRepo));
+		errorCollector.assertEquals(HgPhase.Public, phaseHelper.getPhase(5, null));
+		errorCollector.assertEquals(HgPhase.Public, phaseHelper.getPhase(7, null));
+		errorCollector.assertEquals(HgPhase.Public, phaseHelper.getPhase(8, null));
+		// phase of local-only new revisions shall not change
+		errorCollector.assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(cmt1), cmt1));
+		errorCollector.assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(cmt2), cmt2));
+	}
+	
+	@Test
+	public void testPullFromNonPublishing() throws Exception {
+		// copy, not clone as latter updates phase information
+		File srcRepoLoc = RepoUtils.copyRepoToTempLocation("test-phases", "test-pull-nopub-src");
+		File dstRepoLoc = RepoUtils.initEmptyTempRepo("test-pull-nopub-dst");
+		Map<String,?> props = Collections.singletonMap(Internals.CFG_PROPERTY_CREATE_PHASEROOTS, true);
+		final HgLookup hgLookup = new HgLookup(new BasicSessionContext(props, null));
+		HgRepository srcRepo = hgLookup.detect(srcRepoLoc);		
+		// revisions 6 and 9 are secret, so
+		// index of revisions 4 and 5 won't change, but that of 7 and 8 would
+		Nodeid r7 = srcRepo.getChangelog().getRevision(7);
+		Nodeid r8 = srcRepo.getChangelog().getRevision(8);
+
+		HgRepository dstRepo = hgLookup.detect(dstRepoLoc);
+		HgServer server = new HgServer().publishing(false).start(srcRepoLoc);
+		try {
+			final HgRemoteRepository srcRemote = hgLookup.detect(server.getURL());
+			new HgPullCommand(dstRepo).source(srcRemote).execute();
+		} finally {
+			server.stop();
+		}
+		PhasesHelper phaseHelper = new PhasesHelper(HgInternals.getImplementationRepo(dstRepo));
+		errorCollector.assertEquals(HgPhase.Public, phaseHelper.getPhase(4, null));
+		errorCollector.assertEquals(HgPhase.Draft, phaseHelper.getPhase(5, null));
+		errorCollector.assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(r7), r7));
+		errorCollector.assertEquals(HgPhase.Draft, phaseHelper.getPhase(dstRepo.getChangelog().getRevisionIndex(r8), r8));
+		final RevisionSet dstSecret = phaseHelper.allSecret();
+		errorCollector.assertTrue(dstSecret.toString(), dstSecret.isEmpty());
+	}
 
 	private void checkRepositoriesAreSame(HgRepository srcRepo, HgRepository dstRepo) {
 		// XXX copy of TestPush#checkRepositoriesAreSame
--- a/test/org/tmatesoft/hg/test/TestPush.java	Wed Jul 10 11:53:19 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestPush.java	Wed Jul 10 16:41:49 2013 +0200
@@ -255,6 +255,13 @@
 	}
 
 	
+	/**
+	 * XXX doesn't check the case when we push child of a draft revision which is
+	 * known as public on server ((presentLocalDrafts \ outgoing) leaves bogus draft revision, 
+	 * the parent one of the child added and pushed)
+	 * For the time being, TestPull.testPullFromPublishing covers this case (as both push and
+	 * pull share same phase update functionality)
+	 */
 	@Test
 	public void testPushToPublishingServer() throws Exception {
 		// copy, not clone as latter updates phase information