All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] worktree: add per-worktree config files
@ 2018-09-23 17:04 Nguyễn Thái Ngọc Duy
  2018-09-23 20:47 ` Eric Sunshine
                   ` (4 more replies)
  0 siblings, 5 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-09-23 17:04 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config files are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present so that it works in any setup.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 11 +++-
 Documentation/git-config.txt           | 26 +++++---
 Documentation/git-worktree.txt         | 37 ++++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 16 ++++-
 cache.h                                |  2 +
 config.c                               |  7 +++
 environment.c                          |  1 +
 setup.c                                |  5 +-
 t/t2029-worktree-config.sh             | 82 ++++++++++++++++++++++++++
 worktree.c                             | 41 +++++++++++++
 worktree.h                             |  6 ++
 12 files changed, 231 insertions(+), 11 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d85d1a324..c24abf5871 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) are 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.
@@ -371,6 +372,12 @@ advice.*::
 		editor input from the user.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file in that order. In multiple working
+	directory mode, "config" file is shared while
+	"config.worktree" is per-working directory.
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 8e240435be..4870e00b89 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -45,13 +45,15 @@ unset an existing `--type` specifier with `--no-type`.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -131,6 +133,11 @@ from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -281,6 +288,10 @@ $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
@@ -299,9 +310,10 @@ configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..3f9112db56 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,43 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+directories. If the config variables `core.bare` or `core.worktree`
+are already present in the config file, they will be applied to the
+main working directory only.
+
+In order to have configuration specific to working directories, you
+can turn on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Git before
+version 2.20.0 will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and
+`core.worktree` is gone. If you have them before, you need to move
+them to the `config.worktree` of the main working directory. You may
+also take this opportunity to move other configuration that you do not
+want to share to all working directories:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working directory, unless
+   you are sure you always use sparse checkout for all working
+   directories.
+
+When `git config --worktree` is used to set a configuration variable
+in multiple working directory setup, `extensions.worktreeConfig` will
+be automatically set. The two variables `core.worktree` and
+`core.bare` if present will be moved to `config.worktree` of the main
+working tree.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..36fcca8087 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -275,6 +280,9 @@ worktrees/<id>/locked::
 	or manually by `git worktree prune`. The file may contain a string
 	explaining why the repository is locked.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/builtin/config.c b/builtin/config.c
index 97b58c4aea..337db7b552 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -24,6 +25,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, type;
 static char *default_value;
@@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error(_("only one config file at a time"));
 		usage_builtin_config();
@@ -645,7 +649,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1]) {
+			migrate_worktree_config(the_repository);
+			given_config_source.file = git_pathdup("config.worktree");
+		} else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				prefix_filename(prefix, given_config_source.file);
diff --git a/cache.h b/cache.h
index d508f3d4f8..9e9b917e99 100644
--- a/cache.h
+++ b/cache.h
@@ -957,11 +957,13 @@ extern int grafts_replace_parents;
 extern int repository_format_precious_objects;
 extern char *repository_format_partial_clone;
 extern const char *core_partial_clone_filter_default;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
 	char *partial_clone; /* value of extensions.partialclone */
+	int worktree_config;
 	int is_bare;
 	int hash_algo;
 	char *work_tree;
diff --git a/config.c b/config.c
index 3461993f0a..24f68a5208 100644
--- a/config.c
+++ b/config.c
@@ -1695,6 +1695,13 @@ static int do_git_config_sequence(const struct config_options *opts,
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 3f3c8746c2..268310b3dc 100644
--- a/environment.c
+++ b/environment.c
@@ -33,6 +33,7 @@ int ref_paranoia = -1;
 int repository_format_precious_objects;
 char *repository_format_partial_clone;
 const char *core_partial_clone_filter_default;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index b24c811c1c..6169193946 100644
--- a/setup.c
+++ b/setup.c
@@ -423,7 +423,9 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else
+		} else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
+		else
 			string_list_append(&data->unknown_extensions, ext);
 	} else if (strcmp(var, "core.bare") == 0) {
 		data->is_bare = git_config_bool(var, value);
@@ -466,6 +468,7 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 
 	repository_format_precious_objects = candidate->precious_objects;
 	repository_format_partial_clone = candidate->partial_clone;
+	repository_format_worktree_config = candidate->worktree_config;
 	string_list_clear(&candidate->unknown_extensions, 0);
 	if (!has_common) {
 		if (candidate->is_bare != -1) {
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000000..4ebdf13cf9
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+cmp_config() {
+	if [ "$1" = "-C" ]; then
+		shift &&
+		GD="-C $1" &&
+		shift
+	else
+		GD=
+	fi &&
+	echo "$1" >expected &&
+	shift &&
+	git $GD config "$@" >actual &&
+	test_cmp expected actual
+}
+
+test_expect_success 'setup' '
+	test_commit start &&
+	git config --worktree per.worktree is-ok &&
+	git worktree add wt1 &&
+	git worktree add wt2 &&
+	git config --worktree per.worktree is-ok &&
+	cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	cmp_config also-shared that.is &&
+	cmp_config -C wt1 also-shared that.is &&
+	cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 for-wt1 this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	git config core.bare true &&
+	cmp_config true core.bare &&
+	cmp_config -C wt1 true core.bare &&
+	cmp_config -C wt2 true core.bare &&
+	git config --unset core.bare
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+	git config core.bare true &&
+	git config --worktree foo.bar true &&
+	cmp_config true extensions.worktreeConfig &&
+	cmp_config true foo.bar &&
+	cmp_config true core.bare &&
+	! git -C wt1 config core.bare
+'
+
+test_done
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..a239f99bdc 100644
--- a/worktree.c
+++ b/worktree.c
@@ -5,6 +5,7 @@
 #include "worktree.h"
 #include "dir.h"
 #include "wt-status.h"
+#include "config.h"
 
 void free_worktrees(struct worktree **worktrees)
 {
@@ -508,3 +509,43 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
 	free_worktrees(worktrees);
 	return ret;
 }
+
+void migrate_worktree_config(struct repository *r)
+{
+	struct strbuf worktree_path = STRBUF_INIT;
+	struct strbuf main_path = STRBUF_INIT;
+	struct repository_format format;
+
+	assert(repository_format_worktree_config == 0);
+
+	strbuf_git_common_path(&worktree_path, r, "config.worktree");
+	strbuf_git_path(&main_path, "config");
+
+	read_repository_format(&format, main_path.buf);
+	assert(format.worktree_config == 0);
+
+	if (format.is_bare >= 0) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.bare", "true");
+		git_config_set_in_file(main_path.buf,
+				       "core.bare", NULL);
+	}
+	if (format.work_tree) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.worktree",
+				       format.work_tree);
+		git_config_set_in_file(main_path.buf,
+				       "core.worktree", NULL);
+	}
+
+	git_config_set_in_file(main_path.buf,
+			       "extensions.worktreeConfig", "true");
+	if (format.version == 0)
+		git_config_set_in_file(main_path.buf,
+				       "core.repositoryFormatVersion", "1");
+
+	repository_format_worktree_config = 1;
+
+	strbuf_release(&main_path);
+	strbuf_release(&worktree_path);
+}
diff --git a/worktree.h b/worktree.h
index df3fc30f73..b27b407d1b 100644
--- a/worktree.h
+++ b/worktree.h
@@ -108,4 +108,10 @@ extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Called to add extensions.worktreeConfig to $GIT_DIR/config and move
+ * main worktree specific config variables to $GIT_DIR/config.worktree.
+ */
+void migrate_worktree_config(struct repository *r);
+
 #endif
-- 
2.19.0.647.gb9a6049235


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2018-09-23 20:47 ` Eric Sunshine
  2018-09-24 14:21 ` Taylor Blau
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 34+ messages in thread
From: Eric Sunshine @ 2018-09-23 20:47 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Sun, Sep 23, 2018 at 1:04 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> A new repo extension is added, worktreeConfig. When it is present:
>
>  - Repository config reading by default includes $GIT_DIR/config _and_
>    $GIT_DIR/config.worktree. "config" file remains shared in multiple
>    worktree setup.
>
>  - The special treatment for core.bare and core.worktree, to stay
>    effective only in main worktree, is gone. These config files are
>    supposed to be in config.worktree.

"These config _settings_ are supposed to be..."

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> -is used to store the configuration for that repository, and
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) are each

s/are/in/

> +repository is used to store the configuration for that repository, and
>  `$HOME/.gitconfig` is used to store a per-user configuration as
> diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
> @@ -0,0 +1,82 @@
> +cmp_config() {
> +       if [ "$1" = "-C" ]; then

Style:

    if test "$1" = "-C"
    then
        ...

> +}
> +
> +test_expect_success 'setup' '
> +       test_commit start &&
> +       git config --worktree per.worktree is-ok &&

Did you want:

    cmp_config false extensions.worktreeConfig

at this point in the test or something? It's not clear what the
intention is here.

> +       git worktree add wt1 &&
> +       git worktree add wt2 &&
> +       git config --worktree per.worktree is-ok &&
> +       cmp_config true extensions.worktreeConfig
> +'
> +test_expect_success 'core.bare no longer for main only' '
> +       git config core.bare true &&
> +       cmp_config true core.bare &&
> +       cmp_config -C wt1 true core.bare &&
> +       cmp_config -C wt2 true core.bare &&
> +       git config --unset core.bare

Should this be?

    test_when_finished "git config --unset core.bare" &&

or perhaps test_config()?

> +'

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2018-09-23 20:47 ` Eric Sunshine
@ 2018-09-24 14:21 ` Taylor Blau
  2018-09-25 15:57   ` Duy Nguyen
  2018-09-29 13:53   ` Duy Nguyen
  2018-09-25 21:26 ` Junio C Hamano
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 34+ messages in thread
From: Taylor Blau @ 2018-09-24 14:21 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

On Sun, Sep 23, 2018 at 07:04:38PM +0200, Nguyễn Thái Ngọc Duy wrote:
> A new repo extension is added, worktreeConfig. When it is present:

I was initially scratching my head at why this should be a repository
extension, but...

>  - The special treatment for core.bare and core.worktree, to stay
>    effective only in main worktree, is gone. These config files are
>    supposed to be in config.worktree.

This seems to be sufficient justification for it. A destructive action
(such as migrating configuration from one location to another, as you
have implemented in the patch below) should be something that the user
opts into, rather than having happen automatically.

> This extension is most useful in multiple worktree setup because you
> now have an option to store per-worktree config (which is either
> .git/config.worktree for main worktree, or
> .git/worktrees/xx/config.worktree for linked ones).

This sounds quite useful for these situations.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 8d85d1a324..c24abf5871 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  ------------------
>
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> -is used to store the configuration for that repository, and
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) are each
> +repository is used to store the configuration for that repository, and

Typo: 'are each'.

>  ENVIRONMENT
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index e2ee9fc21b..3f9112db56 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -204,6 +204,43 @@ working trees, it can be used to identify worktrees. For example if
>  you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
>  then "ghi" or "def/ghi" is enough to point to the former working tree.
>
> +CONFIGURATION FILE
> +------------------
> +By default, the repository "config" file is shared across all working
> +directories. If the config variables `core.bare` or `core.worktree`
> +are already present in the config file, they will be applied to the
> +main working directory only.
> +
> +In order to have configuration specific to working directories, you
> +can turn on "worktreeConfig" extension, e.g.:
> +
> +------------
> +$ git config extensions.worktreeConfig true
> +------------

Good, this matches my expectation from above.

> @@ -24,6 +25,7 @@ static char key_delim = ' ';
>  static char term = '\n';
>
>  static int use_global_config, use_system_config, use_local_config;
> +static int use_worktree_config;
>  static struct git_config_source given_config_source;
>  static int actions, type;
>  static char *default_value;
> @@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
>  	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
>  	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
>  	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
> +	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
>  	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
>  	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
>  	OPT_GROUP(N_("Action")),
> @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  			     PARSE_OPT_STOP_AT_NON_OPTION);
>
>  	if (use_global_config + use_system_config + use_local_config +
> +	    use_worktree_config +
>  	    !!given_config_source.file + !!given_config_source.blob > 1) {

I feel like we're getting into "let's extract a function" territory,
here, since this line is growing in width. Perhaps:

  static int config_files_count()
  {
    return use_global_config + use_system_config + use_local_config +
  		use_worktree_config +
  		!!given_config_source.file +
  		!!given_config_source.blob;
  }

Simplifying the call to:

> diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
> new file mode 100755
> index 0000000000..4ebdf13cf9
> --- /dev/null
> +++ b/t/t2029-worktree-config.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description="config file in multi worktree"
> +
> +. ./test-lib.sh
> +
> +cmp_config() {
> +	if [ "$1" = "-C" ]; then
> +		shift &&
> +		GD="-C $1" &&
> +		shift
> +	else
> +		GD=
> +	fi &&
> +	echo "$1" >expected &&
> +	shift &&
> +	git $GD config "$@" >actual &&
> +	test_cmp expected actual
> +}

This cmp_config seems generally useful, perhaps beyond t2029. What do
you think about putting it in t/test-lib-functions.sh instead?

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-24 14:21 ` Taylor Blau
@ 2018-09-25 15:57   ` Duy Nguyen
  2018-09-29 13:53   ` Duy Nguyen
  1 sibling, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-25 15:57 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Git Mailing List

On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau <me@ttaylorr.com> wrote:
> > +cmp_config() {
> > +     if [ "$1" = "-C" ]; then
> > +             shift &&
> > +             GD="-C $1" &&
> > +             shift
> > +     else
> > +             GD=
> > +     fi &&
> > +     echo "$1" >expected &&
> > +     shift &&
> > +     git $GD config "$@" >actual &&
> > +     test_cmp expected actual
> > +}
>
> This cmp_config seems generally useful, perhaps beyond t2029. What do
> you think about putting it in t/test-lib-functions.sh instead?

Good point. t1300 (and I think some t13xx) does the same. Other tests also do

    test "$(git config...)" = blah

which can also use this function to have better error reporting when
things go wrong.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2018-09-23 20:47 ` Eric Sunshine
  2018-09-24 14:21 ` Taylor Blau
@ 2018-09-25 21:26 ` Junio C Hamano
  2018-09-26 15:48   ` Duy Nguyen
  2018-09-26 18:25 ` [PATCH] worktree: add per-worktree config files Ævar Arnfjörð Bjarmason
  2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  4 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2018-09-25 21:26 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This extension is most useful in multiple worktree setup because you
> now have an option to store per-worktree config (which is either
> .git/config.worktree for main worktree, or
> .git/worktrees/xx/config.worktree for linked ones).

Heh.  "This is useful if you have multiple" is true without saying,
because this is totally useless if you have only a single worktree.
I'd suggest writing the above without "most useful".

> This design places a bet on the assumption that the majority of config
> variables are shared so it is the default mode. A safer move would be
> default writes go to per-worktree file, so that accidental changes are
> isolated.

Warning: devil's advocate mode on.

Done in either way, this will confuse the users.  What is the reason
why people are led to think it is a good idea to use multiple
worktrees, even when they need different settings?  What do they
want out of "multiple worktrees linked to a single repository" as
opposed to just a simple "as many clones as necessary"?  Reduced
disk footprint?  Is there a better way to achieve that without the
downside of multiple worktrees (e.g. configuration need to be
uniform)?

> (*) "git config --worktree" points back to "config" file when this
>     extension is not present so that it works in any setup.

Shouldn't it barf and error out instead?  A user who hasn't enabled
the extension uses --worktree option and misled to believe that the
setting affects only a single worktree, even though the change is
made globally---that does not sound like a great end-user experience.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-25 21:26 ` Junio C Hamano
@ 2018-09-26 15:48   ` Duy Nguyen
  2018-09-26 17:40     ` Junio C Hamano
  2018-09-27 15:24     ` Wherefor worktrees? Marc Branchaud
  0 siblings, 2 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-26 15:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Tue, Sep 25, 2018 at 11:26 PM Junio C Hamano <gitster@pobox.com> wrote:
> Warning: devil's advocate mode on.

Oh good. I need to a good dose of sanity anyway.

> Done in either way, this will confuse the users.  What is the reason
> why people are led to think it is a good idea to use multiple
> worktrees, even when they need different settings?  What do they
> want out of "multiple worktrees linked to a single repository" as
> opposed to just a simple "as many clones as necessary"?  Reduced
> disk footprint?

I believe the main selling point of multiple worktrees is sharing
refs. You could easily avoid expensive clones with --local, but
synchronizing between different clones is not very convenient. Other
than that, different worktrees tend to behave like separate clones.

This leaves a gray area where other things should be shared or not. I
think the preference (or default mode) is still _not_ shared (*).
Sharing more things (besides refs and object database) is just a new
opportunity popping up when we implement multiple worktrees. Since
multiple worktrees (or clones before its time) are grouped together,
sometimes you would like to share common configuration. We could sort
of achieve this already with includeIf but again not as convenient.

(*) real life is not that simple. Since refs are shared, including
_remotes_ refs, so configuration related to remotes should also be
shared, or it will be a maintenance nightmare. Which is why
$GIT_DIR/config so far has been shared besides the two exceptions that
are core.bare and core.worktree. And I really like to get rid of these
exceptions.

> Is there a better way to achieve that without the
> downside of multiple worktrees (e.g. configuration need to be
> uniform)?

Is there a better way to achieve sharing refs between clones? I gave
it a minute but couldn't come up with a good answer :(

> > (*) "git config --worktree" points back to "config" file when this
> >     extension is not present so that it works in any setup.
>
> Shouldn't it barf and error out instead?

The intention is a uniform interface/api that works with both single
and multiple worktrees configurations. Otherwise in your scripts you
would need to write "if single worktree, do this, else do that". If
"git config --worktree" works with both, the existing scripts can be
audited and updated just a bit, adding "--worktree" where the config
should not be shared, and we're done.

> A user who hasn't enabled
> the extension uses --worktree option and misled to believe that the
> setting affects only a single worktree, even though the change is
> made globally---that does not sound like a great end-user experience.

I was talking about a single worktree. But I think here you meant the
user has multiple worktrees, but the extension is not enabled. I'm
probably not clear in the commit message, but "git config" can detect
that the extension has not been enabled, automatically enable it (and
move core.bare and core.worktree (if present) to the main worktree's
private config), so "git config --worktree" will never share the
change.

But perhaps the user should be made aware of this situation and asked
to explicitly enable the extension instead? It's certainly a more
conservative approach.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-26 15:48   ` Duy Nguyen
@ 2018-09-26 17:40     ` Junio C Hamano
  2018-09-27 15:24     ` Wherefor worktrees? Marc Branchaud
  1 sibling, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2018-09-26 17:40 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List

Duy Nguyen <pclouds@gmail.com> writes:

> I believe the main selling point of multiple worktrees is sharing
> refs. You could easily avoid expensive clones with --local, but
> synchronizing between different clones is not very convenient. Other
> than that, different worktrees tend to behave like separate clones.

OK.  Even with the enforced limitation that no single branch can be
checked out in multiple worktrees at the same time, it is more
convenient as you can "merge" other branch and trust that the result
on the checked-out branch in your worktree is immediately visible to
other worktrees.

> user has multiple worktrees, but the extension is not enabled. I'm

Exactly.  "config --worktree" in your script will silently break
other worktrees; it wanted to affect only the current worktree, but
it changed settings to all the others.

> probably not clear in the commit message, but "git config" can detect
> that the extension has not been enabled, automatically enable it (and
> move core.bare and core.worktree (if present) to the main worktree's
> private config), so "git config --worktree" will never share the
> change.

I wonder if that is good enough.  The user in one worktree did
"config --worktree" and all the worktrees now start diverging in
their config---but that is true _only_ when they use "--worktree"
option to say "I want this to be set differently from others".  All
the other calls to "git config" will stil be shared.

So from a cursory thinking, it sounds OK to me, but somebody else
may think of bad ramifications after a good night sleep.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2018-09-25 21:26 ` Junio C Hamano
@ 2018-09-26 18:25 ` Ævar Arnfjörð Bjarmason
  2018-09-27 17:24   ` Duy Nguyen
  2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  4 siblings, 1 reply; 34+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-09-26 18:25 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git


On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:

> +extensions.worktreeConfig::
> +	If set, by default "git config" reads from both "config" and
> +	"config.worktree" file in that order.

How does this interact with config that's now only used if it's in
.git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
will that be inherited across the two of these?

> In multiple working
> +	directory mode, "config" file is shared while
> +	"config.worktree" is per-working directory.

"But then how will it work with more than one?" I found myself thinking
before reading some more and remembering .git/worktree. Shouldn't we
consistently say:

    [...]"config" and "worktrees/<worktree name>/config"[...]

Or something like that?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Wherefor worktrees?
  2018-09-26 15:48   ` Duy Nguyen
  2018-09-26 17:40     ` Junio C Hamano
@ 2018-09-27 15:24     ` Marc Branchaud
  2018-09-27 16:36       ` Duy Nguyen
  1 sibling, 1 reply; 34+ messages in thread
From: Marc Branchaud @ 2018-09-27 15:24 UTC (permalink / raw)
  To: Duy Nguyen, Junio C Hamano; +Cc: Git Mailing List

On 2018-09-26 11:48 AM, Duy Nguyen wrote:
> 
> I believe the main selling point of multiple worktrees is sharing
> refs. You could easily avoid expensive clones with --local, but
> synchronizing between different clones is not very convenient. Other
> than that, different worktrees tend to behave like separate clones.

Sharing hooks is also useful, but yes mainly the refs.

I love being able to work in more than one branch at a time.  I often 
have a couple of ongoing big, messy topics, and being able to easily 
jump onto some release branch for a quick bugfix, without having to 
first stash things or finish an interactive rebase or fix a conflicting 
merge, is a godsend.

And the reason I use worktrees for this, instead of clones, is for the 
shared refs.  It makes sense to me that I'm working with different 
checkouts from a single repo, with all my local branches and local tags. 
  "git fetch" updates the remote refs regardless of which worktree I'm 
in when I run it.  The setup is lightweight and efficient; it's just how 
I want to work.

Having used git-new-workdir for a long time, it's main deficiency for me 
is submodules (the shared bisection state didn't bother me much).  It 
would be nice if all my worktrees' submodules also shared refs.  That's 
"nice", but not "essential".  Mainly it would be convenient if a 
recursive-submodule fetch performed in one worktree updated the 
submodule refs in my other worktrees.  Similarly, if I create a local 
branch in a submodule in one worktree, it would be nice to see that 
branch in the submodule in other worktrees.  Again, "nice", but probably 
just because I've lived with git-new-workdir's limitations for so long 
that I'm used to them.

That said, I really appreciate Duy's work here -- thanks!  Git deserves 
to have a cool feature like worktrees be part of its standard toolkit.

		M.


> This leaves a gray area where other things should be shared or not. I
> think the preference (or default mode) is still _not_ shared (*).
> Sharing more things (besides refs and object database) is just a new
> opportunity popping up when we implement multiple worktrees. Since
> multiple worktrees (or clones before its time) are grouped together,
> sometimes you would like to share common configuration. We could sort
> of achieve this already with includeIf but again not as convenient.
> 
> (*) real life is not that simple. Since refs are shared, including
> _remotes_ refs, so configuration related to remotes should also be
> shared, or it will be a maintenance nightmare. Which is why
> $GIT_DIR/config so far has been shared besides the two exceptions that
> are core.bare and core.worktree. And I really like to get rid of these
> exceptions.
> 
>> Is there a better way to achieve that without the
>> downside of multiple worktrees (e.g. configuration need to be
>> uniform)?
> 
> Is there a better way to achieve sharing refs between clones? I gave
> it a minute but couldn't come up with a good answer :(
> 
>>> (*) "git config --worktree" points back to "config" file when this
>>>      extension is not present so that it works in any setup.
>>
>> Shouldn't it barf and error out instead?
> 
> The intention is a uniform interface/api that works with both single
> and multiple worktrees configurations. Otherwise in your scripts you
> would need to write "if single worktree, do this, else do that". If
> "git config --worktree" works with both, the existing scripts can be
> audited and updated just a bit, adding "--worktree" where the config
> should not be shared, and we're done.
> 
>> A user who hasn't enabled
>> the extension uses --worktree option and misled to believe that the
>> setting affects only a single worktree, even though the change is
>> made globally---that does not sound like a great end-user experience.
> 
> I was talking about a single worktree. But I think here you meant the
> user has multiple worktrees, but the extension is not enabled. I'm
> probably not clear in the commit message, but "git config" can detect
> that the extension has not been enabled, automatically enable it (and
> move core.bare and core.worktree (if present) to the main worktree's
> private config), so "git config --worktree" will never share the
> change.
> 
> But perhaps the user should be made aware of this situation and asked
> to explicitly enable the extension instead? It's certainly a more
> conservative approach.
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Wherefor worktrees?
  2018-09-27 15:24     ` Wherefor worktrees? Marc Branchaud
@ 2018-09-27 16:36       ` Duy Nguyen
  0 siblings, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-27 16:36 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Junio C Hamano, Git Mailing List

On Thu, Sep 27, 2018 at 5:24 PM Marc Branchaud <marcnarc@xiplink.com> wrote:
>
> On 2018-09-26 11:48 AM, Duy Nguyen wrote:
> >
> > I believe the main selling point of multiple worktrees is sharing
> > refs. You could easily avoid expensive clones with --local, but
> > synchronizing between different clones is not very convenient. Other
> > than that, different worktrees tend to behave like separate clones.
>
> Sharing hooks is also useful

Well yes, but for hooks I think a better way is moving hook management
back to config files. I think we have a rough idea what to do, and
AEvar highlighted the mand roadblocks elsewhere. Hopefully it will
materialize someday.

> Having used git-new-workdir for a long time, it's main deficiency for me
> is submodules (the shared bisection state didn't bother me much).  It
> would be nice if all my worktrees' submodules also shared refs.  That's
> "nice", but not "essential".

Heh I've been thinking about this a bit too. I thought separate refs
was a requirement for submodules, but if you don't actively use
branches in submodules and go with detached HEAD, it might work.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-26 18:25 ` [PATCH] worktree: add per-worktree config files Ævar Arnfjörð Bjarmason
@ 2018-09-27 17:24   ` Duy Nguyen
  2018-09-27 18:34     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 34+ messages in thread
From: Duy Nguyen @ 2018-09-27 17:24 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List

On Wed, Sep 26, 2018 at 8:25 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:
>
> > +extensions.worktreeConfig::
> > +     If set, by default "git config" reads from both "config" and
> > +     "config.worktree" file in that order.
>
> How does this interact with config that's now only used if it's in
> .git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
> will that be inherited across the two of these?

Er... we can't? If I remember correctly we don't have any enforcement
on where what config vars must or must not go. The only exception is
core.bare and core.worktree which is only read from $GIT_DIR/config
because of the way they are involved in .git directory discovery. If I
put remote "foo" in ~/.gitconfig, "git remote" happily reports remote
"foo" to me.

To sum up, we always inherit config from higher levels, with
/etc/gitconfig being the highest and $GIT_DIR/config the lowest. It's
up to the user to share config between repos responsibly. This patch
only adds one more level, $GIT_DIR/config.worktree which is now the
lowest level.

> > In multiple working
> > +     directory mode, "config" file is shared while
> > +     "config.worktree" is per-working directory.
>
> "But then how will it work with more than one?" I found myself thinking
> before reading some more and remembering .git/worktree. Shouldn't we
> consistently say:
>
>     [...]"config" and "worktrees/<worktree name>/config"[...]
>
> Or something like that?

Point taken. Maybe I'm trying to hide implementation details too much.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-27 17:24   ` Duy Nguyen
@ 2018-09-27 18:34     ` Ævar Arnfjörð Bjarmason
  2018-09-27 18:49       ` Duy Nguyen
  2018-09-29  6:36       ` Duy Nguyen
  0 siblings, 2 replies; 34+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-09-27 18:34 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List


On Thu, Sep 27 2018, Duy Nguyen wrote:

> On Wed, Sep 26, 2018 at 8:25 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:
>>
>> > +extensions.worktreeConfig::
>> > +     If set, by default "git config" reads from both "config" and
>> > +     "config.worktree" file in that order.
>>
>> How does this interact with config that's now only used if it's in
>> .git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
>> will that be inherited across the two of these?
>
> Er... we can't? If I remember correctly we don't have any enforcement
> on where what config vars must or must not go. The only exception is
> core.bare and core.worktree which is only read from $GIT_DIR/config
> because of the way they are involved in .git directory discovery. If I
> put remote "foo" in ~/.gitconfig, "git remote" happily reports remote
> "foo" to me.
>
> To sum up, we always inherit config from higher levels, with
> /etc/gitconfig being the highest and $GIT_DIR/config the lowest. It's
> up to the user to share config between repos responsibly. This patch
> only adds one more level, $GIT_DIR/config.worktree which is now the
> lowest level.

I see I'm misremembering most of the details here. I thought that if I put:

    [remote "whatever]
    url = ...

Into my ~/.gitconfig that it wouldn't work, but it does, e.g. here in my
~/g/git:

    $ grep -A1 whatever .git/config
    $
    $ grep -A1 whatever ~/.gitconfig
    [remote "whatever"]
        url = git@github.com:test/git.git

But there's still some special casing for .git/config going on,
e.g. here:

    $ git config remote.origin.url
    git@github.com:git/git.git
    $ git config remote.whatever.url
    git@github.com:test/git.git
    $ git remote get-url origin
    git@github.com:git/git.git
    $ git remote get-url whatever
    fatal: No such remote 'whatever'

And:

    $ git remote set-url whatever git@github.com:test2/git.git
    fatal: No such remote 'whatever'

So there is some special casing of .git/config somewhere. I looked into
this ages ago, and don't remember where that's done.

I was wondering if these patches introduced any unwanted edge cases in
this regard, e.g. if you're using the per-worktree config, and you have
remotes in .git/config, does "git remote get/set-url" do the right
thing?

Then if we have remotes in .git/config, should we add new remotes to
.git/config or the per-worktree file? I'd lean towards .git/config,
since remotes are orthagonal to worktrees, and closely tied with refs
which are shared no matter what this extension says, but maybe there's a
good argument for doing it the other way around.



>> > In multiple working
>> > +     directory mode, "config" file is shared while
>> > +     "config.worktree" is per-working directory.
>>
>> "But then how will it work with more than one?" I found myself thinking
>> before reading some more and remembering .git/worktree. Shouldn't we
>> consistently say:
>>
>>     [...]"config" and "worktrees/<worktree name>/config"[...]
>>
>> Or something like that?
>
> Point taken. Maybe I'm trying to hide implementation details too much.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-27 18:34     ` Ævar Arnfjörð Bjarmason
@ 2018-09-27 18:49       ` Duy Nguyen
  2018-09-29  6:36       ` Duy Nguyen
  1 sibling, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-27 18:49 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List

On Thu, Sep 27, 2018 at 8:34 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> ...
>
> So there is some special casing of .git/config somewhere. I looked into
> this ages ago, and don't remember where that's done.

Thanks! At least know I have some clues to look into (and will do).
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-27 18:34     ` Ævar Arnfjörð Bjarmason
  2018-09-27 18:49       ` Duy Nguyen
@ 2018-09-29  6:36       ` Duy Nguyen
  1 sibling, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-29  6:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List

On Thu, Sep 27, 2018 at 8:34 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> I see I'm misremembering most of the details here. I thought that if I put:
>
>     [remote "whatever]
>     url = ...
>
> Into my ~/.gitconfig that it wouldn't work, but it does, e.g. here in my
> ~/g/git:
>
>     $ grep -A1 whatever .git/config
>     $
>     $ grep -A1 whatever ~/.gitconfig
>     [remote "whatever"]
>         url = git@github.com:test/git.git
>
> But there's still some special casing for .git/config going on,
> e.g. here:
>
>     $ git config remote.origin.url
>     git@github.com:git/git.git
>     $ git config remote.whatever.url
>     git@github.com:test/git.git
>     $ git remote get-url origin
>     git@github.com:git/git.git
>     $ git remote get-url whatever
>     fatal: No such remote 'whatever'
>
> And:
>
>     $ git remote set-url whatever git@github.com:test2/git.git
>     fatal: No such remote 'whatever'
>
> So there is some special casing of .git/config somewhere. I looked into
> this ages ago, and don't remember where that's done.

To conclude this thread. Yes some code does know about where the
config variable is from and it looks like only "git remote" and "git
upload-pack" takes advantage of it [1] [2].

I considered documentation improvement for the git-remote part, but I
don't see anywhere I can fit "some remote attributes can be shared
from ~/.gitconfig, but a remote can only be added from repo-level
config" in. Jeff already documented it well. I found one minor problem
there in the documentation, which I will send separately.

Thanks again for making me aware of this.

[1] e459b073fb (remote rename: more carefully determine whether a
remote is configured - 2017-01-19)
[2] 20b20a22f8 (upload-pack: provide a hook for running pack-objects -
2016-05-18)
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] worktree: add per-worktree config files
  2018-09-24 14:21 ` Taylor Blau
  2018-09-25 15:57   ` Duy Nguyen
@ 2018-09-29 13:53   ` Duy Nguyen
  1 sibling, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-29 13:53 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Git Mailing List

On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau <me@ttaylorr.com> wrote:
> > @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> >                            PARSE_OPT_STOP_AT_NON_OPTION);
> >
> >       if (use_global_config + use_system_config + use_local_config +
> > +         use_worktree_config +
> >           !!given_config_source.file + !!given_config_source.blob > 1) {
>
> I feel like we're getting into "let's extract a function" territory,
> here, since this line is growing in width. Perhaps:
>
>   static int config_files_count()
>   {
>     return use_global_config + use_system_config + use_local_config +
>                 use_worktree_config +
>                 !!given_config_source.file +
>                 !!given_config_source.blob;
>   }
>
> Simplifying the call to:

I think there's a bigger problem here because we have no good way to
describe that a bunch of options are mutually exclusive. We get by ok
if the number of options is two, but when it gets bigger (and this is
not the only place), it gets messier. Besides the long "if" condition,
I consider the error message "only one config file at a time"
inadequate. We should tell the user what exact options are
conflicting.

So, I'm going to bury my head in the sand this time and ignore the
problem, including adding config_files_count(). It could be a
interesting mini project if someone wants to jump in. Meanwhile it'll
go to the very bottom of my long long backlog.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 0/2] Per-worktree config files
  2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2018-09-26 18:25 ` [PATCH] worktree: add per-worktree config files Ævar Arnfjörð Bjarmason
@ 2018-09-29 15:30 ` Nguyễn Thái Ngọc Duy
  2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
                     ` (2 more replies)
  4 siblings, 3 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-09-29 15:30 UTC (permalink / raw)
  To: pclouds; +Cc: git, Taylor Blau, Junio C Hamano, Ævar Arnfjörð

Besides various doc/test changes. v2 has two code changes

- "git config --worktree" on a multiple worktree setup _without_ the
  new extension will now barf. The user is supposed to RTFM and enable
  the extension manually.

- I failed to update setup code to pick up config from per-worktree
  files. Granted, core.bare and core.worktree don't have much use in
  this context (except perhaps submodules). But let's make sure we
  have proper code path to handle it.

config.worktree should have a new scope, CONFIG_SCOPE_WORKTREE, on top
of CONFIG_SCOPE_REPO. But we would need to go over scope-sensitive
code (upload-pack and remote) to update if necessary. I don't think
it's worth getting into that just yet.

Range diff

-:  ---------- > 1:  8f334e0020 t1300: extract and use test_cmp_config()
1:  12a999e517 ! 2:  99337a52f2 worktree: add per-worktree config files
    @@ -9,7 +9,7 @@
            worktree setup.
     
          - The special treatment for core.bare and core.worktree, to stay
    -       effective only in main worktree, is gone. These config files are
    +       effective only in main worktree, is gone. These config settings are
            supposed to be in config.worktree.
     
         This extension is most useful in multiple worktree setup because you
    @@ -38,7 +38,8 @@
         isolated.
     
         (*) "git config --worktree" points back to "config" file when this
    -        extension is not present so that it works in any setup.
    +        extension is not present and there is only one worktree so that it
    +        works in any both single and multiple worktree setups.
     
      diff --git a/Documentation/config.txt b/Documentation/config.txt
      --- a/Documentation/config.txt
    @@ -50,7 +51,7 @@
     -the Git commands' behavior. The `.git/config` file in each repository
     -is used to store the configuration for that repository, and
     +the Git commands' behavior. The files `.git/config` and optionally
    -+`config.worktree` (see `extensions.worktreeConfig` below) are each
    ++`config.worktree` (see `extensions.worktreeConfig` below) 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`
    @@ -61,9 +62,10 @@
      
     +extensions.worktreeConfig::
     +	If set, by default "git config" reads from both "config" and
    -+	"config.worktree" file in that order. In multiple working
    -+	directory mode, "config" file is shared while
    -+	"config.worktree" is per-working directory.
    ++	"config.worktree" file from GIT_DIR in that order. In
    ++	multiple working directory mode, "config" file is shared while
    ++	"config.worktree" is per-working directory (i.e., it's in
    ++	GIT_COMMON_DIR/worktrees/<id>/config.worktree)
     +
      core.fileMode::
      	Tells Git if the executable bit of files in the working tree
    @@ -140,12 +142,12 @@
     +CONFIGURATION FILE
     +------------------
     +By default, the repository "config" file is shared across all working
    -+directories. If the config variables `core.bare` or `core.worktree`
    -+are already present in the config file, they will be applied to the
    -+main working directory only.
    ++trees. If the config variables `core.bare` or `core.worktree` are
    ++already present in the config file, they will be applied to the main
    ++working trees only.
     +
    -+In order to have configuration specific to working directories, you
    -+can turn on "worktreeConfig" extension, e.g.:
    ++In order to have configuration specific to working trees, you can turn
    ++on "worktreeConfig" extension, e.g.:
     +
     +------------
     +$ git config extensions.worktreeConfig true
    @@ -153,26 +155,19 @@
     +
     +In this mode, specific configuration stays in the path pointed by `git
     +rev-parse --git-path config.worktree`. You can add or update
    -+configuration in this file with `git config --worktree`. Git before
    -+version 2.20.0 will refuse to access repositories with this extension.
    ++configuration in this file with `git config --worktree`. Older Git
    ++versions may will refuse to access repositories with this extension.
     +
    -+Note that in this file, the exception for `core.bare` and
    -+`core.worktree` is gone. If you have them before, you need to move
    -+them to the `config.worktree` of the main working directory. You may
    -+also take this opportunity to move other configuration that you do not
    -+want to share to all working directories:
    ++Note that in this file, the exception for `core.bare` and `core.worktree`
    ++is gone. If you have them in $GIT_DIR/config before, you must move
    ++them to the `config.worktree` of the main working tree. You may also
    ++take this opportunity to review and move other configuration that you
    ++do not want to share to all working trees:
     +
     + - `core.worktree` and `core.bare` should never be shared
     +
    -+ - `core.sparseCheckout` is recommended per working directory, unless
    -+   you are sure you always use sparse checkout for all working
    -+   directories.
    -+
    -+When `git config --worktree` is used to set a configuration variable
    -+in multiple working directory setup, `extensions.worktreeConfig` will
    -+be automatically set. The two variables `core.worktree` and
    -+`core.bare` if present will be moved to `config.worktree` of the main
    -+working tree.
    ++ - `core.sparseCheckout` is recommended per working tree, unless you
    ++   are sure you always use sparse checkout for all working trees.
     +
      DETAILS
      -------
    @@ -248,10 +243,13 @@
     +		struct worktree **worktrees = get_worktrees(0);
     +		if (repository_format_worktree_config)
     +			given_config_source.file = git_pathdup("config.worktree");
    -+		else if (worktrees[0] && worktrees[1]) {
    -+			migrate_worktree_config(the_repository);
    -+			given_config_source.file = git_pathdup("config.worktree");
    -+		} else
    ++		else if (worktrees[0] && worktrees[1])
    ++			die(_("--worktree cannot be used with multiple "
    ++			      "working trees unless the config\n"
    ++			      "extension worktreeConfig is enabled. "
    ++			      "Please read \"CONFIGURATION FILE\"\n"
    ++			      "section in \"git help worktree\" for details"));
    ++		else
     +			given_config_source.file = git_pathdup("config");
     +		free_worktrees(worktrees);
     +	} else if (given_config_source.file) {
    @@ -284,6 +282,10 @@
      	if (repo_config && !access_or_die(repo_config, R_OK, 0))
      		ret += git_config_from_file(fn, repo_config, data);
      
    ++	/*
    ++	 * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
    ++	 * But let's not complicate things before it's actually needed.
    ++	 */
     +	if (repository_format_worktree_config) {
     +		char *path = git_pathdup("config.worktree");
     +		if (!access_or_die(path, R_OK, 0))
    @@ -310,6 +312,27 @@
      diff --git a/setup.c b/setup.c
      --- a/setup.c
      +++ b/setup.c
    +@@
    + 	initialized = 1;
    + }
    + 
    ++static int read_worktree_config(const char *var, const char *value, void *vdata)
    ++{
    ++	struct repository_format *data = vdata;
    ++
    ++	if (strcmp(var, "core.bare") == 0) {
    ++		data->is_bare = git_config_bool(var, value);
    ++	} else if (strcmp(var, "core.worktree") == 0) {
    ++		if (!value)
    ++			return config_error_nonbool(var);
    ++		data->work_tree = xstrdup(value);
    ++	}
    ++	return 0;
    ++}
    ++
    + static int check_repo_format(const char *var, const char *value, void *vdata)
    + {
    + 	struct repository_format *data = vdata;
     @@
      			if (!value)
      				return config_error_nonbool(var);
    @@ -319,16 +342,40 @@
     +			data->worktree_config = git_config_bool(var, value);
     +		else
      			string_list_append(&data->unknown_extensions, ext);
    - 	} else if (strcmp(var, "core.bare") == 0) {
    - 		data->is_bare = git_config_bool(var, value);
    +-	} else if (strcmp(var, "core.bare") == 0) {
    +-		data->is_bare = git_config_bool(var, value);
    +-	} else if (strcmp(var, "core.worktree") == 0) {
    +-		if (!value)
    +-			return config_error_nonbool(var);
    +-		data->work_tree = xstrdup(value);
    + 	}
    +-	return 0;
    ++
    ++	return read_worktree_config(var, value, vdata);
    + }
    + 
    + static int check_repository_format_gently(const char *gitdir, struct repository_format *candidate, int *nongit_ok)
     @@
      
      	repository_format_precious_objects = candidate->precious_objects;
      	repository_format_partial_clone = candidate->partial_clone;
     +	repository_format_worktree_config = candidate->worktree_config;
      	string_list_clear(&candidate->unknown_extensions, 0);
    ++
    ++	if (repository_format_worktree_config) {
    ++		/*
    ++		 * pick up core.bare and core.worktree from per-worktree
    ++		 * config if present
    ++		 */
    ++		strbuf_addf(&sb, "%s/config.worktree", gitdir);
    ++		git_config_from_file(read_worktree_config, sb.buf, candidate);
    ++		strbuf_release(&sb);
    ++		has_common = 0;
    ++	}
    ++
      	if (!has_common) {
      		if (candidate->is_bare != -1) {
    + 			is_bare_repository_cfg = candidate->is_bare;
     
      diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
      new file mode 100755
    @@ -341,150 +388,76 @@
     +
     +. ./test-lib.sh
     +
    -+cmp_config() {
    -+	if [ "$1" = "-C" ]; then
    -+		shift &&
    -+		GD="-C $1" &&
    -+		shift
    -+	else
    -+		GD=
    -+	fi &&
    -+	echo "$1" >expected &&
    -+	shift &&
    -+	git $GD config "$@" >actual &&
    -+	test_cmp expected actual
    -+}
    -+
     +test_expect_success 'setup' '
    -+	test_commit start &&
    -+	git config --worktree per.worktree is-ok &&
    ++	test_commit start
    ++'
    ++
    ++test_expect_success 'config --worktree in single worktree' '
    ++	git config --worktree foo.bar true &&
    ++	test_cmp_config true foo.bar
    ++'
    ++
    ++test_expect_success 'add worktrees' '
     +	git worktree add wt1 &&
    -+	git worktree add wt2 &&
    -+	git config --worktree per.worktree is-ok &&
    -+	cmp_config true extensions.worktreeConfig
    ++	git worktree add wt2
    ++'
    ++
    ++test_expect_success 'config --worktree without extension' '
    ++	test_must_fail git config --worktree foo.bar false
    ++'
    ++
    ++test_expect_success 'enable worktreeConfig extension' '
    ++	git config extensions.worktreeConfig true &&
    ++	test_cmp_config true extensions.worktreeConfig
     +'
     +
     +test_expect_success 'config is shared as before' '
     +	git config this.is shared &&
    -+	cmp_config shared this.is &&
    -+	cmp_config -C wt1 shared this.is &&
    -+	cmp_config -C wt2 shared this.is
    ++	test_cmp_config shared this.is &&
    ++	test_cmp_config -C wt1 shared this.is &&
    ++	test_cmp_config -C wt2 shared this.is
     +'
     +
     +test_expect_success 'config is shared (set from another worktree)' '
     +	git -C wt1 config that.is also-shared &&
    -+	cmp_config also-shared that.is &&
    -+	cmp_config -C wt1 also-shared that.is &&
    -+	cmp_config -C wt2 also-shared that.is
    ++	test_cmp_config also-shared that.is &&
    ++	test_cmp_config -C wt1 also-shared that.is &&
    ++	test_cmp_config -C wt2 also-shared that.is
     +'
     +
     +test_expect_success 'config private to main worktree' '
     +	git config --worktree this.is for-main &&
    -+	cmp_config for-main this.is &&
    -+	cmp_config -C wt1 shared this.is &&
    -+	cmp_config -C wt2 shared this.is
    ++	test_cmp_config for-main this.is &&
    ++	test_cmp_config -C wt1 shared this.is &&
    ++	test_cmp_config -C wt2 shared this.is
     +'
     +
     +test_expect_success 'config private to linked worktree' '
     +	git -C wt1 config --worktree this.is for-wt1 &&
    -+	cmp_config for-main this.is &&
    -+	cmp_config -C wt1 for-wt1 this.is &&
    -+	cmp_config -C wt2 shared this.is
    ++	test_cmp_config for-main this.is &&
    ++	test_cmp_config -C wt1 for-wt1 this.is &&
    ++	test_cmp_config -C wt2 shared this.is
     +'
     +
     +test_expect_success 'core.bare no longer for main only' '
    -+	git config core.bare true &&
    -+	cmp_config true core.bare &&
    -+	cmp_config -C wt1 true core.bare &&
    -+	cmp_config -C wt2 true core.bare &&
    -+	git config --unset core.bare
    ++	test_config core.bare true &&
    ++	test "$(git rev-parse --is-bare-repository)" = true &&
    ++	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
    ++	test "$(git -C wt2 rev-parse --is-bare-repository)" = true
     +'
     +
    -+test_expect_success 'config.worktree no longer read without extension' '
    -+	git config --unset extensions.worktreeConfig &&
    -+	cmp_config shared this.is &&
    -+	cmp_config -C wt1 shared this.is &&
    -+	cmp_config -C wt2 shared this.is
    ++test_expect_success 'per-worktree core.bare is picked up' '
    ++	git -C wt1 config --worktree core.bare true &&
    ++	test "$(git rev-parse --is-bare-repository)" = false &&
    ++	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
    ++	test "$(git -C wt2 rev-parse --is-bare-repository)" = false
     +'
     +
    -+test_expect_success 'config --worktree migrate core.bare and core.worktree' '
    -+	git config core.bare true &&
    -+	git config --worktree foo.bar true &&
    -+	cmp_config true extensions.worktreeConfig &&
    -+	cmp_config true foo.bar &&
    -+	cmp_config true core.bare &&
    -+	! git -C wt1 config core.bare
    ++test_expect_success 'config.worktree no longer read without extension' '
    ++	git config --unset extensions.worktreeConfig &&
    ++	test_cmp_config shared this.is &&
    ++	test_cmp_config -C wt1 shared this.is &&
    ++	test_cmp_config -C wt2 shared this.is
     +'
     +
     +test_done
    -
    - diff --git a/worktree.c b/worktree.c
    - --- a/worktree.c
    - +++ b/worktree.c
    -@@
    - #include "worktree.h"
    - #include "dir.h"
    - #include "wt-status.h"
    -+#include "config.h"
    - 
    - void free_worktrees(struct worktree **worktrees)
    - {
    -@@
    - 	free_worktrees(worktrees);
    - 	return ret;
    - }
    -+
    -+void migrate_worktree_config(struct repository *r)
    -+{
    -+	struct strbuf worktree_path = STRBUF_INIT;
    -+	struct strbuf main_path = STRBUF_INIT;
    -+	struct repository_format format;
    -+
    -+	assert(repository_format_worktree_config == 0);
    -+
    -+	strbuf_git_common_path(&worktree_path, r, "config.worktree");
    -+	strbuf_git_path(&main_path, "config");
    -+
    -+	read_repository_format(&format, main_path.buf);
    -+	assert(format.worktree_config == 0);
    -+
    -+	if (format.is_bare >= 0) {
    -+		git_config_set_in_file(worktree_path.buf,
    -+				       "core.bare", "true");
    -+		git_config_set_in_file(main_path.buf,
    -+				       "core.bare", NULL);
    -+	}
    -+	if (format.work_tree) {
    -+		git_config_set_in_file(worktree_path.buf,
    -+				       "core.worktree",
    -+				       format.work_tree);
    -+		git_config_set_in_file(main_path.buf,
    -+				       "core.worktree", NULL);
    -+	}
    -+
    -+	git_config_set_in_file(main_path.buf,
    -+			       "extensions.worktreeConfig", "true");
    -+	if (format.version == 0)
    -+		git_config_set_in_file(main_path.buf,
    -+				       "core.repositoryFormatVersion", "1");
    -+
    -+	repository_format_worktree_config = 1;
    -+
    -+	strbuf_release(&main_path);
    -+	strbuf_release(&worktree_path);
    -+}
    -
    - diff --git a/worktree.h b/worktree.h
    - --- a/worktree.h
    - +++ b/worktree.h
    -@@
    - 				     const char *fmt, ...)
    - 	__attribute__((format (printf, 2, 3)));
    - 
    -+/*
    -+ * Called to add extensions.worktreeConfig to $GIT_DIR/config and move
    -+ * main worktree specific config variables to $GIT_DIR/config.worktree.
    -+ */
    -+void migrate_worktree_config(struct repository *r);
    -+
    - #endif


Nguyễn Thái Ngọc Duy (2):
  t1300: extract and use test_cmp_config()
  worktree: add per-worktree config files

 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 30 ++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t1300-config.sh                      | 79 +++++++-------------------
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 t/test-lib-functions.sh                | 24 ++++++++
 12 files changed, 253 insertions(+), 78 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

-- 
2.19.0.341.g3acb95d729


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v2 1/2] t1300: extract and use test_cmp_config()
  2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
