Browse Source

Revert "CommitBuilder should check for duplicate parents"

This reverts commit 6bc48cdc62.

Until git v1.7.10.2~29^2~1 (builtin/merge.c: reduce parents early,
2012-04-17), C git merge would make merge commits with duplicate parents
when asked to with a series of commands like the following:

  git checkout origin/master
  git merge --no-ff origin/master

Nowadays "git merge" removes redundant parents more aggressively
(whenever one parent is an ancestor of another and not just when
duplicates exist) but merges with duplicate parents are still permitted
and can be created with git fast-import or git commit-tree and history
viewers need to be able to cope with them.

CommitBuilder is an interface analagous to commit-tree, so it should
allow duplicate parents.  (That said, an option to automatically remove
redundant parents would be useful.)

Reported-by: Dave Borowitz <dborowitz@google.com>
Change-Id: Ia682238397eb1de8541802210fa875fdd50f62f0
Signed-off-by: Jonathan Nieder <jrn@google.com>
tags/v4.0.0.201503231230-m1
Jonathan Nieder 9 years ago
parent
commit
5967b65838

+ 0
- 108
org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SimpleMergeTest.java View File

import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;


import java.io.IOException; import java.io.IOException;
import java.util.Arrays;
import java.util.List;


import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.dircache.DirCacheBuilder; import org.eclipse.jgit.dircache.DirCacheBuilder;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase;
import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.TreeWalk;
import org.junit.Test; import org.junit.Test;
assertEquals("02ba32d3649e510002c21651936b7077aa75ffa9",ourMerger.getResultTreeId().name()); assertEquals("02ba32d3649e510002c21651936b7077aa75ffa9",ourMerger.getResultTreeId().name());
} }


@Test
public void testDuplicateParents() throws Exception {
ObjectId commitId;
RevCommit newCommit;
final ObjectInserter ow = db.newObjectInserter();
RevWalk rw = new RevWalk(db);
ObjectId parentA = db.resolve("a");
ObjectId parentB = db.resolve("b");
ObjectId[] parentIds_AA = new ObjectId[] { parentA, parentA };
ObjectId[] parentIds_AB = new ObjectId[] { parentA, parentB };
ObjectId[] parentIds_BA = new ObjectId[] { parentB, parentA };
ObjectId[] parentIds_BBAB = new ObjectId[] { parentB, parentB, parentA,
parentB };

try {
commitId = commit(ow, db.readDirCache(), parentA, parentA);
fail("an expected exception did not occur");
} catch (IllegalArgumentException e) {
//
}

commitId = commit(ow, db.readDirCache(), parentA, parentB);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

commitId = commit(ow, db.readDirCache(), parentB, parentA);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

try {
commitId = commit(ow, db.readDirCache(), parentIds_AA);
fail("an expected exception did not occur");
} catch (IllegalArgumentException e) {
//
}

commitId = commit(ow, db.readDirCache(), parentIds_AB);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

commitId = commit(ow, db.readDirCache(), parentIds_BA);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

try {
commitId = commit(ow, db.readDirCache(), parentIds_BBAB);
fail("an expected exception did not occur");
} catch (IllegalArgumentException e) {
//
}

try {
commitId = commit(ow, db.readDirCache(),
Arrays.asList(parentIds_AA));
fail("an expected exception did not occur");
} catch (IllegalArgumentException e) {
//
}

commitId = commit(ow, db.readDirCache(), parentIds_AB);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

commitId = commit(ow, db.readDirCache(), parentIds_BA);
newCommit = rw.parseCommit(commitId);
assertEquals(2, newCommit.getParentCount());

try {
commitId = commit(ow, db.readDirCache(), parentIds_BBAB);
fail("an expected exception did not occur");
} catch (IllegalArgumentException e) {
//
}
}

