Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diff rename detection cycle fixes #1659

Merged
merged 3 commits into from
Jun 18, 2013
Merged

Conversation

arrbee
Copy link
Member

@arrbee arrbee commented Jun 18, 2013

As @ethomson has pointed out in #1656, diff rename detection has problems when you introduce a cycle of renames (a->b, b->c, c->a) because processing the earlier rename of a->b would alter the diff records so that when it was time process b->c, the b record was no longer the appropriate match.

Instead of making huge changes, this PR fixes this by tweaking a few details:

  1. When looking for the closest match to a given record, if we displace an earlier located match, then note that we "bumped" a record and repeat the initial phase of finding closest matches until no records get bumped.
  2. When updating records in place, always make the target record into the RENAME record (i.e. make this a true reverse mapping, even for data updates) so we always preserve records that are sources if possible.
  3. If we are forced to overwrite a source record (which now only happens when we have two split MODIFIED records), update the best match mappings to point the unprocessed mapping at the new record location.

This PR includes @ethomson's previously failing tests from #1656 and contains my fixes for the issue. Hopefully he will look this over and give it his blessing.

Edward Thomson and others added 3 commits June 18, 2013 09:39
This makes the diff rename tracking code more careful about the
order in which it processes renames and more thorough in updating
the mapping of correct renames when an earlier rename update
alters the index of a later matched pair.
@arrbee
Copy link
Member Author

arrbee commented Jun 18, 2013

As an optimization, the loop that finds the best matches (which is rerun if a record is bumped) could be adjusted so that records which are not bumped after a run are marked as "finalized" and don't have to be reprocessed on later times through the loop. The code doesn't do that now. I'm not sure if it would actually be a win for the additional complexity.

@ethomson
Copy link
Member

So much better than my fix would have been.

vmg pushed a commit that referenced this pull request Jun 18, 2013
Diff rename detection cycle fixes
@vmg vmg merged commit 84ba494 into libgit2:development Jun 18, 2013
@arrbee arrbee deleted the rename-cycle-fixes branch June 18, 2013 23:48
phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants