git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Current state of Git worktree used with submodules?
@ 2016-07-19 20:59 Lars Schneider
  2016-07-20  4:14 ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Lars Schneider @ 2016-07-19 20:59 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller, Michael Haggerty

Hi,

some time ago Michael wrote in a blog post [1]:
"It's not recommended to use git worktree with a repository that contains submodules."

Plus "Documentation/git-worktree.txt" states:
"Multiple checkout in general is still experimental, and the support
for submodules is incomplete. It is NOT recommended to make multiple
checkouts of a superproject."

I wonder about the current state of this limitation. Is the statement still valid? 
If yes, do you know if someone is working on this? If nobody is working on this, do
you have some pointers for me what the main problems are?

Thank you,
Lars


[1] https://github.com/blog/2042-git-2-5-including-multiple-worktrees-and-triangular-workflows

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

* Re: Current state of Git worktree used with submodules?
  2016-07-19 20:59 Current state of Git worktree used with submodules? Lars Schneider
@ 2016-07-20  4:14 ` Duy Nguyen
  2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
  0 siblings, 1 reply; 31+ messages in thread
From: Duy Nguyen @ 2016-07-20  4:14 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Git Mailing List, Stefan Beller, Michael Haggerty

On Tue, Jul 19, 2016 at 10:59 PM, Lars Schneider
<larsxschneider@gmail.com> wrote:
> Hi,
>
> some time ago Michael wrote in a blog post [1]:
> "It's not recommended to use git worktree with a repository that contains submodules."
>
> Plus "Documentation/git-worktree.txt" states:
> "Multiple checkout in general is still experimental, and the support
> for submodules is incomplete. It is NOT recommended to make multiple
> checkouts of a superproject."
>
> I wonder about the current state of this limitation. Is the statement still valid?

Yes.

> If yes, do you know if someone is working on this? If nobody is working on this, do
> you have some pointers for me what the main problems are?

The blocker is config file being shared (you do not want to share
core.worktree and submodule.*). I made some progress last weekend,
just needed to add some tests to see if submodule works as expected
and will post the series soon. Then you can take over if you want ;)

Note that I only try to make submodules work with multi worktree, not
work optimally. A more ambitious goal is sharing submodule repos, so
you can keep disk usage down...

> [1] https://github.com/blog/2042-git-2-5-including-multiple-worktrees-and-triangular-workflows
-- 
Duy

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

* [PATCH v4 0/4] Split .git/config in multiple worktree setup
  2016-07-20  4:14 ` Duy Nguyen
@ 2016-07-20 17:24   ` Nguyễn Thái Ngọc Duy
  2016-07-20 17:24     ` [PATCH v4 1/4] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
                       ` (3 more replies)
  0 siblings, 4 replies; 31+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-20 17:24 UTC (permalink / raw)
  To: git
  Cc: max, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, Nguyễn Thái Ngọc Duy

On Wed, Jul 20, 2016 at 6:14 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> If yes, do you know if someone is working on this? If nobody is working on this, do
>> you have some pointers for me what the main problems are?
>
> The blocker is config file being shared (you do not want to share
> core.worktree and submodule.*). I made some progress last weekend,
> just needed to add some tests to see if submodule works as expected
> and will post the series soon. Then you can take over if you want ;)

Here it is. I'm going to describe some more for new people. Let's
start with the problem, then the high level solution and finally
what's done in submodule. These are separated by ^----$

Multipl worktrees in its current form share the config file. The only
exceptions are core.bare and core.worktree which will be applied for
the main worktree only, if present in the config file.

This does not make submodules happy because submodules use
core.worktree to link back to the real repos stored inside .git dir of
the super modules. This is not so bad right now because the
submodule's worktree would be "main" worktree and core.worktree sticks
to it. If one day "git submodule add" initialize the worktree as a
linked worktree, problem arises.

The second problem is more real. When you initialize submodules,
submodule info is stored in the supermodule's config file, which is
shared. So the secondary worktree will not be able to initialize its
own submodules (and may be confused by the existing submodule.*
section).

----

So we need to split the config file into two logical parts: a shared
part and a worktree-specific one. This makes everybody happy even
though it's not easy.

What this series does is adding "git config --worktree" which writes
to the worktree-specific part, while "git config" writes to the shared
part. "git config" as a read operation will read the shared part
first, then the worktree specific part.

Now. In current multiple worktrees setup, both the shared and private
parts are in the same file, "config". And "git config --worktree" will
refuse to work if you have more than one worktree. For it to work with
multiple worktree, you need to enable extensions.worktreeConfig (in
config file).

This extension designates the file "config.worktree" as storage for
the private part. It can be .git/config.worktree for main worktree, or
.git/worktrees/xxx/config.worktree for linked ones.

Before enabling extensions.worktreeConfig (or soon after it), you need
to move core.bare and core.worktree to .git/config.worktree because
the exceptions above are gone. If they are present in "config" file,
they are _shared_ (and hell follows)

If you have followed through the first four iterations, v4 [1] has
very close design. The main difference is: in v4, "config" is
per-worktree and the shared part is split away, hidden in
.git/common/config. This leads to the migration and backward
compatibility problems.

The new design is free of that because "config" remains shared while
the private is hidden away. The risk is "git config" by default writes
to the shared part. Accidentally sharing something may be more
dangerous than accidentally _not_ sharing something like v3 [1] (which
defaults to per-worktree). I've thought about this and I'm willing to
take the new direction, bettting that 90% of the time people want to
share, so it's a rare problem.

----

With all that in place, what does a command have to do to take
advantage of it?

- Whenever you need to write a per-worktree config (you decide it!),
  use "git config --worktree". That's it. You don't really need to
  care where it ends up to. This is what 3/4 is, for submodule.

- Avoid "git config /path/to/.git/config" because that may or may not
  be the right place (there's also config.worktree now). Builtin
  commands have this worse because if you look at _another_ repo, then
  you may need to go through repo detection and stuff before you can
  read its config. I just fall back to running "git config" in 2/4.

So that's it. It seems to be running ok. But I know very little about
submodules to test it properly.

The only problem left that I have to work out is config deletion. I
suppose we could follow the chain backward again: try to delete in
per-worktree config file first. If not found, try again in the shared
config file...

[1] http://thread.gmane.org/gmane.comp.version-control.git/281906/focus=284803
-- 
2.9.1.566.gbd532d4


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

* [PATCH v4 1/4] worktree: add per-worktree config files
  2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
@ 2016-07-20 17:24     ` Nguyễn Thái Ngọc Duy
  2016-07-26  0:59       ` Stefan Beller
  2016-07-20 17:24     ` [PATCH v4 2/4] submodule: update core.worktree using git-config Nguyễn Thái Ngọc Duy
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 31+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-20 17:24 UTC (permalink / raw)
  To: git
  Cc: max, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, 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         | 31 ++++++++++++++
 Documentation/gitrepository-layout.txt |  8 ++++
 builtin/config.c                       | 18 +++++++-
 cache.h                                |  2 +
 config.c                               |  7 ++++
 environment.c                          |  1 +
 setup.c                                |  5 ++-
 t/t2028-worktree-config.sh (new +x)    | 77 ++++++++++++++++++++++++++++++++++
 10 files changed, 175 insertions(+), 11 deletions(-)
 create mode 100755 t/t2028-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 16dc22d..7d64da0 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.
@@ -264,6 +265,12 @@ advice.*::
 		show directions on how to proceed from the current state.
 --
 
+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 f163113..9dfdb6a 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -47,13 +47,15 @@ checks or transformations are performed on the value.
 
 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:
@@ -133,6 +135,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.
@@ -253,6 +260,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
@@ -268,9 +279,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 7c4cfb0..41350db 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -111,6 +111,37 @@ OPTIONS
 --expire <time>::
 	With `prune`, only expire unused working trees older than <time>.
 
+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 XXX 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.
+
 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 577ee84..6cfdb4c 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'.
@@ -276,6 +281,9 @@ worktrees/<id>/link::
 	file. It is used to detect if the linked repository is
 	manually removed.
 
+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 1d7c6ef..535707c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -4,6 +4,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>]"),
@@ -23,6 +24,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, types;
 static const char *get_color_slot, *get_colorbool_slot;
@@ -57,6 +59,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")),
@@ -491,6 +494,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_with_options(builtin_config_usage, builtin_config_options);
@@ -525,7 +529,19 @@ 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) {
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else {
+			struct worktree **worktrees = get_worktrees();
+			if (worktrees[0] && worktrees[1])
+				die(_("Per-worktree configuration requires extensions.worktreeConfig\n"
+				      "Please read section CONFIGURATION in `git help worktree` before\n"
+				      "enabling it."));
+			free_worktrees(worktrees);
+			given_config_source.file = git_pathdup("config");
+		}
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				xstrdup(prefix_filename(prefix,
diff --git a/cache.h b/cache.h
index f1dc289..606500e 100644
--- a/cache.h
+++ b/cache.h
@@ -757,10 +757,12 @@ extern int grafts_replace_parents;
 #define GIT_REPO_VERSION 0
 #define GIT_REPO_VERSION_READ 1
 extern int repository_format_precious_objects;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
+	int worktree_config;
 	int is_bare;
 	char *work_tree;
 	struct string_list unknown_extensions;
diff --git a/config.c b/config.c
index bea937e..99ff6be 100644
--- a/config.c
+++ b/config.c
@@ -1254,6 +1254,13 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
 	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 ca72464..b4d56ef 100644
--- a/environment.c
+++ b/environment.c
@@ -26,6 +26,7 @@ int warn_ambiguous_refs = 1;
 int warn_on_object_refname_ambiguity = 1;
 int ref_paranoia = -1;
 int repository_format_precious_objects;
+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 6d0e0c9..75c784f 100644
--- a/setup.c
+++ b/setup.c
@@ -389,6 +389,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			;
 		else if (!strcmp(ext, "preciousobjects"))
 			data->precious_objects = git_config_bool(var, value);
+		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) {
@@ -432,8 +434,9 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 	}
 
 	repository_format_precious_objects = candidate.precious_objects;
+	repository_format_worktree_config = candidate.worktree_config;
 	string_list_clear(&candidate.unknown_extensions, 0);
-	if (!has_common) {
+	if (!has_common || repository_format_worktree_config) {
 		if (candidate.is_bare != -1) {
 			is_bare_repository_cfg = candidate.is_bare;
 			if (is_bare_repository_cfg == 1)
diff --git a/t/t2028-worktree-config.sh b/t/t2028-worktree-config.sh
new file mode 100755
index 0000000..34067df
--- /dev/null
+++ b/t/t2028-worktree-config.sh
@@ -0,0 +1,77 @@
+#!/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 &&
+	test_must_fail git config --worktree per.worktree is-not-ok &&
+	git config extensions.worktreeConfig true
+'
+
+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 fails in multi worktree without extension' '
+	test_must_fail git config --worktree foo.bar true
+'
+
+test_done
-- 
2.9.1.566.gbd532d4


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

* [PATCH v4 2/4] submodule: update core.worktree using git-config
  2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
  2016-07-20 17:24     ` [PATCH v4 1/4] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2016-07-20 17:24     ` Nguyễn Thái Ngọc Duy
  2016-07-20 22:04       ` Stefan Beller
  2016-07-20 17:24     ` [PATCH v4 3/4] submodule: support running in multiple worktree setup Nguyễn Thái Ngọc Duy
  2016-07-20 17:24     ` [PATCH v4 4/4] t2029: some really basic tests for submodules in multi worktree Nguyễn Thái Ngọc Duy
  3 siblings, 1 reply; 31+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-20 17:24 UTC (permalink / raw)
  To: git
  Cc: max, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, Nguyễn Thái Ngọc Duy

To access a separate repository, the first step should be read its
config file to determine if this repository layout is supported or
not, or if we understand all repo extensions, of it is a linked
worktree. Only then should know where to update the config file.

Unfortunately, our C code base is not ready for doing all that in the
same process. The repo detection is not meant to be used for peeking
in other repository, and config code would read config.worktree that
is in _current_ $GIT_DIR.

For now, let's spawn a new process and let all that done separately.

PS. submodule-helper also updates core.worktree. But in there, we
create a new clone, we know what is the initial repository layout, so
we know we can simply update "config" file without risks.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 submodule.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index abc2ac2..b912871 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1128,7 +1128,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 {
 	struct strbuf file_name = STRBUF_INIT;
 	struct strbuf rel_path = STRBUF_INIT;
+	struct strbuf path = STRBUF_INIT;
 	const char *real_work_tree = xstrdup(real_path(work_tree));
+	struct child_process cp = CHILD_PROCESS_INIT;
 
 	/* Update gitfile */
 	strbuf_addf(&file_name, "%s/.git", work_tree);
@@ -1136,13 +1138,17 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 		   relative_path(git_dir, real_work_tree, &rel_path));
 
 	/* Update core.worktree setting */
-	strbuf_reset(&file_name);
-	strbuf_addf(&file_name, "%s/config", git_dir);
-	git_config_set_in_file(file_name.buf, "core.worktree",
-			       relative_path(real_work_tree, git_dir,
-					     &rel_path));
+	strbuf_addstr(&path, relative_path(real_work_tree, git_dir,
+					   &rel_path));
+	cp.git_cmd = 1;
+	argv_array_pushl(&cp.args, "-C", work_tree, NULL);
+	argv_array_pushl(&cp.args, "--work-tree", ".", NULL);
+	argv_array_pushl(&cp.args, "config", "core.worktree", path.buf, NULL);
+	if (run_command(&cp) < 0)
+		die(_("failed to update core.worktree for %s"), git_dir);
 
 	strbuf_release(&file_name);
+	strbuf_release(&path);
 	strbuf_release(&rel_path);
 	free((void *)real_work_tree);
 }
-- 
2.9.1.566.gbd532d4


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

* [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
  2016-07-20 17:24     ` [PATCH v4 1/4] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
  2016-07-20 17:24     ` [PATCH v4 2/4] submodule: update core.worktree using git-config Nguyễn Thái Ngọc Duy
@ 2016-07-20 17:24     ` Nguyễn Thái Ngọc Duy
  2016-07-20 23:22       ` Stefan Beller
  2016-07-27  4:10       ` Max Kirillov
  2016-07-20 17:24     ` [PATCH v4 4/4] t2029: some really basic tests for submodules in multi worktree Nguyễn Thái Ngọc Duy
  3 siblings, 2 replies; 31+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-20 17:24 UTC (permalink / raw)
  To: git
  Cc: max, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt | 8 ++++++++
 git-submodule.sh               | 8 ++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 41350db..2a5661d 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -142,6 +142,14 @@ to share to all working directories:
    you are sure you always use sparse checkout for all working
    directories.
 
+ - `submodule.*` in current state should not be shared because the
+   information is tied to a particular version of .gitmodules in a
+   working directory.
+
+ - `remote.*` added by submodules may be per working directory as
+   well, unless you are sure remotes from all possible submodules in
+   history are consistent.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..7b576f5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -261,7 +261,7 @@ or you are unsure what this means choose another name with the '--name' option."
 			esac
 		) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
 	fi
-	git config submodule."$sm_name".url "$realrepo"
+	git config --worktree submodule."$sm_name".url "$realrepo"
 
 	git add $force "$sm_path" ||
 	die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
@@ -461,7 +461,7 @@ Submodule work tree '\$displaypath' contains a .git directory
 			# Remove the whole section so we have a clean state when
 			# the user later decides to init this submodule again
 			url=$(git config submodule."$name".url)
-			git config --remove-section submodule."$name" 2>/dev/null &&
+			git config --worktree --remove-section submodule."$name" 2>/dev/null &&
 			say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
 		fi
 	done
@@ -1106,7 +1106,7 @@ cmd_sync()
 		then
 			displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
 			say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
-			git config submodule."$name".url "$super_config_url"
+			git config --worktree submodule."$name".url "$super_config_url"
 
 			if test -e "$sm_path"/.git
 			then
@@ -1114,7 +1114,7 @@ cmd_sync()
 				sanitize_submodule_env
 				cd "$sm_path"
 				remote=$(get_default_remote)
-				git config remote."$remote".url "$sub_origin_url"
+				git config --worktree remote."$remote".url "$sub_origin_url"
 
 				if test -n "$recursive"
 				then
-- 
2.9.1.566.gbd532d4


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

* [PATCH v4 4/4] t2029: some really basic tests for submodules in multi worktree
  2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
                       ` (2 preceding siblings ...)
  2016-07-20 17:24     ` [PATCH v4 3/4] submodule: support running in multiple worktree setup Nguyễn Thái Ngọc Duy
@ 2016-07-20 17:24     ` Nguyễn Thái Ngọc Duy
  3 siblings, 0 replies; 31+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2016-07-20 17:24 UTC (permalink / raw)
  To: git
  Cc: max, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t2029-worktree-submodule.sh (new +x) | 166 +++++++++++++++++++++++++++++++++
 1 file changed, 166 insertions(+)
 create mode 100755 t/t2029-worktree-submodule.sh

diff --git a/t/t2029-worktree-submodule.sh b/t/t2029-worktree-submodule.sh
new file mode 100755
index 0000000..f96fa50
--- /dev/null
+++ b/t/t2029-worktree-submodule.sh
@@ -0,0 +1,166 @@
+#!/bin/sh
+
+test_description='submodule with multiple worktrees'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	git config extensions.worktreeConfig true &&
+	>t &&
+	git add t &&
+	git commit -m initial &&
+	git branch initial
+'
+
+test_expect_success 'setup - repository in init subdirectory' '
+	mkdir init &&
+	(
+		cd init &&
+		git init &&
+		git config extensions.worktreeConfig true &&
+		echo a >a &&
+		git add a &&
+		git commit -m "submodule commit 1" &&
+		git tag -a -m "rev-1" rev-1
+	)
+'
+
+test_expect_success 'setup - commit with gitlink' '
+	echo a >a &&
+	echo z >z &&
+	git add a init z &&
+	git commit -m "super commit 1"
+'
+
+test_expect_success 'setup - hide init subdirectory' '
+	mv init .subrepo
+'
+
+test_expect_success 'setup - repository to add submodules to' '
+	git init addtest &&
+	git -C addtest config extensions.worktreeConfig true &&
+	git init addtest-ignore &&
+	git -C addtest-ignore config extensions.worktreeConfig true
+'
+
+# The 'submodule add' tests need some repository to add as a submodule.
+# The trash directory is a good one as any. We need to canonicalize
+# the name, though, as some tests compare it to the absolute path git
+# generates, which will expand symbolic links.
+submodurl=$(pwd -P)
+
+listbranches() {
+	git for-each-ref --format='%(refname)' 'refs/heads/*'
+}
+
+inspect() {
+	dir=$1 &&
+	dotdot="${2:-..}" &&
+
+	(
+		cd "$dir" &&
+		listbranches >"$dotdot/heads" &&
+		{ git symbolic-ref HEAD || :; } >"$dotdot/head" &&
+		git rev-parse HEAD >"$dotdot/head-sha1" &&
+		git update-index --refresh &&
+		git diff-files --exit-code &&
+		git clean -n -d -x >"$dotdot/untracked"
+	)
+}
+
+test_expect_success 'submodule add' '
+	echo "refs/heads/master" >expect &&
+	>empty &&
+
+	(
+		cd addtest &&
+		git submodule add -q "$submodurl" submod >actual &&
+		test_must_be_empty actual &&
+		echo "gitdir: ../.git/modules/submod" >expect &&
+		test_cmp expect submod/.git &&
+		(
+			cd submod &&
+			git config core.worktree >actual &&
+			echo "../../../submod" >expect &&
+			test_cmp expect actual &&
+			rm -f actual expect
+		) &&
+		git submodule init
+	) &&
+
+	rm -f heads head untracked &&
+	inspect addtest/submod ../.. &&
+	test_cmp expect heads &&
+	test_cmp expect head &&
+	test_cmp empty untracked
+'
+
+test_expect_success 'submodule.* in supermodule is per-worktree' '
+	(
+		cd addtest &&
+		git config -f .git/config.worktree submodule.submod.url >actual &&
+		echo "$submodurl" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'turn submodule to multiworktree' '
+	(
+		cd addtest/.git/modules/submod &&
+		CORE_WT="$(git config core.worktree)" &&
+		git config -f config.worktree core.worktree "$CORE_WT" &&
+		git config --unset core.worktree &&
+		git config extensions.worktreeConfig true &&
+		git config core.worktree >actual &&
+		echo "$CORE_WT" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'new worktree in submodule' '
+	(
+		cd addtest/submod &&
+		git worktree add submod-elsewhere &&
+		cd submod-elsewhere &&
+		test_must_fail git config core.worktree
+	)
+'
+
+test_expect_success 'new worktree in supermodule' '
+	(
+		cd addtest &&
+		git commit -m initial &&
+		git worktree add super-elsewhere &&
+		cd super-elsewhere &&
+		test_must_fail git config submodule.submode
+	)
+'
+
+test_expect_success 'submodule add in the second worktree' '
+	(
+		cd addtest/super-elsewhere &&
+		git submodule add -q "$submodurl" submod2 >actual &&
+		test_must_be_empty actual &&
+		echo "gitdir: ../../.git/worktrees/super-elsewhere/modules/submod2" >expect &&
+		test_cmp expect submod2/.git &&
+		(
+			cd submod2 &&
+			git config core.worktree >actual &&
+			echo "../../../../../super-elsewhere/submod2" >expect &&
+			test_cmp expect actual &&
+			rm -f actual expect
+		) &&
+		git submodule init
+	)
+'
+
+test_expect_success 'submodule.* in supermodule is per-worktree' '
+	(
+		cd addtest/super-elsewhere &&
+		git config -f ../.git/worktrees/super-elsewhere/config.worktree submodule.submod2.url >actual &&
+		echo "$submodurl" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.9.1.566.gbd532d4


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

* Re: [PATCH v4 2/4] submodule: update core.worktree using git-config
  2016-07-20 17:24     ` [PATCH v4 2/4] submodule: update core.worktree using git-config Nguyễn Thái Ngọc Duy
@ 2016-07-20 22:04       ` Stefan Beller
  2016-07-22 17:15         ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Beller @ 2016-07-20 22:04 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> To access a separate repository, the first step should be read its
> config file to determine if this repository layout is supported or
> not, or if we understand all repo extensions, of it is a linked
> worktree. Only then should know where to update the config file.
>
> Unfortunately, our C code base is not ready for doing all that in the
> same process. The repo detection is not meant to be used for peeking
> in other repository, and config code would read config.worktree that
> is in _current_ $GIT_DIR.
>
> For now, let's spawn a new process and let all that done separately.
>
> PS. submodule-helper also updates core.worktree. But in there, we
> create a new clone, we know what is the initial repository layout, so
> we know we can simply update "config" file without risks.

Right, the submodule--helper update_clone should not be required to convert.

>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  submodule.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index abc2ac2..b912871 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1128,7 +1128,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
>  {
>         struct strbuf file_name = STRBUF_INIT;
>         struct strbuf rel_path = STRBUF_INIT;
> +       struct strbuf path = STRBUF_INIT;
>         const char *real_work_tree = xstrdup(real_path(work_tree));
> +       struct child_process cp = CHILD_PROCESS_INIT;
>
>         /* Update gitfile */
>         strbuf_addf(&file_name, "%s/.git", work_tree);
> @@ -1136,13 +1138,17 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
>                    relative_path(git_dir, real_work_tree, &rel_path));
>
>         /* Update core.worktree setting */
> -       strbuf_reset(&file_name);
> -       strbuf_addf(&file_name, "%s/config", git_dir);
> -       git_config_set_in_file(file_name.buf, "core.worktree",
> -                              relative_path(real_work_tree, git_dir,
> -                                            &rel_path));
> +       strbuf_addstr(&path, relative_path(real_work_tree, git_dir,
> +                                          &rel_path));
> +       cp.git_cmd = 1;
> +       argv_array_pushl(&cp.args, "-C", work_tree, NULL);
> +       argv_array_pushl(&cp.args, "--work-tree", ".", NULL);
> +       argv_array_pushl(&cp.args, "config", "core.worktree", path.buf, NULL);
> +       if (run_command(&cp) < 0)
> +               die(_("failed to update core.worktree for %s"), git_dir);

Do we need to make this conditional on the extensions.worktreeConfig
variable, though? When I just run

    git config --worktree . foo bar
fatal: Per-worktree configuration requires extensions.worktreeConfig
Please read section CONFIGURATION in `git help worktree` before
enabling it.

which would trigger the failure here?

>
>         strbuf_release(&file_name);
> +       strbuf_release(&path);
>         strbuf_release(&rel_path);
>         free((void *)real_work_tree);
>  }
> --
> 2.9.1.566.gbd532d4
>

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 17:24     ` [PATCH v4 3/4] submodule: support running in multiple worktree setup Nguyễn Thái Ngọc Duy
@ 2016-07-20 23:22       ` Stefan Beller
  2016-07-22  0:37         ` Stefan Beller
                           ` (3 more replies)
  2016-07-27  4:10       ` Max Kirillov
  1 sibling, 4 replies; 31+ messages in thread
From: Stefan Beller @ 2016-07-20 23:22 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, Jonathan Nieder
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-worktree.txt | 8 ++++++++
>  git-submodule.sh               | 8 ++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 41350db..2a5661d 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -142,6 +142,14 @@ to share to all working directories:
>     you are sure you always use sparse checkout for all working
>     directories.
>
> + - `submodule.*` in current state should not be shared because the
> +   information is tied to a particular version of .gitmodules in a
> +   working directory.

While the submodule.* settings are copied from the .gitmodules file initially,
they can be changed in the config later. (That was actually the whole
point of it,
so you can change the submodule remotes URL without having to change history.)

And I would think that most submodule related settings (such as remote URL,
name, path, even depth recommendation) should be the same for all worktrees,
and a different value for one worktree is a carefully crafted
exception by the user.

So while the .gitmodules file can diverge in the work trees I do not
think that the
actual remotes for the submodules in the different worktrees differ
though. The change
of the .gitmodule files may be because you checked out an old commit, that
has outdated information on where to get the submodule from.

> +
> + - `remote.*` added by submodules may be per working directory as
> +   well, unless you are sure remotes from all possible submodules in
> +   history are consistent.
> +
>  DETAILS
>  -------
>  Each linked working tree has a private sub-directory in the repository's
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 4ec7546..7b576f5 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -261,7 +261,7 @@ or you are unsure what this means choose another name with the '--name' option."
>                         esac
>                 ) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
>         fi
> -       git config submodule."$sm_name".url "$realrepo"
> +       git config --worktree submodule."$sm_name".url "$realrepo"

This is in cmd_add. Actually I think this should be --not-worktree
(i.e. --local)
as when you add a submodule in one worktree, and then in another,
you may want to have the same URL. However if another worktree
already configured it you want to keep the option.
so rather:

  if git config  submodule."$sm_name".url then
      # it exists, do nothing
  else
    # it does not exist
    git config --local ...

>
>         git add $force "$sm_path" ||
>         die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
> @@ -461,7 +461,7 @@ Submodule work tree '\$displaypath' contains a .git directory
>                         # Remove the whole section so we have a clean state when
>                         # the user later decides to init this submodule again
>                         url=$(git config submodule."$name".url)
> -                       git config --remove-section submodule."$name" 2>/dev/null &&
> +                       git config --worktree --remove-section submodule."$name" 2>/dev/null &&
>                         say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"

This is in cmd_deinit, which is documented as:
           Unregister the given submodules, i.e. remove the whole
           submodule.$name section from .git/config together with their work
           tree. Further calls to git submodule update, git submodule foreach
           and git submodule sync will skip any unregistered submodules until
           they are initialized again, so use this command if you don’t want
           to have a local checkout of the submodule in your work tree
           anymore. If you really want to remove a submodule from the
           repository and commit that use git-rm(1) instead.

So one might wonder if the unregister should be a global unregister
or a worktree specific unregister.

From a users POV there are:
* non existent submodules (no gitlink recorded, no config set,
  no repo in place)
* not initialized submodules (gitlink is recorded, no config set,
  and an empty repo is put in the working tree as a place holder).
* initialized submodules (gitlink is recorded, the config
  submodule .<name>.url is copied from the .gitmodules file to .git/config.
  an empty dir in the working tree as a place holder)
  A user may change the configuration before the next step as the url in
  the .gitmodules file may be wrong and the user doesn't want to
  rewrite history
* existing submodules (gitlink is recorded, the config option is set
  and instead of an empty placeholder dir, we actually have a git
  repo there.)
* matching submodules (the recorded git link matches
  the actual checked out state of the repo!, config option and repo exist)

I made up these terms for these 5 states and they don't appear in any
documentation, but I think that is the exhaustive list of what a submodule
should be, when using the git-submodule command properly.

There can be more things though. As we have three indicators (existence of
gitlink, config option and repo), we can have up to 8 states, I left some out in
the above listing.

* gitlink recorded, no config set, repo is there and maybe even matches the
  recorded git link.
   What a strange thing! git treats that as a "not initialized" from above.
* no gitlink exists, no config exists, but a repo exists:
  That's just an embedded repo, never touch it!
* no gitlink, no repo, but a config exists: just a stray old config
laying around,
  ignore it
* no gitlink, but a config and a repo exists: "A deleted submodule", use
  rm -rf to purge it.

--------
The above was a wall of text to make myself aware of the pitfalls of submodules.
---

A discussion with Jonathan Nieder in office ensued and we came to the following
conclusions:
* Anything except the "existence in the working tree" shall be shared,
i.e. is a repository
  specific, not working tree specific setting.

Currently we use the submodule."<name>".url config option to determine
the existence of
a submodule (init vs non init; if init -> git submodule update will
(maybe fetch) and checkout
accordingly).

As an intermediate step forward we could do:
* introduce a submodule."<name>".existsInWorktree = [true/false]
setting, that decouples
  the check for existence from the url being present. That is the only
option per worktree, all
  other submodule."<name>".* options are shared if not overwritten
manually with care

* another approach would be to have a
submodule.includeInWorktree=<pathspec> option
  which is similar, but has slightly different naming.

Looking back at origin/sb/submodule-default-path (which is in pu for a
long time now),
which allowed a configuration submodule.defaultUpdatePath <pathspec>,
that could be
used with `git submodule update --init-default-path`.

----
So maybe we want to drop that series and first talk about a migration plan from
the current state to a world where we have the existence depending not
on the url
parameter, but a boolean variable submodule.<name>.<good_name>.
Depending on <good_name> a submodule would be ignored or tried to checkout
in e.g. `submodule update`

---
If we want to move the current behavior of submodules forward, we
would want to have
anything but the url as shared variables and then use the url variable
as a per-worktree
existence flag.

Thanks,
Stefan

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 23:22       ` Stefan Beller
@ 2016-07-22  0:37         ` Stefan Beller
  2016-07-22  7:32         ` Jens Lehmann
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 31+ messages in thread
From: Stefan Beller @ 2016-07-22  0:37 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, Jonathan Nieder
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

FYI: I started working on a series that decouples existence of a
submodule from the URL
as a preparatory series to this one. Then we can have the same URL in
all working trees, but
the existence is configured differently for each working tree.

I'll try to send it out tomorrow.

Thanks,
Stefan

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 23:22       ` Stefan Beller
  2016-07-22  0:37         ` Stefan Beller
@ 2016-07-22  7:32         ` Jens Lehmann
  2016-07-22 16:07           ` Stefan Beller
  2016-07-22 16:55         ` Junio C Hamano
  2016-07-22 17:09         ` Duy Nguyen
  3 siblings, 1 reply; 31+ messages in thread
From: Jens Lehmann @ 2016-07-22  7:32 UTC (permalink / raw)
  To: Stefan Beller, Nguyễn Thái Ngọc Duy,
	Jonathan Nieder
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Lars Schneider, Michael Haggerty

Am 21.07.2016 um 01:22 schrieb Stefan Beller:
> So maybe we want to drop that series and first talk about a migration plan from
> the current state to a world where we have the existence depending not
> on the url
> parameter, but a boolean variable submodule.<name>.<good_name>.
> Depending on <good_name> a submodule would be ignored or tried to checkout
> in e.g. `submodule update`

Whoa, that's a very intrusive change with a ton of compatibility
problems waiting to happen. Maybe its simpler to make "git submodule
sync" aware of worktrees and error out with an "you cannot use
submodules with different URLs in a worktree scenario" error when
the URL is going to change? That should make most use cases work
while avoiding the problematic ones.

> If we want to move the current behavior of submodules forward, we
> would want to have
> anything but the url as shared variables and then use the url variable
> as a per-worktree
> existence flag.

Without having though deeply about all submodule variables, I see
them as worktree specific. E.g. "update=none" is used on our CI-
Server to avoid the disk space cost on some checkouts of a certain
superproject while using "update=checkout" on others where their
content is needed.

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22  7:32         ` Jens Lehmann
@ 2016-07-22 16:07           ` Stefan Beller
  0 siblings, 0 replies; 31+ messages in thread
From: Stefan Beller @ 2016-07-22 16:07 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Nguyễn Thái Ngọc Duy, Jonathan Nieder,
	git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Lars Schneider, Michael Haggerty

On Fri, Jul 22, 2016 at 12:32 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 21.07.2016 um 01:22 schrieb Stefan Beller:
>>
>> So maybe we want to drop that series and first talk about a migration plan
>> from
>> the current state to a world where we have the existence depending not
>> on the url
>> parameter, but a boolean variable submodule.<name>.<good_name>.
>> Depending on <good_name> a submodule would be ignored or tried to checkout
>> in e.g. `submodule update`
>
>
> Whoa, that's a very intrusive change with a ton of compatibility
> problems waiting to happen. Maybe its simpler to make "git submodule
> sync" aware of worktrees and error out with an "you cannot use
> submodules with different URLs in a worktree scenario" error when
> the URL is going to change? That should make most use cases work
> while avoiding the problematic ones.

I think fixing sync alone is just a drop of water on the oven.
Actually I can think of scenarios that have different URLs for different
worktrees (think of the automatic CI thing that should only fetch from
an internal server, whereas the dev-checkout fetches from upstream)
Actually each config variable (including the update strategy as you
mention below, but also the depth, branch, path) may be different in
one work tree.

I do not want to forbid the existence of different settings (URLs)
per worktree. Rather I think a different setting is a user decision,
hence they will want to run "git config --worktree ..."

And one of the unfortunate things is the coupling of existence of a
submodule and the URL. If that were to be decoupled, you could do
a "git config --worktree submodule.<name>.exists true" (or it is wrapped
fancily in "git submodule init") and the URL would not have to be copied
from the .gitmodules file.

I agree that this is a breaking change, which is why I'd guard it with
a config option such that the user can make the choice if they want
to go with the old behavior or the new behavior.


>
>> If we want to move the current behavior of submodules forward, we
>> would want to have
>> anything but the url as shared variables and then use the url variable
>> as a per-worktree
>> existence flag.
>
>
> Without having though deeply about all submodule variables, I see
> them as worktree specific. E.g. "update=none" is used on our CI-
> Server to avoid the disk space cost on some checkouts of a certain
> superproject while using "update=checkout" on others where their
> content is needed.

But this is a conscious user choice, so you would have configured
that on a per-worktree basis anyway?
i.e. it seems to me as if "update=checkout" is a default that is good
for all but one worktree, so why would you want to configure that n times
instead of just once as default?
The non default behavior is then overwritten in the specific worktree.

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 23:22       ` Stefan Beller
  2016-07-22  0:37         ` Stefan Beller
  2016-07-22  7:32         ` Jens Lehmann
@ 2016-07-22 16:55         ` Junio C Hamano
  2016-07-22 17:40           ` Stefan Beller
  2016-07-22 17:09         ` Duy Nguyen
  3 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2016-07-22 16:55 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Nguyễn Thái Ngọc Duy, Jonathan Nieder,
	git@vger.kernel.org, Max Kirillov, Michael J Gruber, Jens Lehmann,
	Lars Schneider, Michael Haggerty

Stefan Beller <sbeller@google.com> writes:

> From a users POV there are:
> * non existent submodules (no gitlink recorded, no config set,
>   no repo in place)
> * not initialized submodules (gitlink is recorded, no config set,
>   and an empty repo is put in the working tree as a place holder).

This is no different from what you later call "embedded".  The only
difference is that embedded thing hasn't seen its initial commit.

> * initialized submodules (gitlink is recorded, the config
>   submodule .<name>.url is copied from the .gitmodules file to .git/config.
>   an empty dir in the working tree as a place holder)
>   A user may change the configuration before the next step as the url in
>   the .gitmodules file may be wrong and the user doesn't want to
>   rewrite history

i.e. what "submodule init" gives you.

> * existing submodules (gitlink is recorded, the config option is set
>   and instead of an empty placeholder dir, we actually have a git
>   repo there.)

i.e. what "submodule update" after "submodule init" gives you.

> * matching submodules (the recorded git link matches
>   the actual checked out state of the repo!, config option and repo exist)

Is this any different from "existing" case for the purpose of
discussing the interaction between a submodule (and its checkout)
and having possibly multiple worktrees of its superproject?

I agree that when a top-level superproject has multiple worktrees
these multiple worktrees may want to have the same submodule in
different states, but I'd imagine that they want to share the same
physical repository (i.e. $GIT_DIR/modules/$name of the primary
worktree of the superproject)---is everybody involved in the
discussion share this assumption?

Assuming that everybody is on the same page, that means "do we have
the repository for that submodule, and if so where in our local
filesystem?" is a bit of information shared across the worktrees of
the superproject.  And the "name" used to identify the submodule is
also shared across these worktrees of the superproject, as it is
meant to be a unique (within the superproject) identifier for that
"other" project it uses, no matter where in the superproject's
working tree (note: this is "working tree", not "worktree") it would
be checked out, and where the upstream URL to get further updates to
the submodule is (i.e. that URL may change over time if they relocate,
or it may even change when the user who works on the superproject
decides to use a different mirror).

What can be different between the instantiation of the same
submodule in these multiple worktrees, and how they should be
recorded?

 * submodule.$name.URL?  I am not sure if we want to have different
   "upstreams" depending on the worktree of the superproject.  While
   there is no fundamental reason to forbid it, having to maintain a
   single local repository for a submodule would mean they would
   need to be treated as separate "remotes" in the submodule
   repository.

 * submodule.$name.path of course can be different depending on
   which commit of the superproject is checked out in the worktree,
   as the superproject may move the submodule binding site across
   its versions.

 * submodule.$name.update, submodule.$name.ignore,
   submodule.$name.branch, etc. would need to be all different among
   worktrees of the superproject, as that is the whole point of
   being able to work on separate branches of the superproject in
   separate worktrees.

Somewhere in this discussion thread, you present the conclusion of
your discussion with Jonathan Nieder that there needs a separate
"should the submodule directory be populated?" bit, which currently
is tied to submodule.$name.URL in $GIT_DIR/config.  I tend to agree
that knowing where you get other people's update of that submodule
repository should come from and wanting to have/keep a checkout of
that submodule in the working tree of a particular worktree are two
different things, so such a separate bit would be needed, and that
would belong to per-worktree configuration.



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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 23:22       ` Stefan Beller
                           ` (2 preceding siblings ...)
  2016-07-22 16:55         ` Junio C Hamano
@ 2016-07-22 17:09         ` Duy Nguyen
  2016-07-22 17:25           ` Stefan Beller
  3 siblings, 1 reply; 31+ messages in thread
From: Duy Nguyen @ 2016-07-22 17:09 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Thu, Jul 21, 2016 at 1:22 AM, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
> <pclouds@gmail.com> wrote:
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  Documentation/git-worktree.txt | 8 ++++++++
>>  git-submodule.sh               | 8 ++++----
>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
>> index 41350db..2a5661d 100644
>> --- a/Documentation/git-worktree.txt
>> +++ b/Documentation/git-worktree.txt
>> @@ -142,6 +142,14 @@ to share to all working directories:
>>     you are sure you always use sparse checkout for all working
>>     directories.
>>
>> + - `submodule.*` in current state should not be shared because the
>> +   information is tied to a particular version of .gitmodules in a
>> +   working directory.
>
> While the submodule.* settings are copied from the .gitmodules file initially,
> they can be changed in the config later. (That was actually the whole
> point of it,
> so you can change the submodule remotes URL without having to change history.)
>
> And I would think that most submodule related settings (such as remote URL,
> name, path, even depth recommendation) should be the same for all worktrees,
> and a different value for one worktree is a carefully crafted
> exception by the user.
>
> So while the .gitmodules file can diverge in the work trees I do not
> think that the
> actual remotes for the submodules in the different worktrees differ
> though. The change
> of the .gitmodule files may be because you checked out an old commit, that
> has outdated information on where to get the submodule from.

I just quickly glanced through the rest of this mail because, as a
submodule ignorant, it's just mumbo jumbo to me. But what I see here
is, there may be problems if we choose to share some submodule info,
but I haven't seen any good thing from sharing any submodule info at
all.

I can imagine long term you may want to just clone a submodule repo
once and share it across worktrees that use it, so maybe it's just me
not seeing things and this may be a step towards that.

Anyway, I assume some people will be working on the submodule side.
And because I have not heard any bad thing about the new config
design, I'm going to drop submodule patches from this series and focus
on polishing config stuff.
-- 
Duy

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

* Re: [PATCH v4 2/4] submodule: update core.worktree using git-config
  2016-07-20 22:04       ` Stefan Beller
@ 2016-07-22 17:15         ` Duy Nguyen
  0 siblings, 0 replies; 31+ messages in thread
From: Duy Nguyen @ 2016-07-22 17:15 UTC (permalink / raw)
  To: Stefan Beller
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

On Thu, Jul 21, 2016 at 12:04 AM, Stefan Beller <sbeller@google.com> wrote:
>> diff --git a/submodule.c b/submodule.c
>> index abc2ac2..b912871 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -1128,7 +1128,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
>>  {
>>         struct strbuf file_name = STRBUF_INIT;
>>         struct strbuf rel_path = STRBUF_INIT;
>> +       struct strbuf path = STRBUF_INIT;
>>         const char *real_work_tree = xstrdup(real_path(work_tree));
>> +       struct child_process cp = CHILD_PROCESS_INIT;
>>
>>         /* Update gitfile */
>>         strbuf_addf(&file_name, "%s/.git", work_tree);
>> @@ -1136,13 +1138,17 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
>>                    relative_path(git_dir, real_work_tree, &rel_path));
>>
>>         /* Update core.worktree setting */
>> -       strbuf_reset(&file_name);
>> -       strbuf_addf(&file_name, "%s/config", git_dir);
>> -       git_config_set_in_file(file_name.buf, "core.worktree",
>> -                              relative_path(real_work_tree, git_dir,
>> -                                            &rel_path));
>> +       strbuf_addstr(&path, relative_path(real_work_tree, git_dir,
>> +                                          &rel_path));
>> +       cp.git_cmd = 1;
>> +       argv_array_pushl(&cp.args, "-C", work_tree, NULL);
>> +       argv_array_pushl(&cp.args, "--work-tree", ".", NULL);
>> +       argv_array_pushl(&cp.args, "config", "core.worktree", path.buf, NULL);
>> +       if (run_command(&cp) < 0)
>> +               die(_("failed to update core.worktree for %s"), git_dir);
>
> Do we need to make this conditional on the extensions.worktreeConfig
> variable, though? When I just run
>
>     git config --worktree . foo bar
> fatal: Per-worktree configuration requires extensions.worktreeConfig
> Please read section CONFIGURATION in `git help worktree` before
> enabling it.
>
> which would trigger the failure here?

It was intended, but I was probably just paranoid. The thinking back
then was, you are switching from "share whole config" to "not share
something". This should not be taken lightly and you should examine
your config file and decide what to share, before making the switch.
It's dangerous!

But then, if everything has been shared before (assuming there are
more than one worktree) and you are probably happy with it (or you
would have made done something to unshare), so it's probably good to
keep on sharing. Which means we can set extensions.worktreeConfig
automatically here (when "git config --worktree" is used) instead of
dying. We would need to move core.bare and core.worktree to main
worktree, but that's manageable.

So in short, you would not see this message in this context in future again.
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22 17:09         ` Duy Nguyen
@ 2016-07-22 17:25           ` Stefan Beller
  2016-07-22 17:42             ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Beller @ 2016-07-22 17:25 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Fri, Jul 22, 2016 at 10:09 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>
> I just quickly glanced through the rest of this mail because, as a
> submodule ignorant, it's just mumbo jumbo to me. But what I see here
> is, there may be problems if we choose to share some submodule info,
> but I haven't seen any good thing from sharing any submodule info at
> all.

Okay. :(

I assume the sharing is beneficial. (As a work-tree ignorant) I thought
we had this main work tree, which also holds the repository, whereas
the other working trees have a light weight implementation (e.g. just
a .git file pointing back to the main working tree/git dir).

So in a way my mental model is more like the config sharing here:
You can configure things in ~/.gitconfig for example that have effects
on more than one repo. Similarly you would want to configure things
in one repo, that has effect on more than one working tree?

And my assumption was to have the repository specific parts be shared,
whereas the working tree specific things should not be shared.

By working tree specific I strongly mean:

* existence in the working tree
* the checked out sha1
* submodule.$name.path

By repository specific I strongly mean:

* the submodule URL

I am not sure about:

* submodule.$name.update, submodule.$name.ignore,
   submodule.$name.branch,
  These have to be able to be different across working trees, but do we
  require them to be set for each working tree individually?  I thought a
  repo wide setup with defaults may be ok?

>
> I can imagine long term you may want to just clone a submodule repo
> once and share it across worktrees that use it, so maybe it's just me
> not seeing things and this may be a step towards that.

Just as Junio put it:
> I agree that when a top-level superproject has multiple worktrees
> these multiple worktrees may want to have the same submodule in
> different states, but I'd imagine that they want to share the same
> physical repository (i.e. $GIT_DIR/modules/$name of the primary
> worktree of the superproject)---is everybody involved in the
> discussion share this assumption?

I agree with that as well.

>
> Anyway, I assume some people will be working on the submodule side.

Once the discussion comes to a rough agreement, I'll give it a shot.

> And because I have not heard any bad thing about the new config
> design, I'm going to drop submodule patches from this series and focus
> on polishing config stuff.

Oh, sorry for not focusing on that part. The design of git config --worktree
is sound IMO.

> --
> Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22 16:55         ` Junio C Hamano
@ 2016-07-22 17:40           ` Stefan Beller
  2016-07-25 15:46             ` Junio C Hamano
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Beller @ 2016-07-22 17:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc, Jonathan Nieder,
	git@vger.kernel.org, Max Kirillov, Michael J Gruber, Jens Lehmann,
	Lars Schneider, Michael Haggerty

On Fri, Jul 22, 2016 at 9:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> From a users POV there are:
>> * non existent submodules (no gitlink recorded, no config set,
>>   no repo in place)
>> * not initialized submodules (gitlink is recorded, no config set,
>>   and an empty repo is put in the working tree as a place holder).

I meant empty directory, not empty repo.

>
> This is no different from what you later call "embedded".  The only
> difference is that embedded thing hasn't seen its initial commit.

That did not occur to me.
The "not initialized" is what you'd get via

    git clone --no-recurse repo-with-submodules

whereas the "embedded" could come from

   git clone <repo with no submodules> tmp
   cd tmp && git clone <another repo, maybe unrelated>

>
>> * initialized submodules (gitlink is recorded, the config
>>   submodule .<name>.url is copied from the .gitmodules file to .git/config.
>>   an empty dir in the working tree as a place holder)
>>   A user may change the configuration before the next step as the url in
>>   the .gitmodules file may be wrong and the user doesn't want to
>>   rewrite history
>
> i.e. what "submodule init" gives you.

Right.

>
>> * existing submodules (gitlink is recorded, the config option is set
>>   and instead of an empty placeholder dir, we actually have a git
>>   repo there.)
>
> i.e. what "submodule update" after "submodule init" gives you.

Right.

>
>> * matching submodules (the recorded git link matches
>>   the actual checked out state of the repo!, config option and repo exist)
>
> Is this any different from "existing" case for the purpose of
> discussing the interaction between a submodule (and its checkout)
> and having possibly multiple worktrees of its superproject?

I don't think so.

>
> I agree that when a top-level superproject has multiple worktrees
> these multiple worktrees may want to have the same submodule in
> different states, but I'd imagine that they want to share the same
> physical repository (i.e. $GIT_DIR/modules/$name of the primary
> worktree of the superproject)---is everybody involved in the
> discussion share this assumption?

At least me agrees.

>
> Assuming that everybody is on the same page, that means "do we have
> the repository for that submodule, and if so where in our local
> filesystem?" is a bit of information shared across the worktrees of
> the superproject.  And the "name" used to identify the submodule is
> also shared across these worktrees of the superproject, as it is
> meant to be a unique (within the superproject) identifier for that
> "other" project it uses, no matter where in the superproject's
> working tree (note: this is "working tree", not "worktree") it would
> be checked out, and where the upstream URL to get further updates to
> the submodule is (i.e. that URL may change over time if they relocate,
> or it may even change when the user who works on the superproject
> decides to use a different mirror).

I agree.

>
> What can be different between the instantiation of the same
> submodule in these multiple worktrees, and how they should be
> recorded?
>
>  * submodule.$name.URL?  I am not sure if we want to have different
>    "upstreams" depending on the worktree of the superproject.  While
>    there is no fundamental reason to forbid it, having to maintain a
>    single local repository for a submodule would mean they would
>    need to be treated as separate "remotes" in the submodule
>    repository.

You can only have a remote if the the submodule repo exists already.
I guess that can be made a requirement.

So setting up the worktrees and submodule URLs in the config and
then doing the clone of said submodule is maybe not encouraged.

>
>  * submodule.$name.path of course can be different depending on
>    which commit of the superproject is checked out in the worktree,
>    as the superproject may move the submodule binding site across
>    its versions.

Right.

>
>  * submodule.$name.update, submodule.$name.ignore,
>    submodule.$name.branch, etc. would need to be all different among
>    worktrees of the superproject, as that is the whole point of
>    being able to work on separate branches of the superproject in
>    separate worktrees.

What do you mean by "would need". The ability to be different or rather
the veto of an 'inheritance' of defaults from the repository configuration?

>
> Somewhere in this discussion thread, you present the conclusion of
> your discussion with Jonathan Nieder that there needs a separate
> "should the submodule directory be populated?" bit, which currently
> is tied to submodule.$name.URL in $GIT_DIR/config.

I'll try to get the discussion back on list and whenever Jonathan starts talking
off list, I'll poke him with a stick.

>  I tend to agree
> that knowing where you get other people's update of that submodule
> repository should come from and wanting to have/keep a checkout of
> that submodule in the working tree of a particular worktree are two
> different things, so such a separate bit would be needed, and that
> would belong to per-worktree configuration.
>

Okay. How would you disentangle these two things?

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22 17:25           ` Stefan Beller
@ 2016-07-22 17:42             ` Duy Nguyen
  2016-07-25 23:25               ` Stefan Beller
  0 siblings, 1 reply; 31+ messages in thread
From: Duy Nguyen @ 2016-07-22 17:42 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Fri, Jul 22, 2016 at 7:25 PM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Jul 22, 2016 at 10:09 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>
>> I just quickly glanced through the rest of this mail because, as a
>> submodule ignorant, it's just mumbo jumbo to me. But what I see here
>> is, there may be problems if we choose to share some submodule info,
>> but I haven't seen any good thing from sharing any submodule info at
>> all.
>
> Okay. :(

Didn't mean to make you feel sad :)


> I assume the sharing is beneficial. (As a work-tree ignorant) I thought
> we had this main work tree, which also holds the repository, whereas
> the other working trees have a light weight implementation (e.g. just
> a .git file pointing back to the main working tree/git dir).

The main worktree is special for historical reason. But from the user
point of view (and even developer's at a certain level) they should be
treated equally. Think of it like cloning the same repo multiple
times. Only now you save disk space because there's only one object
database.

> So in a way my mental model is more like the config sharing here
> You can configure things in ~/.gitconfig for example that have effects
> on more than one repo. Similarly you would want to configure things
> in one repo, that has effect on more than one working tree?
>
> And my assumption was to have the repository specific parts be shared,
> whereas the working tree specific things should not be shared.

I think that's a good assumption. Although I would rather be not
sharing by default and let the user initiate it when they want to
share something. Like ~/..gitconfig, we never write anything there
unless the user asks us to explicitly (with git config --user).
Accidental share could have negative effect.

>> I can imagine long term you may want to just clone a submodule repo
>> once and share it across worktrees that use it, so maybe it's just me
>> not seeing things and this may be a step towards that.
>
> Just as Junio put it:
>> I agree that when a top-level superproject has multiple worktrees
>> these multiple worktrees may want to have the same submodule in
>> different states, but I'd imagine that they want to share the same
>> physical repository (i.e. $GIT_DIR/modules/$name of the primary
>> worktree of the superproject)---is everybody involved in the
>> discussion share this assumption?
>
> I agree with that as well.

Yeah. We have a long way to go though. As I see it, you may need ref
namespace as well (so they look like separate clones), which has never
been used on the client side before. Either that or odb alternates...

>> And because I have not heard any bad thing about the new config
>> design, I'm going to drop submodule patches from this series and focus
>> on polishing config stuff.
>
> Oh, sorry for not focusing on that part. The design of git config --worktree
> is sound IMO.

This makes me happy (I know other people can still find flaws in it,
and I'm ok with that). This config split thing has been wrecking my
brain for a long time, find the the "right" way to do with minimum
impacts :)
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22 17:40           ` Stefan Beller
@ 2016-07-25 15:46             ` Junio C Hamano
  0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2016-07-25 15:46 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Nguyễn Thái Ngọc, Jonathan Nieder,
	git@vger.kernel.org, Max Kirillov, Michael J Gruber, Jens Lehmann,
	Lars Schneider, Michael Haggerty

Stefan Beller <sbeller@google.com> writes:

> On Fri, Jul 22, 2016 at 9:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>  * submodule.$name.update, submodule.$name.ignore,
>>    submodule.$name.branch, etc. would need to be all different among
>>    worktrees of the superproject, as that is the whole point of
>>    being able to work on separate branches of the superproject in
>>    separate worktrees.
>
> What do you mean by "would need". The ability to be different or rather
> the veto of an 'inheritance' of defaults from the repository configuration?

They have to be able to represent different settings per worktree
that checks out different branches/commits of superproject.  They
may happen to be set the same, but they do not have to be.

Is what I meant.

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-22 17:42             ` Duy Nguyen
@ 2016-07-25 23:25               ` Stefan Beller
  2016-07-26 17:20                 ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Beller @ 2016-07-25 23:25 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Fri, Jul 22, 2016 at 10:42 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Jul 22, 2016 at 7:25 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Fri, Jul 22, 2016 at 10:09 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>>
>>> I just quickly glanced through the rest of this mail because, as a
>>> submodule ignorant, it's just mumbo jumbo to me. But what I see here
>>> is, there may be problems if we choose to share some submodule info,
>>> but I haven't seen any good thing from sharing any submodule info at
>>> all.
>>
>> Okay. :(
>
> Didn't mean to make you feel sad :)

I was using the :( a bit carelessly here. I was quite surprised that you
"haven't seen any good thing from sharing any submodule info at all."

So what is the design philosophy in worktrees? How much independence does
one working tree have?

So here is what I did:
 *  s/git submodule init/git submodule update --init/
 * added a test_pause to the last test on the last line
 * Then:

$ find . |grep da5e6058
./addtest/.git/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
./addtest/.git/worktrees/super-elsewhere/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
./addtest/.git/worktrees/super-elsewhere/modules/submod2/objects/08/da5e6058267d6be703ae058d173ce38ed53066
./.git/objects/08/da5e6058267d6be703ae058d173ce38ed53066

The last entry is the "upstream" for the addtest clone, so that is fine.
However inside the ./addtest/ (and its worktrees, which currently are
embedded in there?) we only want to have one object store for a given
submodule?

>
>> I assume the sharing is beneficial. (As a work-tree ignorant) I thought
>> we had this main work tree, which also holds the repository, whereas
>> the other working trees have a light weight implementation (e.g. just
>> a .git file pointing back to the main working tree/git dir).
>
> The main worktree is special for historical reason. But from the user
> point of view (and even developer's at a certain level) they should be
> treated equally. Think of it like cloning the same repo multiple
> times. Only now you save disk space because there's only one object
> database.

That's what we want for submodules too, see above?

>
>> So in a way my mental model is more like the config sharing here
>> You can configure things in ~/.gitconfig for example that have effects
>> on more than one repo. Similarly you would want to configure things
>> in one repo, that has effect on more than one working tree?
>>
>> And my assumption was to have the repository specific parts be shared,
>> whereas the working tree specific things should not be shared.
>
> I think that's a good assumption. Although I would rather be not
> sharing by default and let the user initiate it when they want to
> share something. Like ~/..gitconfig, we never write anything there
> unless the user asks us to explicitly (with git config --user).
> Accidental share could have negative effect.

Okay, got it.

>
>>> I can imagine long term you may want to just clone a submodule repo
>>> once and share it across worktrees that use it, so maybe it's just me
>>> not seeing things and this may be a step towards that.
>>
>> Just as Junio put it:
>>> I agree that when a top-level superproject has multiple worktrees
>>> these multiple worktrees may want to have the same submodule in
>>> different states, but I'd imagine that they want to share the same
>>> physical repository (i.e. $GIT_DIR/modules/$name of the primary
>>> worktree of the superproject)---is everybody involved in the
>>> discussion share this assumption?
>>
>> I agree with that as well.
>
> Yeah. We have a long way to go though. As I see it, you may need ref
> namespace as well (so they look like separate clones), which has never
> been used on the client side before. Either that or odb alternates...
>
>>> And because I have not heard any bad thing about the new config
>>> design, I'm going to drop submodule patches from this series and focus
>>> on polishing config stuff.
>>
>> Oh, sorry for not focusing on that part. The design of git config --worktree
>> is sound IMO.
>
> This makes me happy (I know other people can still find flaws in it,
> and I'm ok with that). This config split thing has been wrecking my
> brain for a long time, find the the "right" way to do with minimum
> impacts :)

After playing with this series a bit more, I actually like the UI as it is an
easy mental model "submodules behave completely independent".

However in 3/4 you said:

+ - `submodule.*` in current state should not be shared because the
+   information is tied to a particular version of .gitmodules in a
+   working directory.

This is already a problem with say different branches/versions.
That has been solved by duplicating that information to .git/config
as a required step. (I don't like that approach, as it is super confusing
IMHO)

+
+ - `remote.*` added by submodules may be per working directory as
+   well, unless you are sure remotes from all possible submodules in
+   history are consistent.
+

Same as above.

I planned to have a series out last Friday, but it took longer than expected
as it was unclear how to best proceed and I ran into problems solving
"trivial issues".

I am back to the drawing board for the submodule side of things,
but I guess this series could be used once we figure out how to
have just one object database for a submodule.

Thanks,
Stefan

> --
> Duy

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

* Re: [PATCH v4 1/4] worktree: add per-worktree config files
  2016-07-20 17:24     ` [PATCH v4 1/4] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
@ 2016-07-26  0:59       ` Stefan Beller
  2016-07-26 15:04         ` Duy Nguyen
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Beller @ 2016-07-26  0:59 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

On Wed, Jul 20, 2016 at 10:24 AM, 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.
>
> 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>

I like the user facing design, but how am I supposed to use it internally?

Say I want to read a value preferably from the worktree I'd do a
    /*
     * maybe I don't even have to set it to 1 as
     * the user is supposed to do that?
     */
    repository_format_worktree_config = 1;
    git_config_get_{string,bool,int} (... as usual ...)

and if I want to read the value globally I would set the variable to 0
and read? (I would need to restore it, so I'll have a temporary variable
to keep the original value of repository_format_worktree_config)

Thanks,
Stefan


> ---
>  Documentation/config.txt               | 11 ++++-
>  Documentation/git-config.txt           | 26 ++++++++----
>  Documentation/git-worktree.txt         | 31 ++++++++++++++
>  Documentation/gitrepository-layout.txt |  8 ++++
>  builtin/config.c                       | 18 +++++++-
>  cache.h                                |  2 +
>  config.c                               |  7 ++++
>  environment.c                          |  1 +
>  setup.c                                |  5 ++-
>  t/t2028-worktree-config.sh (new +x)    | 77 ++++++++++++++++++++++++++++++++++
>  10 files changed, 175 insertions(+), 11 deletions(-)
>  create mode 100755 t/t2028-worktree-config.sh
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 16dc22d..7d64da0 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.
> @@ -264,6 +265,12 @@ advice.*::
>                 show directions on how to proceed from the current state.
>  --
>
> +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 f163113..9dfdb6a 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -47,13 +47,15 @@ checks or transformations are performed on the value.
>
>  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:
> @@ -133,6 +135,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.
> @@ -253,6 +260,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
> @@ -268,9 +279,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 7c4cfb0..41350db 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -111,6 +111,37 @@ OPTIONS
>  --expire <time>::
>         With `prune`, only expire unused working trees older than <time>.
>
> +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 XXX 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.
> +
>  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 577ee84..6cfdb4c 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'.
> @@ -276,6 +281,9 @@ worktrees/<id>/link::
>         file. It is used to detect if the linked repository is
>         manually removed.
>
> +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 1d7c6ef..535707c 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -4,6 +4,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>]"),
> @@ -23,6 +24,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, types;
>  static const char *get_color_slot, *get_colorbool_slot;
> @@ -57,6 +59,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")),
> @@ -491,6 +494,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_with_options(builtin_config_usage, builtin_config_options);
> @@ -525,7 +529,19 @@ 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) {
> +               if (repository_format_worktree_config)
> +                       given_config_source.file = git_pathdup("config.worktree");
> +               else {
> +                       struct worktree **worktrees = get_worktrees();
> +                       if (worktrees[0] && worktrees[1])
> +                               die(_("Per-worktree configuration requires extensions.worktreeConfig\n"
> +                                     "Please read section CONFIGURATION in `git help worktree` before\n"
> +                                     "enabling it."));
> +                       free_worktrees(worktrees);
> +                       given_config_source.file = git_pathdup("config");
> +               }
> +       } else if (given_config_source.file) {
>                 if (!is_absolute_path(given_config_source.file) && prefix)
>                         given_config_source.file =
>                                 xstrdup(prefix_filename(prefix,
> diff --git a/cache.h b/cache.h
> index f1dc289..606500e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -757,10 +757,12 @@ extern int grafts_replace_parents;
>  #define GIT_REPO_VERSION 0
>  #define GIT_REPO_VERSION_READ 1
>  extern int repository_format_precious_objects;
> +extern int repository_format_worktree_config;
>
>  struct repository_format {
>         int version;
>         int precious_objects;
> +       int worktree_config;
>         int is_bare;
>         char *work_tree;
>         struct string_list unknown_extensions;
> diff --git a/config.c b/config.c
> index bea937e..99ff6be 100644
> --- a/config.c
> +++ b/config.c
> @@ -1254,6 +1254,13 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
>         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 ca72464..b4d56ef 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -26,6 +26,7 @@ int warn_ambiguous_refs = 1;
>  int warn_on_object_refname_ambiguity = 1;
>  int ref_paranoia = -1;
>  int repository_format_precious_objects;
> +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 6d0e0c9..75c784f 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -389,6 +389,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
>                         ;
>                 else if (!strcmp(ext, "preciousobjects"))
>                         data->precious_objects = git_config_bool(var, value);
> +               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) {
> @@ -432,8 +434,9 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
>         }
>
>         repository_format_precious_objects = candidate.precious_objects;
> +       repository_format_worktree_config = candidate.worktree_config;
>         string_list_clear(&candidate.unknown_extensions, 0);
> -       if (!has_common) {
> +       if (!has_common || repository_format_worktree_config) {
>                 if (candidate.is_bare != -1) {
>                         is_bare_repository_cfg = candidate.is_bare;
>                         if (is_bare_repository_cfg == 1)
> diff --git a/t/t2028-worktree-config.sh b/t/t2028-worktree-config.sh
> new file mode 100755
> index 0000000..34067df
> --- /dev/null
> +++ b/t/t2028-worktree-config.sh
> @@ -0,0 +1,77 @@
> +#!/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 &&
> +       test_must_fail git config --worktree per.worktree is-not-ok &&
> +       git config extensions.worktreeConfig true
> +'
> +
> +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 fails in multi worktree without extension' '
> +       test_must_fail git config --worktree foo.bar true
> +'
> +
> +test_done
> --
> 2.9.1.566.gbd532d4
>

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

* Re: [PATCH v4 1/4] worktree: add per-worktree config files
  2016-07-26  0:59       ` Stefan Beller
@ 2016-07-26 15:04         ` Duy Nguyen
  0 siblings, 0 replies; 31+ messages in thread
From: Duy Nguyen @ 2016-07-26 15:04 UTC (permalink / raw)
  To: Stefan Beller
  Cc: git@vger.kernel.org, Max Kirillov, Junio C Hamano,
	Michael J Gruber, Jens Lehmann, Lars Schneider, Michael Haggerty

On Tue, Jul 26, 2016 at 2:59 AM, Stefan Beller <sbeller@google.com> wrote:
> I like the user facing design, but how am I supposed to use it internally?
>
> Say I want to read a value preferably from the worktree I'd do a
>     /*
>      * maybe I don't even have to set it to 1 as
>      * the user is supposed to do that?
>      */
>     repository_format_worktree_config = 1;
>     git_config_get_{string,bool,int} (... as usual ...)
>
> and if I want to read the value globally I would set the variable to 0
> and read? (I would need to restore it, so I'll have a temporary variable
> to keep the original value of repository_format_worktree_config)

I would understand if you need an api to write to worktree config or
the shared one. But choosing to _read_ from a specific source sounds
wrong. The common rule should apply everywhere: read from worktree
first, if not found, read again from shared config. Why do you need
this?
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-25 23:25               ` Stefan Beller
@ 2016-07-26 17:20                 ` Duy Nguyen
  2016-07-26 18:15                   ` Stefan Beller
  0 siblings, 1 reply; 31+ messages in thread
From: Duy Nguyen @ 2016-07-26 17:20 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Tue, Jul 26, 2016 at 1:25 AM, Stefan Beller <sbeller@google.com> wrote:
> So what is the design philosophy in worktrees? How much independence does
> one working tree have?

git-worktree started out as an alternative for git-stash: hmm.. i need
to make some changes in another branch, okay let's leave this worktree
(with all its messy stuff) as-is, create another worktree, make those
changes, then delete the worktree and go back here. There's already
another way of doing that without git-stash: you clone the repo, fix
your stuff, push back and delete the new repo.

I know I have not really answered your questions. But I think it gives
an idea what are the typical use cases for multiple worktrees. How
much independence would need to be decided case-by-case, I think.

> So here is what I did:
>  *  s/git submodule init/git submodule update --init/
>  * added a test_pause to the last test on the last line
>  * Then:
>
> $ find . |grep da5e6058
> ./addtest/.git/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
> ./addtest/.git/worktrees/super-elsewhere/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
> ./addtest/.git/worktrees/super-elsewhere/modules/submod2/objects/08/da5e6058267d6be703ae058d173ce38ed53066
> ./.git/objects/08/da5e6058267d6be703ae058d173ce38ed53066
>
> The last entry is the "upstream" for the addtest clone, so that is fine.
> However inside the ./addtest/ (and its worktrees, which currently are
> embedded in there?) we only want to have one object store for a given
> submodule?

How to store stuff in .git is the implementation details that the user
does not care about. As long as we keep the behavior the same (they
can still "git submodule init" and stuff in the new worktree), sharing
the same object store makes sense (pros: lower disk consumption, cons:
none).


> After playing with this series a bit more, I actually like the UI as it is an
> easy mental model "submodules behave completely independent".
>
> However in 3/4 you said:
>
> + - `submodule.*` in current state should not be shared because the
> +   information is tied to a particular version of .gitmodules in a
> +   working directory.
>
> This is already a problem with say different branches/versions.
> That has been solved by duplicating that information to .git/config
> as a required step. (I don't like that approach, as it is super confusing
> IMHO)

Hmm.. I didn't realize this. But then I have never given much thought
about submodules, probably because I have an alternative solution for
it (or some of its use cases) anyway :)

OK so it's already a problem. But if we keep sharing submodule stuff
in .git/config, there's a _new_ problem: when you "submodule init" a
worktree, .git/config is now tailored for the current worktree, when
you move back to the previous worktree, you need to "submodule init"
again. So moving to multiple worktrees setup changes how the user uses
submodule, not good in my opinion.

If you have a grand plan to make submodule work at switching branches
(without reinit) and if it happens to work the same way when we have
multiple worktrees, great.

> I am back to the drawing board for the submodule side of things,
> but I guess this series could be used once we figure out how to
> have just one object database for a submodule.

I would leave this out for now. Let's make submodule work with
multiple worktrees first (and see how the users react to this). Then
we can try to share object database. Object database and refs are tied
closely together so you may run into other problems soon.
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-26 17:20                 ` Duy Nguyen
@ 2016-07-26 18:15                   ` Stefan Beller
  2016-07-27 14:37                     ` Jakub Narębski
  2016-07-27 15:40                     ` Duy Nguyen
  0 siblings, 2 replies; 31+ messages in thread
From: Stefan Beller @ 2016-07-26 18:15 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Tue, Jul 26, 2016 at 10:20 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Jul 26, 2016 at 1:25 AM, Stefan Beller <sbeller@google.com> wrote:
>> So what is the design philosophy in worktrees? How much independence does
>> one working tree have?
>
> git-worktree started out as an alternative for git-stash: hmm.. i need
> to make some changes in another branch, okay let's leave this worktree
> (with all its messy stuff) as-is, create another worktree, make those
> changes, then delete the worktree and go back here. There's already
> another way of doing that without git-stash: you clone the repo, fix
> your stuff, push back and delete the new repo.
>
> I know I have not really answered your questions. But I think it gives
> an idea what are the typical use cases for multiple worktrees. How
> much independence would need to be decided case-by-case, I think.

Thanks!


>
>> So here is what I did:
>>  *  s/git submodule init/git submodule update --init/
>>  * added a test_pause to the last test on the last line
>>  * Then:
>>
>> $ find . |grep da5e6058
>> ./addtest/.git/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
>> ./addtest/.git/worktrees/super-elsewhere/modules/submod/objects/08/da5e6058267d6be703ae058d173ce38ed53066
>> ./addtest/.git/worktrees/super-elsewhere/modules/submod2/objects/08/da5e6058267d6be703ae058d173ce38ed53066
>> ./.git/objects/08/da5e6058267d6be703ae058d173ce38ed53066
>>
>> The last entry is the "upstream" for the addtest clone, so that is fine.
>> However inside the ./addtest/ (and its worktrees, which currently are
>> embedded in there?) we only want to have one object store for a given
>> submodule?
>
> How to store stuff in .git is the implementation details that the user
> does not care about.

They do unfortunately. :(
Some teams here are trying to migrate from the repo[1] tool to submodules,
and they usually have large code bases. (e.g. The Android Open Source
Project[2], put into a superproject has a .git dir size of 17G. The
17G are partitioned as follows:

.../.git$ du --max-depth=1 -h
    44K ./hooks
    32K ./refs
    36K ./logs
    17G ./modules
    4.0K ./branches
    8.0K ./info
    4.7M ./objects
    17G .

i.e. roughly all in submodules.

So our users do care about both what is on disk, as well
as what goes over the wire (network traffic).

My sudden interest in worktrees came up when I learned the
`--reference` flag for submodule operations is broken for
our use case, and instead of fixing the `--reference` flag,
I think the worktree approach is generally saner (i.e. with the
references you may have nasty gc issues IIUC, but in the
worktree world gc knows about all the working trees, detached
heads and branches.)

[1] https://source.android.com/source/developing.html
[2] https://android.googlesource.com/

> As long as we keep the behavior the same (they
> can still "git submodule init" and stuff in the new worktree), sharing
> the same object store makes sense (pros: lower disk consumption, cons:
> none).

So I think the current workflow for submodules
may need some redesign anyway as the submodule
commands were designed with a strict "one working
tree only" assumption.

Submodule URLs  are stored in 3 places:
 A) In the .gitmodules file of the superproject
 B) In the option submodule.<name>.URL in the superproject
 C) In the remote.origin.URL in the submodule

A) is a recommendation from the superproject to make life
of downstream easier to find and setup the whole thing.
You can ignore that if you want, though generally a caring
upstream provides good URLs here.

C) is where we actually fetch from (and hope it has all
the sha1s that are recorded as gitlinks in the superproejct)

B) seems like a hack to enable the workflow as below:

Current workflow for handling submodule URLs:
 1) Clone the superproject
 2) Run git submodule init on desired submodules
 3) Inspect .git/config to see if any submodule URL needs adaption
 4) Run git submodule update to obtain the submodules from
    the configured place
 5) In case of superproject adapting the URL
    -> git submodule sync, which overwrites the submodule.<name>.URL in the
    superprojects .git/config as well as configuring the
remote."$remote".url in the submodule
 6) In case of users desire to change the URL
    -> No one command to solve it; possible workaround: edit
    .gitmodules and git submodule sync, or configure  the submodule.<name>.URL
    in the superprojects .git/config as well as configuring the
remote."$remote".url in
    the submodule separately. Although just changing the submodules remote works
    just as well (until you remove and re-clone the submodule)

One could imagine another workflow:
 1) clone the superproject, which creates empty repositories for the
    submodules
 (2) from the prior workflow is gone
 3) instead of inspecting .git/config you can directly manipulate the
    remote.$remote.url configuration in the submodule.
 4) Run git submodule update to obtain the submodules from
    the configured place

The current workflow is setup that way because historically you had
the submodules .git dir inside the submodule, which would be gone
if you deleted a submodule. So if you later checkout an earlier version'
that had a submodule, you are missing the objects and more importantly
configuration where to get them from.

This is now fixed by keeping the actual submodules git dir inside
the superprojects git dir.


>
>
>> After playing with this series a bit more, I actually like the UI as it is an
>> easy mental model "submodules behave completely independent".
>>
>> However in 3/4 you said:
>>
>> + - `submodule.*` in current state should not be shared because the
>> +   information is tied to a particular version of .gitmodules in a
>> +   working directory.
>>
>> This is already a problem with say different branches/versions.
>> That has been solved by duplicating that information to .git/config
>> as a required step. (I don't like that approach, as it is super confusing
>> IMHO)
>
> Hmm.. I didn't realize this. But then I have never given much thought
> about submodules, probably because I have an alternative solution for
> it (or some of its use cases) anyway :)

What is that?

>
> OK so it's already a problem. But if we keep sharing submodule stuff
> in .git/config, there's a _new_ problem: when you "submodule init" a
> worktree, .git/config is now tailored for the current worktree, when
> you move back to the previous worktree, you need to "submodule init"
> again.

"Moving back" sounds like you use the worktree feature for short lived
things only. (e.g. in the man page you refer to the hot fix your boss wants
you to make urgently).

I thought the worktree feature is more useful for long term "branches",
e.g. I have one worktree of git now that tracks origin/master so I can
use that to "make install" to repair my local broken version of git.

(I could have a worktree "continuous integration", where I only run tests
in. I could have one worktree for Documentation changes only.)

This long lived stuff probably doesn't make sense for the a single
repository, but in combination with submodules (which is another way
to approach the "sparse/narrow" desire of a large project), I think
that makes sense, because the "continuous integration" shares a lot
of submodules with my "regular everyday hacking" or the "I need to
test my colleague work now" worktree.

> So moving to multiple worktrees setup changes how the user uses
> submodule, not good in my opinion.

Because the submodule user API is built on the strong assumption
of "one working tree only", we have to at least slightly adapt.

So instead of cloning a submodule in a worktree we could just
setup a submodule worktree as well there?
(i.e. that saves us network as well as disk)

>
> If you have a grand plan to make submodule work at switching branches
> (without reinit) and if it happens to work the same way when we have
> multiple worktrees, great.

Eh, I am still working on the master plan. ;)
The insights on how worktrees handles stuff helps me shape it though. :)

If you switch a branch (or to any sha1), the submodule currently stays
"as-is" and may be updated using "submodule update", which goes through
the list of existing (checked out) submodules and checks them out to the
sha1 pointed to by the superprojects gitlink.

>
>> I am back to the drawing board for the submodule side of things,
>> but I guess this series could be used once we figure out how to
>> have just one object database for a submodule.
>
> I would leave this out for now. Let's make submodule work with
> multiple worktrees first (and see how the users react to this). Then
> we can try to share object database. Object database and refs are tied
> closely together so you may run into other problems soon.

I see. The normal for submodules is to be in detached HEAD though.

The user can of course checkout branches or things in there, but
the "submodule update" operations do not go to a branch for you.


----
Another (slightly offtopic) observation on the similarity of worktree
and submodules: There is no good way implemented to remove one.

For submodules there is deinit both removes the working tree as well
as the configuration indicating the existence (Note: the git dir still exists
for the submodule). Though that sounds like what we need to save us
network traffic the next time we need the submodule. Although going
through the code I need to test that a bit more later today to see how
fail safe it is.
On the submodule side, it often gets confusing what you want to remove
(local checkout of the submodule, or the gitlink or both).

For worktrees there is no "worktree rm" as it would probably promise
a bit more than the man pages suggestion of rm -rf $worktree && git
worktree prune.

Thanks,
Stefan

> --
> Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-20 17:24     ` [PATCH v4 3/4] submodule: support running in multiple worktree setup Nguyễn Thái Ngọc Duy
  2016-07-20 23:22       ` Stefan Beller
@ 2016-07-27  4:10       ` Max Kirillov
  2016-07-27 14:40         ` Jakub Narębski
  2016-07-27 14:49         ` Duy Nguyen
  1 sibling, 2 replies; 31+ messages in thread
From: Max Kirillov @ 2016-07-27  4:10 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger

Hi.

On Wed, Jul 20, 2016 at 07:24:18PM +0200, Nguyễn Thái Ngọc Duy wrote:
> + - `remote.*` added by submodules may be per working directory as
> +   well, unless you are sure remotes from all possible submodules in
> +   history are consistent.
...
> @@ -1114,7 +1114,7 @@ cmd_sync()
>  				sanitize_submodule_env
>  				cd "$sm_path"
>  				remote=$(get_default_remote)
> -				git config remote."$remote".url "$sub_origin_url"
> +				git config --worktree remote."$remote".url "$sub_origin_url"
>  
>  				if test -n "$recursive"
>  				then

I don't think remote.* should be per-worktree. 

* note that it is sumodule repository, not superproject. It
  does not even have to have multiple worktrees.
* it is quite bad to have it different in worktree, because
  git fetch then results in different ref updates depending
  on where it was called. So whatever issue it was intended
  to solve, it hardly made things better.
* I'm not sure I know all use cases of "submodule sync",
  but as far as I understand, it should be called when the
  submodule repository stays the "same" (however user
  defines the "same"), but older url does not work for some
  reason. Then I think it is correct to change the remote
  url for all worktrees.

-- 
Max

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-26 18:15                   ` Stefan Beller
@ 2016-07-27 14:37                     ` Jakub Narębski
  2016-07-27 16:53                       ` Stefan Beller
  2016-07-27 15:40                     ` Duy Nguyen
  1 sibling, 1 reply; 31+ messages in thread
From: Jakub Narębski @ 2016-07-27 14:37 UTC (permalink / raw)
  To: Stefan Beller, Duy Nguyen
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

W dniu 2016-07-26 o 20:15, Stefan Beller pisze:
> On Tue, Jul 26, 2016 at 10:20 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Tue, Jul 26, 2016 at 1:25 AM, Stefan Beller <sbeller@google.com> wrote:
>>> So what is the design philosophy in worktrees? How much independence does
>>> one working tree have?
>>
>> git-worktree started out as an alternative for git-stash: hmm.. i need
>> to make some changes in another branch, okay let's leave this worktree
>> (with all its messy stuff) as-is, create another worktree, make those
>> changes, then delete the worktree and go back here. There's already
>> another way of doing that without git-stash: you clone the repo, fix
>> your stuff, push back and delete the new repo.
>>
>> I know I have not really answered your questions. But I think it gives
>> an idea what are the typical use cases for multiple worktrees. How
>> much independence would need to be decided case-by-case, I think.
> 
> Thanks!

Hopefully the Git User's Survey 2016 would answer what people really
use worktrees for, and what use submodules for.  You are welcome to
submit proposed questions for the survey:
  http://thread.gmane.org/gmane.comp.version-control.git/299032

> My sudden interest in worktrees came up when I learned the
> `--reference` flag for submodule operations is broken for
> our use case, and instead of fixing the `--reference` flag,
> I think the worktree approach is generally saner (i.e. with the
> references you may have nasty gc issues IIUC, but in the
> worktree world gc knows about all the working trees, detached
> heads and branches.)

I think the problem with `--reference` is that it does not
setup backreferences to prevent gc removing borrowed objects;
which is a hard problem to solve, except for limited cases...
like git-worktree.
 
> So I think the current workflow for submodules
> may need some redesign anyway as the submodule
> commands were designed with a strict "one working
> tree only" assumption.
> 
> Submodule URLs  are stored in 3 places:
>  A) In the .gitmodules file of the superproject
>  B) In the option submodule.<name>.URL in the superproject
>  C) In the remote.origin.URL in the submodule
> 
> A) is a recommendation from the superproject to make life
> of downstream easier to find and setup the whole thing.
> You can ignore that if you want, though generally a caring
> upstream provides good URLs here.

Also, this URL might have change if the repository moves
to other server; even when checking out ancient version
we usually want to use current URL, not the one in currently
checked-out .gitmodules file.
 
> C) is where we actually fetch from (and hope it has all
> the sha1s that are recorded as gitlinks in the superproject)

Is it? Or is it only the case if you do `git fetch` or
equivalent from within inside of submodule? You can fetch
updates using `git submodule ...` from supermodule, isn't it?
But I might be wrong here.

Also: if .git file is gitfile link, do submodule even has
it's own configuration file?

> 
> B) seems like a hack to enable the workflow as below:

It has overloaded meaning, being used both for current URL
of submodule as seen in supermodule, AND that submodule
is checked out / needs to be checked out in the worktree
of a supermodule.  There might be the case when you check
out (in given worktree) a version of a supermodule that
do not include submodule at all, but you want to know that
when going back, this submodule is to be checked out (or not).

The second information needs to be per-worktree. How to
solve it, be it per-worktree configuration (not shared),
or a special configuration variable, or worktree having
unshared copy of configuration -- this what is discussed.

> Current workflow for handling submodule URLs:
>  1) Clone the superproject
>  2) Run git submodule init on desired submodules

Or 1-2) clone the superproject recursively, with all its
submodules.

>  3) Inspect .git/config to see if any submodule URL needs adaption

Which is usually not needed.

>  4) Run git submodule update to obtain the submodules from
>     the configured place