@Test @Test
public void testTrivialTwoWay_disjointhistories() throws IOException { public void testTrivialTwoWay_disjointhistories() throws IOException {
Merger ourMerger = MergeStrategy.SIMPLE_TWO_WAY_IN_CORE.newMerger(db); Merger ourMerger = MergeStrategy.SIMPLE_TWO_WAY_IN_CORE.newMerger(db);
odi.flush(); odi.flush();
return id; return id;
} }

private ObjectId commit(final ObjectInserter odi, final DirCache treeB,
final AnyObjectId parentId1, final AnyObjectId parentId2)
throws Exception {
final CommitBuilder c = new CommitBuilder();
c.setTreeId(treeB.writeTree(odi));
c.setAuthor(new PersonIdent("A U Thor", "a.u.thor", 1L, 0));
c.setCommitter(c.getAuthor());
c.setParentIds(parentId1, parentId2);
c.setMessage("Tree " + c.getTreeId().name());
ObjectId id = odi.insert(c);
odi.flush();
return id;
}

private ObjectId commit(final ObjectInserter odi, final DirCache treeB,
List<ObjectId> parents) throws Exception {
final CommitBuilder c = new CommitBuilder();
c.setTreeId(treeB.writeTree(odi));
c.setAuthor(new PersonIdent("A U Thor", "a.u.thor", 1L, 0));
c.setCommitter(c.getAuthor());
c.setParentIds(parents);
c.setMessage("Tree " + c.getTreeId().name());
ObjectId id = odi.insert(c);
odi.flush();
return id;
}
} }

+ 26
- 0
org.eclipse.jgit.test/tst/org/eclipse/jgit/revplot/PlotCommitListTest.java View File

test.noMoreCommits(); test.noMoreCommits();
} }


// test a history where a merge commit has two time the same parent
@Test
public void testDuplicateParents() throws Exception {
final RevCommit m1 = commit();
final RevCommit m2 = commit(m1);
final RevCommit m3 = commit(m2, m2);

final RevCommit s1 = commit(m2);
final RevCommit s2 = commit(s1);

PlotWalk pw = new PlotWalk(db);
pw.markStart(pw.lookupCommit(m3));
pw.markStart(pw.lookupCommit(s2));
PlotCommitList<PlotLane> pcl = new PlotCommitList<PlotLane>();
pcl.source(pw);
pcl.fillTo(Integer.MAX_VALUE);

CommitListAssert test = new CommitListAssert(pcl);
test.commit(s2).nrOfPassingLanes(0);
test.commit(s1).nrOfPassingLanes(0);
test.commit(m3).nrOfPassingLanes(1);
test.commit(m2).nrOfPassingLanes(0);
test.commit(m1).nrOfPassingLanes(0);
test.noMoreCommits();
}

/** /**
* The graph shows the problematic original positioning. Due to this some * The graph shows the problematic original positioning. Due to this some
* lanes are no straight lines here, but they are with the new layout code) * lanes are no straight lines here, but they are with the new layout code)

+ 0
- 1
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties View File

downloadCancelled=Download cancelled downloadCancelled=Download cancelled
downloadCancelledDuringIndexing=Download cancelled during indexing downloadCancelledDuringIndexing=Download cancelled during indexing
duplicateAdvertisementsOf=duplicate advertisements of {0} duplicateAdvertisementsOf=duplicate advertisements of {0}
duplicateParents=The following parent was specified multiple times: {0}
duplicateRef=Duplicate ref: {0} duplicateRef=Duplicate ref: {0}
duplicateRemoteRefUpdateIsIllegal=Duplicate remote ref update is illegal. Affected remote name: {0} duplicateRemoteRefUpdateIsIllegal=Duplicate remote ref update is illegal. Affected remote name: {0}
duplicateStagesNotAllowed=Duplicate stages not allowed duplicateStagesNotAllowed=Duplicate stages not allowed

+ 0
- 1
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java View File

/***/ public String downloadCancelled; /***/ public String downloadCancelled;
/***/ public String downloadCancelledDuringIndexing; /***/ public String downloadCancelledDuringIndexing;
/***/ public String duplicateAdvertisementsOf; /***/ public String duplicateAdvertisementsOf;
/***/ public String duplicateParents;
/***/ public String duplicateRef; /***/ public String duplicateRef;
/***/ public String duplicateRemoteRefUpdateIsIllegal; /***/ public String duplicateRemoteRefUpdateIsIllegal;
/***/ public String duplicateStagesNotAllowed; /***/ public String duplicateStagesNotAllowed;

