* [Patch 0/3] includeIf series for worktrees @ 2021-07-12 22:31 randall.becker 2021-07-12 22:31 ` [Patch 1/3] config.c: add conditional include based on worktree randall.becker ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: randall.becker @ 2021-07-12 22:31 UTC (permalink / raw) To: git; +Cc: Randall S. Becker From: "Randall S. Becker" <rsbecker@nexbridge.com> Add support for includeIf with a worktree: prefix. This permits conditional includes that are specific to individual worktrees. The set of patches differ slightly from gitdir: as worktrees to not have .git/ directories, so matching repo/ is somewhat problematic - suggestings on dealing with that case are more than welcome. Also added worktree test condition to verify that onbranch: works in a worktree scenario as different from the main repository. Randall S. Becker (3): config.c: add conditional include based on worktree. Documentation/config.txt: add worktree includeIf conditionals. t1305: add tests for includeIf:worktree. Documentation/config.txt | 11 +++++- config.c | 63 ++++++++++++++++++++++++++++++ t/t1305-config-include.sh | 81 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 1 deletion(-) -- 2.32.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch 1/3] config.c: add conditional include based on worktree. 2021-07-12 22:31 [Patch 0/3] includeIf series for worktrees randall.becker @ 2021-07-12 22:31 ` randall.becker 2021-07-13 13:03 ` Johannes Schindelin 2021-07-12 22:31 ` [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals randall.becker 2021-07-12 22:31 ` [Patch 3/3] t1305: add tests for includeIf:worktree randall.becker 2 siblings, 1 reply; 9+ messages in thread From: randall.becker @ 2021-07-12 22:31 UTC (permalink / raw) To: git; +Cc: Randall S. Becker From: "Randall S. Becker" <rsbecker@nexbridge.com> This enhancement extends the [includeIf] semantics to include conditional inclusion based on whether the conditional is within a specific worktree or case-insensitive worktree. The [includeIf "worktree:path"] and [includeIf "worktree/i:path"] and analogous to the gitdir: and gitdir/i: conditions, respectively. Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> --- config.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/config.c b/config.c index f9c400ad30..e2b2364579 100644 --- a/config.c +++ b/config.c @@ -272,6 +272,64 @@ static int include_by_gitdir(const struct config_options *opts, return ret; } +static int include_by_worktree(const struct config_options *opts, + const char *cond, size_t cond_len, int icase) +{ + struct strbuf text = STRBUF_INIT; + struct strbuf pattern = STRBUF_INIT; + int ret = 0, prefix; + const char *worktree; + int already_tried_absolute = 0; + + if (the_repository->worktree) + worktree = the_repository->worktree; + else + goto done; + + strbuf_realpath(&text, worktree, 1); + strbuf_add(&pattern, cond, cond_len); + prefix = prepare_include_condition_pattern(&pattern); + +again: + if (prefix < 0) + goto done; + + if (prefix > 0) { + /* + * perform literal matching on the prefix part so that + * any wildcard character in it can't create side effects. + */ + if (text.len < prefix) + goto done; + if (!icase && strncmp(pattern.buf, text.buf, prefix)) + goto done; + if (icase && strncasecmp(pattern.buf, text.buf, prefix)) + goto done; + } + + ret = !wildmatch(pattern.buf + prefix, text.buf + prefix, + WM_PATHNAME | (icase ? WM_CASEFOLD : 0)); + + if (!ret && !already_tried_absolute) { + /* + * We've tried e.g. matching worktree:~/work, but if + * ~/work is a symlink to /mnt/storage/work + * strbuf_realpath() will expand it, so the rule won't + * match. Let's match against a + * strbuf_add_absolute_path() version of the path, + * which'll do the right thing + */ + strbuf_reset(&text); + strbuf_add_absolute_path(&text, worktree); + already_tried_absolute = 1; + goto again; + } +done: + strbuf_release(&pattern); + strbuf_release(&text); + return ret; +} + static int include_by_branch(const char *cond, size_t cond_len) { int flags; @@ -300,6 +358,11 @@ 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, "worktree:", &cond, &cond_len)) + return include_by_worktree(opts, cond, cond_len, 0); + else if (skip_prefix_mem(cond, cond_len, "worktree/i:", &cond, + &cond_len)) + return include_by_worktree(opts, cond, cond_len, 1); else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) return include_by_branch(cond, cond_len); -- 2.32.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch 1/3] config.c: add conditional include based on worktree. 2021-07-12 22:31 ` [Patch 1/3] config.c: add conditional include based on worktree randall.becker @ 2021-07-13 13:03 ` Johannes Schindelin 0 siblings, 0 replies; 9+ messages in thread From: Johannes Schindelin @ 2021-07-13 13:03 UTC (permalink / raw) To: randall.becker; +Cc: git, Randall S. Becker Hi Randall, On Mon, 12 Jul 2021, randall.becker@nexbridge.ca wrote: > @@ -300,6 +358,11 @@ 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, "worktree:", &cond, &cond_len)) > + return include_by_worktree(opts, cond, cond_len, 0); > + else if (skip_prefix_mem(cond, cond_len, "worktree/i:", &cond, > + &cond_len)) > + return include_by_worktree(opts, cond, cond_len, 1); Thank you for not forgetting the `/i` case. Ciao, Dscho > else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) > return include_by_branch(cond, cond_len); > > -- > 2.32.0 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals. 2021-07-12 22:31 [Patch 0/3] includeIf series for worktrees randall.becker 2021-07-12 22:31 ` [Patch 1/3] config.c: add conditional include based on worktree randall.becker @ 2021-07-12 22:31 ` randall.becker 2021-07-14 1:04 ` Junio C Hamano 2021-07-12 22:31 ` [Patch 3/3] t1305: add tests for includeIf:worktree randall.becker 2 siblings, 1 reply; 9+ messages in thread From: randall.becker @ 2021-07-12 22:31 UTC (permalink / raw) To: git; +Cc: Randall S. Becker From: "Randall S. Becker" <rsbecker@nexbridge.com> Documentation of the worktree and worktree/i conditionals is add based on gitdir rules except that the trailing / form of the path is not supported. Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> --- Documentation/config.txt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index bf82766a6a..7e951937ae 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -143,7 +143,16 @@ refer to linkgit:gitignore[5] for details. For convenience: `gitdir/i`:: This is the same as `gitdir` except that matching is done - case-insensitively (e.g. on case-insensitive file systems) + case-insensitively (e.g. on case-insensitive file systems). + +`worktree`:: + This is similar to `gitdir` except that matching is done with + the path of a worktree instead of the main repository. Unlike + `gitdir`, the trailing / form of the worktree path is not supported. + +`worktree/i`:: + This is the same as `worktree` except that matching is done + case-insensitively (e.g. on case-insensitive file systems). `onbranch`:: The data that follows the keyword `onbranch:` is taken to be a -- 2.32.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals. 2021-07-12 22:31 ` [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals randall.becker @ 2021-07-14 1:04 ` Junio C Hamano 2021-07-14 13:46 ` Randall S. Becker 0 siblings, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2021-07-14 1:04 UTC (permalink / raw) To: randall.becker; +Cc: git, Randall S. Becker randall.becker@nexbridge.ca writes: > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > Documentation of the worktree and worktree/i conditionals is add based on > gitdir rules except that the trailing / form of the path is not supported. > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > --- > Documentation/config.txt | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index bf82766a6a..7e951937ae 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -143,7 +143,16 @@ refer to linkgit:gitignore[5] for details. For convenience: > > `gitdir/i`:: > This is the same as `gitdir` except that matching is done > - case-insensitively (e.g. on case-insensitive file systems) > + case-insensitively (e.g. on case-insensitive file systems). > + > +`worktree`:: > + This is similar to `gitdir` except that matching is done with > + the path of a worktree instead of the main repository. Unlike > + `gitdir`, the trailing / form of the worktree path is not supported. It is not immediately obvious what "the trailing / form" means. Does it refer to the 4th item in the 4-bullet list in the description just above the patch context (I am trying to make a guess here)? The problem I perceive in this description is that there is no phrase "trailing" in the vicinity of what readers have read so far; readers who are not exactly familiar with the system may need a bit more assurance that they guessed correctly. Unlike `gitdir`, `**` will not be automatically added to a pattern that ends with `/` would be easier to give that assurance, albeit more verbosely. Assuming that I guessed correctly, is this a deliberate design decision not to "automatically add ** after a pattern that ends with a slash", and if so why? I would have thought that "in the worktrees that I create inside /var/tmp/, please enable these configuration variables" would be a fairly natural thing to ask, and I do not immediately see a reason why we want to apply different syntax rules between "gitdir" and "worktree". Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals. 2021-07-14 1:04 ` Junio C Hamano @ 2021-07-14 13:46 ` Randall S. Becker 2021-07-14 17:10 ` Junio C Hamano 0 siblings, 1 reply; 9+ messages in thread From: Randall S. Becker @ 2021-07-14 13:46 UTC (permalink / raw) To: 'Junio C Hamano'; +Cc: git On July 13, 2021 9:05 PM. Junio C Hamano wrote: >randall.becker@nexbridge.ca writes: > >> From: "Randall S. Becker" <rsbecker@nexbridge.com> >> >> Documentation of the worktree and worktree/i conditionals is add based >> on gitdir rules except that the trailing / form of the path is not supported. >> >> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> >> --- >> Documentation/config.txt | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt index >> bf82766a6a..7e951937ae 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -143,7 +143,16 @@ refer to linkgit:gitignore[5] for details. For convenience: >> >> `gitdir/i`:: >> This is the same as `gitdir` except that matching is done >> - case-insensitively (e.g. on case-insensitive file systems) >> + case-insensitively (e.g. on case-insensitive file systems). >> + >> +`worktree`:: >> + This is similar to `gitdir` except that matching is done with >> + the path of a worktree instead of the main repository. Unlike >> + `gitdir`, the trailing / form of the worktree path is not supported. > >It is not immediately obvious what "the trailing / form" means. > >Does it refer to the 4th item in the 4-bullet list in the description just above the patch context (I am trying to make a guess here)? > >The problem I perceive in this description is that there is no phrase "trailing" in the vicinity of what readers have read so far; readers who >are not exactly familiar with the system may need a bit more assurance that they guessed correctly. > > Unlike `gitdir`, `**` will not be automatically added to a > pattern that ends with `/` > >would be easier to give that assurance, albeit more verbosely. > >Assuming that I guessed correctly, is this a deliberate design decision not to "automatically add ** after a pattern that ends with a slash", >and if so why? I would have thought that "in the worktrees that I create inside /var/tmp/, please enable these configuration variables" >would be a fairly natural thing to ask, and I do not immediately see a reason why we want to apply different syntax rules between "gitdir" >and "worktree". The reason for this comes down to what is in *the_repository. Essentially, the_repository->gitdir always has a /path/to/.git directory with full qualification. the_repository->worktree does not have /.git added for obvious reasons, so the /path/to is bare of the trailing /. This causes a trailing pattern /to/path/** match to fail. I could copy the value into a working buffer but that seemed a bit clunky. So using the available information, the syntax rules need to be different between the two, unless the value of worktree is augmented. I was unsure which way the team wanted to go on this. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals. 2021-07-14 13:46 ` Randall S. Becker @ 2021-07-14 17:10 ` Junio C Hamano 2021-07-14 17:30 ` Randall S. Becker 0 siblings, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2021-07-14 17:10 UTC (permalink / raw) To: Randall S. Becker; +Cc: git "Randall S. Becker" <rsbecker@nexbridge.com> writes: >>Assuming that I guessed correctly, is this a deliberate design >>decision not to "automatically add ** after a pattern that ends >>with a slash", and if so why? I would have thought that "in the >>worktrees that I create inside /var/tmp/, please enable these >>configuration variables" would be a fairly natural thing to ask, >>and I do not immediately see a reason why we want to apply >>different syntax rules between "gitdir" and "worktree". > The reason for this comes down to what is in >*the_repository. Sorry, but I still do not understand. > Essentially, the_repository->gitdir always has a /path/to/.git > directory with full qualification. Yes. > the_repository->worktree does not have /.git added > for obvious reasons, so the /path/to is bare of the trailing >/. It may be the case, but /path/to/.git does not have trailing slash, either, so I do not see the relevance. When you say [includeIf "gitdir:/path/"], the "behave as if ** is added after the slash at the end" rule kicks in, and the pattern "/path/**" is used to see if it matches "/path/to/.git" and it does, right? When you say [includeIf "worktree:/path/"], wouldn't the resulting "/path/**" match "/path/to"? By the way, I think [PATCH 1/3] should turn the body of include_by_gitdir() to a common helper function that - accepts a path to a directory and a pattern - turns it into a relpath - prepares the pattern with prepare_include_condition_pattern() - do the match include_by_gitdir() does. and make include_by_gitdir() a very thin wrapper that passes opts->git_dir to that common helper. Then you do not have to copy the entire function to create your new include_by_worktree(); it can be another very thin wrapper that passes the_repository->worktree instead of opts->git_dir to the common helper, as there is no other difference in these two functions. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals. 2021-07-14 17:10 ` Junio C Hamano @ 2021-07-14 17:30 ` Randall S. Becker 0 siblings, 0 replies; 9+ messages in thread From: Randall S. Becker @ 2021-07-14 17:30 UTC (permalink / raw) To: 'Junio C Hamano'; +Cc: git On July 14, 2021 1:10 PM, Junio C Hamano >"Randall S. Becker" <rsbecker@nexbridge.com> writes: > >>>Assuming that I guessed correctly, is this a deliberate design >>>decision not to "automatically add ** after a pattern that ends with a >>>slash", and if so why? I would have thought that "in the worktrees >>>that I create inside /var/tmp/, please enable these configuration >>>variables" would be a fairly natural thing to ask, and I do not >>>immediately see a reason why we want to apply different syntax rules >>>between "gitdir" and "worktree". > >> The reason for this comes down to what is in *the_repository. > >Sorry, but I still do not understand. > >> Essentially, the_repository->gitdir always has a /path/to/.git >> directory with full qualification. > >Yes. > >> the_repository->worktree does not have /.git added for obvious >>reasons, so the /path/to is bare of the trailing /. > >It may be the case, but /path/to/.git does not have trailing slash, either, so I do not see the relevance. > >When you say [includeIf "gitdir:/path/"], the "behave as if ** is added after the slash at the end" rule kicks in, and the pattern "/path/**" is >used to see if it matches "/path/to/.git" and it does, right? When you say [includeIf "worktree:/path/"], wouldn't the resulting "/path/**" >match "/path/to"? I think I over-complicated the first test case and got myself into a mess. Will fix that. >By the way, I think [PATCH 1/3] should turn the body of >include_by_gitdir() to a common helper function that > > - accepts a path to a directory and a pattern > - turns it into a relpath > - prepares the pattern with prepare_include_condition_pattern() > - do the match include_by_gitdir() does. > >and make include_by_gitdir() a very thin wrapper that passes >opts->git_dir to that common helper. Then you do not have to copy >the entire function to create your new include_by_worktree(); it can be another very thin wrapper that passes the_repository->worktree >instead of opts->git_dir to the common helper, as there is no other difference in these two functions. That sounds like a plan. Will go for it in V2. -Randall ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch 3/3] t1305: add tests for includeIf:worktree. 2021-07-12 22:31 [Patch 0/3] includeIf series for worktrees randall.becker 2021-07-12 22:31 ` [Patch 1/3] config.c: add conditional include based on worktree randall.becker 2021-07-12 22:31 ` [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals randall.becker @ 2021-07-12 22:31 ` randall.becker 2 siblings, 0 replies; 9+ messages in thread From: randall.becker @ 2021-07-12 22:31 UTC (permalink / raw) To: git; +Cc: Randall S. Becker From: "Randall S. Becker" <rsbecker@nexbridge.com> The tests are a subset of those for gitdir:, taking into account that the worktree: form does not support the trailing / at this time in pattern matches. Some resets of the .git/config file are done to restrict the set of includeIf paths being evaluated that conflict with prior subtests. Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> --- t/t1305-config-include.sh | 81 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index ccbb116c01..fe1ad106c3 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -309,6 +309,69 @@ test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icas ) ' +test_expect_success 'conditional worktree include, unanchored' ' + ( + cd foo && + # Must add a commit for worktree add + git commit --allow-empty --allow-empty-message && + sed -i "/includeIf/,\$d" .git/config && + git worktree add ../foo.wt && + echo "[includeIf \"worktree:foo.wt\"]path=bar" >>.git/config && + echo "[test]one=1" >.git/bar && + cd ../foo.wt && + echo 1 >expect && + git config test.one >actual && + test_cmp expect actual + ) +' + +test_expect_success 'conditional worktree include, $HOME expansion' ' + ( + cd foo && + echo "[includeIf \"worktree:~/foo.wt\"]path=bar2" >>.git/config && + echo "[test]two=2" >.git/bar2 && + cd ../foo.wt && + echo 2 >expect && + git config test.two >actual && + test_cmp expect actual + ) +' + +test_expect_success 'conditional worktree include, full pattern' ' + ( + cd foo && + echo "[includeIf \"worktree:**/foo.wt\"]path=bar3" >>.git/config && + echo "[test]three=3" >.git/bar3 && + cd ../foo.wt && + echo 3 >expect && + git config test.three >actual && + test_cmp expect actual + ) +' + +test_expect_success 'conditional worktree include, relative path' ' + echo "[includeIf \"worktree:./foo.wt\"]path=bar4" >>.gitconfig && + echo "[test]four=4" >bar4 && + ( + cd foo.wt && + echo 4 >expect && + git config test.four >actual && + test_cmp expect actual + ) +' + +test_expect_success 'conditional worktree include, both unanchored, icase' ' + ( + cd foo && + echo "[includeIf \"worktree/i:FOO.WT\"]path=bar5" >>.git/config && + echo "[test]five=5" >.git/bar5 && + cd ../foo.wt && + echo 5 >expect && + git config test.five >actual && + test_cmp expect actual + ) +' + test_expect_success 'conditional include, onbranch' ' echo "[includeIf \"onbranch:foo-branch\"]path=bar9" >>.git/config && echo "[test]nine=9" >.git/bar9 && @@ -348,6 +411,24 @@ test_expect_success 'conditional include, onbranch, implicit /** for /' ' test_cmp expect actual ' +test_expect_success 'conditional worktree include, onbranch' ' + ( + cd foo && + sed -i "/includeIf/,\$d" .git/config && + echo "[includeIf \"onbranch:foo.wt2\"]path=bar12" >>.git/config && + echo "[test]twelve=12" >.git/bar12 + ) && + ( + cd foo.wt && + git checkout -b main && + test_must_fail git config test.twelve && + git checkout -b foo.wt2 && + echo 12 >expect && + git config test.twelve >actual && + test_cmp expect actual + ) +' + test_expect_success 'include cycles are detected' ' git init --bare cycle && git -C cycle config include.path cycle && -- 2.32.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-14 17:30 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-12 22:31 [Patch 0/3] includeIf series for worktrees randall.becker 2021-07-12 22:31 ` [Patch 1/3] config.c: add conditional include based on worktree randall.becker 2021-07-13 13:03 ` Johannes Schindelin 2021-07-12 22:31 ` [Patch 2/3] Documentation/config.txt: add worktree includeIf conditionals randall.becker 2021-07-14 1:04 ` Junio C Hamano 2021-07-14 13:46 ` Randall S. Becker 2021-07-14 17:10 ` Junio C Hamano 2021-07-14 17:30 ` Randall S. Becker 2021-07-12 22:31 ` [Patch 3/3] t1305: add tests for includeIf:worktree randall.becker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).