Re: [PATCH 2/5] config.c: move worktree-specific variables to .git/worktrees/...

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Dec 2, 2015 at 2:13 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> .git/info/config.worktree is a pattern list that splits .git/config in
> to sets: the worktree set matches the patterns, the commmon set does
> not.
>
> In normal worktrees, both sets are stored in .git/config. The
> config.worktree has no effect. Nothing is changed.
>
> In linked worktrees, the common and worktree sets are read from and
> saved to .git/config and .git/config.worktree respectively. Config
> keys in .git/config that belong to the worktree set is ignored. Those
> are for the main worktree only. Similarly, keys not matching the
> patterns come from .git/config, duplicate keys from
> .git/config.worktree are ignored.
>
> The effect is similar to the $GIT_DIR/$GIT_COMMON_DIR split, we can
> define that some vars can be shared and some cannot. And as a result
> of the $GIT_DIR/$GIT_COMMON_DIR split, config.worktree is actually
> found at .git/worktrees/<id>/config.worktree.

Why does this worktree-specific file need/have a .worktree suffix?

> Throwing the exclude mechanism into this means reading config files
> will be slower. But unless somebody reads thousands of keys, it should
> not be noticable. The nice thing is we don't have to introduce yet
> another pattern syntax.
>
> In future, we might want to have a shared config file to contain
> common worktree-specific settings, so that we have some good defaults,
> but still allow customization. Or we could twist the above logic a
> bit: for linked worktrees, read _all_ variables in config.worktree
> regardless of the patterns. But let's wait and see..
>
> Helped-by: Max Kirillov <max@max630.net>
> Helped-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/config.c b/config.c
> @@ -89,6 +91,73 @@ static long config_buf_ftell(struct config_source *conf)
> +static void load_info_config_worktree(void)
> +{
> +       struct exclude_list *el = &config_local;
> +       struct strbuf sb = STRBUF_INIT;
> +       int i, lineno = 1;
> +       char *buf, *entry;
> +       size_t size;
> +
> +       clear_exclude_list(el);
> +
> +       if (strbuf_read_file(&sb,
> +                            git_path("info/config.worktree"),
> +                            128) <= 0) {
> +               strbuf_release(&sb);
> +               return;
> +       }
> +       strbuf_addch(&sb, '\n');
> +       el->filebuf = buf = strbuf_detach(&sb, &size);
> +
> +       for (i = 0; i < size; i++)
> +               if (buf[i] == '.')
> +                       buf[i] = '/';
> +               else
> +                       buf[i] = tolower(buf[i]);
> +
> +       entry = buf;
> +       for (i = 0; i < size; i++) {
> +               if (buf[i] == '\n') {
> +                       if (entry != buf + i && entry[0] != '#') {
> +                               buf[i - (i && buf[i-1] == '\r')] = 0;
> +                               trim_trailing_spaces(entry);
> +                               add_exclude(entry, "", 0, el, lineno);
> +                       }
> +                       lineno++;
> +                       entry = buf + i + 1;
> +               }
> +       }
> +
> +       /*
> +        * avoid base name matching because it may confusion in

s/may/may cause/

> +        * non-directory context.
> +        */
> +       for (i = 0; i < el->nr; i++)
> +               el->excludes[i]->flags &= ~EXC_FLAG_NODIR;
> +}
> +
> +static int is_config_local(const char *key_)
> +{
> +       static struct strbuf key = STRBUF_INIT;
> +       int i, dtype;
> +
> +       if (!config_local.nr)
> +               return 0;
> +
> +       strbuf_reset(&key);
> +       strbuf_addstr(&key, key_);

Why does 'key' need to be static considering that it is overwritten on
each call and its value is never accessed after the function returns?

> +       for (i = 0; i < key.len; i++) {
> +               if (key.buf[i] == '.')
> +                       key.buf[i] = '/';
> +               else
> +                       key.buf[i] = tolower(key.buf[i]);
> +       }
> +       dtype = DT_REG;
> +       return is_excluded_from_list(key.buf, key.len, "", &dtype,
> +                                    &config_local) > 0;
> +}
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -198,4 +198,30 @@ test_expect_success 'local clone from linked checkout' '
> +test_expect_success 'setting worktree.foo goes to config.worktree' '
> +       echo worKtree.Foo >> .git/info/config.worktree &&

Perhaps? s/>> />/

> +       git worktree add wt.foo HEAD &&
> +       git config woRKtree.FOO barrrr &&
> +       git --git-dir=wt.foo/.git config woRKtree.FOO bar &&
> +       cat >expect <<\EOF &&
> +[woRKtree]
> +       FOO = bar
> +EOF
> +       test_cmp expect .git/worktrees/wt.foo/config.worktree &&
> +       git --git-dir=wt.foo/.git config woRktree.foo >actual2 &&
> +       echo bar >expect2 &&
> +       test_cmp expect2 actual2 &&
> +       test_path_is_missing .git/config.worktree &&
> +       git config WORKTREE.FOO >actual3 &&
> +       echo barrrr >expect3 &&
> +       test_cmp expect3 actual3
> +'
> +
> +test_expect_success 'shared config still goes to config' '
> +       git config random.key randomValue &&
> +       git --git-dir=wt.foo/.git config random.key >actual &&

What about also testing the opposite scenario?

    git --git-dir=wt.foo/.git  config random.key randomValue &&
    git config random.key >actual &&

> +       echo randomValue >expect &&
> +       test_cmp expect actual
> +'
> +
>  test_done
> --
> 2.2.0.513.g477eb31