Or 2+4) run `git submodule update --init`

>  5) In case of superproject adapting the URL
>     -> git submodule sync, which overwrites the submodule.<name>.URL in the
>     superprojects .git/config as well as configuring the
>     remote."$remote".url in the submodule

This takes information from current .gitmodules, isn't it?

>  6) In case of users desire to change the URL
>     -> No one command to solve it; possible workaround: edit
>     .gitmodules and git submodule sync, or configure  the submodule.<name>.URL
>     in the superprojects .git/config as well as configuring the remote."$remote".url in
>     the submodule separately. Although just changing the submodules remote works
>     just as well (until you remove and re-clone the submodule)
[...]


> "Moving back" sounds like you use the worktree feature for short lived
> things only. (e.g. in the man page you refer to the hot fix your boss wants
> you to make urgently).
> 
> I thought the worktree feature is more useful for long term "branches",
> e.g. I have one worktree of git now that tracks origin/master so I can
> use that to "make install" to repair my local broken version of git.
> 
> (I could have a worktree "continuous integration", where I only run tests
> in. I could have one worktree for Documentation changes only.)
> 
> This long lived stuff probably doesn't make sense for the a single
> repository, but in combination with submodules (which is another way
> to approach the "sparse/narrow" desire of a large project), I think
> that makes sense, because the "continuous integration" shares a lot
> of submodules with my "regular everyday hacking" or the "I need to
> test my colleague work now" worktree.

