Re: [PATCH 1/2] submodule: create helper to build paths to submodule gitdirs
On 08/10, Junio C Hamano wrote:
> Brandon Williams writes:
>
> > Introduce a helper function "submodule_name_to_gitdir()" (and the
> > submodule--helper subcommand "gitdir") which constructs a path to a
> > submodule's gitdir, located in the provided repository's "modules"
> > directory.
> >
> > This consolidates the logic needed to build up a path into a
> > repository's "modules" directory, abstracting away the fact that
> > submodule git directories are stored in a repository's common gitdir.
> > This makes it easier to adjust how submodules gitdir are stored in the
> > "modules" directory in a future patch.
> >
> > Signed-off-by: Brandon Williams
> > ---
> > ...
> > @@ -2018,7 +2039,7 @@ static int connect_gitdir_workingtree(int argc, const
> > char **argv, const char *p
> > name = argv[1];
> > path = argv[2];
> >
> > - strbuf_addf(, "%s/modules/%s", get_git_dir(), name);
> > + submodule_name_to_gitdir(, the_repository, name);
> > sm_gitdir = absolute_pathdup(sb.buf);
> >
> > connect_work_tree_and_git_dir(path, sm_gitdir, 0);
>
> This function goes away with 1c866b98 ("submodule--helper: replace
> connect-gitdir-workingtree by ensure-core-worktree", 2018-08-03) in
> sb/submodule-update-in-c topic. git-submodule.sh has simlar
> conflicts.
>
> I guess its replacement function does not care as deeply as its
> predecessor used to about where the submodule's $GIT_DIR is, so the
> correct resolution may be just to ignore the change made to this
> caller to the new name-to-gitdir function.
Well that patch still cares about where the gitdir is except it
initializes a "struct repository" for the submodule and then builds a
path to the config using:
cfg_file = xstrfmt("%s/config", subrepo.gitdir);
hmm...I didn't get a chance to look at that series but that line looks
wrong. It probably should be more like:
cfg_file = repo_git_path(, "config");
I'll go comment on that series.
>
> It would have been nicer to see a bit better inter-developer
> coordination, especially between two who sit practically next to
> each other ;-)
>
> Thanks.
--
Brandon Williams
Re: [PATCH 1/2] submodule: create helper to build paths to submodule gitdirs
Brandon Williams writes:
> Introduce a helper function "submodule_name_to_gitdir()" (and the
> submodule--helper subcommand "gitdir") which constructs a path to a
> submodule's gitdir, located in the provided repository's "modules"
> directory.
>
> This consolidates the logic needed to build up a path into a
> repository's "modules" directory, abstracting away the fact that
> submodule git directories are stored in a repository's common gitdir.
> This makes it easier to adjust how submodules gitdir are stored in the
> "modules" directory in a future patch.
>
> Signed-off-by: Brandon Williams
> ---
> ...
> @@ -2018,7 +2039,7 @@ static int connect_gitdir_workingtree(int argc, const
> char **argv, const char *p
> name = argv[1];
> path = argv[2];
>
> - strbuf_addf(, "%s/modules/%s", get_git_dir(), name);
> + submodule_name_to_gitdir(, the_repository, name);
> sm_gitdir = absolute_pathdup(sb.buf);
>
> connect_work_tree_and_git_dir(path, sm_gitdir, 0);
This function goes away with 1c866b98 ("submodule--helper: replace
connect-gitdir-workingtree by ensure-core-worktree", 2018-08-03) in
sb/submodule-update-in-c topic. git-submodule.sh has simlar
conflicts.
I guess its replacement function does not care as deeply as its
predecessor used to about where the submodule's $GIT_DIR is, so the
correct resolution may be just to ignore the change made to this
caller to the new name-to-gitdir function.
It would have been nicer to see a bit better inter-developer
coordination, especially between two who sit practically next to
each other ;-)
Thanks.
Re: [PATCH 1/2] submodule: create helper to build paths to submodule gitdirs
On 08/08, Stefan Beller wrote:
> On Wed, Aug 8, 2018 at 3:33 PM Brandon Williams wrote:
> >
> > Introduce a helper function "submodule_name_to_gitdir()" (and the
> > submodule--helper subcommand "gitdir") which constructs a path to a
> > submodule's gitdir, located in the provided repository's "modules"
> > directory.
>
> Makes sense.
>
> >
> > This consolidates the logic needed to build up a path into a
> > repository's "modules" directory, abstracting away the fact that
> > submodule git directories are stored in a repository's common gitdir.
> > This makes it easier to adjust how submodules gitdir are stored in the
> > "modules" directory in a future patch.
>
> and yet, all places that we touch were and still are broken for old-style
> submodules that have their git directory inside the working tree?
> Do we need to pay attention to those, too?
This series only tries to address the issues with submodules stored in
$GITDIR/modules/ and places in our codebase that explicitly reference
submodules stored there.
For those old-old-style submodules, wouldn't the absorb submodule
functions handle that migration?
>
>
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 8b5ad59bde..053747d290 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
>
> > @@ -577,7 +578,7 @@ 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
> > + if ! $(git config -f "$(git submodule--helper gitdir
> > "$name")/config" core.worktree) 2>/dev/null
>
> This will collide with origin/sb/submodule-update-in-c specifically
> 1c866b9831d (submodule--helper: replace connect-gitdir-workingtree
> by ensure-core-worktree, 2018-08-03), but as that removes these lines,
> it should be easy to resolve the conflict.
--
Brandon Williams
Re: [PATCH 1/2] submodule: create helper to build paths to submodule gitdirs
On Wed, Aug 8, 2018 at 3:33 PM Brandon Williams wrote:
>
> Introduce a helper function "submodule_name_to_gitdir()" (and the
> submodule--helper subcommand "gitdir") which constructs a path to a
> submodule's gitdir, located in the provided repository's "modules"
> directory.
Makes sense.
>
> This consolidates the logic needed to build up a path into a
> repository's "modules" directory, abstracting away the fact that
> submodule git directories are stored in a repository's common gitdir.
> This makes it easier to adjust how submodules gitdir are stored in the
> "modules" directory in a future patch.
and yet, all places that we touch were and still are broken for old-style
submodules that have their git directory inside the working tree?
Do we need to pay attention to those, too?
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8b5ad59bde..053747d290 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -577,7 +578,7 @@ 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
> + if ! $(git config -f "$(git submodule--helper gitdir
> "$name")/config" core.worktree) 2>/dev/null
This will collide with origin/sb/submodule-update-in-c specifically
1c866b9831d (submodule--helper: replace connect-gitdir-workingtree
by ensure-core-worktree, 2018-08-03), but as that removes these lines,
it should be easy to resolve the conflict.
[PATCH 1/2] submodule: create helper to build paths to submodule gitdirs
Introduce a helper function "submodule_name_to_gitdir()" (and the
submodule--helper subcommand "gitdir") which constructs a path to a
submodule's gitdir, located in the provided repository's "modules"
directory.
This consolidates the logic needed to build up a path into a
repository's "modules" directory, abstracting away the fact that
submodule git directories are stored in a repository's common gitdir.
This makes it easier to adjust how submodules gitdir are stored in the
"modules" directory in a future patch.
Signed-off-by: Brandon Williams
---
builtin/submodule--helper.c | 28 +++--
dir.c| 2 +-
git-submodule.sh | 7 +++--
repository.c | 3 +-
submodule.c | 53
submodule.h | 7 +
t/t7410-submodule-checkout-to.sh | 6 ++--
7 files changed, 75 insertions(+), 31 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a3c4564c6c..5bfd2d0be9 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -906,6 +906,21 @@ static int module_name(int argc, const char **argv, const
char *prefix)
return 0;
}
+static int module_gitdir(int argc, const char **argv, const char *prefix)
+{
+ struct strbuf gitdir = STRBUF_INIT;
+
+ if (argc != 2)
+ usage(_("git submodule--helper gitdir "));
+
+ submodule_name_to_gitdir(, the_repository, argv[1]);
+
+ printf("%s\n", gitdir.buf);
+
+ strbuf_release();
+ return 0;
+}
+
struct sync_cb {
const char *prefix;
unsigned int flags;
@@ -1268,18 +1283,24 @@ static int add_possible_reference_from_superproject(
* standard layout with .git/(modules/)+/objects
*/
if (ends_with(alt->path, "/objects")) {
+ struct repository alternate;
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
strbuf_add(, alt->path, strlen(alt->path) -
strlen("objects"));
+ repo_init(, sb.buf, NULL);
+
/*
* We need to end the new path with '/' to mark it as a dir,
* otherwise a submodule name containing '/' will be broken
* as the last part of a missing submodule reference would
* be taken as a file name.
*/
- strbuf_addf(, "modules/%s/", sas->submodule_name);
+ strbuf_reset();
+ submodule_name_to_gitdir(, , sas->submodule_name);
+ strbuf_addch(, '/');
+ repo_clear();
sm_alternate = compute_alternate_path(sb.buf, );
if (sm_alternate) {
@@ -1392,7 +1413,7 @@ static int module_clone(int argc, const char **argv,
const char *prefix)
usage_with_options(git_submodule_helper_usage,
module_clone_options);
- strbuf_addf(, "%s/modules/%s", get_git_dir(), name);
+ submodule_name_to_gitdir(, the_repository, name);
sm_gitdir = absolute_pathdup(sb.buf);
strbuf_reset();
@@ -2018,7 +2039,7 @@ static int connect_gitdir_workingtree(int argc, const
char **argv, const char *p
name = argv[1];
path = argv[2];
- strbuf_addf(, "%s/modules/%s", get_git_dir(), name);
+ submodule_name_to_gitdir(, the_repository, name);
sm_gitdir = absolute_pathdup(sb.buf);
connect_work_tree_and_git_dir(path, sm_gitdir, 0);
@@ -2040,6 +2061,7 @@ struct cmd_struct {
static struct cmd_struct commands[] = {
{"list", module_list, 0},
{"name", module_name, 0},
+ {"gitdir", module_gitdir, 0},
{"clone", module_clone, 0},
{"update-clone", update_clone, 0},
{"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
diff --git a/dir.c b/dir.c
index 21e6f2520a..7a9827ea4b 100644
--- a/dir.c
+++ b/dir.c
@@ -3053,7 +3053,7 @@ static void connect_wt_gitdir_in_nested(const char
*sub_worktree,
strbuf_reset(_wt);
strbuf_reset(_gd);
strbuf_addf(_wt, "%s/%s", sub_worktree, sub->path);
- strbuf_addf(_gd, "%s/modules/%s", sub_gitdir, sub->name);
+ submodule_name_to_gitdir(_gd, , sub->name);
connect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 1);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index 8b5ad59bde..053747d290 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -252,12 +252,13 @@ Use -f if you really want to add it." >&2
fi
else
- if test -d ".git/modules/$sm_name"
+ sm_gitdir="$(git submodule--helper gitdir "$sm_name")"
+ if test -d "$sm_gitdir"
then
if test -z "$force"
then
