Re: [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree

[Max Kirillov <max@max630.net>]

On Thu, Mar 26, 2015 at 07:04:24PM +0700, Nguyễn Thái Ngọc Duy wrote:
> When you define $GIT_DIR/info/config.worktree, which contains of
> gitignore-style patterns (*), config variables that match these
> patterns will be saved in $GIT_DIR/config.worktree instead of
> $GIT_DIR/config.

Should it rather be in GIT_COMMON_DIR? As far as I
understand, its meaning is "variables which we allow to use
per-worktree because we intend to have them different in
different worktrees, and sure no bad issues
can happen because this. It is not hardcored in git because
the list is going to extend, and we'd like to allow older
versions of git (and other git implementations) to be still
able to understand newer repositories". So there should be
no sense to make the list worktree-specific.

Also, probably the per-worktree variables should be searched
for in both common config and per-worktree config, and the
main repository should not have config.worktree, to be able
to work with implementations which are not aware of the
whole multiple worktrees feature. And in worktrees, if the
variable is not defined in config.wortree, the default
vaalue should come from common config. This though has
downside that worktree cannot use the more global vlue for
variable implicitly.

-- 
Max

> On the surface, they are just like any other variables. You can read
> or modify them with `git config` command, or config_* API. Underneath,
> they are always stored in $GIT_DIR/config.worktree instead of
> $GIT_DIR/config (and never in $HOME/.gitconfig or /etc/gitconfig)
> 
> The reason for this split is, as the name implies, for
> worktree-specific configuration. When the same repo is attached to
> more than one worktree, we can't use $GIT_DIR/config to store worktree
> specific stuff because it's shared across worktrees.
> 
> (*) Config variable names are separated by dots. However as this is a
> quick and dirty patch to experiment with submodules and multiple
> worktrees, you must substitute dots with slashes when writing these
> patterns, for now. E.g. write "remote/*/foo" instead of "remote.*.foo"
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Something for Max to play with if he's still experimenting with
>  multiple worktree support for submodules :-) It's far from perfect,
>  but good enough for this purpose.
> 
>  Documentation/config.txt |  6 ++++-
>  builtin/config.c         |  8 ++++++
>  config.c                 | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
>  t/t1300-repo-config.sh   | 34 ++++++++++++++++++++++++
>  4 files changed, 114 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2700a1b..6d00f49 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2,11 +2,15 @@ CONFIGURATION FILE
>  ------------------
>  
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> +the Git commands' behavior. The `.git/config` and `.git/config.worktree`
> +files in each repository
>  is used to store the configuration for that repository, and
>  `$HOME/.gitconfig` is used to store a per-user configuration as
>  fallback values for the `.git/config` file. The file `/etc/gitconfig`
>  can be used to store a system-wide default configuration.
> +If `.git/info/core.worktree` exists, it contains gitignore-style
> +patterns. Variables that match these patterns can only be contained in
> +`.git/config.worktree`.
>  
>  The configuration variables are used by both the Git plumbing
>  and the porcelains. The variables are divided into sections, wherein
> diff --git a/builtin/config.c b/builtin/config.c
> index 8cc2604..4ca8fc2 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -555,6 +555,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  			usage_with_options(builtin_config_usage, builtin_config_options);
>  		}
>  
> +	/*
> +	 * For set operations, --local could be either config or
> +	 * config.worktree. Let config.c determine the path based on
> +	 * config keys.
> +	 */
> +	if (use_local_config && actions != ACTION_LIST)
> +		given_config_source.file = NULL;
> +
>  	if (actions == ACTION_LIST) {
>  		check_argc(argc, 0, 0);
>  		if (git_config_with_options(show_all_config, NULL,
> diff --git a/config.c b/config.c
> index 15a2983..f68eb6a 100644
> --- a/config.c
> +++ b/config.c
> @@ -12,6 +12,7 @@
>  #include "quote.h"
>  #include "hashmap.h"
>  #include "string-list.h"
> +#include "dir.h"
>  
>  struct config_source {
>  	struct config_source *prev;
> @@ -37,6 +38,7 @@ struct config_source {
>  };
>  
>  static struct config_source *cf;
> +static struct exclude_list config_local;
>  
>  static int zlib_compression_seen;
>  
> @@ -84,6 +86,34 @@ static long config_buf_ftell(struct config_source *conf)
>  	return conf->u.buf.pos;
>  }
>  
> +static int is_config_local(const char *key_)
> +{
> +	static struct strbuf key = STRBUF_INIT;
> +	static int loaded;
> +	int i, dtype;
> +
> +	if (!loaded) {
> +		add_excludes_from_file_to_list(git_path("info/config.worktree"),
> +					       "", 0, &config_local, 0);
> +		loaded = 1;
> +	}
> +
> +	if (!config_local.nr)
> +		return 0;
> +
> +	strbuf_reset(&key);
> +	strbuf_addstr(&key, key_);
> +	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;
> +}
> +
>  #define MAX_INCLUDE_DEPTH 10
>  static const char include_depth_advice[] =
>  "exceeded maximum include depth (%d) while including\n"
> @@ -1167,6 +1197,15 @@ int git_config_system(void)
>  	return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
>  }
>  
> +static int config_local_filter(const char *var, const char *value, void *data)
> +{
> +	struct config_include_data *inc = data;
> +
> +	if (!is_config_local(var))
> +		return error("%s does not belong to config.worktree", var);
> +	return inc->fn(var, value, inc->data);
> +}
> +
>  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  {
>  	int ret = 0, found = 0;
> @@ -1192,8 +1231,19 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  	}
>  
>  	if (repo_config && !access_or_die(repo_config, R_OK, 0)) {
> +		char *wt_config;
>  		ret += git_config_from_file(fn, repo_config, data);
>  		found += 1;
> +		wt_config = git_pathdup("config.worktree");
> +		if (!access_or_die(wt_config, R_OK, 0)) {
> +			struct config_include_data inc = CONFIG_INCLUDE_INIT;
> +			inc.fn = fn;
> +			inc.data = data;
> +			ret += git_config_from_file(config_local_filter,
> +						    wt_config, &inc);
> +			found += 1;
> +		}
> +		free(wt_config);
>  	}
>  
>  	switch (git_config_from_parameters(fn, data)) {
> @@ -1932,6 +1982,23 @@ int git_config_set_multivar_in_file(const char *config_filename,
>  
>  	store.multi_replace = multi_replace;
>  
> +	if (is_config_local(key)) {
> +		if (!config_filename)
> +			config_filename = filename_buf = git_pathdup("config.worktree");
> +		/* cheap trick, but should work 90% of time */
> +		else if (!ends_with(config_filename, ".worktree"))
> +			die("%s can only be stored in %s",
> +			    key, git_path("config.worktree"));
> +		else {
> +			struct strbuf sb = STRBUF_INIT;
> +			strbuf_addstr(&sb, real_path(config_filename));
> +			if (strcmp_icase(sb.buf,
> +					 real_path(git_path("config.worktree"))))
> +				die("%s can only be stored in %s",
> +				    key, git_path("config.worktree"));
> +			strbuf_release(&sb);
> +		}
> +	}
>  	if (!config_filename)
>  		config_filename = filename_buf = git_pathdup("config");
>  
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 938fc8b..c01effb 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -1179,4 +1179,38 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
>  	  "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600"
>  '
>  
> +test_expect_success 'setting worktree.foo goes to config.worktree' '
> +	test_when_finished "rm .git/config.worktree" &&
> +	echo "worktree/*" >.git/info/config.worktree &&
> +	git config wOrktree.foo bar &&
> +	cat >expect <<\EOF &&
> +[wOrktree]
> +	foo = bar
> +EOF
> +	test_cmp expect .git/config.worktree &&
> +	test "`git config woRktree.foo`" = bar
> +'
> +
> +test_expect_success 'setting --local worktree.foo goes to config.worktree' '
> +	test_when_finished "rm .git/config.worktree" &&
> +	echo "worktree/*" >.git/info/config.worktree &&
> +	git config --local worKtree.fooo barr &&
> +	cat >expect <<\EOF &&
> +[worKtree]
> +	fooo = barr
> +EOF
> +	test_cmp expect .git/config.worktree &&
> +	test "`git config worktreE.fooo`" = barr
> +'
> +
> +test_expect_success 'setting worktree.foo outside config.worktree' '
> +	echo "worktree/*" >.git/info/config.worktree &&
> +	test_must_fail git config --global Worktree.foo bar 2>err &&
> +	grep config.worktree err &&
> +	test_must_fail git config --system workTree.foo bar 2>err &&
> +	grep config.worktree err &&
> +	test_must_fail git config --file foo.worktree worktree.foo bar 2>err &&
> +	grep config.worktree err
> +'
> +
>  test_done
> -- 
> 2.3.0.rc1.137.g477eb31
>