Skip to content

Conditional includes#4244

Closed
pks-t wants to merge 8 commits into
libgit2:masterfrom
pks-t:pks/conditional-includes
Closed

Conditional includes#4244
pks-t wants to merge 8 commits into
libgit2:masterfrom
pks-t:pks/conditional-includes

Conversation

@pks-t
Copy link
Copy Markdown
Member

@pks-t pks-t commented May 24, 2017

A first implementation of conditional includes. I'd mainly like to get some feedback on the implementation, as these conditionals require us to break backwards compatibility for configuration backends. This results from the requirement to have a repository struct available when parsing the config, as otherwise we wouldn't be able to resolve the conditions correctly. Obviously, parsing configurations still works when no repository is available -- in that case, we will simply treat all conditions as non-matching.

@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Jun 6, 2017

Will hold this PR until #4250 is merged.

@pks-t pks-t force-pushed the pks/conditional-includes branch 3 times, most recently from 6a53219 to a857842 Compare June 9, 2017 07:18
pks-t added 8 commits July 21, 2017 11:29
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.
To implement conditional includes, we require certain information from
the repository in whose context a file is being parsed. To have access
to this, pass down the repository to the parser to make it available.
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.
@pks-t pks-t force-pushed the pks/conditional-includes branch from a857842 to 51e4a64 Compare July 21, 2017 10:12
@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Jul 21, 2017

Rebased on top of #4250 and fixed conflicts. I consider this PR ready now

@pks-t pks-t closed this Aug 25, 2017
@pks-t pks-t deleted the pks/conditional-includes branch August 25, 2017 14:30
@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Aug 25, 2017

Oh, I've accidentally closed this PR. Will create a new one.

@pks-t pks-t mentioned this pull request Aug 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant