Skip to content

rename detection: don't try to detect submodule renames#4119

Merged
ethomson merged 3 commits intomasterfrom
ethomson/submodule_renames
Feb 10, 2017
Merged

rename detection: don't try to detect submodule renames#4119
ethomson merged 3 commits intomasterfrom
ethomson/submodule_renames

Conversation

@ethomson
Copy link
Copy Markdown
Member

@ethomson ethomson commented Feb 9, 2017

Consider the case where some branch deletes a submodule, then adds a new submodule with a different path.

Since these two paths have the same mode, they will be eligible for rename detection, and we will try to load the blobs to analyze their contents. This will, of course, fail since not only are they not blobs but the objects are in a different repository.

Make sure that the things we're looking at are actually blobs when considering them for rename detection (in diff or merge).

Edward Thomson added 2 commits February 9, 2017 18:13
Test that shows that submodules are incorrectly considered in renames,
and `git_merge_trees` will fail to lookup the submodule as a blob.
Copy link
Copy Markdown
Member

@carlosmn carlosmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread src/diff_tform.c Outdated
@@ -554,7 +554,7 @@ static int similarity_measure(
*score = -1;

/* don't try to compare files of different types */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is now a bit out of sync, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Willfix.

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.

2 participants