[RFC PATCH 0/3] submodules with no working tree shall unset core.worktree

[RFC PATCH 0/3] submodules with no working tree shall unset core.worktree

[Stefan Beller]

The first patch teaches checkout/reset (with --recurse-submodules) to unset
the core.worktree config when the new state of the superprojects working tree
doesn't contain the submodules working tree.

The last patch is teaching "git submodule deinit" to unset the core.worktree
setting as well. It turned out this one is tricky, as for that we also
have to set it in the counter part, such as "submodule update".

Thanks,
Stefan

Stefan Beller (3):
  submodule: unset core.worktree if no working tree is present
  submodule: ensure core.worktree is set after update
  submodule deinit: unset core.worktree

 builtin/submodule--helper.c | 26 ++++++++++++++++++++++++++
 git-submodule.sh            |  5 +++++
 submodule.c                 | 14 ++++++++++++++
 submodule.h                 |  2 ++
 t/lib-submodule-update.sh   |  5 +++--
 t/t7400-submodule-basic.sh  |  5 +++++
 6 files changed, 55 insertions(+), 2 deletions(-)

-- 
2.18.0.rc1.244.gcf134e6275-goog

[PATCH 1/3] submodule: unset core.worktree if no working tree is present

[Stefan Beller]

When a submodules work tree is removed, we should unset its core.worktree
setting as the worktree is no longer present. This is not just in line
with the conceptual view of submodules, but it fixes an inconvenience
for looking at submodules that are not checked out:

    git clone --recurse-submodules git://github.com/git/git && cd git &&
    git checkout --recurse-submodules v2.13.0
    git -C .git/modules/sha1collisiondetection log
    fatal: cannot chdir to '../../../sha1collisiondetection': \
        No such file or directory

With this patch applied, the final call to git log works instead of dying
in its setup, as the checkout will unset the core.worktree setting such
that following log will be run in a bare repository.

This patch covers all commands that are in the unpack machinery, i.e.
checkout, read-tree, reset. A follow up patch will address
"git submodule deinit", which will also make use of the new function
submodule_unset_core_worktree(), which is why we expose it in this patch.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c               | 14 ++++++++++++++
 submodule.h               |  2 ++
 t/lib-submodule-update.sh |  3 ++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/submodule.c b/submodule.c
index 939d6870ecd..e127c074b04 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1532,6 +1532,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
 	return ret;
 }
 
+void submodule_unset_core_worktree(const struct submodule *sub)
+{
+	char *config_path = xstrfmt("%s/modules/%s/config",
+				    get_git_common_dir(), sub->name);
+
+	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+		warning(_("Could not unset core.worktree setting in submodule '%s'"),
+			  sub->path);
+
+	free(config_path);
+}
+
 static const char *get_super_prefix_or_empty(void)
 {
 	const char *s = get_super_prefix();
@@ -1697,6 +1709,8 @@ int submodule_move_head(const char *path,
 
 			if (is_empty_dir(path))
 				rmdir_or_warn(path);
+
+			submodule_unset_core_worktree(sub);
 		}
 	}
 out:
diff --git a/submodule.h b/submodule.h
index 7856b8a0b3d..4644683e6cb 100644
--- a/submodule.h
+++ b/submodule.h
@@ -121,6 +121,8 @@ extern int submodule_move_head(const char *path,
 			       const char *new_head,
 			       unsigned flags);
 
+void submodule_unset_core_worktree(const struct submodule *sub);
+
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific environment variables, but
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 1f38a85371a..12cd4e9233e 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
 			git branch -t remove_sub1 origin/remove_sub1 &&
 			$command remove_sub1 &&
 			test_superproject_content origin/remove_sub1 &&
-			! test -e sub1
+			! test -e sub1 &&
+			test_must_fail git config -f .git/modules/sub1/config core.worktree
 		)
 	'
 	# ... absorbing a .git directory along the way.
-- 
2.18.0.rc1.244.gcf134e6275-goog

[PATCH 2/3] submodule: ensure core.worktree is set after update

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 24 ++++++++++++++++++++++++
 git-submodule.sh            |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bd250ca2164..dffc55ed8ee 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1860,6 +1860,29 @@ static int check_name(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *name, *path;
+	char *sm_gitdir;
+
+	if (argc != 3)
+		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
+
+	name = argv[1];
+	path = argv[2];
+
+	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
+	sm_gitdir = absolute_pathdup(sb.buf);
+
+	connect_work_tree_and_git_dir(path, sm_gitdir, 0);
+
+	strbuf_release(&sb);
+	free(sm_gitdir);
+
+	return 0;
+}
+
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -1873,6 +1896,7 @@ static struct cmd_struct commands[] = {
 	{"name", module_name, 0},
 	{"clone", module_clone, 0},
 	{"update-clone", update_clone, 0},
+	{"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
 	{"relative-path", resolve_relative_path, 0},
 	{"resolve-relative-url", resolve_relative_url, 0},
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
diff --git a/git-submodule.sh b/git-submodule.sh
index 78073cd87d1..6596a77c5ef 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -615,6 +615,11 @@ cmd_update()
 			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
 		fi
 
+		if ! $(git config -f "$(git rev-parse --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
+		then
+			git submodule--helper connect-gitdir-workingtree $name $sm_path
+		fi
+
 		if test "$subsha1" != "$sha1" || test -n "$force"
 		then
 			subforce=$force
-- 
2.18.0.rc1.244.gcf134e6275-goog

Re: [PATCH 2/3] submodule: ensure core.worktree is set after update

[SZEDER Gábor]

> +static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	const char *name, *path;
> +	char *sm_gitdir;
> +
> +	if (argc != 3)
> +		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");

So this aborts when it's invoked with the wrong number of cmdline
arguments.

> +
> +	name = argv[1];
> +	path = argv[2];
> +
> +	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
> +	sm_gitdir = absolute_pathdup(sb.buf);
> +
> +	connect_work_tree_and_git_dir(path, sm_gitdir, 0);
> +
> +	strbuf_release(&sb);
> +	free(sm_gitdir);
> +
> +	return 0;
> +}
> +
>  #define SUPPORT_SUPER_PREFIX (1<<0)
>  
>  struct cmd_struct {

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 78073cd87d1..6596a77c5ef 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -615,6 +615,11 @@ cmd_update()
>  			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
>  		fi
>  
> +		if ! $(git config -f "$(git rev-parse --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
> +		then
> +			git submodule--helper connect-gitdir-workingtree $name $sm_path

The path to the submodule, $sm_path, may contain spaces, so it must
be quoted.

I'm sure you would have noticed this already, had you checked this
'submodule--helper's exit code :)  In t7406 the test 'submodule update
properly revives a moved submodule' does update a submodule with a
space in its name, and thus executes 'submodule--helper' with one more
argument than expected, causing it to abort, but since there is no
error checking, 'git submodule update' continues anyway, and in the
end the test tends to pass[1].

I think it would be prudent to check the exit code of all
'submodule--helper' executions.


[1] I wrote "tends to", because e.g. on Travis CI the aborting
    'submodule--helper' often dumps its core, and the extra 'core'
    file shows up in the output of 'git status' and 'test_cmp' notices
    it.


> +		fi
> +
>  		if test "$subsha1" != "$sha1" || test -n "$force"
>  		then
>  			subforce=$force
> -- 
> 2.18.0.rc1.244.gcf134e6275-goog
> 
> 

[PATCH 3/3] submodule deinit: unset core.worktree

[Stefan Beller]

When a submodule is deinit'd, the working tree is gone, so the setting of
core.worktree is bogus. Unset it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 ++
 t/lib-submodule-update.sh   | 2 +-
 t/t7400-submodule-basic.sh  | 5 +++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dffc55ed8ee..19480902681 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -980,6 +980,8 @@ static void deinit_submodule(const char *path, const char *prefix,
 		if (!(flags & OPT_QUIET))
 			printf(format, displaypath);
 
+		submodule_unset_core_worktree(sub);
+
 		strbuf_release(&sb_rm);
 	}
 
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 12cd4e9233e..aa5ac03325a 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
 	then
 		mkdir -p submodule_update/.git/modules/sub1/modules &&
 		cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
-		GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+		# core.worktree is unset for sub2 as it is not checked out
 	fi &&
 	# indicate we are interested in the submodule:
 	git -C submodule_update config submodule.sub1.url "bogus" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index be946fb1fd1..c6e1f749639 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -993,6 +993,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
 	rmdir init
 '
 
+test_expect_success 'submodule deinit should unset core.worktree' '
+	test_path_is_file .git/modules/example/config &&
+	test_must_fail git config -f .git/modules/example/config core.worktree
+'
+
 test_expect_success 'submodule deinit from subdirectory' '
 	git submodule update --init &&
 	git config submodule.example.foo bar &&
-- 
2.18.0.rc1.244.gcf134e6275-goog

Re: [RFC PATCH 0/3] submodules with no working tree shall unset core.worktree

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> The first patch teaches checkout/reset (with --recurse-submodules) to unset
> the core.worktree config when the new state of the superprojects working tree
> doesn't contain the submodules working tree.

Are there two cases of "doesn't contain working tree of a submodule"?

The superproject's commit may not have a gitlink to a specific
submodule in its tree (i.e. there is no way to even "submodule init"
with such a commit in the superproject's history).  Or there may be
a gitlink but the user chose not to check it out in the working
tree.

Do they need to be treated differently, or can they be treated the
same way?

Also, is the "submodule" feature supposed to play well with multiple
worktree feature?  Let's imagine that you have two worktrees for a
single superproject, and the branches of the superproject these two
worktrees check out are different ones (which is the more sensible
set-up than checking out the same branch twice).  Further imagine
that the superproject started using a single submodule sometime in
the past and keeps using it throughout its life since then.

 1. if both of these two branches have the submodule, and two
    worktrees both are interested in having the submodule checked
    out via "submodule init/update", where does core.worktree point
    at?  Do we have two copies of the variable?

 2. what if one branch predates the use of the submodule in the
    superproject while the other branch is newer and uses the
    submodule?  Where does core.worktree point at?

Thanks.

> The last patch is teaching "git submodule deinit" to unset the core.worktree
> setting as well. It turned out this one is tricky, as for that we also
> have to set it in the counter part, such as "submodule update".
>
> Thanks,
> Stefan
>
> Stefan Beller (3):
>   submodule: unset core.worktree if no working tree is present
>   submodule: ensure core.worktree is set after update
>   submodule deinit: unset core.worktree
>
>  builtin/submodule--helper.c | 26 ++++++++++++++++++++++++++
>  git-submodule.sh            |  5 +++++
>  submodule.c                 | 14 ++++++++++++++
>  submodule.h                 |  2 ++
>  t/lib-submodule-update.sh   |  5 +++--
>  t/t7400-submodule-basic.sh  |  5 +++++
>  6 files changed, 55 insertions(+), 2 deletions(-)

Re: [RFC PATCH 0/3] submodules with no working tree shall unset core.worktree

[Stefan Beller]

On Wed, Jun 13, 2018 at 11:00 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Beller <sbeller@google.com> writes:
>
> > The first patch teaches checkout/reset (with --recurse-submodules) to unset
> > the core.worktree config when the new state of the superprojects working tree
> > doesn't contain the submodules working tree.
>
> Are there two cases of "doesn't contain working tree of a submodule"?
>
> The superproject's commit may not have a gitlink to a specific
> submodule in its tree (i.e. there is no way to even "submodule init"
> with such a commit in the superproject's history).

... for example git.git before 2.13 did not have a gitlink for its
sha1collision submodule ...

>   Or there may be
> a gitlink but the user chose not to check it out in the working
> tree.

This is when the submodule is not active or de-initialized or not initialized
to begin with. However there is an empty directory as a place holder.
As long as the empty place holder is there, we do not run into trouble.

> Do they need to be treated differently, or can they be treated the
> same way?

I would think they are the same, as both cases do not have a working tree
for the submodule.

> Also, is the "submodule" feature supposed to play well with multiple
> worktree feature?

It is a long term goal to get this working.

>  Let's imagine that you have two worktrees for a
> single superproject, and the branches of the superproject these two
> worktrees check out are different ones (which is the more sensible
> set-up than checking out the same branch twice).

(git-worktree even prevents you to checkout the same branch),
sounds all very sensible up to here.


> Further imagine
> that the superproject started using a single submodule sometime in
> the past and keeps using it throughout its life since then.
>
>  1. if both of these two branches have the submodule, and two
>     worktrees both are interested in having the submodule checked
>     out via "submodule init/update", where does core.worktree point
>     at?  Do we have two copies of the variable?

Currently in the real world (aka in origin/master), the submodules are
completely duplicated, and do not know about the worktree feature
themselves:

    $ cd .git && find . |grep config |grep sha1co
    ./modules/sha1collisiondetection/config
    ./worktrees/git/modules/sha1collisiondetection/config

(Yes I do have a worktree called "git".)

So for the sake of this patch series this should be okay, as there
is only ever one working tree for a submodule currently.

The current state of affairs is really bad as the submodule is
there twice with all objects and the rest.

>  2. what if one branch predates the use of the submodule in the
>     superproject while the other branch is newer and uses the
>     submodule?  Where does core.worktree point at?

As we have two copies of the repository, one of them points at the
correct place, and the other would be unset.

---

In an ideal world we would want the submodules to use the worktree
feature along-side the superproject, e.g. if the superproject has the
two worktrees as above, the submodule would also get these.

However I have not thought about this future too deeply, as there
are a couple bumps along the road:

Supposedly the submodules main (common) git dir would stay in the
common git dir of the superproject at /modules/<name>.
However where would we put the submodules non-main worktree
git dirs? There are two places, as seen from the superprojects:

  .git/modules/<submodule-name>/worktrees/<worktree-name>
  .git/worktrees/<worktree-name>/modules/<submodule-name>

As worktrees can be removed pretty easily ("git worktree prune"),
I would not put the main parts of a submodule into a worktree part
of the superproject (The user could be in a non-main worktree
when they first ask for the submodule -- we have to make sure
the main git dir goes into the superprojects main git dir under
modules/).

An advantage for
   .git/worktrees/<worktree-name>/modules/<submodule-name>
would be that the worktree of the submodule is coupled to the
worktree of the superproject, i.e. if the user wipes the superprojects
worktree, the submodules accompanying worktree is gone automatically.

An advantage for
  .git/modules/<submodule-name>/worktrees/<worktree-name>
is to have any submodule related things in one place in
  .git/modules/<submodule-name>

Cleanup of the worktrees would be a bit harder as we'd have to
remember to prune the worktrees for all submodules as well
when the superproject removes one of its worktrees.

It is relatively rare to delete the submodule git directory compared to
removing a worktree, such that
  .git/modules/<submodule-name>/worktrees/<worktree-name>
sounds a bit more promising to me.

However the worktree command needs to figure out the worktrees:
    /path/to/super/worktree/submodule $ git worktree list

This would look at its git dir, which is

 .git/modules/<submodule-name>/worktrees/<specific-worktree>

discover the common dir at

 .git/modules/<submodule-name>

and then proceed in listing worktrees by exploring worktrees/.
If we had put it in the other form of
   .git/worktrees/<worktree-name>/modules/<submodule-name>
we'd discover the common git dir at
  .git/modules/<submodule-name>
and list it from there.

However currently we use the existence of directories (both for
submodules as well as worktrees) to infer some knowledge about
the existence of submodules and worktrees, so we cannot just declare
one version or the other to be the future, but we'd have to add some
form of linking, I would think.

Stefan

[PATCH 0/3] submodules with no working tree shall unset core.worktree

[Stefan Beller]

v2:
* fixed quoting issues for white space'd names/paths

v1:
The first patch teaches checkout/reset (with --recurse-submodules) to unset
the core.worktree config when the new state of the superprojects working tree
doesn't contain the submodules working tree.

The last patch is teaching "git submodule deinit" to unset the core.worktree
setting as well. It turned out this one is tricky, as for that we also
have to set it in the counter part, such as "submodule update".

Thanks,
Stefan

Stefan Beller (3):
  submodule: unset core.worktree if no working tree is present
  submodule: ensure core.worktree is set after update
  submodule deinit: unset core.worktree

 builtin/submodule--helper.c | 26 ++++++++++++++++++++++++++
 git-submodule.sh            |  5 +++++
 submodule.c                 | 14 ++++++++++++++
 submodule.h                 |  2 ++
 t/lib-submodule-update.sh   |  5 +++--
 t/t7400-submodule-basic.sh  |  5 +++++
 6 files changed, 55 insertions(+), 2 deletions(-)

-- 
2.18.0.rc1.244.gcf134e6275-goog

[PATCH 1/3] submodule: unset core.worktree if no working tree is present

[Stefan Beller]

When a submodules work tree is removed, we should unset its core.worktree
setting as the worktree is no longer present. This is not just in line
with the conceptual view of submodules, but it fixes an inconvenience
for looking at submodules that are not checked out:

    git clone --recurse-submodules git://github.com/git/git && cd git &&
    git checkout --recurse-submodules v2.13.0
    git -C .git/modules/sha1collisiondetection log
    fatal: cannot chdir to '../../../sha1collisiondetection': \
        No such file or directory

With this patch applied, the final call to git log works instead of dying
in its setup, as the checkout will unset the core.worktree setting such
that following log will be run in a bare repository.

This patch covers all commands that are in the unpack machinery, i.e.
checkout, read-tree, reset. A follow up patch will address
"git submodule deinit", which will also make use of the new function
submodule_unset_core_worktree(), which is why we expose it in this patch.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c               | 14 ++++++++++++++
 submodule.h               |  2 ++
 t/lib-submodule-update.sh |  3 ++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/submodule.c b/submodule.c
index 939d6870ecd..e127c074b04 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1532,6 +1532,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
 	return ret;
 }
 
+void submodule_unset_core_worktree(const struct submodule *sub)
+{
+	char *config_path = xstrfmt("%s/modules/%s/config",
+				    get_git_common_dir(), sub->name);
+
+	if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+		warning(_("Could not unset core.worktree setting in submodule '%s'"),
+			  sub->path);
+
+	free(config_path);
+}
+
 static const char *get_super_prefix_or_empty(void)
 {
 	const char *s = get_super_prefix();
@@ -1697,6 +1709,8 @@ int submodule_move_head(const char *path,
 
 			if (is_empty_dir(path))
 				rmdir_or_warn(path);
+
+			submodule_unset_core_worktree(sub);
 		}
 	}
 out:
diff --git a/submodule.h b/submodule.h
index 7856b8a0b3d..4644683e6cb 100644
--- a/submodule.h
+++ b/submodule.h
@@ -121,6 +121,8 @@ extern int submodule_move_head(const char *path,
 			       const char *new_head,
 			       unsigned flags);
 
+void submodule_unset_core_worktree(const struct submodule *sub);
+
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific environment variables, but
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 1f38a85371a..12cd4e9233e 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
 			git branch -t remove_sub1 origin/remove_sub1 &&
 			$command remove_sub1 &&
 			test_superproject_content origin/remove_sub1 &&
-			! test -e sub1
+			! test -e sub1 &&
+			test_must_fail git config -f .git/modules/sub1/config core.worktree
 		)
 	'
 	# ... absorbing a .git directory along the way.
-- 
2.18.0.rc1.244.gcf134e6275-goog

[PATCH 2/3] submodule: ensure core.worktree is set after update

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 24 ++++++++++++++++++++++++
 git-submodule.sh            |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bd250ca2164..dffc55ed8ee 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1860,6 +1860,29 @@ static int check_name(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *name, *path;
+	char *sm_gitdir;
+
+	if (argc != 3)
+		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
+
+	name = argv[1];
+	path = argv[2];
+
+	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
+	sm_gitdir = absolute_pathdup(sb.buf);
+
+	connect_work_tree_and_git_dir(path, sm_gitdir, 0);
+
+	strbuf_release(&sb);
+	free(sm_gitdir);
+
+	return 0;
+}
+
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -1873,6 +1896,7 @@ static struct cmd_struct commands[] = {
 	{"name", module_name, 0},
 	{"clone", module_clone, 0},
 	{"update-clone", update_clone, 0},
+	{"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
 	{"relative-path", resolve_relative_path, 0},
 	{"resolve-relative-url", resolve_relative_url, 0},
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
diff --git a/git-submodule.sh b/git-submodule.sh
index 78073cd87d1..6bd0db02b33 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -615,6 +615,11 @@ cmd_update()
 			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
 		fi
 
+		if ! $(git config -f "$(git rev-parse --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
+		then
+			git submodule--helper connect-gitdir-workingtree "$name" "$sm_path"
+		fi
+
 		if test "$subsha1" != "$sha1" || test -n "$force"
 		then
 			subforce=$force
-- 
2.18.0.rc1.244.gcf134e6275-goog

[PATCH 3/3] submodule deinit: unset core.worktree

[Stefan Beller]

When a submodule is deinit'd, the working tree is gone, so the setting of
core.worktree is bogus. Unset it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 2 ++
 t/lib-submodule-update.sh   | 2 +-
 t/t7400-submodule-basic.sh  | 5 +++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index dffc55ed8ee..19480902681 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -980,6 +980,8 @@ static void deinit_submodule(const char *path, const char *prefix,
 		if (!(flags & OPT_QUIET))
 			printf(format, displaypath);
 
+		submodule_unset_core_worktree(sub);
+
 		strbuf_release(&sb_rm);
 	}
 
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 12cd4e9233e..aa5ac03325a 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
 	then
 		mkdir -p submodule_update/.git/modules/sub1/modules &&
 		cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
-		GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+		# core.worktree is unset for sub2 as it is not checked out
 	fi &&
 	# indicate we are interested in the submodule:
 	git -C submodule_update config submodule.sub1.url "bogus" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 812db137b8d..48fd14fae6e 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -993,6 +993,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
 	rmdir init
 '
 
+test_expect_success 'submodule deinit should unset core.worktree' '
+	test_path_is_file .git/modules/example/config &&
+	test_must_fail git config -f .git/modules/example/config core.worktree
+'
+
 test_expect_success 'submodule deinit from subdirectory' '
 	git submodule update --init &&
 	git config submodule.example.foo bar &&
-- 
2.18.0.rc1.244.gcf134e6275-goog