Bug 518591 - Merges that could have been fast-forwarded cause the res…

…t of the history to be ignored

When retrieving the log of a given file, merges are analyzed to
determine whether a particular branch of history is needed or not. If
the end result of a file after a merge is the same as the end result
of one of the merge's parent branches, that means the merge's other
parent branches can be ignored as the other branches' histories have
effectively been discarded in favour of the other branch. In such
cases, we flag the other parents to be ignored when traversing the
history so that they won't be considered and included in the returned
log.

B
|\
| B
|/
A

However, if a merge is actaully a fast-forward merge like the graph
above, then we shouldn't ignore the other (seemingly irrelevant)
parent branch as it actually contains the rest of the history of the
file. Thus, if the merge's parent commits have a common ancestor that
is actually identical to one of the merges' parent commits, that
common ancestor should not be flagged to be ignored by the traversal
or the rest of the history will end up being ignored by the
traversal.

modules/orionode/lib/git/commit.js

@@ -169,8 +169,10 @@ function getCommitLog(req, res) {
 								if (entries[i] !== null && entries[i].id().equal(entryId)) {
 									// if the entry id is the same as a parent, that means
 									// the content from the other branches should be ignored
+									// as they have been discarded in favour of the changes
+									// of this particular branch of history
 									for (var j = 0; j < entries.length; j++) {
-										if (i !== j) {
+										if (i !== j && (base === null || !parents[j].id().equal(base))) {
 											ignore.push(parents[j].id());
 										}
 									}

modules/orionode/test/test-git-api.js

@@ -3037,6 +3037,96 @@ maybeDescribe("git", function() {
 				});
 			});
 
+			/**
+			 * Confirm the parent history information of the following graph.
+			 * The merge commit has B as its first parent and C as its second
+			 * parent. It keeps the changes from C after the merge.
+			 * Commits A, B, and C all modify the file.
+			 * 
+			 * Actual repository history:
+			 * 
+			 * M
+			 * |\
+			 * | \
+			 * |  \
+			 * U   C
+			 * |  /
+			 * | /
+			 * |/
+			 * B
+			 * |
+			 * A
+			 * 
+			 * Returned history for the file:
+			 * 
+			 * C
+			 * |
+			 * B
+			 * |
+			 * A
+			 */
+			it("bug518591", function(finished) {
+				var commitA, commitB, commitC, local;
+				var name = "test.txt";
+				var testName = "bug518591";
+				var fullPath = path.join(WORKSPACE, testName);
+				var client = new GitClient(testName);
+				client.init();
+				// create the file at commit A
+				client.setFileContents(name, "A");
+				client.stage(name);
+				client.commit();
+				client.start().then(function(commit) {
+					commitA = commit.Id;
+					// modify to B
+					client.setFileContents(name, "B");
+					client.stage(name);
+					client.commit();
+					return client.start();
+				})
+				.then(function(commit) {
+					commitB = commit.Id;
+					// create commit C
+					client.setFileContents(name, "C");
+					client.stage(name);
+					client.commit();
+					return client.start();
+				})
+				.then(function(commit) {
+					commitC = commit.Id;
+					// branch
+					client.createBranch("other");
+					// reset to B
+					client.reset("HARD", commitB);
+					return client.start();
+				})
+				.then(function() {
+					return git.Repository.open(fullPath);
+				})
+				.then(function(repo) {
+					// merge the other branch without fast-forwarding using NodeGit APIs
+					return repo.mergeBranches("HEAD", "other", null, git.Merge.PREFERENCE.NO_FASTFORWARD);
+				})
+				.then(function() {
+					client.log("master", "master", name);
+					return client.start();
+				})
+				.then(function(log) {
+					assert.equal(log.Children.length, 3);
+					assert.equal(log.Children[0].Id, commitC);
+					assert.equal(log.Children[0].Parents.length, 1);
+					assert.equal(log.Children[0].Parents[0].Name, commitB);
+					assert.equal(log.Children[1].Id, commitB);
+					assert.equal(log.Children[1].Parents.length, 1);
+					assert.equal(log.Children[1].Parents[0].Name, commitA);
+					assert.equal(log.Children[2].Id, commitA);
+					finished();
+				})
+				.catch(function(err) {
+					finished(err);
+				});
+			});
+
 			/**
 			 * Confirm the parent history information of the following graph.
 			 * We want to make sure that the extraneous commits A, B, C, and D