Conditional includes#4332
Conversation
a857842 to
7089a9b
Compare
|
Rebased to fix conflicts. |
This function has previously been implemented in Windows-specific path handling code as `path__is_absolute`. As we will need this functionality in other parts, extract the logic into "path.h" alongside with a non-Windows implementation.
This function has previously been implemented in Windows-specific path handling code as `path__is_dirsep`. As we will need this functionality in other parts, extract the logic into "path.h" alongside with a non-Windows implementation.
Several functions to retrieve variables from a repository only return immutable values, which allows us to actually constify the passed-in repository parameter. Do so to help a later patch, which will only have access to a constant repository.
Our current configuration logic is completely oblivious of any repository, but only cares for actual file paths. Unfortunately, we are forced to break this assumption by the introduction of conditional includes, which are evaluated in the context of a repository. Right now, only one conditional exists with "gitdir:" -- it will only include the configuration if the current repository's git directory matches the value passed to "gitdir:". To support these conditionals, we have to break our API and make the repository available when opening a configuration file. This commit extends the `open` call of configuration backends to include another repository and adjusts existing code to have it available. This includes the user-visible functions `git_config_add_file_ondisk` and `git_config_add_backend`.
The logic inside this function will be required later on, when implementing conditional includes. Extract it into its own function to ease the implementation.
The reader machinery will be extended to handle conditional includes. The only conditions that currently exist all match the against the git directory of the repository the config file belongs to. As such, we need to have access to the repository when reading configuration files to properly handle these conditions. One specialty of thes conditional includes is that the actual pattern may also be a relative pattern starting with "./". In this case, we have to match the pattern against the path relative to the config file which is currently being parsed. So besides the repository, we also have to pass down the path to the current config file that is being parsed.
fc9c473 to
3ace427
Compare
Upstream git.git has implemented the ability to include other configuration files based on conditions. Right now, this only includes the ability to include a file based on the gitdir-location of the repository the currently parsed configuration file belongs to. This commit implements handling these conditional includes for the case-sensitive "gitdir" condition.
Next to the "gitdir" conditional for including other configuration files, there's also a "gitdir/i" conditional. In contrast to the former one, path matching with "gitdir/i" is done case-insensitively. This commit implements the case-insensitive condition.
3ace427 to
f7d837c
Compare
|
I've fixed the issue on Windows now, but tests on macOS currently fail. I have no idea why that is and am unfortunately unable to debug into that issue due to not having a working macOS setup right now. :( |
|
The error on macOS comes from a peculiarity of how it handles temporary paths. The whereas the The reason we get different paths is that we perform a whole bunch of |
We put our repository in the temporary directory which makes macOS map the path into a virtual path. `realpath(3)` can resolve it and we do so during repository opening, but that makes its path have a different prefix from the sandbox path clar thinks we have. Resolve the sandbox path before putting it into the test config files so the paths match as expected.
| } diskfile_readonly_backend; | ||
|
|
||
| static int config_read(git_strmap *values, struct config_file *file, git_config_level_t level, int depth); | ||
| static int config_read(const git_repository *repo, git_strmap *values, struct config_file *file, git_config_level_t level, int depth); |
There was a problem hiding this comment.
The values map is the output parameter, isn't it? The repository should come later in the arg list.
| return 0; | ||
|
|
||
| condition = git__substrdup(section + strlen("includeIf."), | ||
| strlen(section) - strlen("includeIf.") - strlen(".path")); |
There was a problem hiding this comment.
This might just be me, but I prefer the method used in the normalisation which searches for the first and last '.' to figure out how long the middle part is without making us count hard-coded strings.
|
I went ahead and did the arg-order change. Yay for features. |
|
The parameter reordering makes sense. Thanks for improving and merging! |
|
Why pass a repo object into git_config_functions and not just a path? This would allow to construct git_config objects w/o intantiating a repo first. |
|
Because we don't know what else git is going to come up with and adding a path would mean that we're going to have to constantly change the interface. |
I've accidentally closed #4244 as I have deleted my branch. Re-opening here.