config: fix adding files if their parent directory is a file#4898
Merged
Conversation
When we try to add a configuration file with `git_config_add_file_ondisk`, we treat nonexisting files as empty. We do this by performing a stat call, ignoring ENOENT errors. This works just fine in case the file or any of its parents simply does not exist, but there is also the case where any of the parent directories is not a directory, but a file. So e.g. trying to add a configuration file "/dev/null/.gitconfig" will fail, as `errno` will be ENOTDIR instead of ENOENT. Catch ENOTDIR in addition to ENOENT to fix the issue. Add a test that verifies we are able to add configuration files with such an invalid path file just fine.
Member
|
Seems reasonable to me. Thanks! |
Member
|
Thanks. This works great for reading, but I wonder if we might leave users with a confusing message if they try to write. Part of the reason we treat nonexistent paths as empty config files is to allow you to e.g. grab the default set of configuration files and write into the appropriate one which might mean writing to the global one (tbh I think that's the only likely scenario where this would happen). It's probably fine, but I wanted to point this possible unexpected case. |
Member
Author
|
On Thu, Nov 29, 2018 at 02:39:07AM -0800, Carlos Martín Nieto wrote:
Thanks. This works great for reading, but I wonder if we might
leave users with a confusing message if they try to write. Part
of the reason we treat nonexistent paths as empty config files
is to allow you to e.g. grab the default set of configuration
files and write into the appropriate one which might mean
writing to the global one (tbh I think that's the only likely
scenario where this would happen).
It's probably fine, but I wanted to point this possible
unexpected case.
Yeah, I had this in mind already. Thing is that this is really no
different to what we currently have. E.g. HOME=/home/pks and
/home/pks does not exist. We will add the empty configuration
file /home/pks/.gitconfig just fine, but writing to it will fail
as we are not going to create the parent missing directory.
|
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
3 participants
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.
When we try to add a configuration file with
git_config_add_file_ondisk, wetreat nonexisting files as empty. We do this by performing a stat call, ignoring
ENOENT errors. This works just fine in case the file or any of its parents
simply does not exist, but there is also the case where any of the parent
directories is not a directory, but a file. So e.g. trying to add a
configuration file "/dev/null/.gitconfig" will fail, as
errnowill be ENOTDIRinstead of ENOENT.
Catch ENOTDIR in addition to ENOENT to fix the issue. Add a test that verifies
we are able to add configuration files with such an invalid path file just fine.
This should probably fix #4897.