Re-design how the core.longPaths config setting is read#4031
Merged
dscho merged 2 commits intogit-for-windows:mainfrom Sep 22, 2022
Merged
Re-design how the core.longPaths config setting is read#4031dscho merged 2 commits intogit-for-windows:mainfrom
core.longPaths config setting is read#4031dscho merged 2 commits intogit-for-windows:mainfrom
Conversation
Member
Author
|
It might be too late for this PR to be merged before v2.38.0 is due. I'm not sure. Opinions? |
There was a problem hiding this comment.
On one hand, I normally don't like merging something that touches so many files so late in the release cycle.
On the other hand, the change seems clear. I especially like how you removed the global in favor of the method call. I also think that we aren't going to get any more confidence in this except by shipping it, since I doubt we are using long paths anywhere in our CI chain.
derrickstolee
approved these changes
Sep 22, 2022
Let's avoid requiring `git_config()` to have been called all the time. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is no longer necessary (and was fragile to begin with, as many code paths might have been missed). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
4488b44 to
04a77c9
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For years now, we carry a rather large patch that essentially forces all commands to call the default config parser, first thing.
However, this is a bit fragile because too often, code changes, and either config calls are lost, or new locations are missed that would require the same treatment.
Let's overhaul the design to determine the
core.longPathsvalue only when needed, via the cached config. This is much more robust, as it no longer requires this ginormous patch and as no new locations can be missed.