From: Brandon Williams <bmwill@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree
Date: Fri, 10 Aug 2018 14:47:03 -0700 [thread overview]
Message-ID: <20180810214703.GB211322@google.com> (raw)
In-Reply-To: <20180803222322.261813-7-sbeller@google.com>
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 <sbeller@google.com>
> ---
> 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 <name> <path>");
> +
> + path = argv[1];
> +
> + sub = submodule_from_path(the_repository, &null_oid, path);
> + if (!sub)
> + BUG("We could get the submodule handle before?");
> +
> + if (repo_submodule_init(&subrepo, the_repository, path))
> + die(_("could not get a repository handle for submodule '%s'"), path);
> +
> + if (!repo_config_get_string(&subrepo, "core.worktree", &cw)) {
> + 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-bmwill@google.com/T/#t
This lines should probably be more like:
cfg_file = repo_git_path(&subrepo, "config");
> +
> + abs_path = absolute_pathdup(path);
> + rel_path = relative_path(abs_path, subrepo.gitdir, &sb);
> +
> + git_config_set_in_file(cfg_file, "core.worktree", rel_path);
> +
> + free(cfg_file);
> + free(abs_path);
> + strbuf_release(&sb);
> + }
> +
> + 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> <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 {
> @@ -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
>
--
Brandon Williams
next prev parent reply other threads:[~2018-08-10 21:47 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-03 22:23 [PATCH 0/7] Resend of sb/submodule-update-in-c Stefan Beller
2018-08-03 22:23 ` [PATCH 1/7] git-submodule.sh: align error reporting for update mode to use path Stefan Beller
2018-08-03 22:23 ` [PATCH 2/7] git-submodule.sh: rename unused variables Stefan Beller
2018-08-03 22:23 ` [PATCH 3/7] builtin/submodule--helper: factor out submodule updating Stefan Beller
2018-08-03 22:23 ` [PATCH 4/7] builtin/submodule--helper: store update_clone information in a struct Stefan Beller
2018-08-03 22:23 ` [PATCH 5/7] builtin/submodule--helper: factor out method to update a single submodule Stefan Beller
2018-08-03 22:23 ` [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree Stefan Beller
2018-08-10 21:47 ` Brandon Williams [this message]
2018-08-10 21:52 ` Stefan Beller
2018-08-10 22:02 ` Brandon Williams
2018-08-03 22:23 ` [PATCH 7/7] submodule--helper: introduce new update-module-mode helper Stefan Beller
2018-08-03 22:36 ` [PATCH 0/7] Resend of sb/submodule-update-in-c Junio C Hamano
2018-08-13 22:42 ` Stefan Beller
2018-08-13 22:42 ` [PATCH 1/7] git-submodule.sh: align error reporting for update mode to use path Stefan Beller
2018-08-13 22:42 ` [PATCH 2/7] git-submodule.sh: rename unused variables Stefan Beller
2018-08-13 22:42 ` [PATCH 3/7] builtin/submodule--helper: factor out submodule updating Stefan Beller
2018-08-13 22:42 ` [PATCH 4/7] builtin/submodule--helper: store update_clone information in a struct Stefan Beller
2018-08-13 22:42 ` [PATCH 5/7] builtin/submodule--helper: factor out method to update a single submodule Stefan Beller
2018-08-13 22:42 ` [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree Stefan Beller
2018-08-13 22:42 ` [PATCH 7/7] submodule--helper: introduce new update-module-mode helper Stefan Beller
2018-08-18 16:10 ` Duy Nguyen
2018-08-20 19:44 ` Stefan Beller
2018-08-14 21:01 ` [PATCH 0/7] Resend of sb/submodule-update-in-c Junio C Hamano
2018-08-14 21:24 ` Stefan Beller
Reply instructions:
You may reply publicly 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180810214703.GB211322@google.com \
--to=bmwill@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sbeller@google.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).