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

Conditional includes #4244

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@pks-t
Member

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

This comment has been minimized.

Show comment
Hide comment
@pks-t

pks-t Jun 6, 2017

Member

Will hold this PR until #4250 is merged.

Member

pks-t commented Jun 6, 2017

Will hold this PR until #4250 is merged.

pks-t added some commits May 23, 2017

repository: constify several repo parameters for getters
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.
path: expose `git_path_is_absolute`
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.
path: expose `git_path_is_dirsep`
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.
config: pass repository when opening config files
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`.
config_file: extract function to parse include path
The logic inside this function will be required later on, when
implementing conditional includes. Extract it into its own function to
ease the implementation.
config_file: pass repository down to the parser
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.
config_file: implement conditional "gitdir" includes
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.
config_file: implement "gitdir/i" conditional
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

This comment has been minimized.

Show comment
Hide comment
@pks-t

pks-t Jul 21, 2017

Member

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

Member

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-t:pks/conditional-includes branch Aug 25, 2017

@pks-t

This comment has been minimized.

Show comment
Hide comment
@pks-t

pks-t Aug 25, 2017

Member

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

Member

pks-t commented Aug 25, 2017

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

@pks-t pks-t referenced this pull request Aug 25, 2017

Merged

Conditional includes #4332

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment