| Message ID | 3573995264441c50ea9529b3ee926d1ed3ec4ac8.1559330905.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] config: learn the "onbranch:" includeIf condition | expand |
Hi Denton, On Fri, 31 May 2019, Denton Liu wrote: > Currently, if a user wishes to have individual settings per branch, they > are required to manually keep track of the settings in their head and > manually set the options on the command-line or change the config at > each branch. > > Teach config the "onbranch:" includeIf condition so that it can > conditionally include configuration files if the branch that is checked > out in the current worktree matches the pattern given. This looks good to me. Thanks, Dscho > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > > Thanks for taking a look, Johannes. I've modified the patch to match > with wildcards now. > > Changes since v1: > > * Allow onbranch: to specify a pattern instead of just a static > comparison > > Interdiff against v1: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 3b9fbe1860..93aa4323c6 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -145,9 +145,11 @@ refer to linkgit:gitignore[5] for details. For convenience: > case-insensitively (e.g. on case-insensitive file sytems) > > `onbranch`:: > - The data that follows the keyword `onbranch:` is taken to be a > - name of a local branch. If we are in a worktree where that > - branch is currently checked out, the include condition is met. > + The data that follows the keyword `onbranch:` is taken to be a pattern > + with standard globbing wildcards and two additional ones, `**/` and > + `/**`, that can match multiple path components. If we are in a worktree > + where the name of the branch that is currently checked out matches the > + pattern, the include condition is met. > > A few more notes on matching via `gitdir` and `gitdir/i`: > > diff --git a/config.c b/config.c > index 954e84e13a..48bb435739 100644 > --- a/config.c > +++ b/config.c > @@ -268,6 +268,8 @@ static int include_by_gitdir(const struct config_options *opts, > static int include_by_branch(const char *cond, size_t cond_len) > { > int flags; > + int ret; > + struct strbuf pattern = STRBUF_INIT; > const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flags); > const char *shortname; > > @@ -275,7 +277,10 @@ static int include_by_branch(const char *cond, size_t cond_len) > !skip_prefix(refname, "refs/heads/", &shortname)) > return 0; > > - return !strncmp(cond, shortname, cond_len); > + strbuf_add(&pattern, cond, cond_len); > + ret = !wildmatch(pattern.buf, shortname, WM_PATHNAME); > + strbuf_release(&pattern); > + return ret; > } > > static int include_condition_is_true(const struct config_options *opts, > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh > index 062db08ad9..05c7def1f2 100755 > --- a/t/t1305-config-include.sh > +++ b/t/t1305-config-include.sh > @@ -314,6 +314,7 @@ test_expect_success 'conditional include, onbranch' ' > cd bar && > echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && > echo "[test]nine=9" >.git/bar9 && > + git checkout -b master && > test_must_fail git config test.nine && > git checkout -b foo-branch && > echo 9 >expect && > @@ -322,6 +323,25 @@ test_expect_success 'conditional include, onbranch' ' > ) > ' > > +test_expect_success 'conditional include, onbranch, wildcard' ' > + ( > + cd bar && > + echo "[includeIf \"onbranch:?oo-*/**\"]path=bar10" >>.git/config && > + echo "[test]ten=10" >.git/bar10 && > + git checkout -b not-foo-branch/a && > + test_must_fail git config test.ten && > + > + echo 10 >expect && > + git checkout -b foo-branch/a/b/c && > + git config test.ten >actual && > + test_cmp expect actual && > + > + git checkout -b moo-bar/a && > + git config test.ten >actual && > + test_cmp expect actual > + ) > +' > + > test_expect_success 'include cycles are detected' ' > cat >.gitconfig <<-\EOF && > [test]value = gitconfig > > Documentation/config.txt | 12 ++++++++++++ > config.c | 21 +++++++++++++++++++++ > t/t1305-config-include.sh | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 66 insertions(+) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 7e2a6f61f5..93aa4323c6 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -144,6 +144,13 @@ refer to linkgit:gitignore[5] for details. For convenience: > This is the same as `gitdir` except that matching is done > case-insensitively (e.g. on case-insensitive file sytems) > > +`onbranch`:: > + The data that follows the keyword `onbranch:` is taken to be a pattern > + with standard globbing wildcards and two additional ones, `**/` and > + `/**`, that can match multiple path components. If we are in a worktree > + where the name of the branch that is currently checked out matches the > + pattern, the include condition is met. > + > A few more notes on matching via `gitdir` and `gitdir/i`: > > * Symlinks in `$GIT_DIR` are not resolved before matching. > @@ -206,6 +213,11 @@ Example > [includeIf "gitdir:/path/to/group/"] > path = foo.inc > > + ; include only if we are in a worktree where foo-branch is > + ; currently checked out > + [includeIf "onbranch:foo-branch"] > + path = foo.inc > + > Values > ~~~~~~ > > diff --git a/config.c b/config.c > index 296a6d9cc4..48bb435739 100644 > --- a/config.c > +++ b/config.c > @@ -19,6 +19,7 @@ > #include "utf8.h" > #include "dir.h" > #include "color.h" > +#include "refs.h" > > struct config_source { > struct config_source *prev; > @@ -264,6 +265,24 @@ static int include_by_gitdir(const struct config_options *opts, > return ret; > } > > +static int include_by_branch(const char *cond, size_t cond_len) > +{ > + int flags; > + int ret; > + struct strbuf pattern = STRBUF_INIT; > + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flags); > + const char *shortname; > + > + if (!refname || !(flags & REF_ISSYMREF) || > + !skip_prefix(refname, "refs/heads/", &shortname)) > + return 0; > + > + strbuf_add(&pattern, cond, cond_len); > + ret = !wildmatch(pattern.buf, shortname, WM_PATHNAME); > + strbuf_release(&pattern); > + return ret; > +} > + > static int include_condition_is_true(const struct config_options *opts, > const char *cond, size_t cond_len) > { > @@ -272,6 +291,8 @@ static int include_condition_is_true(const struct config_options *opts, > return include_by_gitdir(opts, cond, cond_len, 0); > else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) > return include_by_gitdir(opts, cond, cond_len, 1); > + else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) > + return include_by_branch(cond, cond_len); > > /* unknown conditionals are always false */ > return 0; > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh > index 579a86b7f8..05c7def1f2 100755 > --- a/t/t1305-config-include.sh > +++ b/t/t1305-config-include.sh > @@ -309,6 +309,39 @@ test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icas > ) > ' > > +test_expect_success 'conditional include, onbranch' ' > + ( > + cd bar && > + echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && > + echo "[test]nine=9" >.git/bar9 && > + git checkout -b master && > + test_must_fail git config test.nine && > + git checkout -b foo-branch && > + echo 9 >expect && > + git config test.nine >actual && > + test_cmp expect actual > + ) > +' > + > +test_expect_success 'conditional include, onbranch, wildcard' ' > + ( > + cd bar && > + echo "[includeIf \"onbranch:?oo-*/**\"]path=bar10" >>.git/config && > + echo "[test]ten=10" >.git/bar10 && > + git checkout -b not-foo-branch/a && > + test_must_fail git config test.ten && > + > + echo 10 >expect && > + git checkout -b foo-branch/a/b/c && > + git config test.ten >actual && > + test_cmp expect actual && > + > + git checkout -b moo-bar/a && > + git config test.ten >actual && > + test_cmp expect actual > + ) > +' > + > test_expect_success 'include cycles are detected' ' > cat >.gitconfig <<-\EOF && > [test]value = gitconfig > -- > 2.22.0.rc1.169.g49223abbf8 > >
Hi Denton, On Fri, 31 May 2019, Johannes Schindelin wrote: > On Fri, 31 May 2019, Denton Liu wrote: > > > Currently, if a user wishes to have individual settings per branch, > > they are required to manually keep track of the settings in their head > > and manually set the options on the command-line or change the config > > at each branch. > > > > Teach config the "onbranch:" includeIf condition so that it can > > conditionally include configuration files if the branch that is > > checked out in the current worktree matches the pattern given. > > This looks good to me. ... actually... > > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh > > index 579a86b7f8..05c7def1f2 100755 > > --- a/t/t1305-config-include.sh > > +++ b/t/t1305-config-include.sh > > @@ -309,6 +309,39 @@ test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icas > > ) > > ' > > > > +test_expect_success 'conditional include, onbranch' ' > > + ( > > + cd bar && This `bar` is a side effect of an earlier test case. To be precise, of this test case: test_expect_success SYMLINKS 'conditional include, gitdir matching symlink' ' ln -s foo bar && ( cd bar && echo "[includeIf \"gitdir:bar/\"]path=bar7" >>.git/config && echo "[test]seven=7" >.git/bar7 && echo 7 >expect && git config test.seven >actual && test_cmp expect actual ) ' As you can see, this test case is marked with the `SYMLINKS` prerequisite, and hence can be skipped. I got a crazy idea, though: how about `cd foo` instead of `cd bar`? It should still work, as `bar` is just a symbolic link (unless that test case has been skipped, in which case `bar` does not even exist) to `foo`. And while this still relies on a side effect of an earlier test case ('conditional include, both unanchored', to be precise), at least that test case cannot be skipped because of a missing prerequisite. The same `s/cd bar/cd foo` applies to the second test case in this patch, too, of course. Thanks, Dscho > > + echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && > > + echo "[test]nine=9" >.git/bar9 && > > + git checkout -b master && > > + test_must_fail git config test.nine && > > + git checkout -b foo-branch && > > + echo 9 >expect && > > + git config test.nine >actual && > > + test_cmp expect actual > > + ) > > +' > > + > > +test_expect_success 'conditional include, onbranch, wildcard' ' > > + ( > > + cd bar && > > + echo "[includeIf \"onbranch:?oo-*/**\"]path=bar10" >>.git/config && > > + echo "[test]ten=10" >.git/bar10 && > > + git checkout -b not-foo-branch/a && > > + test_must_fail git config test.ten && > > + > > + echo 10 >expect && > > + git checkout -b foo-branch/a/b/c && > > + git config test.ten >actual && > > + test_cmp expect actual && > > + > > + git checkout -b moo-bar/a && > > + git config test.ten >actual && > > + test_cmp expect actual > > + ) > > +' > > + > > test_expect_success 'include cycles are detected' ' > > cat >.gitconfig <<-\EOF && > > [test]value = gitconfig > > -- > > 2.22.0.rc1.169.g49223abbf8 > > > > > >
On Sat, Jun 1, 2019 at 2:34 AM Denton Liu <liu.denton@gmail.com> wrote: > > Currently, if a user wishes to have individual settings per branch, they > are required to manually keep track of the settings in their head and > manually set the options on the command-line or change the config at > each branch. > > Teach config the "onbranch:" includeIf condition so that it can > conditionally include configuration files if the branch that is checked > out in the current worktree matches the pattern given. Just wondering, anybody wants special settings on detached head (and if so, what syntax should it be)? > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 7e2a6f61f5..93aa4323c6 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -144,6 +144,13 @@ refer to linkgit:gitignore[5] for details. For convenience: > This is the same as `gitdir` except that matching is done > case-insensitively (e.g. on case-insensitive file sytems) > > +`onbranch`:: > + The data that follows the keyword `onbranch:` is taken to be a pattern > + with standard globbing wildcards and two additional ones, `**/` and > + `/**`, that can match multiple path components. If we are in a worktree > + where the name of the branch that is currently checked out matches the > + pattern, the include condition is met. Supporting transforming "foo/" to "foo/**" would be nice (gitdir and gitdir/i do this). Useful when you organize your branches into "subdirs". > @@ -264,6 +265,24 @@ static int include_by_gitdir(const struct config_options *opts, > return ret; > } > > +static int include_by_branch(const char *cond, size_t cond_len) > +{ > + int flags; > + int ret; > + struct strbuf pattern = STRBUF_INIT; > + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flags); > + const char *shortname; > + > + if (!refname || !(flags & REF_ISSYMREF) || > + !skip_prefix(refname, "refs/heads/", &shortname)) We probably don't even need to check more than !refname. Symbolic refs from HEAD must be refs/heads/* or it's an error (fsck will complain). But it's probably also ok to stay strict. > + return 0; > + > + strbuf_add(&pattern, cond, cond_len); > + ret = !wildmatch(pattern.buf, shortname, WM_PATHNAME); > + strbuf_release(&pattern); > + return ret; > +}
diff --git a/Documentation/config.txt b/Documentation/config.txt index 7e2a6f61f5..93aa4323c6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -144,6 +144,13 @@ refer to linkgit:gitignore[5] for details. For convenience: This is the same as `gitdir` except that matching is done case-insensitively (e.g. on case-insensitive file sytems) +`onbranch`:: + The data that follows the keyword `onbranch:` is taken to be a pattern + with standard globbing wildcards and two additional ones, `**/` and + `/**`, that can match multiple path components. If we are in a worktree + where the name of the branch that is currently checked out matches the + pattern, the include condition is met. + A few more notes on matching via `gitdir` and `gitdir/i`: * Symlinks in `$GIT_DIR` are not resolved before matching. @@ -206,6 +213,11 @@ Example [includeIf "gitdir:/path/to/group/"] path = foo.inc + ; include only if we are in a worktree where foo-branch is + ; currently checked out + [includeIf "onbranch:foo-branch"] + path = foo.inc + Values ~~~~~~ diff --git a/config.c b/config.c index 296a6d9cc4..48bb435739 100644 --- a/config.c +++ b/config.c @@ -19,6 +19,7 @@ #include "utf8.h" #include "dir.h" #include "color.h" +#include "refs.h" struct config_source { struct config_source *prev; @@ -264,6 +265,24 @@ static int include_by_gitdir(const struct config_options *opts, return ret; } +static int include_by_branch(const char *cond, size_t cond_len) +{ + int flags; + int ret; + struct strbuf pattern = STRBUF_INIT; + const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flags); + const char *shortname; + + if (!refname || !(flags & REF_ISSYMREF) || + !skip_prefix(refname, "refs/heads/", &shortname)) + return 0; + + strbuf_add(&pattern, cond, cond_len); + ret = !wildmatch(pattern.buf, shortname, WM_PATHNAME); + strbuf_release(&pattern); + return ret; +} + static int include_condition_is_true(const struct config_options *opts, const char *cond, size_t cond_len) { @@ -272,6 +291,8 @@ static int include_condition_is_true(const struct config_options *opts, return include_by_gitdir(opts, cond, cond_len, 0); else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) return include_by_gitdir(opts, cond, cond_len, 1); + else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) + return include_by_branch(cond, cond_len); /* unknown conditionals are always false */ return 0; diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 579a86b7f8..05c7def1f2 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -309,6 +309,39 @@ test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icas ) ' +test_expect_success 'conditional include, onbranch' ' + ( + cd bar && + echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && + echo "[test]nine=9" >.git/bar9 && + git checkout -b master && + test_must_fail git config test.nine && + git checkout -b foo-branch && + echo 9 >expect && + git config test.nine >actual && + test_cmp expect actual + ) +' + +test_expect_success 'conditional include, onbranch, wildcard' ' + ( + cd bar && + echo "[includeIf \"onbranch:?oo-*/**\"]path=bar10" >>.git/config && + echo "[test]ten=10" >.git/bar10 && + git checkout -b not-foo-branch/a && + test_must_fail git config test.ten && + + echo 10 >expect && + git checkout -b foo-branch/a/b/c && + git config test.ten >actual && + test_cmp expect actual && + + git checkout -b moo-bar/a && + git config test.ten >actual && + test_cmp expect actual + ) +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig
Currently, if a user wishes to have individual settings per branch, they are required to manually keep track of the settings in their head and manually set the options on the command-line or change the config at each branch. Teach config the "onbranch:" includeIf condition so that it can conditionally include configuration files if the branch that is checked out in the current worktree matches the pattern given. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Thanks for taking a look, Johannes. I've modified the patch to match with wildcards now. Changes since v1: * Allow onbranch: to specify a pattern instead of just a static comparison Interdiff against v1: diff --git a/Documentation/config.txt b/Documentation/config.txt index 3b9fbe1860..93aa4323c6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -145,9 +145,11 @@ refer to linkgit:gitignore[5] for details. For convenience: case-insensitively (e.g. on case-insensitive file sytems) `onbranch`:: - The data that follows the keyword `onbranch:` is taken to be a - name of a local branch. If we are in a worktree where that - branch is currently checked out, the include condition is met. + The data that follows the keyword `onbranch:` is taken to be a pattern + with standard globbing wildcards and two additional ones, `**/` and + `/**`, that can match multiple path components. If we are in a worktree + where the name of the branch that is currently checked out matches the + pattern, the include condition is met. A few more notes on matching via `gitdir` and `gitdir/i`: diff --git a/config.c b/config.c index 954e84e13a..48bb435739 100644 --- a/config.c +++ b/config.c @@ -268,6 +268,8 @@ static int include_by_gitdir(const struct config_options *opts, static int include_by_branch(const char *cond, size_t cond_len) { int flags; + int ret; + struct strbuf pattern = STRBUF_INIT; const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, &flags); const char *shortname; @@ -275,7 +277,10 @@ static int include_by_branch(const char *cond, size_t cond_len) !skip_prefix(refname, "refs/heads/", &shortname)) return 0; - return !strncmp(cond, shortname, cond_len); + strbuf_add(&pattern, cond, cond_len); + ret = !wildmatch(pattern.buf, shortname, WM_PATHNAME); + strbuf_release(&pattern); + return ret; } static int include_condition_is_true(const struct config_options *opts, diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 062db08ad9..05c7def1f2 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -314,6 +314,7 @@ test_expect_success 'conditional include, onbranch' ' cd bar && echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && echo "[test]nine=9" >.git/bar9 && + git checkout -b master && test_must_fail git config test.nine && git checkout -b foo-branch && echo 9 >expect && @@ -322,6 +323,25 @@ test_expect_success 'conditional include, onbranch' ' ) ' +test_expect_success 'conditional include, onbranch, wildcard' ' + ( + cd bar && + echo "[includeIf \"onbranch:?oo-*/**\"]path=bar10" >>.git/config && + echo "[test]ten=10" >.git/bar10 && + git checkout -b not-foo-branch/a && + test_must_fail git config test.ten && + + echo 10 >expect && + git checkout -b foo-branch/a/b/c && + git config test.ten >actual && + test_cmp expect actual && + + git checkout -b moo-bar/a && + git config test.ten >actual && + test_cmp expect actual + ) +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig Documentation/config.txt | 12 ++++++++++++ config.c | 21 +++++++++++++++++++++ t/t1305-config-include.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+)