@ 2018-09-29 15:30   ` Nguyễn Thái Ngọc Duy
  2018-09-30  4:05     ` Eric Sunshine
  2018-09-30 12:31     ` SZEDER Gábor
  2018-09-29 15:30   ` [PATCH v2 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2 siblings, 2 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-09-29 15:30 UTC (permalink / raw)
  To: pclouds; +Cc: git, Taylor Blau, Junio C Hamano, Ævar Arnfjörð

In many config-related tests it's common to check if a config variable
has expected value and we want to print the differences when the test
fails. Doing it the normal way is three lines of shell code. Let's add
a function do to all this (and a little more).

This function has uses outside t1300 as well but I'm not going to
convert them all. And it will be used in the next commit where
per-worktree config feature is introduced.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t1300-config.sh       | 79 ++++++++++-------------------------------
 t/test-lib-functions.sh | 24 +++++++++++++
 2 files changed, 43 insertions(+), 60 deletions(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index cdf1fed5d1..00c2b0f0eb 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -76,15 +76,11 @@ EOF
 test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	echo Second >expect &&
-	git config cores.whatever >actual &&
-	test_cmp expect actual
+	test_cmp_config Second cores.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	echo Second >expect &&
-	git config CoReS.WhAtEvEr >actual &&
-	test_cmp expect actual
+	test_cmp_config Second CoReS.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '
@@ -94,12 +90,8 @@ test_expect_success 'subsections are not canonicalized by git-config' '
 	[section "SubSection"]
 	key = two
 	EOF
-	echo one >expect &&
-	git config section.subsection.key >actual &&
-	test_cmp expect actual &&
-	echo two >expect &&
-	git config section.SubSection.key >actual &&
-	test_cmp expect actual
+	test_cmp_config one section.subsection.key &&
+	test_cmp_config two section.SubSection.key
 '
 
 cat > .git/config <<\EOF
@@ -212,9 +204,7 @@ test_expect_success 'really really mean test' '
 '
 
 test_expect_success 'get value' '
-	echo alpha >expect &&
-	git config beta.haha >actual &&
-	test_cmp expect actual
+	test_cmp_config alpha beta.haha
 '
 
 cat > expect << EOF
@@ -251,15 +241,11 @@ test_expect_success 'non-match' '
 '
 
 test_expect_success 'non-match value' '
-	echo wow >expect &&
-	git config --get nextsection.nonewline !for >actual &&
-	test_cmp expect actual
+	test_cmp_config wow --get nextsection.nonewline !for
 '
 
 test_expect_success 'multi-valued get returns final one' '
-	echo "wow2 for me" >expect &&
-	git config --get nextsection.nonewline >actual &&
-	test_cmp expect actual
+	test_cmp_config "wow2 for me" --get nextsection.nonewline
 '
 
 test_expect_success 'multi-valued get-all returns all' '
@@ -520,21 +506,11 @@ test_expect_success 'editing stdin is an error' '
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
-	(
-		cd x &&
-		echo strasse >expect &&
-		git config --get --file ../other-config ein.bahn >actual &&
-		test_cmp expect actual
-	)
-
+	test_cmp_config -C x strasse --get --file ../other-config ein.bahn
 '
 
 test_expect_success 'refer config from subdirectory via --file' '
-	(
-		cd x &&
-		git config --file=../other-config --get ein.bahn >actual &&
-		test_cmp expect actual
-	)
+	test_cmp_config -C x strasse --file=../other-config --get ein.bahn
 '
 
 cat > expect << EOF
@@ -688,16 +664,13 @@ test_expect_success numbers '
 
 test_expect_success '--int is at least 64 bits' '
 	git config giga.watts 121g &&
-	echo 129922760704 >expect &&
-	git config --int --get giga.watts >actual &&
-	test_cmp expect actual
+	echo  >expect &&
+	test_cmp_config 129922760704 --int --get giga.watts
 '
 
 test_expect_success 'invalid unit' '
 	git config aninvalid.unit "1auto" &&
-	echo 1auto >expect &&
-	git config aninvalid.unit >actual &&
-	test_cmp expect actual &&
+	test_cmp_config 1auto aninvalid.unit &&
 	test_must_fail git config --int --get aninvalid.unit 2>actual &&
 	test_i18ngrep "bad numeric config value .1auto. for .aninvalid.unit. in file .git/config: invalid unit" actual
 '
@@ -1039,9 +1012,7 @@ test_expect_success '--null --get-regexp' '
 
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
-	echo "foo 	  bar" >expect &&
-	git config section.val >actual &&
-	test_cmp expect actual
+	test_cmp_config "foo 	  bar" section.val
 '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
@@ -1808,21 +1779,15 @@ big = 1M
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
-	git config --type=int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=int --type=int core.big
 '
 
 test_expect_success 'identical legacy --type specifiers are allowed' '
-	git config --int --int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --int core.big
 '
 
 test_expect_success 'identical mixed --type specifiers are allowed' '
-	git config --int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --type=int core.big
 '
 
 test_expect_success 'non-identical modern --type specifiers are not allowed' '
@@ -1841,21 +1806,15 @@ test_expect_success 'non-identical mixed --type specifiers are not allowed' '
 '
 
 test_expect_success '--type allows valid type specifiers' '
-	echo "true" >expect &&
-	git config --type=bool core.foo >actual &&
-	test_cmp expect actual
+	test_cmp_config true  --type=bool core.foo
 '
 
 test_expect_success '--no-type unsets type specifiers' '
-	echo "10" >expect &&
-	git config --type=bool --no-type core.number >actual &&
-	test_cmp expect actual
+	test_cmp_config 10 --type=bool --no-type core.number
 '
 
 test_expect_success 'unset type specifiers may be reset to conflicting ones' '
-	echo 1048576 >expect &&
-	git config --type=bool --no-type --type=int core.big >actual &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=bool --no-type --type=int core.big
 '
 
 test_expect_success '--type rejects unknown specifiers' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index d82fac9d79..4cd7fb8fdf 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -747,6 +747,30 @@ test_cmp() {
 	$GIT_TEST_CMP "$@"
 }
 
+# similar to test_cmp but $2 is a config key instead of actual value
+# it can also accept -C to read from a different repo, e.g.
+#
+#     test_cmp_config -C xyz foo core.bar
+#
+# is sort of equivalent of
+#
+#     test "foo" = "$(git -C xyz core.bar)"
+
+test_cmp_config() {
+	if [ "$1" = "-C" ]
+	then
+		shift &&
+		GD="-C $1" &&
+		shift
+	else
+		GD=
+	fi &&
+	echo "$1" >expected &&
+	shift &&
+	git $GD config "$@" >actual &&
+	test_cmp expected actual
+}
+
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin() {
-- 
2.19.0.341.g3acb95d729


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v2 2/2] worktree: add per-worktree config files
  2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
@ 2018-09-29 15:30   ` Nguyễn Thái Ngọc Duy
  2018-09-30  4:32     ` Eric Sunshine
  2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2 siblings, 1 reply; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-09-29 15:30 UTC (permalink / raw)
  To: pclouds; +Cc: git, Taylor Blau, Junio C Hamano, Ævar Arnfjörð

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config settings are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present and there is only one worktree so that it
    works in any both single and multiple worktree setups.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 30 ++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 10 files changed, 210 insertions(+), 18 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d85d1a324..44407e69db 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) 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.
@@ -371,6 +372,13 @@ advice.*::
 		editor input from the user.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file from GIT_DIR in that order. In
+	multiple working directory mode, "config" file is shared while
+	"config.worktree" is per-working directory (i.e., it's in
+	GIT_COMMON_DIR/worktrees/<id>/config.worktree)
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 8e240435be..4870e00b89 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -45,13 +45,15 @@ unset an existing `--type` specifier with `--no-type`.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -131,6 +133,11 @@ from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -281,6 +288,10 @@ $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
@@ -299,9 +310,10 @@ configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..aa88278dde 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+trees. If the config variables `core.bare` or `core.worktree` are
+already present in the config file, they will be applied to the main
+working trees only.
+
+In order to have configuration specific to working trees, you can turn
+on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Older Git
+versions may will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and `core.worktree`
+is gone. If you have them in $GIT_DIR/config before, you must move
+them to the `config.worktree` of the main working tree. You may also
+take this opportunity to review and move other configuration that you
+do not want to share to all working trees:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working tree, unless you
+   are sure you always use sparse checkout for all working trees.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..36fcca8087 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -275,6 +280,9 @@ worktrees/<id>/locked::
 	or manually by `git worktree prune`. The file may contain a string
 	explaining why the repository is locked.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/builtin/config.c b/builtin/config.c
index 97b58c4aea..84385ef165 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -24,6 +25,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, type;
 static char *default_value;
@@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error(_("only one config file at a time"));
 		usage_builtin_config();
@@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1])
+			die(_("--worktree cannot be used with multiple "
+			      "working trees unless the config\n"
+			      "extension worktreeConfig is enabled. "
+			      "Please read \"CONFIGURATION FILE\"\n"
+			      "section in \"git help worktree\" for details"));
+		else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				prefix_filename(prefix, given_config_source.file);
diff --git a/cache.h b/cache.h
index d508f3d4f8..9e9b917e99 100644
--- a/cache.h
+++ b/cache.h
@@ -957,11 +957,13 @@ extern int grafts_replace_parents;
 extern int repository_format_precious_objects;
 extern char *repository_format_partial_clone;
 extern const char *core_partial_clone_filter_default;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
 	char *partial_clone; /* value of extensions.partialclone */
