From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Stefan Beller <sbeller@google.com>,
allan.jensen@qt.io, git <git@vger.kernel.org>
Subject: Re: [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)
Date: Fri, 07 Sep 2018 19:04:01 -0700 [thread overview]
Message-ID: <xmqqmussvj72.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180908000946.GA225427@aiede.svl.corp.google.com> (Jonathan Nieder's message of "Fri, 7 Sep 2018 17:09:46 -0700")
Jonathan Nieder <jrnieder@gmail.com> writes:
> It is late in the release cycle, so revert the whole 3-patch series.
> We can try again later for 2.20.
>
> Reported-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
> Helped-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Stefan Beller wrote:
>> Jonathan Nieder wrote:
>
>>> I think we
>>> should revert e98317508c0 in "master" (for 2.19) and keep making use
>>> of that 'second try' in "next" (for 2.20).
>>
>> Actually I'd rather revert the whole topic leading up to
>> 7e25437d35a (Merge branch 'sb/submodule-core-worktree', 2018-07-18)
>> as the last patch in there doesn't work well without e98317508c0 IIRC.
>>
>> And having only the first patch would bring an inconsistent state as
>> then different commands behave differently w.r.t. setting core.worktree.
>
> Like this (generated using "git revert -m1)?
OK. Thanks for taking care of it.
>
> 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, 2 insertions(+), 55 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b56028ba9d..f6fb8991f3 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1123,8 +1123,6 @@ 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);
> }
>
> @@ -2005,29 +2003,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 {
> @@ -2041,7 +2016,6 @@ 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 f7fd80345c..1cb2c0a31b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -580,11 +580,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
> diff --git a/submodule.c b/submodule.c
> index 50cbf5f13e..a2b266fbfa 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1534,18 +1534,6 @@ 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();
> @@ -1711,8 +1699,6 @@ 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 7d476cefa7..e452919aa4 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -127,8 +127,6 @@ 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 5b56b23166..016391723c 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
> - # core.worktree is unset for sub2 as it is not checked out
> + GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
> fi &&
> # indicate we are interested in the submodule:
> git -C submodule_update config submodule.sub1.url "bogus" &&
> @@ -709,8 +709,7 @@ 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_must_fail git config -f .git/modules/sub1/config core.worktree
> + ! test -e sub1
> )
> '
> # ... absorbing a .git directory along the way.
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 7d3d984210..c0ffc1022a 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -984,11 +984,6 @@ 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 &&
next prev parent reply other threads:[~2018-09-08 2:06 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-07 9:52 Old submodules broken in 2.19rc1 and 2.19rc2 Allan Sandfeld Jensen
2018-09-07 15:03 ` Jeff King
2018-09-07 15:18 ` Allan Sandfeld Jensen
2018-09-07 20:14 ` Jonathan Nieder
2018-09-07 17:08 ` Stefan Beller
2018-09-07 20:20 ` Jonathan Nieder
2018-09-07 22:33 ` Allan Sandfeld Jensen
2018-09-07 22:35 ` Jonathan Nieder
2018-09-07 22:45 ` Stefan Beller
2018-09-08 0:09 ` [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2) Jonathan Nieder
2018-09-08 2:04 ` Junio C Hamano [this message]
2018-09-08 18:39 ` Johannes Sixt
2018-09-10 17:11 ` Stefan Beller
2018-09-11 19:49 ` Junio C Hamano
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
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=xmqqmussvj72.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=allan.jensen@qt.io \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).