git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org, seanwbehan@riseup.net, bmwill@google.com,
	hvoigt@hvoigt.net
Subject: Re: [PATCH 5/5] submodule: fixup nested submodules after moving the submodule
Date: Tue, 27 Mar 2018 17:07:14 -0700
Message-ID: <20180327170714.81b37f84cb4c544d76f34701@google.com> (raw)
In-Reply-To: <20180327213918.77851-6-sbeller@google.com>

On Tue, 27 Mar 2018 14:39:18 -0700
Stefan Beller <sbeller@google.com> wrote:

> connect_work_tree_and_git_dir is used to connect a submodule worktree with
> its git directory and vice versa after events that require a reconnection
> such as moving around the working tree. As submodules can have nested
> submoduled themselves, we'd also want to fix the nested submodules when

s/submoduled/submodules

> asked to. Add an option to recurse into the nested submodules and connect
> them as well.
> 
> As submodules are identified by their name (which determines their git
> directory in relation to their superprojects git directory) internally

s/superprojects/superproject's/

> and by their path in the working tree of the superproject, we need to
> make sure that the mapping of name <-> path is kept intact. We can do
> that in the git-mv command by writing out the gitmodules file and first

s/and //

> and then force a reload of the submodule config machinery.

s/force/forcing/

> 
> Signed-off-by: Stefan Beller <sbeller@google.com>

[snip]

