Re: [PATCH v6 1/1] config: add conditional include
From: "Duy Nguyen" On Sat, Feb 25, 2017 at 5:08 AM, Philip Oakley wrote: +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 `includeIf`could include one other file, and that users can have multiple `includeIf` 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
Re: [PATCH v6 1/1] config: add conditional include
On Fri, Feb 24, 2017 at 08:14:25PM +0700, Nguyễn Thái Ngọc Duy wrote: > +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; > +} 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.
Re: [PATCH v6 1/1] config: add conditional include
On Sat, Feb 25, 2017 at 5:08 AM, Philip Oakley wrote: >> +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 `includeIf`could include one other > file, and that users can have multiple `includeIf` 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"? -- Duy
Re: [PATCH v6 1/1] config: add conditional include
From: "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..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 --- Documentation/config.txt | 61 + config.c | 97 +++ t/t1305-config-include.sh | 56 +++ 3 files changed, 214 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 015346c417..6c0cd2a273 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -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 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 `includeIf`could include one other file, and that users can have multiple `includeIf` lines. +a `includeIf..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
On 24/02/17 13:14, Nguyễn Thái Ngọc Duy wrote: [snip] > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Documentation/config.txt | 61 + > config.c | 97 > +++ > t/t1305-config-include.sh | 56 +++ > 3 files changed, 214 insertions(+) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 015346c417..6c0cd2a273 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -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..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"] s/include-if/includeIf/ > + path = /path/to/foo.inc > + > + ; include for all repositories inside /path/to/group > + [include-if "gitdir:/path/to/group/"] ditto > + 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
Nguyễn Thái Ngọc Duy writes: > +Conditional includes > + > + > +You can include one config file from another conditionally by setting > +a `includeIf..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. s/match/&es/, I think. > ++ > +The .git location which may be auto-discovered, or come from s/ which//? > +`$GIT_DIR` environment variable. If the repository auto discovered via s/If the /In a/? > +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. > @@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct > config_include_data *inc > return 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? > +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. > + 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.
[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..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 --- Documentation/config.txt | 61 + config.c | 97 +++ t/t1305-config-include.sh | 56 +++ 3 files changed, 214 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 015346c417..6c0cd2a273 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -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..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 ~~ diff --git a/config.c b/config.c index c6b874a7bf..ad16802c8a 100644 --- a/config.c +++ b/config.c @@ -13,6 +13,7 @@ #include "hashmap.h" #include "string-list.h" #include "utf8.h" +#include "dir.h" struc
[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..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 --- Documentation/config.txt | 61 + config.c | 97 +++ t/t1305-config-include.sh | 56 +++ 3 files changed, 214 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 015346c417..6c0cd2a273 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -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..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 ~~ diff --git a/config.c b/config.c index c6b874a7bf..ad16802c8a 100644 --- a/config.c +++ b/config.c @@ -13,6 +13,7 @@ #include "hashmap.h" #include "string-list.h" #include "utf8.h" +#include "dir.h" struc