+	int worktree_config;
 	int is_bare;
 	int hash_algo;
 	char *work_tree;
diff --git a/config.c b/config.c
index 3461993f0a..b3025164d2 100644
--- a/config.c
+++ b/config.c
@@ -1695,6 +1695,17 @@ static int do_git_config_sequence(const struct config_options *opts,
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	/*
+	 * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
+	 * But let's not complicate things before it's actually needed.
+	 */
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 3f3c8746c2..268310b3dc 100644
--- a/environment.c
+++ b/environment.c
@@ -33,6 +33,7 @@ int ref_paranoia = -1;
 int repository_format_precious_objects;
 char *repository_format_partial_clone;
 const char *core_partial_clone_filter_default;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index b24c811c1c..1be5037f12 100644
--- a/setup.c
+++ b/setup.c
@@ -402,6 +402,20 @@ void setup_work_tree(void)
 	initialized = 1;
 }
 
+static int read_worktree_config(const char *var, const char *value, void *vdata)
+{
+	struct repository_format *data = vdata;
+
+	if (strcmp(var, "core.bare") == 0) {
+		data->is_bare = git_config_bool(var, value);
+	} else if (strcmp(var, "core.worktree") == 0) {
+		if (!value)
+			return config_error_nonbool(var);
+		data->work_tree = xstrdup(value);
+	}
+	return 0;
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
 	struct repository_format *data = vdata;
@@ -423,16 +437,13 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else
+		} else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
+		else
 			string_list_append(&data->unknown_extensions, ext);
-	} else if (strcmp(var, "core.bare") == 0) {
-		data->is_bare = git_config_bool(var, value);
-	} else if (strcmp(var, "core.worktree") == 0) {
-		if (!value)
-			return config_error_nonbool(var);
-		data->work_tree = xstrdup(value);
 	}
-	return 0;
+
+	return read_worktree_config(var, value, vdata);
 }
 
 static int check_repository_format_gently(const char *gitdir, struct repository_format *candidate, int *nongit_ok)
@@ -466,7 +477,20 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 
 	repository_format_precious_objects = candidate->precious_objects;
 	repository_format_partial_clone = candidate->partial_clone;
+	repository_format_worktree_config = candidate->worktree_config;
 	string_list_clear(&candidate->unknown_extensions, 0);
+
+	if (repository_format_worktree_config) {
+		/*
+		 * pick up core.bare and core.worktree from per-worktree
+		 * config if present
+		 */
+		strbuf_addf(&sb, "%s/config.worktree", gitdir);
+		git_config_from_file(read_worktree_config, sb.buf, candidate);
+		strbuf_release(&sb);
+		has_common = 0;
+	}
+
 	if (!has_common) {
 		if (candidate->is_bare != -1) {
 			is_bare_repository_cfg = candidate->is_bare;
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000000..286121d8de
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit start
+'
+
+test_expect_success 'config --worktree in single worktree' '
+	git config --worktree foo.bar true &&
+	test_cmp_config true foo.bar
+'
+
+test_expect_success 'add worktrees' '
+	git worktree add wt1 &&
+	git worktree add wt2
+'
+
+test_expect_success 'config --worktree without extension' '
+	test_must_fail git config --worktree foo.bar false
+'
+
+test_expect_success 'enable worktreeConfig extension' '
+	git config extensions.worktreeConfig true &&
+	test_cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	test_cmp_config also-shared that.is &&
+	test_cmp_config -C wt1 also-shared that.is &&
+	test_cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 for-wt1 this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	test_config core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = true
+'
+
+test_expect_success 'per-worktree core.bare is picked up' '
+	git -C wt1 config --worktree core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = false &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = false
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_done
-- 
2.19.0.341.g3acb95d729


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 1/2] t1300: extract and use test_cmp_config()
  2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
@ 2018-09-30  4:05     ` Eric Sunshine
  2018-09-30 12:31     ` SZEDER Gábor
  1 sibling, 0 replies; 34+ messages in thread
From: Eric Sunshine @ 2018-09-30  4:05 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Git List, Taylor Blau, Junio C Hamano,
	Ævar Arnfjörð Bjarmason

On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
> +# similar to test_cmp but $2 is a config key instead of actual value
> +# it can also accept -C to read from a different repo, e.g.

Minor: maybe say that "-C <dir>" changes to <dir> for the git-config invocation.

> +#     test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of
> +#
> +#     test "foo" = "$(git -C xyz core.bar)"

Should be: $(git -C xyz config core.bar)

> +test_cmp_config() {
> +       if [ "$1" = "-C" ]
> +       then

Style:

    if test "$1" = "-C"
    then
        ...

> +               shift &&
> +               GD="-C $1" &&
> +               shift
> +       else
> +               GD=
> +       fi &&
> +       echo "$1" >expected &&

If $1 starts with a hyphen, 'echo' might try interpreting it as an
option. Use printf instead:

    printf "%s\n" "$1" &&

> +       shift &&
> +       git $GD config "$@" >actual &&
> +       test_cmp expected actual

Please choose names other than "actual" and "expected" since those are
likely to clobber files of the same name which the test has set up
itself. (Or, at the very least, document that this function clobbers
those files -- but using better filenames is preferable.)

> +}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 2/2] worktree: add per-worktree config files
  2018-09-29 15:30   ` [PATCH v2 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2018-09-30  4:32     ` Eric Sunshine
  2018-09-30  7:15       ` Duy Nguyen
  0 siblings, 1 reply; 34+ messages in thread
From: Eric Sunshine @ 2018-09-30  4:32 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Git List, Taylor Blau, Junio C Hamano,
	Ævar Arnfjörð Bjarmason

On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> A new repo extension is added, worktreeConfig. When it is present:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  The Git configuration file contains a number of variables that affect
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) in each
> +repository is used to store the configuration for that repository, and

s/is used/are/used/

>  `$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.
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For example if
> +CONFIGURATION FILE
> +------------------
> +In this mode, specific configuration stays in the path pointed by `git
> +rev-parse --git-path config.worktree`. You can add or update
> +configuration in this file with `git config --worktree`. Older Git
> +versions may will refuse to access repositories with this extension.

s/may will/will/

> diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
> @@ -275,6 +280,9 @@ worktrees/<id>/locked::
> +worktrees/<id>/config.worktree::
> +       Working directory specific configuration file.

I wonder if this deserves a quick mention in
Documentation/git-worktree.txt since other worktree-related files,
such as "worktrees/<id>/locked", are mentioned there.

> diff --git a/builtin/config.c b/builtin/config.c
> @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> +       else if (use_worktree_config) {
> +               struct worktree **worktrees = get_worktrees(0);
> +               if (repository_format_worktree_config)
> +                       given_config_source.file = git_pathdup("config.worktree");
> +               else if (worktrees[0] && worktrees[1])
> +                       die(_("--worktree cannot be used with multiple "
> +                             "working trees unless the config\n"
> +                             "extension worktreeConfig is enabled. "
> +                             "Please read \"CONFIGURATION FILE\"\n"
> +                             "section in \"git help worktree\" for details"));
> +               else
> +                       given_config_source.file = git_pathdup("config");

I'm not sure I understand the purpose of allowing --worktree when only
a single worktree is present and extensions.worktreeConfig is not set.
Can you talk about it a bit more?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 2/2] worktree: add per-worktree config files
  2018-09-30  4:32     ` Eric Sunshine
@ 2018-09-30  7:15       ` Duy Nguyen
  2018-09-30  7:24         ` Eric Sunshine
  0 siblings, 1 reply; 34+ messages in thread
From: Duy Nguyen @ 2018-09-30  7:15 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Git Mailing List, Taylor Blau, Junio C Hamano,
	Ævar Arnfjörð Bjarmason

On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> > +       else if (use_worktree_config) {
> > +               struct worktree **worktrees = get_worktrees(0);
> > +               if (repository_format_worktree_config)
> > +                       given_config_source.file = git_pathdup("config.worktree");
> > +               else if (worktrees[0] && worktrees[1])
> > +                       die(_("--worktree cannot be used with multiple "
> > +                             "working trees unless the config\n"
> > +                             "extension worktreeConfig is enabled. "
> > +                             "Please read \"CONFIGURATION FILE\"\n"
> > +                             "section in \"git help worktree\" for details"));
> > +               else
> > +                       given_config_source.file = git_pathdup("config");
>
> I'm not sure I understand the purpose of allowing --worktree when only
> a single worktree is present and extensions.worktreeConfig is not set.
> Can you talk about it a bit more?

Unified API. If I write a script to do stuff and want it to work in
multiple worktrees as well, I should not need to do "if single
worktree, use "git config --local", if multiple use "git config
--worktree"". By using "git config --worktree" I tell git "this config
is per-worktree" and git should do the right thing, single worktree or
not.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 2/2] worktree: add per-worktree config files
  2018-09-30  7:15       ` Duy Nguyen
@ 2018-09-30  7:24         ` Eric Sunshine
  2018-09-30  7:36           ` Duy Nguyen
  0 siblings, 1 reply; 34+ messages in thread
From: Eric Sunshine @ 2018-09-30  7:24 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Git List, Taylor Blau, Junio C Hamano,
	Ævar Arnfjörð Bjarmason

