Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 510074

Summary:	Git log will not find merge commits on Node
Product:	[ECD] Orion	Reporter:	Remy Suen <remy.suen>
Component:	Git	Assignee:	Remy Suen <remy.suen>
Status:	RESOLVED FIXED	QA Contact:
Severity:	major
Priority:	P3	CC:	steve_northover
Version:	13.0
Target Milestone:	15.0
Hardware:	All
OS:	All
Whiteboard:
Bug Depends on:
Bug Blocks:	510062

Description Remy Suen

2017-01-08 01:02:18 EST

With the rewrite to use fileHistoryWalk(*) in bug 507334 for searching for commits that affect a given file, a regression was introduced wherein merge commits are no longer being displayed in the file's log.

I have submitted a failing test case to NodeGit but I'm not sure when the bug will get fixed... :(

https://github.com/nodegit/nodegit/pull/1184

When it does get fixed, we will need to upgrade our NodeGit dependency to pick up the fix.

In the meantime, I am not sure what we want to do here. I am inclined to say we revert the fix for bug 507334 myself.

Comment 1 Steve Northover

2017-03-30 16:22:34 EDT

Remy, what is the status of this problem?  Should we revert the other code?

Comment 2 Remy Suen

2017-04-01 00:09:02 EDT

(In reply to Steve Northover from comment #1)
> Remy, what is the status of this problem?

It is still broken.

> Should we revert the other code?

I think we should. Sadly, performance will degrade significantly but slow and correct is better than fast and wrong.

Comment 3 Steve Northover

2017-04-01 11:36:54 EDT

Please revert for now and engage with SSQ to see whether there is something more that can be done.

Comment 4 Remy Suen

2017-04-03 06:23:20 EDT

(In reply to Steve Northover from comment #3)
> Please revert for now and engage with SSQ to see whether there is something
> more that can be done.

I haven't reverted the code yet but I made some simple comparisons.

New code:
modules/orionode/package.json - 4 seconds
README.md - 3 seconds

Old code:
modules/orionode/package.json - 5 seconds
README.md - 90 seconds

Comment 5 Remy Suen

2017-04-03 07:54:55 EDT

As outlined in bug 507334 comment 4, the reason the old code was slow was because it was running a full diff for a given commit and its parent. So, in order to avoid doing this, we must look for an alternate way of determining whether a file has been modified or not.

To achieve this, we should instead look at an individual tree entry's properties. If the file's tree entry has its id changed, then we know that it is now pointing at a new blob which means that tree entry has been modified when compared to the previous parent commit. However, this doesn't cover the rename case so I think some additional logic may be required to consider this case.

1. Get id of tree entry at test.txt from commit A.
2. Get id of tree entry at test.txt from commit B.
3. If they differ, then it has been modified.
4. Recurse to 1 with commit B with test.txt.

1. Get id of tree entry at test.txt from commit A.
2. Get id of tree entry at test.txt from commit B.
3. If the entry doesn't exist at commit B, then the file was newly created in commit A. We check the diff of commit A to verify this. If the diff indicates that the file has been created, recurse into 1 with commit B with test.txt.
4. However, if the diff indicates that it has been renamed, then recurse into commit B with original.txt instead so that we appropriately follow the rename as we go further in the history.

Comment 6 Remy Suen

2017-04-05 03:59:55 EDT

Proposed solution in comment 5 pushed to master.
https://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=07a3e0b556f4c78512f4122cbc7c5cacdf3fbccc

Renames will be tracked in a separate bug if that feature is desired.
https://dev.eclipse.org/mhonarc/lists/orion-dev/msg04070.html