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

Avoid using diff to figure out files that changed in a revision. #10948

Closed
wants to merge 1 commit into from

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented May 10, 2018

In a history with a complex merge structure, trying to figure out
which files changed using git diff appears to be fraught with peril,
since it tends to show changes on both sides of the merge. With
e.g. PR 7579 where the branch was long-lived this ends up considering
most of the repository as changed and causes the command to fail. A
simple approach that seems to be reliable is to use git log
--name-only, with --first-parent to handle merges in a predicatable
way.


This change is Reviewable

In a history with a complex merge structure, trying to figure out
which files changed using git diff appears to be fraught with peril,
since it tends to show changes on both sides of the merge. With
e.g. PR 7579 where the branch was long-lived this ends up considering
most of the repository as changed and causes the command to fail. A
simple approach that seems to be reliable is to use git log
--name-only, with --first-parent to handle merges in a predicatable
way.
@gsnedders
Copy link
Member

Do we not want something analogue to git diff $(git merge-base origin/master FETCH_HEAD)..FETCH_HEAD? Talking to @jgraham it sounds like the problem is in finding the merge-base rather than the diff?

Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

I think this is completely the wrong solution, as it relies on everything on master being the first parent, and given the whole issue here is when branches have weird histories and this just trades it for another issue. See my forthcoming PR for another attempt at fixing this.

@gsnedders
Copy link
Member

That PR is now #10950

@jgraham
Copy link
Contributor Author

jgraham commented May 10, 2018

I think irespective of the other changes, this is safer in that it's less likely to include random crap because git log allows us to take one branch in the face of a merge whereas git diff doesn't. Having said that, if you have random merges in your history we should just reject the PR.

@jgraham jgraham closed this Aug 14, 2018
@sideshowbarker sideshowbarker deleted the improve_files_affected branch November 22, 2018 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants