[PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-13 Thread Stefan Beller
e98317508c0 (submodule: ensure core.worktree is set after update,
2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir
as that ensures both the 'core.worktree' configuration is set as well as
setting up correct gitlink file pointing at the git directory.

We do not need to check for the gitlink in this part of the cmd_update
in git-submodule.sh, as the initial call to update-clone will have ensured
that. So we can reduce the work to only (check and potentially) set the
'core.worktree' setting.

While at it move the check from shell to C as that proves to be useful in
a follow up patch, as we do not need the 'name' in shell now.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/submodule--helper.c | 64 +++--
 git-submodule.sh|  7 ++--
 2 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 8b1088ab58a..648e1330c15 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int ensure_core_worktree(int argc, const char **argv, const char 
*prefix)
+{
+   const struct submodule *sub;
+   const char *path;
+   char *cw;
+   struct repository subrepo;
+
+   if (argc != 2)
+   BUG("submodule--helper connect-gitdir-workingtree  
");
+
+   path = argv[1];
+
+   sub = submodule_from_path(the_repository, _oid, path);
+   if (!sub)
+   BUG("We could get the submodule handle before?");
+
+   if (repo_submodule_init(, the_repository, path))
+   die(_("could not get a repository handle for submodule '%s'"), 
path);
+
+   if (!repo_config_get_string(, "core.worktree", )) {
+   char *cfg_file, *abs_path;
+   const char *rel_path;
+   struct strbuf sb = STRBUF_INIT;
+
+   cfg_file = repo_git_path(, "config");
+
+   abs_path = absolute_pathdup(path);
+   rel_path = relative_path(abs_path, subrepo.gitdir, );
+
+   git_config_set_in_file(cfg_file, "core.worktree", rel_path);
+
+   free(cfg_file);
+   free(abs_path);
+   strbuf_release();
+   }
+
+   return 0;
+}
+
 static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -2029,29 +2068,6 @@ 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 = argv[1];
-   path = argv[2];
-
-   strbuf_addf(, "%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();
-   free(sm_gitdir);
-
-   return 0;
-}
-
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -2065,7 +2081,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},
+   {"ensure-core-worktree", ensure_core_worktree, 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 8caaf274e25..19d010eac06 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -535,6 +535,8 @@ cmd_update()
do
die_if_unmatched "$quickabort" "$sha1"
 
+   git submodule--helper ensure-core-worktree "$sm_path"
+
name=$(git submodule--helper name "$sm_path") || exit
if ! test -z "$update"
then
@@ -577,11 +579,6 @@ 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.865.gffc8e1a3cd6-goog



Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-10 Thread Brandon Williams
On 08/10, Stefan Beller wrote:
> > > + cfg_file = xstrfmt("%s/config", subrepo.gitdir);
> >
> > As I mentioned here:
> > https://public-inbox.org/git/20180807230637.247200-1-bmw...@google.com/T/#t
> >
> > This lines should probably be more like:
> >
> >   cfg_file = repo_git_path(, "config");
> >
> 
> Why? You did not mention the benefits for writing it this way
> here or on the reference. Care to elaborate?

Its more future proof especially because we have the difference bettwen
commondir and gitdir for worktrees.  Using the "repo_git_path" function
handles path rewritting when using worktrees.  Here (when working with
worktrees) "subrepo.gitdir" refers to the worktree specific gitdir while
"subrepo.commondir" refers to the global common gitdir where the
repository config actually lives.

-- 
Brandon Williams


Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-10 Thread Stefan Beller
> > + cfg_file = xstrfmt("%s/config", subrepo.gitdir);
>
> As I mentioned here:
> https://public-inbox.org/git/20180807230637.247200-1-bmw...@google.com/T/#t
>
> This lines should probably be more like:
>
>   cfg_file = repo_git_path(, "config");
>

Why? You did not mention the benefits for writing it this way
here or on the reference. Care to elaborate?


Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-10 Thread Brandon Williams
On 08/03, Stefan Beller wrote:
> e98317508c0 (submodule: ensure core.worktree is set after update,
> 2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir
> as that ensures both the 'core.worktree' configuration is set as well as
> setting up correct gitlink file pointing at the git directory.
> 
> We do not need to check for the gitlink in this part of the cmd_update
> in git-submodule.sh, as the initial call to update-clone will have ensured
> that. So we can reduce the work to only (check and potentially) set the
> 'core.worktree' setting.
> 
> While at it move the check from shell to C as that proves to be useful in
> a follow up patch, as we do not need the 'name' in shell now.
> 
> Signed-off-by: Stefan Beller 
> ---
>  builtin/submodule--helper.c | 64 +++--
>  git-submodule.sh|  7 ++--
>  2 files changed, 42 insertions(+), 29 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 8b1088ab58a..e7635d5d9ab 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, 
> const char *prefix)
>   return 0;
>  }
>  
> +static int ensure_core_worktree(int argc, const char **argv, const char 
> *prefix)
> +{
> + const struct submodule *sub;
> + const char *path;
> + char *cw;
> + struct repository subrepo;
> +
> + if (argc != 2)
> + BUG("submodule--helper connect-gitdir-workingtree  
> ");
> +
> + path = argv[1];
> +
> + sub = submodule_from_path(the_repository, _oid, path);
> + if (!sub)
> + BUG("We could get the submodule handle before?");
> +
> + if (repo_submodule_init(, the_repository, path))
> + die(_("could not get a repository handle for submodule '%s'"), 
> path);
> +
> + if (!repo_config_get_string(, "core.worktree", )) {
> + char *cfg_file, *abs_path;
> + const char *rel_path;
> + struct strbuf sb = STRBUF_INIT;
> +
> + cfg_file = xstrfmt("%s/config", subrepo.gitdir);

As I mentioned here:
https://public-inbox.org/git/20180807230637.247200-1-bmw...@google.com/T/#t

This lines should probably be more like:

  cfg_file = repo_git_path(, "config");

> +
> + abs_path = absolute_pathdup(path);
> + rel_path = relative_path(abs_path, subrepo.gitdir, );
> +
> + git_config_set_in_file(cfg_file, "core.worktree", rel_path);
> +
> + free(cfg_file);
> + free(abs_path);
> + strbuf_release();
> + }
> +
> + return 0;
> +}
> +
>  static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
>  {
>   int i;
> @@ -2029,29 +2068,6 @@ 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 = argv[1];
> - path = argv[2];
> -
> - strbuf_addf(, "%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();
> - free(sm_gitdir);
> -
> - return 0;
> -}
> -
>  #define SUPPORT_SUPER_PREFIX (1<<0)
>  
>  struct cmd_struct {
> @@ -2065,7 +2081,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},
> + {"ensure-core-worktree", ensure_core_worktree, 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 8caaf274e25..19d010eac06 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -535,6 +535,8 @@ cmd_update()
>   do
>   die_if_unmatched "$quickabort" "$sha1"
>  
> + git submodule--helper ensure-core-worktree "$sm_path"
> +
>   name=$(git submodule--helper name "$sm_path") || exit
>   if ! test -z "$update"
>   then
> @@ -577,11 +579,6 @@ 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 

[PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-03 Thread Stefan Beller
e98317508c0 (submodule: ensure core.worktree is set after update,
2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir
as that ensures both the 'core.worktree' configuration is set as well as
setting up correct gitlink file pointing at the git directory.

We do not need to check for the gitlink in this part of the cmd_update
in git-submodule.sh, as the initial call to update-clone will have ensured
that. So we can reduce the work to only (check and potentially) set the
'core.worktree' setting.

While at it move the check from shell to C as that proves to be useful in
a follow up patch, as we do not need the 'name' in shell now.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 64 +++--
 git-submodule.sh|  7 ++--
 2 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 8b1088ab58a..e7635d5d9ab 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int ensure_core_worktree(int argc, const char **argv, const char 
*prefix)
+{
+   const struct submodule *sub;
+   const char *path;
+   char *cw;
+   struct repository subrepo;
+
+   if (argc != 2)
+   BUG("submodule--helper connect-gitdir-workingtree  
");
+
+   path = argv[1];
+
+   sub = submodule_from_path(the_repository, _oid, path);
+   if (!sub)
+   BUG("We could get the submodule handle before?");
+
+   if (repo_submodule_init(, the_repository, path))
+   die(_("could not get a repository handle for submodule '%s'"), 
path);
+
+   if (!repo_config_get_string(, "core.worktree", )) {
+   char *cfg_file, *abs_path;
+   const char *rel_path;
+   struct strbuf sb = STRBUF_INIT;
+
+   cfg_file = xstrfmt("%s/config", subrepo.gitdir);
+
+   abs_path = absolute_pathdup(path);
+   rel_path = relative_path(abs_path, subrepo.gitdir, );
+
+   git_config_set_in_file(cfg_file, "core.worktree", rel_path);
+
+   free(cfg_file);
+   free(abs_path);
+   strbuf_release();
+   }
+
+   return 0;
+}
+
 static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -2029,29 +2068,6 @@ 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 = argv[1];
-   path = argv[2];
-
-   strbuf_addf(, "%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();
-   free(sm_gitdir);
-
-   return 0;
-}
-
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -2065,7 +2081,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},
+   {"ensure-core-worktree", ensure_core_worktree, 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 8caaf274e25..19d010eac06 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -535,6 +535,8 @@ cmd_update()
do
die_if_unmatched "$quickabort" "$sha1"
 
+   git submodule--helper ensure-core-worktree "$sm_path"
+
name=$(git submodule--helper name "$sm_path") || exit
if ! test -z "$update"
then
@@ -577,11 +579,6 @@ 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.132.g195c49a2227