One thing that git-worktree would be very useful, if it could work
with submodules: you could use separate worktrees to easily test
if the supermodule works with and without its submodules present.
 
[...]
> If you switch a branch (or to any sha1), the submodule currently stays
> "as-is" and may be updated using "submodule update", which goes through
> the list of existing (checked out) submodules and checks them out to the
> sha1 pointed to by the superprojects gitlink.

Which might be simply a problem that submodule UI is not mature enough.
I would like to see automatic switch of submodule contents, if 
configured so.

-- 
Jakub Narębski


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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-27  4:10       ` Max Kirillov
@ 2016-07-27 14:40         ` Jakub Narębski
  2016-07-27 14:49         ` Duy Nguyen
  1 sibling, 0 replies; 31+ messages in thread
From: Jakub Narębski @ 2016-07-27 14:40 UTC (permalink / raw)
  To: Max Kirillov, Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger

W dniu 2016-07-27 o 06:10, Max Kirillov pisze:
> Hi.
> 
> On Wed, Jul 20, 2016 at 07:24:18PM +0200, Nguyễn Thái Ngọc Duy wrote:
>> + - `remote.*` added by submodules may be per working directory as
>> +   well, unless you are sure remotes from all possible submodules in
>> +   history are consistent.
> ...
>> @@ -1114,7 +1114,7 @@ cmd_sync()
>>  				sanitize_submodule_env
>>  				cd "$sm_path"
>>  				remote=$(get_default_remote)
>> -				git config remote."$remote".url "$sub_origin_url"
>> +				git config --worktree remote."$remote".url "$sub_origin_url"
>>  
>>  				if test -n "$recursive"
>>  				then
> 
> I don't think remote.* should be per-worktree. 
> 
> * note that it is sumodule repository, not superproject. It
>   does not even have to have multiple worktrees.
> * it is quite bad to have it different in worktree, because
>   git fetch then results in different ref updates depending
>   on where it was called. So whatever issue it was intended
>   to solve, it hardly made things better.
> * I'm not sure I know all use cases of "submodule sync",
>   but as far as I understand, it should be called when the
>   submodule repository stays the "same" (however user
>   defines the "same"), but older url does not work for some
>   reason. Then I think it is correct to change the remote
>   url for all worktrees.

But... I don't know how sane it is, and if anybody uses it,
but one might want to use different repositories (different
forks) for different branches, and thus different worktrees.
For example the 'next' branch might want to switch to X.Org,
because XFree86 is moribund, but keep the old repo for 'maint',
or something like that ;-)

-- 
Jakub Narębski


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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-27  4:10       ` Max Kirillov
  2016-07-27 14:40         ` Jakub Narębski
@ 2016-07-27 14:49         ` Duy Nguyen
  1 sibling, 0 replies; 31+ messages in thread
From: Duy Nguyen @ 2016-07-27 14:49 UTC (permalink / raw)
  To: Max Kirillov
  Cc: Git Mailing List, Junio C Hamano, Michael J Gruber, Jens Lehmann,
	Lars Schneider, Stefan Beller, Michael Haggerty

On Wed, Jul 27, 2016 at 6:10 AM, Max Kirillov <max@max630.net> wrote:
> Hi.
>
> On Wed, Jul 20, 2016 at 07:24:18PM +0200, Nguyễn Thái Ngọc Duy wrote:
>> + - `remote.*` added by submodules may be per working directory as
>> +   well, unless you are sure remotes from all possible submodules in
>> +   history are consistent.
> ...
>> @@ -1114,7 +1114,7 @@ cmd_sync()
>>                               sanitize_submodule_env
>>                               cd "$sm_path"
>>                               remote=$(get_default_remote)
>> -                             git config remote."$remote".url "$sub_origin_url"
>> +                             git config --worktree remote."$remote".url "$sub_origin_url"
>>
>>                               if test -n "$recursive"
>>                               then
>
> I don't think remote.* should be per-worktree.
>
> * note that it is sumodule repository, not superproject.

Ah.. silly me, I thought all these were about supermodule. Yes it
makes more sense then to share remote.* (just like it's set up after
clone).

