threads / rfc / 48702

RFC patch, 3 partssubmodules with no working tree shall unset core.worktree

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

## tl;dr

11 messages between Jun 12, 2018 and Jun 19, 2018. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Stefan Beller· Jun 12, 2018, 23:58 UTC · lore

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
Stefan Beller· Jun 12, 2018, 23:58 UTC · re: Stefan Beller · lore

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

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(-)
Show changes to 3 files +18 −1

submodule.c, submodule.h, t/lib-submodule-update.sh

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
Stefan Beller· Jun 12, 2018, 23:58 UTC · re: Stefan Beller · lore

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

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

builtin/submodule--helper.c, git-submodule.sh

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
SZEDER Gábor· Jun 16, 2018, 20:26 UTC · re: Stefan Beller · lore

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

Show 8 quoted lines
> +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.

Show 18 quoted lines
> +
> +	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 {
Show 11 quoted lines
> 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.
Show 9 quoted lines
> +		fi
> +
>  		if test "$subsha1" != "$sha1" || test -n "$force"
>  		then
>  			subforce=$force
> -- 
> 2.18.0.rc1.244.gcf134e6275-goog
> 
> 
Stefan Beller· Jun 12, 2018, 23:58 UTC · re: Stefan Beller · lore

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

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(-)
Show changes to 3 files +8 −1

builtin/submodule--helper.c, t/lib-submodule-update.sh, t/t7400-submodule-basic.sh

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
Junio C Hamano· Jun 13, 2018, 18:00 UTC · re: Stefan Beller · lore

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

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.
Show 19 quoted lines
> 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(-)
Stefan Beller· Jun 13, 2018, 18:52 UTC · re: Junio C Hamano · lore

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

On Wed, Jun 13, 2018 at 11:00 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
>
> 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.

Show 8 quoted lines
> 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
Stefan Beller· Jun 19, 2018, 00:06 UTC · re: Stefan Beller · lore

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

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
Stefan Beller· Jun 19, 2018, 00:06 UTC · re: Stefan Beller · lore

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

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(-)
Show changes to 3 files +18 −1

submodule.c, submodule.h, t/lib-submodule-update.sh

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
Stefan Beller· Jun 19, 2018, 00:06 UTC · re: Stefan Beller · lore

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

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

builtin/submodule--helper.c, git-submodule.sh

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
Stefan Beller· Jun 19, 2018, 00:06 UTC · re: Stefan Beller · lore

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

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(-)
Show changes to 3 files +8 −1

builtin/submodule--helper.c, t/lib-submodule-update.sh, t/t7400-submodule-basic.sh

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

← back to recent threads