+ 7
- 37
org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java View File

import java.io.OutputStreamWriter; import java.io.OutputStreamWriter;
import java.io.UnsupportedEncodingException; import java.io.UnsupportedEncodingException;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.text.MessageFormat;
import java.util.List; import java.util.List;


import org.eclipse.jgit.internal.JGitText;

/** /**
* Mutable builder to construct a commit recording the state of a project. * Mutable builder to construct a commit recording the state of a project.
* *
* branch being merged into the current branch. * branch being merged into the current branch.
*/ */
public void setParentIds(AnyObjectId parent1, AnyObjectId parent2) { public void setParentIds(AnyObjectId parent1, AnyObjectId parent2) {
if (!parent1.equals(parent2))
parentIds = new ObjectId[] { parent1.copy(), parent2.copy() };
else
throw new IllegalArgumentException(MessageFormat.format(
JGitText.get().duplicateParents, parent1.getName()));
parentIds = new ObjectId[] { parent1.copy(), parent2.copy() };
} }


/** /**
* the entire list of parents for this commit. * the entire list of parents for this commit.
*/ */
public void setParentIds(ObjectId... newParents) { public void setParentIds(ObjectId... newParents) {
ObjectId[] tmpIds = new ObjectId[newParents.length];

int newParentCount = 0;
for (int i = 0; i < newParents.length; i++) {
for (int j = 0; j < newParentCount; j++)
if (tmpIds[j].equals(newParents[i]))
throw new IllegalArgumentException(MessageFormat.format(
JGitText.get().duplicateParents,
tmpIds[j].getName()));
tmpIds[newParentCount++] = newParents[i].copy();
}
parentIds = tmpIds;
parentIds = new ObjectId[newParents.length];
for (int i = 0; i < newParents.length; i++)
parentIds[i] = newParents[i].copy();
} }


/** /**
* the entire list of parents for this commit. * the entire list of parents for this commit.
*/ */
public void setParentIds(List<? extends AnyObjectId> newParents) { public void setParentIds(List<? extends AnyObjectId> newParents) {
ObjectId[] tmpIds = new ObjectId[newParents.size()];

int newParentCount = 0;
for (AnyObjectId newId : newParents) {
for (int j = 0; j < newParentCount; j++)
if (tmpIds[j].equals(newId))
throw new IllegalArgumentException(MessageFormat.format(
JGitText.get().duplicateParents,
tmpIds[j].getName()));
tmpIds[newParentCount++] = newId.copy();
}
parentIds = tmpIds;
parentIds = new ObjectId[newParents.size()];
for (int i = 0; i < newParents.size(); i++)
parentIds[i] = newParents.get(i).copy();
} }


/** /**
if (parentIds.length == 0) { if (parentIds.length == 0) {
setParentId(additionalParent); setParentId(additionalParent);
} else { } else {
for (int i = 0; i < parentIds.length; i++)
if (parentIds[i].equals(additionalParent))
throw new IllegalArgumentException(MessageFormat.format(
JGitText.get().duplicateParents,
parentIds[i].getName()));
ObjectId[] newParents = new ObjectId[parentIds.length + 1]; ObjectId[] newParents = new ObjectId[parentIds.length + 1];
System.arraycopy(parentIds, 0, newParents, 0, parentIds.length); System.arraycopy(parentIds, 0, newParents, 0, parentIds.length);
newParents[parentIds.length] = additionalParent.copy(); newParents[parentIds.length] = additionalParent.copy();

Loading…
Cancel
Save