>   It does not even have to have multiple worktrees.

But we can turn a submodule into multiple worktrees after "submodule
init" and I don't think sharing remote.* is a problem even in that
case.

> * it is quite bad to have it different in worktree, because
>   git fetch then results in different ref updates depending
>   on where it was called. So whatever issue it was intended
>   to solve, it hardly made things better.
> * I'm not sure I know all use cases of "submodule sync",
>   but as far as I understand, it should be called when the
>   submodule repository stays the "same" (however user
>   defines the "same"), but older url does not work for some
>   reason. Then I think it is correct to change the remote
>   url for all worktrees.
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-26 18:15                   ` Stefan Beller
  2016-07-27 14:37                     ` Jakub Narębski
@ 2016-07-27 15:40                     ` Duy Nguyen
  2016-08-03 21:47                       ` Stefan Beller
  1 sibling, 1 reply; 31+ messages in thread
From: Duy Nguyen @ 2016-07-27 15:40 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Tue, Jul 26, 2016 at 8:15 PM, Stefan Beller <sbeller@google.com> wrote:
>> How to store stuff in .git is the implementation details that the user
>> does not care about.
>
> They do unfortunately. :(

Well.. i mean the structure of .git. If .git gets big, yeah many
people will get pissed.

> My sudden interest in worktrees came up when I learned the
> `--reference` flag for submodule operations is broken for
> our use case, and instead of fixing the `--reference` flag,
> I think the worktree approach is generally saner (i.e. with the

I don't know exactly what that --reference problem is, but keep in
mind you still have to support single-worktree use case. If it's
broken in single-worktree, somebody still has to fix it.

> references you may have nasty gc issues IIUC, but in the
> worktree world gc knows about all the working trees, detached
> heads and branches.)

True, but not yet. git-gc now does not know about all detached heads
and reflogs and have cause grief for some people. This should be fixed
soon.

>> As long as we keep the behavior the same (they
>> can still "git submodule init" and stuff in the new worktree), sharing
>> the same object store makes sense (pros: lower disk consumption, cons:
>> none).
>
> So I think the current workflow for submodules
> may need some redesign anyway as the submodule
> commands were designed with a strict "one working
> tree only" assumption.
>
> Submodule URLs  are stored in 3 places:
>  A) In the .gitmodules file of the superproject
>  B) In the option submodule.<name>.URL in the superproject
>  C) In the remote.origin.URL in the submodule
>
> A) is a recommendation from the superproject to make life
> of downstream easier to find and setup the whole thing.
> You can ignore that if you want, though generally a caring
> upstream provides good URLs here.
>
> C) is where we actually fetch from (and hope it has all
> the sha1s that are recorded as gitlinks in the superproejct)
>
> B) seems like a hack to enable the workflow as below:
>
> Current workflow for handling submodule URLs:
>  1) Clone the superproject
>  2) Run git submodule init on desired submodules
>  3) Inspect .git/config to see if any submodule URL needs adaption
>  4) Run git submodule update to obtain the submodules from
>     the configured place
>  5) In case of superproject adapting the URL
>     -> git submodule sync, which overwrites the submodule.<name>.URL in the
>     superprojects .git/config as well as configuring the
> remote."$remote".url in the submodule
>  6) In case of users desire to change the URL
>     -> No one command to solve it; possible workaround: edit
>     .gitmodules and git submodule sync, or configure  the submodule.<name>.URL
>     in the superprojects .git/config as well as configuring the
> remote."$remote".url in
>     the submodule separately. Although just changing the submodules remote works
>     just as well (until you remove and re-clone the submodule)
>
> One could imagine another workflow:
>  1) clone the superproject, which creates empty repositories for the
>     submodules
>  (2) from the prior workflow is gone
>  3) instead of inspecting .git/config you can directly manipulate the
>     remote.$remote.url configuration in the submodule.
>  4) Run git submodule update to obtain the submodules from
>     the configured place
>
> The current workflow is setup that way because historically you had
> the submodules .git dir inside the submodule, which would be gone
> if you deleted a submodule. So if you later checkout an earlier version'
> that had a submodule, you are missing the objects and more importantly
> configuration where to get them from.
>
> This is now fixed by keeping the actual submodules git dir inside
> the superprojects git dir.