> +static void connect_wt_gitdir_in_nested(const char *sub_worktree,
> +					const char *sub_gitdir,
> +					struct repository *superproject)
> +{
> +	int i;
> +	struct repository subrepo;
> +	struct strbuf sub_wt = STRBUF_INIT;
> +	struct strbuf sub_gd = STRBUF_INIT;
> +	const struct submodule *sub;
> +	const char *super_worktree,
> +		   *sub_path; /* path inside the superproject */
> +
> +	/* subrepo got moved, so superproject has outdated information */
> +	submodule_free(superproject);
> +
> +	super_worktree = real_pathdup(superproject->worktree, 1);
> +
> +	sub_path = sub_worktree + strlen(super_worktree) + 1;
> +
> +	if (repo_submodule_init(&subrepo, superproject, sub_path))
> +		return;
> +
> +	repo_read_index(&subrepo);

From the name of this function and its usage in
connect_work_tree_and_git_dir(), I expected this function to just
iterate through all the files in its workdir (which it is doing in the
"for" loop below) and connect any nested submodules. Why does it need
access to its superproject? (I would think that repo_init() would be
sufficient here instead of repo_submodule_init().)

> +
> +	for (i = 0; i < subrepo.index->cache_nr; i++) {
> +		const struct cache_entry *ce = subrepo.index->cache[i];
> +
> +		if (!S_ISGITLINK(ce->ce_mode))
> +			continue;
> +
> +		while (i + 1 < subrepo.index->cache_nr &&
> +		       !strcmp(ce->name, subrepo.index->cache[i + 1]->name))
> +			/*
> +			 * Skip entries with the same name in different stages
> +			 * to make sure an entry is returned only once.
> +			 */
> +			i++;
> +
> +		sub = submodule_from_path(&subrepo, &null_oid, ce->name);
> +		if (!sub)
> +			/* submodule not checked out? */
> +			continue;
> +
> +		strbuf_reset(&sub_wt);
> +		strbuf_addf(&sub_wt, "%s/%s/.git", sub_worktree, sub->path);
> +
> +		strbuf_reset(&sub_gd);
> +		strbuf_addf(&sub_gd, "%s/modules/%s", sub_gitdir, sub->name);
> +
> +		strbuf_setlen(&sub_wt, sub_wt.len - strlen("/.git"));
> +
> +		if (is_submodule_active(&subrepo, ce->name)) {
> +			connect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);
> +			connect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf, &subrepo);

The modifications of sub_wt and sub_gd should probably go here, since
they are not used unless this "if" block is executed.

> +void connect_work_tree_and_git_dir(const char *work_tree_,
> +				   const char *git_dir_,
> +				   int recurse_into_nested)

How is this function expected to be used? From what I see:
 - if recurse_into_nested is 0, this works regardless of whether the
   work_tree_ and git_dir_ is directly or indirectly a submodule of
   the_repository
 - if recurse_into_nested is 1, work_tree_ and git_dir_ must be directly
   a submodule of the_repository (since it is referenced directly)

This seems confusing - is this expected?

  parent reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-27 21:39 [PATCH 0/5] Moving submodules with nested submodules Stefan Beller
2018-03-27 21:39 ` [PATCH 1/5] submodule.h: drop declaration of connect_work_tree_and_git_dir Stefan Beller
2018-03-27 21:39 ` [PATCH 2/5] submodule-config: allow submodule_free to handle arbitrary repositories Stefan Beller
2018-03-27 22:57   ` Brandon Williams
2018-03-27 22:58   ` [PATCH] grep: remove "repo" arg from non-supporting funcs Jonathan Tan
2018-03-27 23:20     ` Stefan Beller
2018-03-28  0:24       ` Jonathan Tan
2018-03-28  0:35         ` Stefan Beller
2018-03-27 21:39 ` [PATCH 3/5] submodule-config: add repository argument to submodule_from_{name, path} Stefan Beller
2018-03-27 23:04   ` Jonathan Tan
2018-03-27 23:53     ` Stefan Beller
2018-03-27 21:39 ` [PATCH 4/5] submodule-config: remove submodule_from_cache Stefan Beller
2018-03-27 23:07   ` Jonathan Tan
2018-03-27 21:39 ` [PATCH 5/5] submodule: fixup nested submodules after moving the submodule Stefan Beller
2018-03-27 23:25   ` Brandon Williams
2018-03-28  0:07   ` Jonathan Tan [this message]
2018-03-28  0:42     ` Stefan Beller
2018-03-28 17:24       ` [PATCHv2 0/6] Moving submodules with nested submodules Stefan Beller
2018-03-28 17:24         ` [PATCH 1/6] submodule.h: drop declaration of connect_work_tree_and_git_dir Stefan Beller
2018-03-28 17:24         ` [PATCH 2/6] submodule-config: allow submodule_free to handle arbitrary repositories Stefan Beller
2018-03-28 17:24         ` [PATCH 3/6] submodule-config: add repository argument to submodule_from_{name, path} Stefan Beller
2018-03-28 17:24         ` [PATCH 4/6] submodule-config: remove submodule_from_cache Stefan Beller
2018-03-28 17:24         ` [PATCH 5/6] submodule: fixup nested submodules after moving the submodule Stefan Beller
2018-03-28 17:35           ` Brandon Williams
2018-03-28 19:08             ` Stefan Beller
2018-03-28 17:46           ` Jonathan Tan
2018-03-28 17:24         ` [PATCH 6/6] grep: remove "repo" arg from non-supporting funcs Stefan Beller
2018-03-28 17:54         ` [PATCHv2 0/6] Moving submodules with nested submodules Jonathan Tan
2018-03-28 22:35           ` [PATCHv3 " Stefan Beller
2018-03-28 22:35             ` [PATCHv3 1/6] submodule.h: drop declaration of connect_work_tree_and_git_dir Stefan Beller
2018-03-28 22:35             ` [PATCHv3 2/6] grep: remove "repo" arg from non-supporting funcs Stefan Beller
2018-03-28 22:35             ` [PATCHv3 3/6] submodule-config: allow submodule_free to handle arbitrary repositories Stefan Beller
2018-03-28 22:35             ` [PATCHv3 4/6] submodule-config: add repository argument to submodule_from_{name, path} Stefan Beller
2018-03-28 22:35             ` [PATCHv3 5/6] submodule-config: remove submodule_from_cache Stefan Beller
2018-03-28 22:35             ` [PATCHv3 6/6] submodule: fixup nested submodules after moving the submodule Stefan Beller
2018-03-28 23:17             ` [PATCHv3 0/6] Moving submodules with nested submodules Jonathan Tan

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180327170714.81b37f84cb4c544d76f34701@google.com \
    --to=jonathantanmy@google.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=sbeller@google.com \
    --cc=seanwbehan@riseup.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox