Conditional config include
v5 cover Conditional config include
Let's try this again. v4 and before can be found in the original thread [1]. The remaining issues of v4 were
Quoted 8 lines · Jeff King
Quoted 1 line · Duy Nguyen
Ping..
There was some discussion after v4. I think the open issues are:
- the commit message is rather terse (it should describe motivation, and can refer to the docs for the "how")
Fixed
Quoted 8 lines
- the syntax might be more clear as:
[include-if "gitdir:..."] or [include "gitdir-is:..."]
I'll go with include-if. It feels a bit awkward to add -is to every new type of condition.
Quoted 5 lines
- there is an open question of whether we would like to go this route, maintaining backwards compatibility syntactically (and thus having these includes quietly skipped in older versions), or introduce a breaking syntax that could be more convenient, like:
Quoted 7 lines
[if-gitdir(...):section]
conditional-but-no-include-necessary = trueI do not have a strong opinion myself, though I had written the original [include] assuming syntactic compatibility, and I think it has worked out (e.g., you can manipulate include.path via "git config" just as you would any other key).
I'll go with the what we have done, at least it's proven not provoking too bad effects. So no breaking syntax.
[1] http://public-inbox.org/git/[email protected]/
Nguyễn Thái Ngọc Duy (1):
config: add conditional include
Documentation/config.txt | 54 +++++++++++++++++++++++++ config.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
Signature · 1 line
2.11.0.157.gd943d85
PATCH v5 1/1 config: add conditional include
This allows some more flexibility in managing configuration across repositories. The most often seen use case on the mailing list is when the user needs to use different email addresses on different repositories. If these repositories share something that we can use to group them up, then we can set the same configuration for groups automatically.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with include-if.xxx.path to make it clearer that a condition is required.
Similar to include.path, older git versions that don't understand include-if will simply ignore them.
--- Documentation/config.txt | 54 +++++++++++++++++++++++++ config.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 ++++++++++++++++++++++++++ 3 files changed, 210 insertions(+)
Documentation/config.txt +54 -0
@@ -91,6 +91,49 @@ found at the location of the include directive. If the value of the relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a special `include-if.<condition>.path` variable to the name of the +file to be included. The variable is treated the same way as +`include.path`. + +The condition starts with a keyword, followed by a colon and a +pattern. The interpretation of the pattern depends on the keyword. +Supported keywords are: + +`gitdir`:: + The environment variable `GIT_DIR` must match the following + pattern for files to be included. The pattern can contain + standard globbing wildcards and two additional ones, `**/` and + `/**`, that can match multiple path components. Please refer + to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching: + + * Symlinks in `$GIT_DIR` are not resolved before matching. + + * Note that "../" is not special and will match literally, which is + unlikely what you want. Example ~~~~~~~
@@ -119,6 +162,17 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your `$HOME` directory + ; include if $GIT_DIR is /path/to/foo/.git + [include-if "gitdir:/path/to/foo/.git"] + path = /path/to/foo.inc + + ; include for all repositories inside /path/to/group + [include-if "gitdir:/path/to/group/"] + path = /path/to/foo.inc + + ; include for all repositories inside $HOME/to/group + [include-if "gitdir:~/to/group/"] + path = /path/to/foo.inc Values ~~~~~~
config.c +100 -0
121 lines not on this page · raw message
t/t1305-config-include.sh +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.11.0.157.gd943d85
Re: [PATCH v5 1/1] config: add conditional include
Quoted 7 lines · Nguyễn Thái Ngọc Duy
Quoted 4 lines
There was some discussion after v4. I think the open issues are:
- the commit message is rather terse (it should describe motivation, and can refer to the docs for the "how")
This allows some more flexibility in managing configuration across repositories.
That is not an ideal opening to describe motivation without people knowing what "this" is ;-) Of course, the person who wrote the sentence know it already after writing the patch and the subject, but others don't.
Sometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with include-if.xxx.path to make it clearer that a condition is required.
Similar to include.path, older git versions that don't understand include-if will simply ignore them.
or something along that line?
Documentation/config.txt found at the location of the include directive. If the value of the · line 100
Quoted 7 lines
Documentation/config.txt +7
-0+Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a special `include-if.<condition>.path` variable to the name of the +file to be included. The variable is treated the same way as +`include.path`.
Drop "special", as all configuration variables are "special" in their own sense, it does not add any useful information.
I think we avoid '-' in Git-native variable and section names, so "include-if" would become an odd-man-out.
The variable is obviously not treated the same way as include.path ;-)
When includeIf.<condition>.path variable is set in a configuration file, the configuration file named by that variable is included (in a way similar to how include.path works) only if the <condition> holds true.
Documentation/config.txt found at the location of the include directive. If the value of the · line 103
Quoted 2 lines
Documentation/config.txt +2
-0+The condition starts with a keyword, followed by a colon and a +pattern. The interpretation of the pattern depends on the keyword.
"a pattern"? I think it is "some data whose format and meaning depends on the keyword".
Hence...
Documentation/config.txt found at the location of the include directive. If the value of the · line 108
Quoted 5 lines
Documentation/config.txt +5
-0+Supported keywords are: + +`gitdir`:: + The environment variable `GIT_DIR` must match the following + pattern for files to be included. The pattern can contain
The data that follows the keyword gitdir: is used as a glob pattern. If the location of the .git directory (which may be auto-discovered, or come from GIT_DIR environment variable) match the pattern, the <condition> becomes true.
Documentation/config.txt found at the location of the include directive. If the value of the · line 131
Quoted 10 lines
+ ...
+ * If the pattern ends with/,**will be automatically added. ForDocumentation/config.txt +8
-0+ example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching:
As future <keywords> may not be about path or matching at all, this belongs to gitdir:: section (and it would be obvious that that applies to gitdir/i:: because we say "this is the same as...").
Quoted 6 lines
+ * Symlinks in
$GIT_DIRare not resolved before matching.config.c +4
-0+ + * Note that "../" is not special and will match literally, which is + unlikely what you want. +static int prepare_include_condition_pattern (struct strbuf *pat)
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 184
Quoted 10 lines
config.c +10
-0+{ +struct strbuf path =STRBUF_INIT ; +int prefix =0 ; + +/* TODO: maybe support ~user/ too */ +if (pat->buf[0 ] =='~' &&is_dir_sep (pat->buf[1 ])) { +const char *home =getenv ("HOME" ); + +if (!home) +return error (_ ("$HOME is not defined" ));
Instead of half-duplicating it here yourself, can't we let expand_user_path() do its thing?
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 257
Quoted 10 lines
config.c +10
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{ +/* no condition (i.e., "include.path") is always true */ +if (!cond) +return 1 ; + +if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 );
This may be OK for now, but it should be trivial to start from a table with two entries, i.e.
struct include_cond {
const char *keyword;
int (*fn)(const char *, size_t);
};and will show a better way to do things to those who follow your footsteps.
Re: [PATCH v5 1/1] config: add conditional include
Quoted 6 lines · Junio C Hamano
The variable is obviously not treated the same way as include.path ;-)
When includeIf.<condition>.path variable is set in a configuration file, the configuration file named by that variable is included (in a way similar to how include.path works) only if the <condition> holds true.
Yeah. I was thinking "value" and writing "variable" instead (or perhaps I meant to write "variable value" and accidentally'd the last word again).
Quoted 9 lines
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 184
Quoted 6 lines
config.c +6
-0+/* TODO: maybe support ~user/ too */ +if (pat->buf[0 ] =='~' &&is_dir_sep (pat->buf[1 ])) { +const char *home =getenv ("HOME" ); + +if (!home) +return error (_ ("$HOME is not defined" ));Instead of half-duplicating it here yourself, can't we let
expand_user_path()do its thing?
This comment came up in previous iterations. I perform explicit expansion to make sure we don't apply wildcards on the expanded "~". But I guess going with expand_user_path() is better (less confusion for future readers). We can come back to it when that "don't apply wildcards" concern becomes real.
Quoted 14 lines
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 257
Quoted 10 lines
config.c +10
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{ +/* no condition (i.e., "include.path") is always true */ +if (!cond) +return 1 ; + +if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 );This may be OK for now, but it should be trivial to start from a table with two entries, i.e.
Quoted 7 lines
struct include_cond { const char *keyword; int (*fn)(const char *, size_t); };and will show a better way to do things to those who follow your footsteps.
Yeah I don't see a third include coming soon and did not go with that. Let's way for it and refactor then.
Signature · 1 line
Duy
Re: [PATCH v5 1/1] config: add conditional include
Quoted 13 lines · Duy Nguyen
Quoted 13 lines
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 257
Quoted 4 lines
config.c +4
-0+if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 );This may be OK for now, but it should be trivial to start from a table with two entries, i.e.
struct include_cond { const char *keyword; int (*fn)(const char *, size_t); };
Quoted 5 lines
Quoted 2 lines
and will show a better way to do things to those who follow your footsteps.
Yeah I don't see a third include coming soon and did not go with that. Let's way for it and refactor then.
I would have said exactly that in my message if you already had include_by_gitdir() and include_by_gitdir_i() as separate functions.
But I didn't, because the above code gives an excuse to the person who adds the third one to be lazy and add another "else if" for expediency, because making it table-driven would require an extra work to add two wrapper functions that do not have anything to do with the third one being added.
Re: [PATCH v5 1/1] config: add conditional include
Quoted 21 lines · Junio C Hamano
Quoted 13 lines · Duy Nguyen
Quoted 13 lines
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 257
Quoted 4 lines
config.c +4
-0+if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 );This may be OK for now, but it should be trivial to start from a table with two entries, i.e.
struct include_cond { const char *keyword; int (*fn)(const char *, size_t); };Quoted 5 lines
Quoted 2 lines
and will show a better way to do things to those who follow your footsteps.
Yeah I don't see a third include coming soon and did not go with that. Let's way for it and refactor then.
Quoted 8 lines
I would have said exactly that in my message if you already had
include_by_gitdir()andinclude_by_gitdir_i()as separate functions.But I didn't, because the above code gives an excuse to the person who adds the third one to be lazy and add another "else if" for expediency, because making it table-driven would require an extra work to add two wrapper functions that do not have anything to do with the third one being added.
I don't think driving that with a two-entry table is the right thing here. We are as likely to add another "foobar:" entry as we are to add another modifier "/i" modifier to "gitdir:", and it is unclear whether that modifier would be mutually exclusive with "/i".
E.g., imagine we add "/re" for a regex, but allow a case-insensitive regex with an "i", giving something like "gitdir/i/re:". Now you would want to drive it by matching "gitdir" first, and then collecting the "/i/re" independently, to avoid an explosion of matches.
Driving that with a table is much more complex. I'd just as soon keep things as simple as possible for now and worry about flagging it in review when something new gets added.
-Peff
Re: [PATCH v5 1/1] config: add conditional include
Quoted 4 lines · Jeff King
I don't think driving that with a two-entry table is the right thing here. We are as likely to add another "foobar:" entry as we are to add another modifier "/i" modifier to "gitdir:", and it is unclear whether that modifier would be mutually exclusive with "/i".
OK, I didn't take /i as something that was meant as a modifier; I took the "gitdir:" and "gitdir/i:" are totally different tests that are spelled similarly, but for the implementation expediency, called into a single helper function without having a layer that presents the same function signature in the middle to make it drivable by a table.
Let's leave it to the review of a future patch that wants to add a third condition then. At that time, we will have more things to look at to make a better decision.
Re: [PATCH v5 1/1] config: add conditional include
In reply to Nguyễn Thái Ngọc DuyBeing late to the review party here.
Quoted 2 lines
config.c +2
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{
...
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 262
Quoted 5 lines
config.c +5
-0+ +error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond); +/* unknown conditionals are always false */ +return 0 ; +}
Thanks for putting an error message here. I was looking at what is currently queued as origin/nd/conditional-config-include, which doesn't have this error() (yet / not any more?)
I'd strongly suggest to keep the error message here as that way a user can diagnose e.g. a typo in the condition easily.
If we plan to extend this list of conditions in the future, and a user switches between versions of git, then they may see this message on a regular basis (whenever they use the 'old' version).
In that case it may be only a warning() instead of an error(), but I have no strong opinion on that.
---
Reason why I am reviewing this series now:
I thought about extending the config system for submodule usage (see debate at [1]).
My gut reaction was to have a condition for "if a superproject exists" and then include a special config (e.g. "config.super" in the superprojects $GIT_DIR).
However while reviewing these patches I realized
I am not interested in conditional includes, but when setting up the submodules we know for a fact that we have a superproject, so no conditional needed. Instead we need a special markup of paths, i.e. we want to have an easy way to say "$GIT_DIR of superproject". Ideas how to do that?
Thanks,
Stefan
Re: [PATCH v5 1/1] config: add conditional include
Quoted 12 lines · Stefan Beller
Quoted 2 lines
config.c +2
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{...
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 262
Quoted 5 lines
config.c +5
-0+ +error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond); +/* unknown conditionals are always false */ +return 0 ; +}Thanks for putting an error message here. I was looking at what is currently queued as origin/nd/conditional-config-include, which doesn't have this
error()(yet / not any more?)
It's "not any more". It was in the original and I asked for it to be removed during the last review.
Quoted 6 lines
I'd strongly suggest to keep the error message here as that way a user can diagnose e.g. a typo in the condition easily.
If we plan to extend this list of conditions in the future, and a user switches between versions of git, then they may see this message on a regular basis (whenever they use the 'old' version).
That would make it unlike the rest of the config-include mechanism (which quietly ignores things it doesn't understand, like include.foo, or include.foo.path), as well as the config code in general (which ignores misspelt keys).
Some of that "quiet when you don't understand it" is historical necessity. Older versions _can't_ complain about not knowing include.path, because they don't yet know it's worth complaining about. Likewise here, if this ships in v2.13 and a new condition "foo:" ships in v2.14, you get:
v2.12 - no complaint; we don't even understand includeIf at all v2.13 - complain; we know includeIf, but not "foo:" v2.14 - works as expected
Which is kind of weird and inconsistent. But maybe the typo-detection case is more important to get right than consistency across historical versions.
-Peff
Re: [PATCH v5 1/1] config: add conditional include
Quoted 15 lines · Jeff King
Quoted 12 lines · Stefan Beller
Quoted 2 lines
config.c +2
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{...
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 262
Quoted 5 lines
config.c +5
-0+ +error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond); +/* unknown conditionals are always false */ +return 0 ; +}Thanks for putting an error message here. I was looking at what is currently queued as origin/nd/conditional-config-include, which doesn't have this
error()(yet / not any more?)
Quoted 2 lines
It's "not any more". It was in the original and I asked for it to be removed during the last review.
Okay. The joys of contradicting opinions on a mailing list. :)
Quoted 8 lines
Quoted 7 lines
I'd strongly suggest to keep the error message here as that way a user can diagnose e.g. a typo in the condition easily.
If we plan to extend this list of conditions in the future, and a user switches between versions of git, then they may see this message on a regular basis (whenever they use the 'old' version).
Quoted 8 lines
That would make it unlike the rest of the config-include mechanism (which quietly ignores things it doesn't understand, like include.foo, or include.foo.path), as well as the config code in general (which ignores misspelt keys).
Some of that "quiet when you don't understand it" is historical necessity. Older versions _can't_ complain about not knowing include.path, because they don't yet know it's worth complaining about.
agreed
Quoted 10 lines
Likewise here, if this ships in v2.13 and a new condition "foo:" ships in v2.14, you get:
v2.12 - no complaint; we don't even understand includeIf at all v2.13 - complain; we know includeIf, but not "foo:" v2.14 - works as expected
Which is kind of weird and inconsistent. But maybe the typo-detection case is more important to get right than consistency across historical versions.
Oh, I see. I was contemplating a future in which 2.12 is not used anymore.
When looking at other examples, such as url.<...>.insteadOf we also do not warn about typos (well we can't actually).
In diff.<driver>.(command/binary/..) we know the limited set of drivers, which is similar to the situation we have here.
Maybe a compromise between typo checking (edit distance < 2 -> warn; silent for larger distances) and the consistency over time is desired. But this is even more code to write.
So for now I retract my strong opinion and be happy with what is presented as-is for the reasons Peff gave.
Thanks,
Stefan
v6 cover Conditional config include
In reply to Nguyễn Thái Ngọc Duyv6 has lots of text changes, most of which I shamelessly copied from Junio's, with some minor edits. The biggest edit is the mention of .git files, which is probably a good idea since .git files are used by multi worktrees and submodules.
I could not move the "notes about matching" block into the gitdir block though. I needed to indent it once (not twice like the "for convenience" block) but my asciidoc-foo did not manage it so I settled for "a few more notes on matching _with gitdir_" without moving it.
Code changes include renaming include-if to includeIf, and using expand_user_path() instead of rolling my own.
Nguyễn Thái Ngọc Duy (1):
config: add conditional include
Documentation/config.txt | 61 +++++++++++++++++++++++++++++ config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+)
Signature · 1 line
2.11.0.157.gd943d85
PATCH v6 1/1 config: add conditional include
Sometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to update $GIT_DIR/.config is high (especially when you end up cloning often), it may not be the best way to go. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation. Having separate ${HOME}s may add more problems than it solves.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with includeIf.<condition>.path to make it clearer that a condition is required. The new config has the same backward compatibility approach as include.path: older git versions that don't understand includeIf will simply ignore them.
--- Documentation/config.txt | 61 +++++++++++++++++++++++++++++ config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+)
Documentation/config.txt +61 -0
@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a `includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as `include.path`. + +The condition starts with a keyword followed by a colon and some data +whose format and meaning depends on the keyword. Supported keywords +are: + +`gitdir`:: + + The data that follows the keyword `gitdir:` is used as a glob + pattern. If the location of the .git directory match the + pattern, the include condition is met. ++ +The .git location which may be auto-discovered, or come from +`$GIT_DIR` environment variable. If the repository auto discovered via +a .git file (e.g. from submodules, or a linked worktree), the .git +location would be the final location, not where the .git file is. ++ +The pattern can contain standard globbing wildcards and two additional +ones, `**/` and `/**`, that can match multiple path components. Please +refer to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching with `gitdir` and `gitdir/i`: + + * Symlinks in `$GIT_DIR` are not resolved before matching. + + * Note that "../" is not special and will match literally, which is + unlikely what you want. Example ~~~~~~~
@@ -119,6 +169,17 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your `$HOME` directory + ; include if $GIT_DIR is /path/to/foo/.git + [include-if "gitdir:/path/to/foo/.git"] + path = /path/to/foo.inc + + ; include for all repositories inside /path/to/group + [include-if "gitdir:/path/to/group/"] + path = /path/to/foo.inc + + ; include for all repositories inside $HOME/to/group + [include-if "gitdir:~/to/group/"] + path = /path/to/foo.inc Values ~~~~~~
config.c +97 -0
118 lines not on this page · raw message
t/t1305-config-include.sh +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.11.0.157.gd943d85
Re: [PATCH v6 1/1] config: add conditional include
Quoted 6 lines · Nguyễn Thái Ngọc Duy
Documentation/config.txt +6
-0+Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a `includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as `include.path`.
Documentation/config.txt found at the location of the include directive. If the value of the · line 109
Quoted 10 lines
Documentation/config.txt +10
-0+ +The condition starts with a keyword followed by a colon and some data +whose format and meaning depends on the keyword. Supported keywords +are: + +`gitdir`:: + + The data that follows the keyword `gitdir:` is used as a glob + pattern. If the location of the .git directory match the + pattern, the include condition is met.
s/match/&es/, I think.
Quoted 2 lines
++
+The .git location which may be auto-discovered, or come from
s/ which//?
Quoted 1 line
+
$GIT_DIRenvironment variable. If the repository auto discovered via
s/If the /In a/?
Documentation/config.txt found at the location of the include directive. If the value of the · line 114
Quoted 2 lines
Documentation/config.txt +2
-0+a .git file (e.g. from submodules, or a linked worktree), the .git +location would be the final location, not where the .git file is.
OK.
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 178
Quoted 9 lines
config.c +5
-0@@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *increturn ret; } +static int prepare_include_condition_pattern (struct strbuf *pat) +{ + ... +/* TODO: escape wildcards */ +strbuf_add_absolute_path (&path, cf->path);
Is this still TODO? As this one returns the prefix length (which probably wants to be commented before the function) and this codepath computes the prefix to cover the path to here, doesn't caller already do the right thing?
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 248
Quoted 3 lines
config.c +3
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{ +/* no condition (i.e., "include.path") is always true */
Does this want to say "includeIf.path" instead? "include.path" is done by handle_path_include() without involving a call to this function.
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 260
Quoted 12 lines
config.c +12
-0+if (!cond) +return 1 ; + +if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 ); + +error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond); +/* unknown conditionals are always false */ +return 0 ; +}
Thanks.
Re: [PATCH v6 1/1] config: add conditional include
In reply to Nguyễn Thái Ngọc DuyOn 24/02/17 13:14, Nguyễn Thái Ngọc Duy wrote:
[snip]
Quoted 69 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- Documentation/config.txt | 61 +++++++++++++++++++++++++++++ config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+)Documentation/config.txt +50
-0@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a `includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as `include.path`. + +The condition starts with a keyword followed by a colon and some data +whose format and meaning depends on the keyword. Supported keywords +are: + +`gitdir`:: + + The data that follows the keyword `gitdir:` is used as a glob + pattern. If the location of the .git directory match the + pattern, the include condition is met. ++ +The .git location which may be auto-discovered, or come from +`$GIT_DIR` environment variable. If the repository auto discovered via +a .git file (e.g. from submodules, or a linked worktree), the .git +location would be the final location, not where the .git file is. ++ +The pattern can contain standard globbing wildcards and two additional +ones, `**/` and `/**`, that can match multiple path components. Please +refer to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching with `gitdir` and `gitdir/i`: + + * Symlinks in `$GIT_DIR` are not resolved before matching. + + * Note that "../" is not special and will match literally, which is + unlikely what you want. Example ~~~~~~~
Documentation/config.txt Example · line 173
Quoted 5 lines
Documentation/config.txt +2
-0@@ -119,6 +169,17 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your `$HOME` directory + ; include if $GIT_DIR is /path/to/foo/.git + [include-if "gitdir:/path/to/foo/.git"]
s/include-if/includeIf/
Documentation/config.txt Example · line 177
Quoted 4 lines
Documentation/config.txt +4
-0+ path = /path/to/foo.inc + + ; include for all repositories inside /path/to/group + [include-if "gitdir:/path/to/group/"]
ditto
Documentation/config.txt Example · line 181
Quoted 4 lines
Documentation/config.txt +4
-0+ path = /path/to/foo.inc + + ; include for all repositories inside $HOME/to/group + [include-if "gitdir:~/to/group/"]
ditto
ATB,
Ramsay Jones
Re: [PATCH v6 1/1] config: add conditional include
In reply to Nguyễn Thái Ngọc DuyFrom: "Nguyễn Thái Ngọc Duy" <[email protected]>
Quoted 45 lines
Sometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to update $GIT_DIR/.config is high (especially when you end up cloning often), it may not be the best way to go. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation. Having separate ${HOME}s may add more problems than it solves.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with includeIf.<condition>.path to make it clearer that a condition is required. The new config has the same backward compatibility approach as include.path: older git versions that don't understand includeIf will simply ignore them.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Documentation/config.txt| 61 +++++++++++++++++++++++++++++config.c| 97
+++++++++++++++++++++++++++++++++++++++++++++++t/t1305-config-include.sh| 56 +++++++++++++++++++++++++++3 files changed, 214 insertions(+)Documentation/config.txt +0
-0@@ -91,6 +91,56 @@ found at the location of the include directive. If thevalue of the
relative to the configuration file in which the include directive was found. See below for examples.
Documentation/config.txt found at the location of the include directive. If the value of the · line 97
Quoted 4 lines
Documentation/config.txt +4
-0+Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting
On first reading I thought this implied you can only have one includeIf within the config file.
I think it is meant to mean that each includeIfcould include one other file, and that users can have multiple includeIf lines.
Documentation/config.txt found at the location of the include directive. If the value of the · line 100
Quoted 3 lines
Documentation/config.txt +3
-0+a `includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as `include.path`. +
--
Philip
Re: [PATCH v6 1/1] config: add conditional include
Quoted 10 lines · Philip Oakley
Documentation/config.txt found at the location of the include directive. If the value of the · line 97
Quoted 4 lines
Documentation/config.txt +4
-0+Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by settingOn first reading I thought this implied you can only have one
includeIfwithin the config file.
I think it is meant to mean that eachincludeIfcould include one other file, and that users can have multipleincludeIflines.
Yes. Not sure how to put it better though (I basically copied the first paragraph from the unconditional include section above, which shares the same confusion). Perhaps just write "the variable can be specified multiple times"? Or "multiple variables include multiple times, the last variable does not override the previous ones"?
Signature · 1 line
Duy
Re: [PATCH v6 1/1] config: add conditional include
From: "Duy Nguyen" <[email protected]>
Quoted 12 lines · Philip Oakley
Quoted 10 lines
Documentation/config.txt found at the location of the include directive. If the value of the · line 97
Quoted 4 lines
Documentation/config.txt +4
-0+Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by settingOn first reading I thought this implied you can only have one
includeIfwithin the config file.
I think it is meant to mean that eachincludeIfcould include one other file, and that users can have multipleincludeIflines.
Quoted 6 lines
Yes. Not sure how to put it better though (I basically copied the first paragraph from the unconditional include section above, which shares the same confusion). Perhaps just write "the variable can be specified multiple times"? Or "multiple variables include multiple times, the last variable does not override the previous ones"?
My attempt, based on updating the Includes section would be something like:
You can include a config file from another by setting the special include.path variable to the name of the file to be included. The variable takes a pathname as its value, and is subject to tilde expansion. include.path supports multiple key values.
The subtle change was to s/one/a/ at the start, and then add the final short sentence that states that the section's variables can have multiple key values.
I copied the 'multiple key values' phrase from the man page intro for consitency, though 'multivalued' could just as easily be used as it is the term used by the 'Configuration File' section that this is part of https://git-scm.com/docs/git-config#_configuration_file.
Even shorter may be:You can include a config file from another by setting the special include.path variable to the name of the file to be included. The variable (can be multivalued) takes a pathname as its value, and is subject to tilde expansion.
The Conditional Includes would follow suit.
Philip
v6 cover Conditional config include
In reply to Nguyễn Thái Ngọc Duyv6 has lots of text changes, most of which I shamelessly copied from Junio's, with some minor edits. The biggest edit is the mention of .git files, which is probably a good idea since .git files are used by multi worktrees and submodules.
I could not move the "notes about matching" block into the gitdir block though. I needed to indent it once (not twice like the "for convenience" block) but my asciidoc-foo did not manage it so I settled for "a few more notes on matching _with gitdir_" without moving it.
Code changes include renaming include-if to includeIf, and using expand_user_path() instead of rolling my own.
Nguyễn Thái Ngọc Duy (1):
config: add conditional include
Documentation/config.txt | 61 +++++++++++++++++++++++++++++ config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+)
Signature · 1 line
2.11.0.157.gd943d85
And sorry for duplicates :P Somehow I managed to --dry-run correctly the first time, then had _two_ 000*.patch in the command line. And send-email happily let me shoot myself in the foot.
Signature · 1 line
Duy
PATCH v6 1/1 config: add conditional include
In reply to Nguyễn Thái Ngọc DuySometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to update $GIT_DIR/.config is high (especially when you end up cloning often), it may not be the best way to go. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation. Having separate ${HOME}s may add more problems than it solves.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with includeIf.<condition>.path to make it clearer that a condition is required. The new config has the same backward compatibility approach as include.path: older git versions that don't understand includeIf will simply ignore them.
--- Documentation/config.txt | 61 +++++++++++++++++++++++++++++ config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++ 3 files changed, 214 insertions(+)
Documentation/config.txt +61 -0
@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include one config file from another conditionally by setting +a `includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as `include.path`. + +The condition starts with a keyword followed by a colon and some data +whose format and meaning depends on the keyword. Supported keywords +are: + +`gitdir`:: + + The data that follows the keyword `gitdir:` is used as a glob + pattern. If the location of the .git directory match the + pattern, the include condition is met. ++ +The .git location which may be auto-discovered, or come from +`$GIT_DIR` environment variable. If the repository auto discovered via +a .git file (e.g. from submodules, or a linked worktree), the .git +location would be the final location, not where the .git file is. ++ +The pattern can contain standard globbing wildcards and two additional +ones, `**/` and `/**`, that can match multiple path components. Please +refer to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching with `gitdir` and `gitdir/i`: + + * Symlinks in `$GIT_DIR` are not resolved before matching. + + * Note that "../" is not special and will match literally, which is + unlikely what you want. Example ~~~~~~~
@@ -119,6 +169,17 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your `$HOME` directory + ; include if $GIT_DIR is /path/to/foo/.git + [include-if "gitdir:/path/to/foo/.git"] + path = /path/to/foo.inc + + ; include for all repositories inside /path/to/group + [include-if "gitdir:/path/to/group/"] + path = /path/to/foo.inc + + ; include for all repositories inside $HOME/to/group + [include-if "gitdir:~/to/group/"] + path = /path/to/foo.inc Values ~~~~~~
config.c +97 -0
118 lines not on this page · raw message
t/t1305-config-include.sh +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.11.0.157.gd943d85
Re: [PATCH v6 1/1] config: add conditional include
Quoted 5 lines · Nguyễn Thái Ngọc Duy
config.c +5
-0+static int include_condition_is_true (const char *cond,size_t cond_len) +{ +/* no condition (i.e., "include.path") is always true */ +if (!cond) +return 1 ;
config.c static int handle_path_include(const char *path, struct config_include_data *inc · line 260
Quoted 10 lines
config.c +10
-0+ +if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,0 ); +else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len)) +return include_by_gitdir (cond, cond_len,1 ); + +error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond); +/* unknown conditionals are always false */ +return 0 ; +}
This "error()" is a nice warning to people who have misspelled the condition (or are surprised that an old version does not respect their condition). But it seems out of place with the rest of the compatibility strategy, which is to quietly ignore include types we don't understand.
For example, if your patch ships in v2.13 and we add the "foo:" condition in v2.14, then with config like:
[includeif "foo:bar"]
path = whatever
you get:
v2.12: silently ignore
v2.13: loudly ignore
v2.14: works
which seems odd.
If we do keep it, it should probably be warning(). I would expect "error:" to indicate that some requested operation could not be completed, but this is just "by the way, I ignored this garbage".
-Peff
PS I know we try to avoid dashes in the section names, but "includeIf"
and "includeif" just look really ugly to me. Maybe it is just me, though.
v7 cover Conditional config include
In reply to Nguyễn Thái Ngọc Duyv7 changes:
- typo fix here and there
- delete a dead TODO comment
- delete the error on unrecognized condition (we probably want a better, general mechanism for troubleshooting config keys anyway)
- some fix up on include.path document because the "one or many" confusion started from there
I don't have a good answer for Jeff's PS about includeIf ugliness. I agree that includeif is ugly but includeIf looks a bit better. I don't see a better option (if only "include" does not start or end with a vowel...). Maybe includewith? Suggestions are welcome.
Interdiff below (just on patch 3, not the first two)
Documentation/config.txt +14 -11
@@ -93,9 +93,11 @@ was found. See below for examples. Conditional includes ~~~~~~~~~~~~~~~~~~~~ -You can include one config file from another conditionally by setting -a `includeIf.<condition>.path` variable to the name of the file to be -included. The variable's value is treated the same way as `include.path`. +You can include a config file from another conditionally by setting a +`includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as +`include.path`. `includeIf.<condition>path` supports multiple key +values. The condition starts with a keyword followed by a colon and some data whose format and meaning depends on the keyword. Supported keywords
@@ -104,13 +106,14 @@ are: `gitdir`:: The data that follows the keyword `gitdir:` is used as a glob - pattern. If the location of the .git directory match the + pattern. If the location of the .git directory matches the pattern, the include condition is met. + -The .git location which may be auto-discovered, or come from -`$GIT_DIR` environment variable. If the repository auto discovered via -a .git file (e.g. from submodules, or a linked worktree), the .git -location would be the final location, not where the .git file is. +The .git location may be auto-discovered, or come from `$GIT_DIR` +environment variable. If the repository is auto discovered via a .git +file (e.g. from submodules, or a linked worktree), the .git location +would be the final location where the .git directory is, not where the +.git file is. + The pattern can contain standard globbing wildcards and two additional ones, `**/` and `/**`, that can match multiple path components. Please
@@ -169,15 +172,15 @@ Example path = ~/foo ; expand "foo" in your `$HOME` directory ; include if $GIT_DIR is /path/to/foo/.git - [include-if "gitdir:/path/to/foo/.git"] + [includeIf "gitdir:/path/to/foo/.git"] path = /path/to/foo.inc ; include for all repositories inside /path/to/group - [include-if "gitdir:/path/to/group/"] + [includeIf "gitdir:/path/to/group/"] path = /path/to/foo.inc ; include for all repositories inside $HOME/to/group - [include-if "gitdir:~/to/group/"] + [includeIf "gitdir:~/to/group/"] path = /path/to/foo.inc Values
config.c +1 -6
@@ -191,7 +191,6 @@ static int prepare_include_condition_pattern(struct strbuf *pat)return error (_ ("relative config include " "conditionals must come from files" )); -/* TODO: escape wildcards */ strbuf_add_absolute_path (&path, cf->path); slash =find_last_dir_sep (path.buf);if (!slash)
@@ -245,16 +244,12 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)static int include_condition_is_true (const char *cond,size_t cond_len) { -/* no condition (i.e., "include.path") is always true */ -if (!cond) -return 1 ;if (skip_prefix_mem (cond, cond_len,"gitdir:" , &cond, &cond_len))return include_by_gitdir (cond, cond_len,0 );else if (skip_prefix_mem (cond, cond_len,"gitdir/i:" , &cond, &cond_len))return include_by_gitdir (cond, cond_len,1 ); -error (_ ("unrecognized include condition: %.*s" ), (int )cond_len, cond);/* unknown conditionals are always false */ return 0 ; }
@@ -278,7 +273,7 @@ int git_config_include(const char *var, const char *value, void *data) ret =handle_path_include (value, inc);if (!parse_config_key (var,"includeif" , &cond, &cond_len, &key) && -include_condition_is_true (cond, cond_len) && + (cond &&include_condition_is_true (cond, cond_len)) && !strcmp (key,"path" )) ret =handle_path_include (value, inc);
Nguyễn Thái Ngọc Duy (3):
config.txt: clarify multiple key values in include.path config.txt: reflow the second include.path paragraph
config: add conditional include
Documentation/config.txt | 77 +++++++++++++++++++++++++++++++++++---- config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++ 3 files changed, 218 insertions(+), 7 deletions(-)
Signature · 1 line
2.11.0.157.gd943d85
PATCH v7 1/3 config.txt: clarify multiple key values in include.path
The phrasing in this paragraph may give an impression that you can only use it once. Rephrase it a bit.
--- Documentation/config.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/config.txt +2 -2
@@ -79,10 +79,10 @@ escape sequences) are invalid. Includes ~~~~~~~~ -You can include one config file from another by setting the special +You can include a config file from another by setting the special `include.path` variable to the name of the file to be included. The variable takes a pathname as its value, and is subject to tilde -expansion. +expansion. `include.path` supports multiple key values. The included file is expanded immediately, as if its contents had been
Signature · 1 line
2.11.0.157.gd943d85
Re: [PATCH v7 1/3] config.txt: clarify multiple key values in include.path
Quoted 14 lines · Nguyễn Thái Ngọc Duy
The phrasing in this paragraph may give an impression that you can only use it once. Rephrase it a bit.
Helped-by Philip Oakley <[email protected]>Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- Documentation/config.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Documentation/config.txt +0
-0
Documentation/config.txt escape sequences) are invalid. · line 85
Quoted 9 lines
Documentation/config.txt +2
-2@@ -79,10 +79,10 @@ escape sequences) are invalid. Includes ~~~~~~~~ -You can include one config file from another by setting the special +You can include a config file from another by setting the special `include.path` variable to the name of the file to be included. The variable takes a pathname as its value, and is subject to tilde -expansion. +expansion. `include.path` supports multiple key values.
Multiple key values or just simply multiple values? I think it is the latter.
But to the intended target audience, I think
include.path can be given multiple times.
is easier to understand. It's not like you can (or want to) enumerate with "git config --get-all include.path" to learn all the values (for the single key "include.value"), and it is better not to lead readers to think of this in terms of <key,value> in the first place (which is already clarified in the text that follows).
Quoted 2 lines
Documentation/config.txt +0
-0The included file is expanded immediately, as if its contents had been
PATCH v7 2/3 config.txt: reflow the second include.path paragraph
In reply to Nguyễn Thái Ngọc Duy--- Documentation/config.txt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
Documentation/config.txt +4 -5
@@ -84,12 +84,11 @@ You can include a config file from another by setting the special variable takes a pathname as its value, and is subject to tilde expansion. `include.path` supports multiple key values. -The -included file is expanded immediately, as if its contents had been +The included file is expanded immediately, as if its contents had been found at the location of the include directive. If the value of the -`include.path` variable is a relative path, the path is considered to be -relative to the configuration file in which the include directive was -found. See below for examples. +`include.path` variable is a relative path, the path is considered to +be relative to the configuration file in which the include directive +was found. See below for examples. Example
Signature · 1 line
2.11.0.157.gd943d85
PATCH v7 3/3 config: add conditional include
In reply to Nguyễn Thái Ngọc DuySometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to update $GIT_DIR/.config is high (especially when you end up cloning often), it may not be the best way to go. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation. Having separate ${HOME}s may add more problems than it solves.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with includeIf.<condition>.path to make it clearer that a condition is required. The new config has the same backward compatibility approach as include.path: older git versions that don't understand includeIf will simply ignore them.
--- Documentation/config.txt | 64 +++++++++++++++++++++++++++++++++ config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++ 3 files changed, 212 insertions(+)
Documentation/config.txt +64 -0
@@ -90,6 +90,59 @@ found at the location of the include directive. If the value of the be relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include a config file from another conditionally by setting a +`includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as +`include.path`. `includeIf.<condition>path` supports multiple key +values. + +The condition starts with a keyword followed by a colon and some data +whose format and meaning depends on the keyword. Supported keywords +are: + +`gitdir`:: + + The data that follows the keyword `gitdir:` is used as a glob + pattern. If the location of the .git directory matches the + pattern, the include condition is met. ++ +The .git location may be auto-discovered, or come from `$GIT_DIR` +environment variable. If the repository is auto discovered via a .git +file (e.g. from submodules, or a linked worktree), the .git location +would be the final location where the .git directory is, not where the +.git file is. ++ +The pattern can contain standard globbing wildcards and two additional +ones, `**/` and `/**`, that can match multiple path components. Please +refer to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. + +`gitdir/i`:: + This is the same as `gitdir` except that matching is done + case-insensitively (e.g. on case-insensitive file sytems) + +A few more notes on matching via `gitdir` and `gitdir/i`: + + * Symlinks in `$GIT_DIR` are not resolved before matching. + + * Note that "../" is not special and will match literally, which is + unlikely what you want. Example ~~~~~~~
@@ -118,6 +171,17 @@ Example path = foo ; expand "foo" relative to the current file path = ~/foo ; expand "foo" in your `$HOME` directory + ; include if $GIT_DIR is /path/to/foo/.git + [includeIf "gitdir:/path/to/foo/.git"] + path = /path/to/foo.inc + + ; include for all repositories inside /path/to/group + [includeIf "gitdir:/path/to/group/"] + path = /path/to/foo.inc + + ; include for all repositories inside $HOME/to/group + [includeIf "gitdir:~/to/group/"] + path = /path/to/foo.inc Values ~~~~~~
config.c +92 -0
113 lines not on this page · raw message
t/t1305-config-include.sh +56 -0
62 lines not on this page · raw message
Signature · 1 line
2.11.0.157.gd943d85
Re: [PATCH v7 3/3] config: add conditional include
Quoted 40 lines · Nguyễn Thái Ngọc Duy
Sometimes a set of repositories want to share configuration settings among themselves that are distinct from other such sets of repositories. A user may work on two projects, each of which have multiple repositories, and use one user.email for one project while using another for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to update $GIT_DIR/.config is high (especially when you end up cloning often), it may not be the best way to go. Having the settings in ~/.gitconfig, which would work for just one set of repositories, would not well in such a situation. Having separate ${HOME}s may add more problems than it solves.
Extend the include.path mechanism that lets a config file include another config file, so that the inclusion can be done only when some conditions hold. Then ~/.gitconfig can say "include config-project-A only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in absolute path), so you would need to group repositories by directory, or something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes with includeIf.<condition>.path to make it clearer that a condition is required. The new config has the same backward compatibility approach as include.path: older git versions that don't understand includeIf will simply ignore them.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- Documentation/config.txt | 64 +++++++++++++++++++++++++++++++++ config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++ 3 files changed, 212 insertions(+)Documentation/config.txt +0
-0
Documentation/config.txt found at the location of the include directive. If the value of the · line 99
Quoted 10 lines
Documentation/config.txt +7
-0@@ -90,6 +90,59 @@ found at the location of the include directive. If the value of the be relative to the configuration file in which the include directive was found. See below for examples. +Conditional includes +~~~~~~~~~~~~~~~~~~~~ + +You can include a config file from another conditionally by setting a +`includeIf.<condition>.path` variable to the name of the file to be +included. The variable's value is treated the same way as +`include.path`. `includeIf.<condition>path` supports multiple key
----------------------------------------^^
s/<condition>path/<condition>.path/
ATB,
Ramsay Jones
Re: [PATCH v7 3/3] config: add conditional include
In reply to Nguyễn Thái Ngọc DuyQuoted 10 lines · Nguyễn Thái Ngọc Duy
int
git_config_include(const char *var, const char *value, void *data){config.c +2
-0struct config_include_data *inc = data; +const char *cond, *key; +int cond_len;int ret;/* @@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)
config.c int git_config_include(const char *var, const char *value, void *data) · line 281
Quoted 10 lines
config.c +6
-0if (!strcmp (var,"include.path" )) ret =handle_path_include (value, inc); + +if (!parse_config_key (var,"includeif" , &cond, &cond_len, &key) && + (cond &&include_condition_is_true (cond, cond_len)) && + !strcmp (key,"path" )) + ret =handle_path_include (value, inc); +return ret; }
So "includeif.path" (misspelled one without any condition) falls through to "return ret" and gives the value we got from inc->fn(). I am OK with that (i.e. "missing condition is false").
Or we can make it go to handle_path_include(), effectively making the "include.path" a short-hand for "includeIf.path". I am also OK with that (i.e. "missing condition is true").
Or we could even have "include.[<condition>.]path" without "includeIf"? I am not sure if it is a bad idea that paints ourselves in a corner, but somehow I find it tempting.
Re: [PATCH v7 3/3] config: add conditional include
Quoted 16 lines · Junio C Hamano
config.c int git_config_include(const char *var, const char *value, void *data) · line 281
Quoted 12 lines
config.c +6
-0@@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)if (!strcmp (var,"include.path" )) ret =handle_path_include (value, inc); + +if (!parse_config_key (var,"includeif" , &cond, &cond_len, &key) && + (cond &&include_condition_is_true (cond, cond_len)) && + !strcmp (key,"path" )) + ret =handle_path_include (value, inc); +return ret; }So "includeif.path" (misspelled one without any condition) falls through to "return ret" and gives the value we got from
inc->fn(). I am OK with that (i.e. "missing condition is false").
Yeah, I think that is sensible, just as not-understood nonsense like "include.foobar" would be ignored, as well.
Quoted 3 lines
Or we can make it go to
handle_path_include(), effectively making the "include.path" a short-hand for "includeIf.path". I am also OK with that (i.e. "missing condition is true").
I think we want "missing condition is false". Certainly an empty condition like "includeIf..path" is false, as are conditions we don't understand.
Quoted 3 lines
Or we could even have "include.[<condition>.]path" without "includeIf"? I am not sure if it is a bad idea that paints ourselves in a corner, but somehow I find it tempting.
That was how I had originally envisioned the namespace working when I introduced include.path long ago. And I think Duy's v1 used that, but the feedback was that it was not sufficiently obvious that the subsection was a conditional.
-Peff
Re: [PATCH v7 3/3] config: add conditional include
Quoted 4 lines · Jeff King
That was how I had originally envisioned the namespace working when I introduced include.path long ago. And I think Duy's v1 used that, but the feedback was that it was not sufficiently obvious that the subsection was a conditional.
I am not sure about "obviousness", but I agree that we do not know that "conditional include" would be the only thing we want for the second level for "include.path" directive. "include-if.<cond>.path" is better for that reason.
I presume that you could still do
[include "if:gitdir=$path"] path = ...
i.e. design the second level to begin with a token that tells readers what it means (and assign "if:" token for "conditional include"), but I do not think it is worth it.
I also imagine that
[include] condition = ... path = ...
is easier to read and write by end-users, but it probably is not feasible because it requires too invasive change to the current code to teach it to grok such construct.
Between "include-if" and "includeIf", if people find the latter not too ugly, I'd prefer to keep it the way Duy posted. Because of the way "include.path" and "include-if.<cond>.path" work, we can declare that they are not like ordinary configuration variable definition at all but are higher-level directives and that may be a sufficient justification to allow "-" in its name, though, if people find "includeIf" too ugly a name to live.
Re: [PATCH v7 3/3] config: add conditional include
Quoted 5 lines · Junio C Hamano
I am not sure about "obviousness", but I agree that we do not know that "conditional include" would be the only thing we want for the second level for "include.path" directive. "include-if.<cond>.path" is better for that reason.
Quoted 8 lines
I presume that you could still do
[include "if:gitdir=$path"] path = ...i.e. design the second level to begin with a token that tells readers what it means (and assign "if:" token for "conditional include"), but I do not think it is worth it.
Yep, all true.
Quoted 9 lines
I also imagine that
[include] condition = ... path = ...is easier to read and write by end-users, but it probably is not feasible because it requires too invasive change to the current code to teach it to grok such construct.
I am against that as it introduces a dependency in the presence and ordering between two config variables, which can yield some surprises.
Quoted 7 lines
Between "include-if" and "includeIf", if people find the latter not too ugly, I'd prefer to keep it the way Duy posted. Because of the way "include.path" and "include-if.<cond>.path" work, we can declare that they are not like ordinary configuration variable definition at all but are higher-level directives and that may be a sufficient justification to allow "-" in its name, though, if people find "includeIf" too ugly a name to live.
OK. I can live with includeIf.
-Peff
Quoted 4 lines · Nguyễn Thái Ngọc Duy
Nguyễn Thái Ngọc Duy (3):
config.txt: clarify multiple key values in include.path config.txt: reflow the second include.path paragraph
config: add conditional include
Thanks. The primary change looked good (it looked good already in the previous round).
Here is a list of minor suggestions on top.
Documentation/config.txt +2 -3
@@ -82,7 +82,7 @@ Includes You can include a config file from another by setting the special `include.path` variable to the name of the file to be included. The variable takes a pathname as its value, and is subject to tilde -expansion. `include.path` supports multiple key values. +expansion. `include.path` can be given multiple times. The included file is expanded immediately, as if its contents had been found at the location of the include directive. If the value of the
@@ -96,8 +96,7 @@ Conditional includes You can include a config file from another conditionally by setting a `includeIf.<condition>.path` variable to the name of the file to be included. The variable's value is treated the same way as -`include.path`. `includeIf.<condition>path` supports multiple key -values. +`include.path`. `includeIf.<condition>.path` can be given multiple times. The condition starts with a keyword followed by a colon and some data whose format and meaning depends on the keyword. Supported keywords
Quoted 4 lines · Nguyễn Thái Ngọc Duy
I don't have a good answer for Jeff's PS about includeIf ugliness. I agree that includeif is ugly but includeIf looks a bit better. I don't see a better option (if only "include" does not start or end with a vowel...). Maybe includewith? Suggestions are welcome.
I actually think "include-if" _looks_ better, although maybe the inconsistency with "-" is something we don't want to encourage (though I also think the implicit include.<cond>.path was OK, too). Feel free to just ignore me. I will live with it either way.
For those following on the mailing list, there is some discussion at:
https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
I think that is mostly focused around another failing in the error-handling of the config code, and that does not need to be addressed by this series (though of course I'd welcome any fixes).
But there's a test failure that probably does need to be dealt with before this graduates to 'next'.
-Peff
Quoted 10 lines · Jeff King
For those following on the mailing list, there is some discussion at:
https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
I think that is mostly focused around another failing in the error-handling of the config code, and that does not need to be addressed by this series (though of course I'd welcome any fixes).
But there's a test failure that probably does need to be dealt with before this graduates to 'next'.
The patch to fix it is this
t/t1305-config-include.sh +1 -1
@@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' ' test_expect_success 'missing include files are ignored' ' cat >.gitconfig <<-\EOF && - [include]path = foo + [include]path = non-existent [test]value = yes EOF echo yes >expect &&
Junio could you squash this in?
A lot more explanation is in another series [1] I just sent. Not sure if we want to repeat the same in this commit's message.
And this series without that squash, when combined with [1], will fail this test even on Linux. Good thing imo.
Quoted 16 lines · Duy Nguyen
The patch to fix it is this
t/t1305-config-include.sh +1
-1@@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' ' test_expect_success 'missing include files are ignored' ' cat >.gitconfig <<-\EOF && - [include]path = foo + [include]path = non-existent [test]value = yes EOF echo yes >expect &&
Quoted 1 line
Junio could you squash this in?
Sure.
Quoted 28 lines · Duy Nguyen
Quoted 10 lines · Jeff King
For those following on the mailing list, there is some discussion at:
https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
I think that is mostly focused around another failing in the error-handling of the config code, and that does not need to be addressed by this series (though of course I'd welcome any fixes).
But there's a test failure that probably does need to be dealt with before this graduates to 'next'.
The patch to fix it is this
t/t1305-config-include.sh +1
-1@@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' ' test_expect_success 'missing include files are ignored' ' cat >.gitconfig <<-\EOF && - [include]path = foo + [include]path = non-existent [test]value = yes EOF echo yes >expect &&
Quoted 2 lines
Junio could you squash this in?
I said yes, but I won't, as the series already has SQUASH??? for other things, so I'll just queue this at the tip. I _might_ get inclined to do the squashing and cleaning up myself later, but please don't hold your breath.
Quoted 7 lines · Jeff King
For those following on the mailing list, there is some discussion at:
https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
I think that is mostly focused around another failing in the error-handling of the config code, and that does not need to be addressed by this series (though of course I'd welcome any fixes).
Thanks. Without a message like this, the list may have never known about the discussion taken elsewhere. I'd appreciate such a report to appear on list the next time much earlier ;-)
When built with FREAD_READS_DIRECTORIES=Yes on Linux, the error in the test can easily reproduce.
In early days of UNIX it was sometimes handy to be able to read the bytes off of directory to "investigate", but we are not a filesystem application, and I do not offhand see any reason why we should be relying on being able to successfully fopen() a directory for reading. A FILE * successfully opened that just returns EOF when read is totally useless for any purpose anyway.
When the path to be opened from the end user (either from the command line or in a configuration file) is a directory, it is better to diagnose it as a user error, and if the path was computed by our code, it may be a bug.
I am wondering if we should enable this on Linux, at least in DEVELOPER builds but possibly even on the release builds, to catch these problems more easily.
Quoted 10 lines · Junio C Hamano
Quoted 7 lines · Jeff King
For those following on the mailing list, there is some discussion at:
https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
I think that is mostly focused around another failing in the error-handling of the config code, and that does not need to be addressed by this series (though of course I'd welcome any fixes).
Quoted 6 lines
Thanks. Without a message like this, the list may have never known about the discussion taken elsewhere. I'd appreciate such a report to appear on list the next time much earlier ;-)
When built with FREAD_READS_DIRECTORIES=Yes on Linux, the error in the test can easily reproduce.
Heh. I had no idea we had FREAD_READS_DIRECTORIES. I think Duy and I reinvented it in another thread. ;)
I agree that may be worth setting on Linux (though note that we _do_ ignore other stdio read errors in the rest of the code path, which may be something we want to address).
-Peff
Quoted 2 lines · Jeff King
Heh. I had no idea we had
FREAD_READS_DIRECTORIES. I think Duy and I reinvented it in another thread. ;)
You two were not alone. I was planning to reinvent it before I went to bed last night and then found it already was there this morning ;-)
Quoted 3 lines
I agree that may be worth setting on Linux (though note that we _do_ ignore other stdio read errors in the rest of the code path, which may be something we want to address).
Yes, we need an error check on fopen() in git_config_from_file() regardless.