Hmm.. sounds good, but I'm no judge when it comes to submodules :)

>> Hmm.. I didn't realize this. But then I have never given much thought
>> about submodules, probably because I have an alternative solution for
>> it (or some of its use cases) anyway :)
>
> What is that?

Narrow clone (making progress but not there yet). I know it does not
cover all cases (e.g. submodule can provide separate access control,
and even using different dvcs system in theory).

>> OK so it's already a problem. But if we keep sharing submodule stuff
>> in .git/config, there's a _new_ problem: when you "submodule init" a
>> worktree, .git/config is now tailored for the current worktree, when
>> you move back to the previous worktree, you need to "submodule init"
>> again.
>
> "Moving back" sounds like you use the worktree feature for short lived
> things only. (e.g. in the man page you refer to the hot fix your boss wants
> you to make urgently).
>
> I thought the worktree feature is more useful for long term "branches",
> e.g. I have one worktree of git now that tracks origin/master so I can
> use that to "make install" to repair my local broken version of git.

I use it for both. Sometimes you just want to fix something and not
mess up your current worktree.

> (I could have a worktree "continuous integration", where I only run tests
> in. I could have one worktree for Documentation changes only.)
>
> This long lived stuff probably doesn't make sense for the a single
> repository,

It does. You can switch branches in all worktrees. I have a worktree
specifically for building mingw32 stuff (separate config.mak and
stuff). When I'm done with a branch on my normal worktree, I could
move over there, check out the same branch then try mingw32 build. If
it fails I can fix it right right there and update the branch. When
everything is ok, I just move back to my normal worktree and continue.