On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > diff --git a/builtin/config.c b/builtin/config.c
> > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> > > +       else if (use_worktree_config) {
> > > +               struct worktree **worktrees = get_worktrees(0);
> > > +               if (repository_format_worktree_config)
> > > +                       given_config_source.file = git_pathdup("config.worktree");
> > > +               else if (worktrees[0] && worktrees[1])
> > > +                       die(_("--worktree cannot be used with multiple "
> > > +                             "working trees unless the config\n"
> > > +                             "extension worktreeConfig is enabled. "
> > > +                             "Please read \"CONFIGURATION FILE\"\n"
> > > +                             "section in \"git help worktree\" for details"));
> > > +               else
> > > +                       given_config_source.file = git_pathdup("config");
> >
> > I'm not sure I understand the purpose of allowing --worktree when only
> > a single worktree is present and extensions.worktreeConfig is not set.
> > Can you talk about it a bit more?
>
> Unified API. If I write a script to do stuff and want it to work in
> multiple worktrees as well, I should not need to do "if single
> worktree, use "git config --local", if multiple use "git config
> --worktree"". By using "git config --worktree" I tell git "this config
> is per-worktree" and git should do the right thing, single worktree or
> not.

That's what I thought, but I still don't understand how that unified
API is going to help if the script writer happens to have multiple
worktrees but doesn't have extensions.worktreeConfig set, in which
case the command will error out. I don't see how that simplifies
things, but perhaps I'm missing something obvious.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 2/2] worktree: add per-worktree config files
  2018-09-30  7:24         ` Eric Sunshine
@ 2018-09-30  7:36           ` Duy Nguyen
  0 siblings, 0 replies; 34+ messages in thread
From: Duy Nguyen @ 2018-09-30  7:36 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Git Mailing List, Taylor Blau, Junio C Hamano,
	Ævar Arnfjörð Bjarmason

On Sun, Sep 30, 2018 at 9:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen <pclouds@gmail.com> wrote:
> > On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > > diff --git a/builtin/config.c b/builtin/config.c
> > > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> > > > +       else if (use_worktree_config) {
> > > > +               struct worktree **worktrees = get_worktrees(0);
> > > > +               if (repository_format_worktree_config)
> > > > +                       given_config_source.file = git_pathdup("config.worktree");
> > > > +               else if (worktrees[0] && worktrees[1])
> > > > +                       die(_("--worktree cannot be used with multiple "
> > > > +                             "working trees unless the config\n"
> > > > +                             "extension worktreeConfig is enabled. "
> > > > +                             "Please read \"CONFIGURATION FILE\"\n"
> > > > +                             "section in \"git help worktree\" for details"));
> > > > +               else
> > > > +                       given_config_source.file = git_pathdup("config");
> > >
> > > I'm not sure I understand the purpose of allowing --worktree when only
> > > a single worktree is present and extensions.worktreeConfig is not set.
> > > Can you talk about it a bit more?
> >
> > Unified API. If I write a script to do stuff and want it to work in
> > multiple worktrees as well, I should not need to do "if single
> > worktree, use "git config --local", if multiple use "git config
> > --worktree"". By using "git config --worktree" I tell git "this config
> > is per-worktree" and git should do the right thing, single worktree or
> > not.
>
> That's what I thought, but I still don't understand how that unified
> API is going to help if the script writer happens to have multiple
> worktrees but doesn't have extensions.worktreeConfig set, in which
> case the command will error out. I don't see how that simplifies
> things, but perhaps I'm missing something obvious.

No it does not simplify that step. The user has to enable it manually
(at least for now). v1 tried to enable it automatically, but I think
it's safer to go one step at a time.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v2 1/2] t1300: extract and use test_cmp_config()
  2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
  2018-09-30  4:05     ` Eric Sunshine
@ 2018-09-30 12:31     ` SZEDER Gábor
  1 sibling, 0 replies; 34+ messages in thread
From: SZEDER Gábor @ 2018-09-30 12:31 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Taylor Blau, Junio C Hamano, Ævar Arnfjörð

On Sat, Sep 29, 2018 at 05:30:04PM +0200, Nguyễn Thái Ngọc Duy wrote:
> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
> 
> This function has uses outside t1300 as well but I'm not going to
> convert them all. And it will be used in the next commit where
> per-worktree config feature is introduced.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t1300-config.sh       | 79 ++++++++++-------------------------------
>  t/test-lib-functions.sh | 24 +++++++++++++
>  2 files changed, 43 insertions(+), 60 deletions(-)
> 
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index cdf1fed5d1..00c2b0f0eb 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -76,15 +76,11 @@ EOF

>  test_expect_success 'unset type specifiers may be reset to conflicting ones' '
> -	echo 1048576 >expect &&
> -	git config --type=bool --no-type --type=int core.big >actual &&
> -	test_cmp expect actual
> +	test_cmp_config 1048576 --type=bool --no-type --type=int core.big
>  '
>  
>  test_expect_success '--type rejects unknown specifiers' '
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index d82fac9d79..4cd7fb8fdf 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -747,6 +747,30 @@ test_cmp() {
>  	$GIT_TEST_CMP "$@"
>  }
>  
> +# similar to test_cmp but $2 is a config key instead of actual value

This doesn't describe very well the function's parameters: $1
specifies the expected value (as opposed to the file containing the
expected value), and the rest can be all kinds of 'git config'
options, not just a second argument with the config key; e.g. see call
in the above hunk.

Perheps only a brief description and a usage string like this would
sufficiently cover everything?

  # Check that the given config key has the expected value.
  #
  #   test_cmp_config [-C <dir>] <expected-value>
  #                   [<git-config-options>...] <config-key>

> +# it can also accept -C to read from a different repo, e.g.
> +#
> +#     test_cmp_config -C xyz foo core.bar
> +#
> +# is sort of equivalent of

I think this comment should outright say that "is a better alternative
to", because it provides useful output on failure, and because it
doesn't hide 'git config's exit code.

Note that this second point does make a bit of a difference:

  test_cmp_config "" nonexisting.variable

would fail, because

  $ git config nonexisting.variable ; echo $?
  1

and the &&-chain.  However,

  test "" = "$(git config nonexisting.variable)"

would still succeed, because the non-zero exit code is ignored.

I consider this a benefit, as it will protect us from a typo in the
name of a set but empty variable, even though we won't get any error
message.

> +#
> +#     test "foo" = "$(git -C xyz core.bar)"
> +

Nit: unnecessary empty line.

> +test_cmp_config() {
> +	if [ "$1" = "-C" ]
> +	then
> +		shift &&
> +		GD="-C $1" &&
> +		shift
> +	else
> +		GD=
> +	fi &&

I think you could now safely declare GD as local.  The test balloon
01d3a526ad (t0000: check whether the shell supports the "local"
keyword, 2017-10-26) has been out there for 4 releases / almost a
year, and we haven't heard about any issues, and the upcoming hash
translation test helpers use 'local' as well.

> +	echo "$1" >expected &&
> +	shift &&
> +	git $GD config "$@" >actual &&
> +	test_cmp expected actual
> +}
> +
>  # test_cmp_bin - helper to compare binary files
>  
>  test_cmp_bin() {
> -- 
> 2.19.0.341.g3acb95d729
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 0/2] Per-worktree config files
  2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
  2018-09-29 15:30   ` [PATCH v2 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2018-10-02 16:06   ` Nguyễn Thái Ngọc Duy
  2018-10-02 16:06     ` [PATCH v3 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
                       ` (2 more replies)
  2 siblings, 3 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-02 16:06 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, Eric Sunshine, SZEDER Gábor

v3 changes are minor (besides test_cmp_config), mostly document
cleanup. Diff

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 44407e69db..e036ff7b86 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -4,7 +4,7 @@ CONFIGURATION FILE
 The Git configuration file contains a number of variables that affect
 the Git commands' behavior. The files `.git/config` and optionally
 `config.worktree` (see `extensions.worktreeConfig` below) in each
-repository is used to store the configuration for that repository, and
+repository are 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.
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index aa88278dde..408c87c9ef 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -221,7 +221,7 @@ $ git config extensions.worktreeConfig true
 In this mode, specific configuration stays in the path pointed by `git
 rev-parse --git-path config.worktree`. You can add or update
 configuration in this file with `git config --worktree`. Older Git
-versions may will refuse to access repositories with this extension.
+versions will refuse to access repositories with this extension.
 
 Note that in this file, the exception for `core.bare` and `core.worktree`
 is gone. If you have them in $GIT_DIR/config before, you must move
@@ -283,6 +283,9 @@ to `/path/main/.git/worktrees/test-next` then a file named
 `test-next` entry from being pruned.  See
 linkgit:gitrepository-layout[5] for details.
 
+When extensions.worktreeConfig is enabled, the config file
+`.git/worktrees/<id>/config.worktree` is read after `.git/config` is.
+
 LIST OUTPUT FORMAT
 ------------------
 The worktree list command has two output formats.  The default format shows the
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4cd7fb8fdf..2149b88392 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -747,28 +747,27 @@ test_cmp() {
 	$GIT_TEST_CMP "$@"
 }
 
-# similar to test_cmp but $2 is a config key instead of actual value
-# it can also accept -C to read from a different repo, e.g.
+# Check that the given config key has the expected value.
 #
-#     test_cmp_config -C xyz foo core.bar
+#    test_cmp_config [-C <dir>] <expected-value>
+#                    [<git-config-options>...] <config-key>
 #
-# is sort of equivalent of
+# for example to check that the value of core.bar is foo
+#
+#    test_cmp_config foo core.bar
 #
-#     test "foo" = "$(git -C xyz core.bar)"
-
 test_cmp_config() {
-	if [ "$1" = "-C" ]
+	local GD
+	if test "$1" = "-C"
 	then
 		shift &&
 		GD="-C $1" &&
 		shift
-	else
-		GD=
 	fi &&
-	echo "$1" >expected &&
+	printf "%s\n" "$1" >expect.config &&
 	shift &&
-	git $GD config "$@" >actual &&
-	test_cmp expected actual
+	git $GD config "$@" >actual.config &&
+	test_cmp expect.config actual.config
 }
 
 # test_cmp_bin - helper to compare binary files

Nguyễn Thái Ngọc Duy (2):
  t1300: extract and use test_cmp_config()
  worktree: add per-worktree config files

 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 33 +++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t1300-config.sh                      | 79 +++++++-------------------
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 t/test-lib-functions.sh                | 23 ++++++++
 12 files changed, 255 insertions(+), 78 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

-- 
2.19.0.342.gc057aaf40a.dirty


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 1/2] t1300: extract and use test_cmp_config()
  2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
@ 2018-10-02 16:06     ` Nguyễn Thái Ngọc Duy
  2018-10-03  7:46       ` Eric Sunshine
  2018-10-02 16:06     ` [PATCH v3 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2018-10-21 14:02     ` [PATCH v4 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2 siblings, 1 reply; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-02 16:06 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, Eric Sunshine, SZEDER Gábor

In many config-related tests it's common to check if a config variable
has expected value and we want to print the differences when the test
fails. Doing it the normal way is three lines of shell code. Let's add
a function do to all this (and a little more).

This function has uses outside t1300 as well but I'm not going to
convert them all. And it will be used in the next commit where
per-worktree config feature is introduced.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t1300-config.sh       | 79 ++++++++++-------------------------------
 t/test-lib-functions.sh | 23 ++++++++++++
 2 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index cdf1fed5d1..00c2b0f0eb 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -76,15 +76,11 @@ EOF
 test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	echo Second >expect &&
-	git config cores.whatever >actual &&
-	test_cmp expect actual
+	test_cmp_config Second cores.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	echo Second >expect &&
-	git config CoReS.WhAtEvEr >actual &&
-	test_cmp expect actual
+	test_cmp_config Second CoReS.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '
@@ -94,12 +90,8 @@ test_expect_success 'subsections are not canonicalized by git-config' '
 	[section "SubSection"]
 	key = two
 	EOF
-	echo one >expect &&
-	git config section.subsection.key >actual &&
-	test_cmp expect actual &&
-	echo two >expect &&
-	git config section.SubSection.key >actual &&
-	test_cmp expect actual
+	test_cmp_config one section.subsection.key &&
+	test_cmp_config two section.SubSection.key
 '
 
 cat > .git/config <<\EOF
@@ -212,9 +204,7 @@ test_expect_success 'really really mean test' '
 '
 
 test_expect_success 'get value' '
-	echo alpha >expect &&
-	git config beta.haha >actual &&
-	test_cmp expect actual
+	test_cmp_config alpha beta.haha
 '
 
 cat > expect << EOF
@@ -251,15 +241,11 @@ test_expect_success 'non-match' '
 '
 
 test_expect_success 'non-match value' '
-	echo wow >expect &&
-	git config --get nextsection.nonewline !for >actual &&
-	test_cmp expect actual
+	test_cmp_config wow --get nextsection.nonewline !for
 '
 
 test_expect_success 'multi-valued get returns final one' '
-	echo "wow2 for me" >expect &&
-	git config --get nextsection.nonewline >actual &&
-	test_cmp expect actual
+	test_cmp_config "wow2 for me" --get nextsection.nonewline
 '
 
 test_expect_success 'multi-valued get-all returns all' '
@@ -520,21 +506,11 @@ test_expect_success 'editing stdin is an error' '
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
-	(
-		cd x &&
-		echo strasse >expect &&
-		git config --get --file ../other-config ein.bahn >actual &&
-		test_cmp expect actual
-	)
-
+	test_cmp_config -C x strasse --get --file ../other-config ein.bahn
 '
 
 test_expect_success 'refer config from subdirectory via --file' '
-	(
-		cd x &&
-		git config --file=../other-config --get ein.bahn >actual &&
-		test_cmp expect actual
-	)
+	test_cmp_config -C x strasse --file=../other-config --get ein.bahn
 '
 
 cat > expect << EOF
@@ -688,16 +664,13 @@ test_expect_success numbers '
 
 test_expect_success '--int is at least 64 bits' '
 	git config giga.watts 121g &&
-	echo 129922760704 >expect &&
-	git config --int --get giga.watts >actual &&
-	test_cmp expect actual
+	echo  >expect &&
+	test_cmp_config 129922760704 --int --get giga.watts
 '
 
 test_expect_success 'invalid unit' '
 	git config aninvalid.unit "1auto" &&
-	echo 1auto >expect &&
-	git config aninvalid.unit >actual &&
-	test_cmp expect actual &&
+	test_cmp_config 1auto aninvalid.unit &&
 	test_must_fail git config --int --get aninvalid.unit 2>actual &&
 	test_i18ngrep "bad numeric config value .1auto. for .aninvalid.unit. in file .git/config: invalid unit" actual
 '
@@ -1039,9 +1012,7 @@ test_expect_success '--null --get-regexp' '
 
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
-	echo "foo 	  bar" >expect &&
-	git config section.val >actual &&
-	test_cmp expect actual
+	test_cmp_config "foo 	  bar" section.val
 '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
@@ -1808,21 +1779,15 @@ big = 1M
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
-	git config --type=int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=int --type=int core.big
 '
 
 test_expect_success 'identical legacy --type specifiers are allowed' '
-	git config --int --int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --int core.big
 '
 
 test_expect_success 'identical mixed --type specifiers are allowed' '
-	git config --int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --type=int core.big
 '
 
 test_expect_success 'non-identical modern --type specifiers are not allowed' '
@@ -1841,21 +1806,15 @@ test_expect_success 'non-identical mixed --type specifiers are not allowed' '
 '
 
 test_expect_success '--type allows valid type specifiers' '
-	echo "true" >expect &&
-	git config --type=bool core.foo >actual &&
-	test_cmp expect actual
+	test_cmp_config true  --type=bool core.foo
 '
 
 test_expect_success '--no-type unsets type specifiers' '
-	echo "10" >expect &&
-	git config --type=bool --no-type core.number >actual &&
-	test_cmp expect actual
+	test_cmp_config 10 --type=bool --no-type core.number
 '
 
 test_expect_success 'unset type specifiers may be reset to conflicting ones' '
-	echo 1048576 >expect &&
-	git config --type=bool --no-type --type=int core.big >actual &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=bool --no-type --type=int core.big
 '
 
 test_expect_success '--type rejects unknown specifiers' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index d82fac9d79..2149b88392 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -747,6 +747,29 @@ test_cmp() {
 	$GIT_TEST_CMP "$@"
 }
 
+# Check that the given config key has the expected value.
+#
+#    test_cmp_config [-C <dir>] <expected-value>
+#                    [<git-config-options>...] <config-key>
+#
+# for example to check that the value of core.bar is foo
+#
+#    test_cmp_config foo core.bar
+#
+test_cmp_config() {
+	local GD
+	if test "$1" = "-C"
+	then
+		shift &&
+		GD="-C $1" &&
+		shift
+	fi &&
+	printf "%s\n" "$1" >expect.config &&
+	shift &&
+	git $GD config "$@" >actual.config &&
+	test_cmp expect.config actual.config
+}
+
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin() {
-- 
2.19.0.342.gc057aaf40a.dirty


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 2/2] worktree: add per-worktree config files
  2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2018-10-02 16:06     ` [PATCH v3 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
@ 2018-10-02 16:06     ` Nguyễn Thái Ngọc Duy
  2018-10-21 14:02     ` [PATCH v4 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2 siblings, 0 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-02 16:06 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, Eric Sunshine, SZEDER Gábor

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config settings are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present and there is only one worktree so that it
    works in any both single and multiple worktree setups.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 33 +++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 10 files changed, 213 insertions(+), 18 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d85d1a324..e036ff7b86 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) in each
+repository are 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.
@@ -371,6 +372,13 @@ advice.*::
 		editor input from the user.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file from GIT_DIR in that order. In
+	multiple working directory mode, "config" file is shared while
+	"config.worktree" is per-working directory (i.e., it's in
+	GIT_COMMON_DIR/worktrees/<id>/config.worktree)
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 8e240435be..4870e00b89 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -45,13 +45,15 @@ unset an existing `--type` specifier with `--no-type`.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -131,6 +133,11 @@ from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -281,6 +288,10 @@ $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
@@ -299,9 +310,10 @@ configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..408c87c9ef 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+trees. If the config variables `core.bare` or `core.worktree` are
+already present in the config file, they will be applied to the main
+working trees only.
+
+In order to have configuration specific to working trees, you can turn
+on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Older Git
+versions will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and `core.worktree`
+is gone. If you have them in $GIT_DIR/config before, you must move
+them to the `config.worktree` of the main working tree. You may also
+take this opportunity to review and move other configuration that you
+do not want to share to all working trees:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working tree, unless you
+   are sure you always use sparse checkout for all working trees.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -253,6 +283,9 @@ to `/path/main/.git/worktrees/test-next` then a file named
 `test-next` entry from being pruned.  See
 linkgit:gitrepository-layout[5] for details.
 
+When extensions.worktreeConfig is enabled, the config file
+`.git/worktrees/<id>/config.worktree` is read after `.git/config` is.
+
 LIST OUTPUT FORMAT
 ------------------
 The worktree list command has two output formats.  The default format shows the
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..36fcca8087 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -275,6 +280,9 @@ worktrees/<id>/locked::
 	or manually by `git worktree prune`. The file may contain a string
 	explaining why the repository is locked.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/builtin/config.c b/builtin/config.c
index 97b58c4aea..84385ef165 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -24,6 +25,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, type;
 static char *default_value;
@@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error(_("only one config file at a time"));
 		usage_builtin_config();
@@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1])
+			die(_("--worktree cannot be used with multiple "
+			      "working trees unless the config\n"
+			      "extension worktreeConfig is enabled. "
+			      "Please read \"CONFIGURATION FILE\"\n"
+			      "section in \"git help worktree\" for details"));
+		else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				prefix_filename(prefix, given_config_source.file);
diff --git a/cache.h b/cache.h
index d508f3d4f8..9e9b917e99 100644
--- a/cache.h
+++ b/cache.h
@@ -957,11 +957,13 @@ extern int grafts_replace_parents;
 extern int repository_format_precious_objects;
 extern char *repository_format_partial_clone;
 extern const char *core_partial_clone_filter_default;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
 	char *partial_clone; /* value of extensions.partialclone */
+	int worktree_config;
 	int is_bare;
 	int hash_algo;
 	char *work_tree;
diff --git a/config.c b/config.c
index 3461993f0a..b3025164d2 100644
--- a/config.c
+++ b/config.c
@@ -1695,6 +1695,17 @@ static int do_git_config_sequence(const struct config_options *opts,
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	/*
+	 * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
+	 * But let's not complicate things before it's actually needed.
+	 */
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 3f3c8746c2..268310b3dc 100644
--- a/environment.c
+++ b/environment.c
@@ -33,6 +33,7 @@ int ref_paranoia = -1;
 int repository_format_precious_objects;
 char *repository_format_partial_clone;
 const char *core_partial_clone_filter_default;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index b24c811c1c..1be5037f12 100644
--- a/setup.c
+++ b/setup.c
@@ -402,6 +402,20 @@ void setup_work_tree(void)
 	initialized = 1;
 }
 
+static int read_worktree_config(const char *var, const char *value, void *vdata)
+{
+	struct repository_format *data = vdata;
+
+	if (strcmp(var, "core.bare") == 0) {
+		data->is_bare = git_config_bool(var, value);
+	} else if (strcmp(var, "core.worktree") == 0) {
+		if (!value)
+			return config_error_nonbool(var);
+		data->work_tree = xstrdup(value);
+	}
+	return 0;
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
 	struct repository_format *data = vdata;
@@ -423,16 +437,13 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else
+		} else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
+		else
 			string_list_append(&data->unknown_extensions, ext);
-	} else if (strcmp(var, "core.bare") == 0) {
-		data->is_bare = git_config_bool(var, value);
-	} else if (strcmp(var, "core.worktree") == 0) {
-		if (!value)
-			return config_error_nonbool(var);
-		data->work_tree = xstrdup(value);
 	}
-	return 0;
+
+	return read_worktree_config(var, value, vdata);
 }
 
 static int check_repository_format_gently(const char *gitdir, struct repository_format *candidate, int *nongit_ok)
@@ -466,7 +477,20 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 
 	repository_format_precious_objects = candidate->precious_objects;
 	repository_format_partial_clone = candidate->partial_clone;
+	repository_format_worktree_config = candidate->worktree_config;
 	string_list_clear(&candidate->unknown_extensions, 0);
+
+	if (repository_format_worktree_config) {
+		/*
+		 * pick up core.bare and core.worktree from per-worktree
+		 * config if present
+		 */
+		strbuf_addf(&sb, "%s/config.worktree", gitdir);
+		git_config_from_file(read_worktree_config, sb.buf, candidate);
+		strbuf_release(&sb);
+		has_common = 0;
+	}
+
 	if (!has_common) {
 		if (candidate->is_bare != -1) {
 			is_bare_repository_cfg = candidate->is_bare;
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000000..286121d8de
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit start
+'
+
+test_expect_success 'config --worktree in single worktree' '
+	git config --worktree foo.bar true &&
+	test_cmp_config true foo.bar
+'
+
+test_expect_success 'add worktrees' '
+	git worktree add wt1 &&
+	git worktree add wt2
+'
+
+test_expect_success 'config --worktree without extension' '
+	test_must_fail git config --worktree foo.bar false
+'
+
+test_expect_success 'enable worktreeConfig extension' '
+	git config extensions.worktreeConfig true &&
+	test_cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	test_cmp_config also-shared that.is &&
+	test_cmp_config -C wt1 also-shared that.is &&
+	test_cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 for-wt1 this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	test_config core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = true
+'
+
+test_expect_success 'per-worktree core.bare is picked up' '
+	git -C wt1 config --worktree core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = false &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = false
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_done
-- 
2.19.0.342.gc057aaf40a.dirty


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 1/2] t1300: extract and use test_cmp_config()
  2018-10-02 16:06     ` [PATCH v3 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
@ 2018-10-03  7:46       ` Eric Sunshine
  0 siblings, 0 replies; 34+ messages in thread
From: Eric Sunshine @ 2018-10-03  7:46 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Ævar Arnfjörð Bjarmason, Git List, Junio C Hamano,
	Taylor Blau, SZEDER Gábor

On Tue, Oct 2, 2018 at 12:07 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> In many config-related tests it's common to check if a config variable
> has expected value and we want to print the differences when the test
> fails. Doing it the normal way is three lines of shell code. Let's add
> a function do to all this (and a little more).
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -747,6 +747,29 @@ test_cmp() {
> +test_cmp_config() {
> +       local GD

If you re-roll, maybe make this part of the &&-chain to protect
against someone coming along and inserting code above this line
without realizing that the &&-chain is broken.

> +       if test "$1" = "-C"
> +       then
> +               shift &&
> +               GD="-C $1" &&
> +               shift
> +       fi &&
> +       printf "%s\n" "$1" >expect.config &&
> +       shift &&
> +       git $GD config "$@" >actual.config &&
> +       test_cmp expect.config actual.config
> +}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 0/2] Per-worktree config files
  2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2018-10-02 16:06     ` [PATCH v3 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
  2018-10-02 16:06     ` [PATCH v3 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2018-10-21 14:02     ` Nguyễn Thái Ngọc Duy
  2018-10-21 14:02       ` [PATCH v4 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
  2018-10-21 14:02       ` [PATCH v4 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2 siblings, 2 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-21 14:02 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, sunshine, szeder.dev

v4 has just one minor change to unbreak a && chain in test_cmp_config.

Nguyễn Thái Ngọc Duy (2):
  t1300: extract and use test_cmp_config()
  worktree: add per-worktree config files

 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 33 +++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t1300-config.sh                      | 79 +++++++-------------------
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 t/test-lib-functions.sh                | 23 ++++++++
 12 files changed, 255 insertions(+), 78 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

Range-diff against v3:
1:  445f985619 ! 1:  23c2ee8844 t1300: extract and use test_cmp_config()
    @@ -201,7 +201,7 @@
     +#    test_cmp_config foo core.bar
     +#
     +test_cmp_config() {
    -+	local GD
    ++	local GD &&
     +	if test "$1" = "-C"
     +	then
     +		shift &&
2:  4c2d1bb37d = 2:  0d4dc8c6b0 worktree: add per-worktree config files
-- 
2.19.1.647.g708186aaf9


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 1/2] t1300: extract and use test_cmp_config()
  2018-10-21 14:02     ` [PATCH v4 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
@ 2018-10-21 14:02       ` Nguyễn Thái Ngọc Duy
  2018-10-21 14:02       ` [PATCH v4 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-21 14:02 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, sunshine, szeder.dev

In many config-related tests it's common to check if a config variable
has expected value and we want to print the differences when the test
fails. Doing it the normal way is three lines of shell code. Let's add
a function do to all this (and a little more).

This function has uses outside t1300 as well but I'm not going to
convert them all. And it will be used in the next commit where
per-worktree config feature is introduced.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t1300-config.sh       | 79 ++++++++++-------------------------------
 t/test-lib-functions.sh | 23 ++++++++++++
 2 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e2cd50ecfc..9652b241c7 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -76,15 +76,11 @@ EOF
 test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	echo Second >expect &&
-	git config cores.whatever >actual &&
-	test_cmp expect actual
+	test_cmp_config Second cores.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	echo Second >expect &&
-	git config CoReS.WhAtEvEr >actual &&
-	test_cmp expect actual
+	test_cmp_config Second CoReS.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '
@@ -94,12 +90,8 @@ test_expect_success 'subsections are not canonicalized by git-config' '
 	[section "SubSection"]
 	key = two
 	EOF
-	echo one >expect &&
-	git config section.subsection.key >actual &&
-	test_cmp expect actual &&
-	echo two >expect &&
-	git config section.SubSection.key >actual &&
-	test_cmp expect actual
+	test_cmp_config one section.subsection.key &&
+	test_cmp_config two section.SubSection.key
 '
 
 cat > .git/config <<\EOF
@@ -212,9 +204,7 @@ test_expect_success 'really really mean test' '
 '
 
 test_expect_success 'get value' '
-	echo alpha >expect &&
-	git config beta.haha >actual &&
-	test_cmp expect actual
+	test_cmp_config alpha beta.haha
 '
 
 cat > expect << EOF
@@ -251,15 +241,11 @@ test_expect_success 'non-match' '
 '
 
 test_expect_success 'non-match value' '
-	echo wow >expect &&
-	git config --get nextsection.nonewline !for >actual &&
-	test_cmp expect actual
+	test_cmp_config wow --get nextsection.nonewline !for
 '
 
 test_expect_success 'multi-valued get returns final one' '
-	echo "wow2 for me" >expect &&
-	git config --get nextsection.nonewline >actual &&
-	test_cmp expect actual
+	test_cmp_config "wow2 for me" --get nextsection.nonewline
 '
 
 test_expect_success 'multi-valued get-all returns all' '
@@ -520,21 +506,11 @@ test_expect_success 'editing stdin is an error' '
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
-	(
-		cd x &&
-		echo strasse >expect &&
-		git config --get --file ../other-config ein.bahn >actual &&
-		test_cmp expect actual
-	)
-
+	test_cmp_config -C x strasse --get --file ../other-config ein.bahn
 '
 
 test_expect_success 'refer config from subdirectory via --file' '
-	(
-		cd x &&
-		git config --file=../other-config --get ein.bahn >actual &&
-		test_cmp expect actual
-	)
+	test_cmp_config -C x strasse --file=../other-config --get ein.bahn
 '
 
 cat > expect << EOF
@@ -688,16 +664,13 @@ test_expect_success numbers '
 
 test_expect_success '--int is at least 64 bits' '
 	git config giga.watts 121g &&
-	echo 129922760704 >expect &&
-	git config --int --get giga.watts >actual &&
-	test_cmp expect actual
+	echo  >expect &&
+	test_cmp_config 129922760704 --int --get giga.watts
 '
 
 test_expect_success 'invalid unit' '
 	git config aninvalid.unit "1auto" &&
-	echo 1auto >expect &&
-	git config aninvalid.unit >actual &&
-	test_cmp expect actual &&
+	test_cmp_config 1auto aninvalid.unit &&
 	test_must_fail git config --int --get aninvalid.unit 2>actual &&
 	test_i18ngrep "bad numeric config value .1auto. for .aninvalid.unit. in file .git/config: invalid unit" actual
 '
@@ -1039,9 +1012,7 @@ test_expect_success '--null --get-regexp' '
 
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
-	echo "foo 	  bar" >expect &&
-	git config section.val >actual &&
-	test_cmp expect actual
+	test_cmp_config "foo 	  bar" section.val
 '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
@@ -1809,21 +1780,15 @@ big = 1M
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
-	git config --type=int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=int --type=int core.big
 '
 
 test_expect_success 'identical legacy --type specifiers are allowed' '
-	git config --int --int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --int core.big
 '
 
 test_expect_success 'identical mixed --type specifiers are allowed' '
-	git config --int --type=int core.big >actual &&
-	echo 1048576 >expect &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --int --type=int core.big
 '
 
 test_expect_success 'non-identical modern --type specifiers are not allowed' '
@@ -1842,21 +1807,15 @@ test_expect_success 'non-identical mixed --type specifiers are not allowed' '
 '
 
 test_expect_success '--type allows valid type specifiers' '
-	echo "true" >expect &&
-	git config --type=bool core.foo >actual &&
-	test_cmp expect actual
+	test_cmp_config true  --type=bool core.foo
 '
 
 test_expect_success '--no-type unsets type specifiers' '
-	echo "10" >expect &&
-	git config --type=bool --no-type core.number >actual &&
-	test_cmp expect actual
+	test_cmp_config 10 --type=bool --no-type core.number
 '
 
 test_expect_success 'unset type specifiers may be reset to conflicting ones' '
-	echo 1048576 >expect &&
-	git config --type=bool --no-type --type=int core.big >actual &&
-	test_cmp expect actual
+	test_cmp_config 1048576 --type=bool --no-type --type=int core.big
 '
 
 test_expect_success '--type rejects unknown specifiers' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 78d8c3783b..d158c8d0bf 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -747,6 +747,29 @@ test_cmp() {
 	$GIT_TEST_CMP "$@"
 }
 
+# Check that the given config key has the expected value.
+#
+#    test_cmp_config [-C <dir>] <expected-value>
+#                    [<git-config-options>...] <config-key>
+#
+# for example to check that the value of core.bar is foo
+#
+#    test_cmp_config foo core.bar
+#
+test_cmp_config() {
+	local GD &&
+	if test "$1" = "-C"
+	then
+		shift &&
+		GD="-C $1" &&
+		shift
+	fi &&
+	printf "%s\n" "$1" >expect.config &&
+	shift &&
+	git $GD config "$@" >actual.config &&
+	test_cmp expect.config actual.config
+}
+
 # test_cmp_bin - helper to compare binary files
 
 test_cmp_bin() {
-- 
2.19.1.647.g708186aaf9


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 2/2] worktree: add per-worktree config files
  2018-10-21 14:02     ` [PATCH v4 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
  2018-10-21 14:02       ` [PATCH v4 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
@ 2018-10-21 14:02       ` Nguyễn Thái Ngọc Duy
  2018-10-22  4:54         ` Junio C Hamano
  1 sibling, 1 reply; 34+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-10-21 14:02 UTC (permalink / raw)
  To: pclouds; +Cc: avarab, git, gitster, me, sunshine, szeder.dev

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config settings are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present and there is only one worktree so that it
    works in any both single and multiple worktree setups.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 12 +++-
 Documentation/git-config.txt           | 26 ++++++---
 Documentation/git-worktree.txt         | 33 +++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 19 ++++++-
 cache.h                                |  2 +
 config.c                               | 11 ++++
 environment.c                          |  1 +
 setup.c                                | 40 ++++++++++---
 t/t2029-worktree-config.sh             | 79 ++++++++++++++++++++++++++
 10 files changed, 213 insertions(+), 18 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 552827935a..244560a35e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) in each
+repository are 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.
@@ -371,6 +372,13 @@ advice.*::
 		editor input from the user.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file from GIT_DIR in that order. In
+	multiple working directory mode, "config" file is shared while
+	"config.worktree" is per-working directory (i.e., it's in
+	GIT_COMMON_DIR/worktrees/<id>/config.worktree)
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 5e87d82933..1bfe9f56a7 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -45,13 +45,15 @@ unset an existing `--type` specifier with `--no-type`.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -131,6 +133,11 @@ from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -281,6 +288,10 @@ $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
@@ -299,9 +310,10 @@ configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..408c87c9ef 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+trees. If the config variables `core.bare` or `core.worktree` are
+already present in the config file, they will be applied to the main
+working trees only.
+
+In order to have configuration specific to working trees, you can turn
+on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Older Git
+versions will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and `core.worktree`
+is gone. If you have them in $GIT_DIR/config before, you must move
+them to the `config.worktree` of the main working tree. You may also
+take this opportunity to review and move other configuration that you
+do not want to share to all working trees:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working tree, unless you
+   are sure you always use sparse checkout for all working trees.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -253,6 +283,9 @@ to `/path/main/.git/worktrees/test-next` then a file named
 `test-next` entry from being pruned.  See
 linkgit:gitrepository-layout[5] for details.
 
+When extensions.worktreeConfig is enabled, the config file
+`.git/worktrees/<id>/config.worktree` is read after `.git/config` is.
+
 LIST OUTPUT FORMAT
 ------------------
 The worktree list command has two output formats.  The default format shows the
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..36fcca8087 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -275,6 +280,9 @@ worktrees/<id>/locked::
 	or manually by `git worktree prune`. The file may contain a string
 	explaining why the repository is locked.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/builtin/config.c b/builtin/config.c
index 97b58c4aea..84385ef165 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -24,6 +25,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, type;
 static char *default_value;
@@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error(_("only one config file at a time"));
 		usage_builtin_config();
@@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1])
+			die(_("--worktree cannot be used with multiple "
+			      "working trees unless the config\n"
+			      "extension worktreeConfig is enabled. "
+			      "Please read \"CONFIGURATION FILE\"\n"
+			      "section in \"git help worktree\" for details"));
+		else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				prefix_filename(prefix, given_config_source.file);
diff --git a/cache.h b/cache.h
index 59c8a93046..8fd1bb2a57 100644
--- a/cache.h
+++ b/cache.h
@@ -960,11 +960,13 @@ extern int grafts_replace_parents;
 extern int repository_format_precious_objects;
 extern char *repository_format_partial_clone;
 extern const char *core_partial_clone_filter_default;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
 	char *partial_clone; /* value of extensions.partialclone */
+	int worktree_config;
 	int is_bare;
 	int hash_algo;
 	char *work_tree;
diff --git a/config.c b/config.c
index 4051e38823..aa0ed854f6 100644
--- a/config.c
+++ b/config.c
@@ -1695,6 +1695,17 @@ static int do_git_config_sequence(const struct config_options *opts,
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	/*
+	 * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
+	 * But let's not complicate things before it's actually needed.
+	 */
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 3f3c8746c2..268310b3dc 100644
--- a/environment.c
+++ b/environment.c
@@ -33,6 +33,7 @@ int ref_paranoia = -1;
 int repository_format_precious_objects;
 char *repository_format_partial_clone;
 const char *core_partial_clone_filter_default;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index b24c811c1c..1be5037f12 100644
--- a/setup.c
+++ b/setup.c
@@ -402,6 +402,20 @@ void setup_work_tree(void)
 	initialized = 1;
 }
 
+static int read_worktree_config(const char *var, const char *value, void *vdata)
+{
+	struct repository_format *data = vdata;
+
+	if (strcmp(var, "core.bare") == 0) {
+		data->is_bare = git_config_bool(var, value);
+	} else if (strcmp(var, "core.worktree") == 0) {
+		if (!value)
+			return config_error_nonbool(var);
+		data->work_tree = xstrdup(value);
+	}
+	return 0;
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
 	struct repository_format *data = vdata;
@@ -423,16 +437,13 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else
+		} else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
+		else
 			string_list_append(&data->unknown_extensions, ext);
-	} else if (strcmp(var, "core.bare") == 0) {
-		data->is_bare = git_config_bool(var, value);
-	} else if (strcmp(var, "core.worktree") == 0) {
-		if (!value)
-			return config_error_nonbool(var);
-		data->work_tree = xstrdup(value);
 	}
-	return 0;
+
+	return read_worktree_config(var, value, vdata);
 }
 
 static int check_repository_format_gently(const char *gitdir, struct repository_format *candidate, int *nongit_ok)
@@ -466,7 +477,20 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 
 	repository_format_precious_objects = candidate->precious_objects;
 	repository_format_partial_clone = candidate->partial_clone;
+	repository_format_worktree_config = candidate->worktree_config;
 	string_list_clear(&candidate->unknown_extensions, 0);
+
+	if (repository_format_worktree_config) {
+		/*
+		 * pick up core.bare and core.worktree from per-worktree
+		 * config if present
+		 */
+		strbuf_addf(&sb, "%s/config.worktree", gitdir);
+		git_config_from_file(read_worktree_config, sb.buf, candidate);
+		strbuf_release(&sb);
+		has_common = 0;
+	}
+
 	if (!has_common) {
 		if (candidate->is_bare != -1) {
 			is_bare_repository_cfg = candidate->is_bare;
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000000..286121d8de
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit start
+'
+
+test_expect_success 'config --worktree in single worktree' '
+	git config --worktree foo.bar true &&
+	test_cmp_config true foo.bar
+'
+
+test_expect_success 'add worktrees' '
+	git worktree add wt1 &&
+	git worktree add wt2
+'
+
+test_expect_success 'config --worktree without extension' '
+	test_must_fail git config --worktree foo.bar false
+'
+
+test_expect_success 'enable worktreeConfig extension' '
+	git config extensions.worktreeConfig true &&
+	test_cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	test_cmp_config also-shared that.is &&
+	test_cmp_config -C wt1 also-shared that.is &&
+	test_cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	test_cmp_config for-main this.is &&
+	test_cmp_config -C wt1 for-wt1 this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	test_config core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = true
+'
+
+test_expect_success 'per-worktree core.bare is picked up' '
+	git -C wt1 config --worktree core.bare true &&
+	test "$(git rev-parse --is-bare-repository)" = false &&
+	test "$(git -C wt1 rev-parse --is-bare-repository)" = true &&
+	test "$(git -C wt2 rev-parse --is-bare-repository)" = false
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	test_cmp_config shared this.is &&
+	test_cmp_config -C wt1 shared this.is &&
+	test_cmp_config -C wt2 shared this.is
+'
+
+test_done
-- 
2.19.1.647.g708186aaf9


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/2] worktree: add per-worktree config files
  2018-10-21 14:02       ` [PATCH v4 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2018-10-22  4:54         ` Junio C Hamano
  2018-10-22 14:32           ` Duy Nguyen
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2018-10-22  4:54 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: avarab, git, me, sunshine, szeder.dev

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 552827935a..244560a35e 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  ------------------
>  
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> -is used to store the configuration for that repository, and
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) in each
> +repository are 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.
> @@ -371,6 +372,13 @@ advice.*::
>  		editor input from the user.
>  --
>  
> +extensions.worktreeConfig::
> +	If set, by default "git config" reads from both "config" and
> +	"config.worktree" file from GIT_DIR in that order. In
> +	multiple working directory mode, "config" file is shared while
> +	"config.worktree" is per-working directory (i.e., it's in
> +	GIT_COMMON_DIR/worktrees/<id>/config.worktree)
> +

This obviously conflicts with your 59-patch series, but more
importantly

 - I notice that this is the only description of extensions.* key in
   the configuration files.  Don't we have any other extension
   defined, and if so shouldn't we be describing them already (not
   as a part of this series, obviously)?

 - If we are going to describe other extensions.* keys, do we want
   extensions-config.txt file to split this (and others) out and
   later rename it to config/extensions.txt?  Or do we want to
   collect related things together by logically not by name and have
   this extension described in config/worktree.txt instead, perhaps
   separate from other extensions.* keys?


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/2] worktree: add per-worktree config files
  2018-10-22  4:54         ` Junio C Hamano
@ 2018-10-22 14:32           ` Duy Nguyen
  2018-10-25  9:16             ` Junio C Hamano
  0 siblings, 1 reply; 34+ messages in thread
From: Duy Nguyen @ 2018-10-22 14:32 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Taylor Blau, Eric Sunshine, SZEDER Gábor

On Mon, Oct 22, 2018 at 6:54 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 552827935a..244560a35e 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -2,8 +2,9 @@ CONFIGURATION FILE
> >  ------------------
> >
> >  The Git configuration file contains a number of variables that affect
> > -the Git commands' behavior. The `.git/config` file in each repository
> > -is used to store the configuration for that repository, and
> > +the Git commands' behavior. The files `.git/config` and optionally
> > +`config.worktree` (see `extensions.worktreeConfig` below) in each
> > +repository are 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.
> > @@ -371,6 +372,13 @@ advice.*::
> >               editor input from the user.
> >  --
> >
> > +extensions.worktreeConfig::
> > +     If set, by default "git config" reads from both "config" and
> > +     "config.worktree" file from GIT_DIR in that order. In
> > +     multiple working directory mode, "config" file is shared while
> > +     "config.worktree" is per-working directory (i.e., it's in
> > +     GIT_COMMON_DIR/worktrees/<id>/config.worktree)
> > +
>
> This obviously conflicts with your 59-patch series, but more
> importantly
>
>  - I notice that this is the only description of extensions.* key in
>    the configuration files.  Don't we have any other extension
>    defined, and if so shouldn't we be describing them already (not
>    as a part of this series, obviously)?

Right. We have two extensions already but it's described in
technical/repository-format.txt. I'll move this extension there
because it's written "This document will serve as the master list for
extensions." in that document.

>  - If we are going to describe other extensions.* keys, do we want
>    extensions-config.txt file to split this (and others) out and
>    later rename it to config/extensions.txt?  Or do we want to
>    collect related things together by logically not by name and have
>    this extension described in config/worktree.txt instead, perhaps
>    separate from other extensions.* keys?

I think we would go with config/extensions.txt because if grouping
logically, I'm not sure where extensions.preciousObjects and
extensions.partialClone would go.
-- 
Duy

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/2] worktree: add per-worktree config files
  2018-10-22 14:32           ` Duy Nguyen
@ 2018-10-25  9:16             ` Junio C Hamano
  0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2018-10-25  9:16 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Taylor Blau, Eric Sunshine, SZEDER Gábor

Duy Nguyen <pclouds@gmail.com> writes:

>> > +extensions.worktreeConfig::
>> > +     If set, by default "git config" reads from both "config" and
>> > +     "config.worktree" file from GIT_DIR in that order. In
>> > +     multiple working directory mode, "config" file is shared while
>> > +     "config.worktree" is per-working directory (i.e., it's in
>> > +     GIT_COMMON_DIR/worktrees/<id>/config.worktree)
>> > +
>>
>> This obviously conflicts with your 59-patch series, but more
>> importantly
>>
>>  - I notice that this is the only description of extensions.* key in
>>    the configuration files.  Don't we have any other extension
>>    defined, and if so shouldn't we be describing them already (not
>>    as a part of this series, obviously)?
>
> Right. We have two extensions already but it's described in
> technical/repository-format.txt. I'll move this extension there
> because it's written "This document will serve as the master list for
> extensions." in that document.
>
>>  - If we are going to describe other extensions.* keys, do we want
>>    extensions-config.txt file to split this (and others) out and
>>    later rename it to config/extensions.txt?  Or do we want to
>>    collect related things together by logically not by name and have
>>    this extension described in config/worktree.txt instead, perhaps
>>    separate from other extensions.* keys?
>
> I think we would go with config/extensions.txt because if grouping
> logically, I'm not sure where extensions.preciousObjects and
> extensions.partialClone would go.

OK, that sounds sensible.

Other than that, I am getting the feeling that everybody agrees that
the problem this topic addresses is worth addressing, and the design
and the implementation of the solution presented here is sensible.

If so, let's move it forward to 'next' and plan to merge it down to
'master'.  The "extensions.*" description can happen incrementally,.
I'd think.


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2018-10-25  9:16 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-23 17:04 [PATCH] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
2018-09-23 20:47 ` Eric Sunshine
2018-09-24 14:21 ` Taylor Blau
2018-09-25 15:57   ` Duy Nguyen
2018-09-29 13:53   ` Duy Nguyen
2018-09-25 21:26 ` Junio C Hamano
2018-09-26 15:48   ` Duy Nguyen
2018-09-26 17:40     ` Junio C Hamano
2018-09-27 15:24     ` Wherefor worktrees? Marc Branchaud
2018-09-27 16:36       ` Duy Nguyen
2018-09-26 18:25 ` [PATCH] worktree: add per-worktree config files Ævar Arnfjörð Bjarmason
2018-09-27 17:24   ` Duy Nguyen
2018-09-27 18:34     ` Ævar Arnfjörð Bjarmason
2018-09-27 18:49       ` Duy Nguyen
2018-09-29  6:36       ` Duy Nguyen
2018-09-29 15:30 ` [PATCH v2 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
2018-09-29 15:30   ` [PATCH v2 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
2018-09-30  4:05     ` Eric Sunshine
2018-09-30 12:31     ` SZEDER Gábor
2018-09-29 15:30   ` [PATCH v2 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
2018-09-30  4:32     ` Eric Sunshine
2018-09-30  7:15       ` Duy Nguyen
2018-09-30  7:24         ` Eric Sunshine
2018-09-30  7:36           ` Duy Nguyen
2018-10-02 16:06   ` [PATCH v3 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
2018-10-02 16:06     ` [PATCH v3 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
2018-10-03  7:46       ` Eric Sunshine
2018-10-02 16:06     ` [PATCH v3 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
2018-10-21 14:02     ` [PATCH v4 0/2] Per-worktree " Nguyễn Thái Ngọc Duy
2018-10-21 14:02       ` [PATCH v4 1/2] t1300: extract and use test_cmp_config() Nguyễn Thái Ngọc Duy
2018-10-21 14:02       ` [PATCH v4 2/2] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
2018-10-22  4:54         ` Junio C Hamano
2018-10-22 14:32           ` Duy Nguyen
2018-10-25  9:16             ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.