From: Junio C Hamano <gitster@pobox.com> To: Stefan Beller <sbeller@google.com> Cc: git@vger.kernel.org Subject: Re: [PATCH 4/4] submodule deinit: unset core.worktree Date: Sat, 08 Dec 2018 16:03:56 +0900 Message-ID: <xmqqo99w7b4z.fsf@gitster-ct.c.googlers.com> (raw) In-Reply-To: <20181207235425.128568-5-sbeller@google.com> Stefan Beller <sbeller@google.com> writes: > This re-introduces 984cd77ddb (submodule deinit: unset core.worktree, > 2018-06-18), which was reverted as part of f178c13fda (Revert "Merge > branch 'sb/submodule-core-worktree'", 2018-09-07) > > The whole series was reverted as the offending commit e98317508c > (submodule: ensure core.worktree is set after update, 2018-06-18) > was relied on by other commits such as 984cd77ddb. > > Keep the offending commit reverted, but its functionality came back via > 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), such > that we can reintroduce 984cd77ddb now. None of the above three explains the most important thing directly, so readers fail to grasp what the main theme of the three-patch series is, without looking at the commits that were reverted already. Is the theme of the overall series to make sure core.worktree is set to point at the working tree when submodule's working tree is instantiated, and unset it when it is not? 2/4 was also explained (in the original) that it wants to unset and did so when "move_head" gets called. This one does the unset when a submodule is deinited. Are these the only two cases a submodule loses its working tree? If so, the log message for this step should declare victory by ending with something like ... as we covered the only other case in which a submodule loses its working tree in the earlier step (i.e. switching branches of top-level project to move to a commit that did not have the submodule), this makes the code always maintain core.worktree correctly unset when there is no working tree for a submodule. Thanks. I think I agree with what the series wants to do (if I read what it wants to do correctly, that is). > Signed-off-by: Stefan Beller <sbeller@google.com> > --- > builtin/submodule--helper.c | 2 ++ > t/lib-submodule-update.sh | 2 +- > t/t7400-submodule-basic.sh | 5 +++++ > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 31ac30cf2f..672b74db89 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -1131,6 +1131,8 @@ 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); > } > > diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh > index 51d4555549..5b56b23166 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 > - GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree > + # core.worktree is unset for sub2 as it is not checked out > fi && > # indicate we are interested in the submodule: > git -C submodule_update config submodule.sub1.url "bogus" && > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index 76a7cb0af7..aba2d4d6ee 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -984,6 +984,11 @@ 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 index Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-07 23:54 [PATCH 0/4] Stefan Beller 2018-12-07 23:54 ` [PATCH 1/4] submodule update: add regression test with old style setups Stefan Beller 2018-12-09 0:11 ` Junio C Hamano 2018-12-07 23:54 ` [PATCH 2/4] submodule: unset core.worktree if no working tree is present Stefan Beller 2018-12-08 6:44 ` Junio C Hamano 2018-12-07 23:54 ` [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree Stefan Beller 2018-12-08 6:55 ` Junio C Hamano 2018-12-12 22:46 ` Stefan Beller 2018-12-13 3:14 ` Junio C Hamano 2018-12-07 23:54 ` [PATCH 4/4] submodule deinit: unset core.worktree Stefan Beller 2018-12-08 7:03 ` Junio C Hamano [this message] 2018-12-08 5:57 ` [PATCH 0/4] Junio C Hamano 2018-12-12 22:35 ` Stefan Beller 2018-12-13 3:15 ` Junio C Hamano 2018-12-14 23:59 ` [PATCH 0/4] submodules: unset core.worktree when no working tree present Stefan Beller 2018-12-14 23:59 ` [PATCH 1/4] submodule update: add regression test with old style setups Stefan Beller 2018-12-26 18:21 ` Junio C Hamano 2018-12-14 23:59 ` [PATCH 2/4] submodule: unset core.worktree if no working tree is present Stefan Beller 2018-12-26 18:27 ` Junio C Hamano 2018-12-14 23:59 ` [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree Stefan Beller 2018-12-14 23:59 ` [PATCH 4/4] submodule deinit: unset core.worktree Stefan Beller
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=xmqqo99w7b4z.fsf@gitster-ct.c.googlers.com \ --to=gitster@pobox.com \ --cc=git@vger.kernel.org \ --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
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