You can achieve the same thing with multiple clones, but it's
inconvenient (you need to fetch and push...) and multiple clones take
up more space.

>> So moving to multiple worktrees setup changes how the user uses
>> submodule, not good in my opinion.
>
> Because the submodule user API is built on the strong assumption
> of "one working tree only", we have to at least slightly adapt.
>
> So instead of cloning a submodule in a worktree we could just
> setup a submodule worktree as well there?
> (i.e. that saves us network as well as disk)

You still need to clone once then somehow associate the clone with a
submodule, I think.

> For worktrees there is no "worktree rm" as it would probably promise
> a bit more than the man pages suggestion of rm -rf $worktree && git
> worktree prune.

Oh there will be, sooooon :D I prefer not to do that command sequence
manually, especially when "rm -rf" is involved. "git worktree remove"
can refuse to delete when your worktree is dirty so you don't
accidentally rm and cry later.
-- 
Duy

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-27 14:37                     ` Jakub Narębski
@ 2016-07-27 16:53                       ` Stefan Beller
  0 siblings, 0 replies; 31+ messages in thread
From: Stefan Beller @ 2016-07-27 16:53 UTC (permalink / raw)
  To: Jakub Narębski
  Cc: Duy Nguyen, Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

Jakub wrote:
> I think the problem with `--reference` is that it does not
> setup backreferences to prevent gc removing borrowed objects;
> which is a hard problem to solve, except for limited cases...
> like git-worktree.

Right. And instead of solving the reference problem, I'd
rather solve the worktree problem as I think it yields more?

>
>> So I think the current workflow for submodules
>> may need some redesign anyway as the submodule
>> commands were designed with a strict "one working
>> tree only" assumption.
>>
>> Submodule URLs  are stored in 3 places:
>>  A) In the .gitmodules file of the superproject
>>  B) In the option submodule.<name>.URL in the superproject
>>  C) In the remote.origin.URL in the submodule
>>
>> A) is a recommendation from the superproject to make life
>> of downstream easier to find and setup the whole thing.
>> You can ignore that if you want, though generally a caring
>> upstream provides good URLs here.
>
> Also, this URL might have change if the repository moves
> to other server; even when checking out ancient version
> we usually want to use current URL, not the one in currently
> checked-out .gitmodules file.

Right.

>
>> C) is where we actually fetch from (and hope it has all
>> the sha1s that are recorded as gitlinks in the superproject)
>
> Is it? Or is it only the case if you do `git fetch` or
> equivalent from within inside of submodule? You can fetch
> updates using `git submodule ...` from supermodule, isn't it?
> But I might be wrong here.

If you call `submodule update` in the  superproject
it actually just does a `(cd $submodule && git fetch)`.

And in the submodule we have a .git file pointing to
the superprojects ".git/modules/<name>/" which is a full
blown git dir, i.e. it has its own config, HEAD etc.

>
> Also: if .git file is gitfile link, do submodule even has
> it's own configuration file?

Yes they do.

>
>>
>> B) seems like a hack to enable the workflow as below:
>
> It has overloaded meaning, being used both for current URL
> of submodule as seen in supermodule, AND that submodule
> is checked out / needs to be checked out in the worktree
> of a supermodule.  There might be the case when you check
> out (in given worktree) a version of a supermodule that
> do not include submodule at all, but you want to know that
> when going back, this submodule is to be checked out (or not).

I am currently working on solving that with a patch series, that
allows 2 settings. The URL will be used only to overwrite the
URL from the .gitmodules file and another setting will be used
to determine if we want to checkout the submodule.

>
> The second information needs to be per-worktree. How to
> solve it, be it per-worktree configuration (not shared),
> or a special configuration variable, or worktree having
> unshared copy of configuration -- this what is discussed.

>
>> Current workflow for handling submodule URLs:
>>  1) Clone the superproject
>>  2) Run git submodule init on desired submodules
>
> Or 1-2) clone the superproject recursively, with all its
> submodules.

Only if the URLs are setup properly.

>
>>  3) Inspect .git/config to see if any submodule URL needs adaption
>
> Which is usually not needed.

Yeah, I should have added the assertion that the .gitmodules
may be out of date or such for this workflow to make sense.
Usually just go with recursive clone.

>>
>> This long lived stuff probably doesn't make sense for the a single
>> repository, but in combination with submodules (which is another way
>> to approach the "sparse/narrow" desire of a large project), I think
>> that makes sense, because the "continuous integration" shares a lot
>> of submodules with my "regular everyday hacking" or the "I need to
>> test my colleague work now" worktree.
>
> One thing that git-worktree would be very useful, if it could work
> with submodules: you could use separate worktrees to easily test
> if the supermodule works with and without its submodules present.

Oh! Yeah that makes sense!

>
> [...]
>> If you switch a branch (or to any sha1), the submodule currently stays
>> "as-is" and may be updated using "submodule update", which goes through
>> the list of existing (checked out) submodules and checks them out to the
>> sha1 pointed to by the superprojects gitlink.
>
> Which might be simply a problem that submodule UI is not mature enough.
> I would like to see automatic switch of submodule contents, if
> configured so.

Me too. Once upon a time Jens pushed for that with a series found at:
https://github.com/jlehmann/git-submod-enhancements/tree/git-checkout-recurse-submodules

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

* Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup
  2016-07-27 15:40                     ` Duy Nguyen
@ 2016-08-03 21:47                       ` Stefan Beller
  0 siblings, 0 replies; 31+ messages in thread
From: Stefan Beller @ 2016-08-03 21:47 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Jonathan Nieder, git@vger.kernel.org, Max Kirillov,
	Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Michael Haggerty

On Wed, Jul 27, 2016 at 8:40 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Jul 26, 2016 at 8:15 PM, Stefan Beller <sbeller@google.com> wrote:
>>> How to store stuff in .git is the implementation details that the user
>>> does not care about.
>>
>> They do unfortunately. :(
>
> Well.. i mean the structure of .git. If .git gets big, yeah many
> people will get pissed.
>
>> My sudden interest in worktrees came up when I learned the
>> `--reference` flag for submodule operations is broken for
>> our use case, and instead of fixing the `--reference` flag,
>> I think the worktree approach is generally saner (i.e. with the
>
> I don't know exactly what that --reference problem is, but keep in
> mind you still have to support single-worktree use case. If it's
> broken in single-worktree, somebody still has to fix it.

So --reference let's you point to a directory such that you can clone
with less data transmission (borrow objects from the local reference).

For submodules this is not per submodule, i.e.

  git clone --recursive --reference <path>

will only look at that <path> to borrow for the superproject and all submodules.
But submodules are usually different projects, so you don't find their objects
in the superprojects reference path.

One way out would be to extend the path appropriately (assuming the same
submodule structure in the reference repository).

Another way would be to extend the reference mechanism to look for
objects in the given path and any submodule of that path. Then the submodule
layout can change and --reference is still super effective.

My chose way was to look at the submodule support for worktrees, as that
will hopefully be less brittle w.r.t. gc eventually.

>
>> The current workflow is setup that way because historically you had
>> the submodules .git dir inside the submodule, which would be gone
>> if you deleted a submodule. So if you later checkout an earlier version'
>> that had a submodule, you are missing the objects and more importantly
>> configuration where to get them from.
>>
>> This is now fixed by keeping the actual submodules git dir inside
>> the superprojects git dir.
>
> Hmm.. sounds good, but I'm no judge when it comes to submodules :)

yeah I'll try to get feedback from the submodule people. :)

>
>>> Hmm.. I didn't realize this. But then I have never given much thought
>>> about submodules, probably because I have an alternative solution for
>>> it (or some of its use cases) anyway :)
>>
>> What is that?
>
> Narrow clone (making progress but not there yet). I know it does not
> cover all cases (e.g. submodule can provide separate access control,
> and even using different dvcs system in theory).

heh, ok. Yeah ACLs are the big thing here, so we'd rather go with submodules.

>
>>> OK so it's already a problem. But if we keep sharing submodule stuff
>>> in .git/config, there's a _new_ problem: when you "submodule init" a
>>> worktree, .git/config is now tailored for the current worktree, when
>>> you move back to the previous worktree, you need to "submodule init"
>>> again.
>>
>> "Moving back" sounds like you use the worktree feature for short lived
>> things only. (e.g. in the man page you refer to the hot fix your boss wants
>> you to make urgently).
>>
>> I thought the worktree feature is more useful for long term "branches",
>> e.g. I have one worktree of git now that tracks origin/master so I can
>> use that to "make install" to repair my local broken version of git.
>
> I use it for both. Sometimes you just want to fix something and not
> mess up your current worktree.

I tried worktrees in my daily workflow and the issue for me is my editor
that is worktree agnostic.  As I tried using worktree for different git related
patch series', the set of files I need to look at are the same in the
different work trees

When switching branches the files are still at the same place, such that
the editor, that has a bunch of files open, will just reload the files and you
don't need to open/close files in the editor.
With worktrees you need to open/close all files that you intend to touch in
that worktree, which I dislike as an extra step.

>
>> (I could have a worktree "continuous integration", where I only run tests
>> in. I could have one worktree for Documentation changes only.)
>>
>> This long lived stuff probably doesn't make sense for the a single
>> repository,
>
> It does. You can switch branches in all worktrees. I have a worktree
> specifically for building mingw32 stuff (separate config.mak and
> stuff). When I'm done with a branch on my normal worktree, I could
> move over there, check out the same branch then try mingw32 build. If
> it fails I can fix it right right there and update the branch. When
> everything is ok, I just move back to my normal worktree and continue.

So you use different worktrees for different purposes i.e. editing always
happens in the same, but testing or real hot fixes go into a separate
worktree?


>> So instead of cloning a submodule in a worktree we could just
>> setup a submodule worktree as well there?
>> (i.e. that saves us network as well as disk)
>
> You still need to clone once then somehow associate the clone with a
> submodule, I think.

(In the single worktree case) if you already have the submodule cloned,
but delete it from the working tree, you still keep the repo in .git. Later
when you decide to checkout the submodule again, you don't have to clone
it again, but just checkout.

I imagine the same is in multiple worktrees. Although you may want to fetch
a different remote first for a different worktree (as it may have configured a
different remote URL for the worktree)

>
>> For worktrees there is no "worktree rm" as it would probably promise
>> a bit more than the man pages suggestion of rm -rf $worktree && git
>> worktree prune.
>
> Oh there will be, sooooon :D I prefer not to do that command sequence
> manually, especially when "rm -rf" is involved. "git worktree remove"
> can refuse to delete when your worktree is dirty so you don't
> accidentally rm and cry later.

yeah that was my line of thinking :)

Thanks,
Stefan

> --
> Duy

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

end of thread, other threads:[~2016-08-03 21:48 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-19 20:59 Current state of Git worktree used with submodules? Lars Schneider
2016-07-20  4:14 ` Duy Nguyen
2016-07-20 17:24   ` [PATCH v4 0/4] Split .git/config in multiple worktree setup Nguyễn Thái Ngọc Duy
2016-07-20 17:24     ` [PATCH v4 1/4] worktree: add per-worktree config files Nguyễn Thái Ngọc Duy
2016-07-26  0:59       ` Stefan Beller
2016-07-26 15:04         ` Duy Nguyen
2016-07-20 17:24     ` [PATCH v4 2/4] submodule: update core.worktree using git-config Nguyễn Thái Ngọc Duy
2016-07-20 22:04       ` Stefan Beller
2016-07-22 17:15         ` Duy Nguyen
2016-07-20 17:24     ` [PATCH v4 3/4] submodule: support running in multiple worktree setup Nguyễn Thái Ngọc Duy
2016-07-20 23:22       ` Stefan Beller
2016-07-22  0:37         ` Stefan Beller
2016-07-22  7:32         ` Jens Lehmann
2016-07-22 16:07           ` Stefan Beller
2016-07-22 16:55         ` Junio C Hamano
2016-07-22 17:40           ` Stefan Beller
2016-07-25 15:46             ` Junio C Hamano
2016-07-22 17:09         ` Duy Nguyen
2016-07-22 17:25           ` Stefan Beller
2016-07-22 17:42             ` Duy Nguyen
2016-07-25 23:25               ` Stefan Beller
2016-07-26 17:20                 ` Duy Nguyen
2016-07-26 18:15                   ` Stefan Beller
2016-07-27 14:37                     ` Jakub Narębski
2016-07-27 16:53                       ` Stefan Beller
2016-07-27 15:40                     ` Duy Nguyen
2016-08-03 21:47                       ` Stefan Beller
2016-07-27  4:10       ` Max Kirillov
2016-07-27 14:40         ` Jakub Narębski
2016-07-27 14:49         ` Duy Nguyen
2016-07-20 17:24     ` [PATCH v4 4/4] t2029: some really basic tests for submodules in multi worktree Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).