-
Notifications
You must be signed in to change notification settings - Fork 451
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
Update git version to 2.18.0.gvfs.1.26.gf61ade4 #166
Conversation
This updates the git version to 2.18.0 which includes changes to the rename detection to use config setting which was previously turned off with a patch in git.
Dictionary<string, GitConfigSetting> statusConfig = GitConfigHelper.GetSettings( | ||
File.ReadAllLines(Path.Combine(srcRoot, GVFSConstants.DotGit.Config)), | ||
"status"); | ||
"test"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between test.renames and merge.renames?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benpeart can elaborate but this is so the tests will pass when someone has status.renames=true in their global config because our code is not following proper git config hierarchy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check just for our tests? Or is it needed for customer machines as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so that the test cases can pass without enabling someone to bypass the "--no-renames" test in the pre-command hook by setting status.renames=true in their global config.
If I leave this as "status" then if someone set status.renames=true in their global config (an entirely valid setting) we would allow the command to go through instead of blocking it and requiring --no-renames. I only left this in as "test" so that I can make the test cases pass until we get a proper fix that checks all config files, deals with inheritance from diff.renames, etc.
Details on the various config settings, and how they are inherited/overwritten by other settings and command line options are in the patch to git (commit e8b2dc2c)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the details!
This updates the git version to 2.18.0 which includes changes to the rename
detection to use config setting which was previously turned off with a patch
in git.