From: Junio C Hamano <gitster@pobox.com> To: Stefan Beller <sbeller@google.com> Cc: git@vger.kernel.org Subject: Re: [PATCH 2/4] submodule: unset core.worktree if no working tree is present Date: Sat, 08 Dec 2018 15:44:23 +0900 Message-ID: <xmqqwook7c1k.fsf@gitster-ct.c.googlers.com> (raw) In-Reply-To: <20181207235425.128568-3-sbeller@google.com> Stefan Beller <sbeller@google.com> writes: > This reintroduces 4fa4f90ccd (submodule: unset core.worktree if no working > tree is present, 2018-06-12), which was reverted as part of f178c13fda > (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07). > > 4fa4f90ccd was reverted as its followup commit was faulty, but without > the accompanying change of the followup, we'd have an incomplete workflow > of setting `core.worktree` again, when it is needed such as checking out > a revision that contains a submodule. > > So re-introduce that commit as-is, focusing on fixing up the followup I was hoping to hear (given what 0/4 claimed) a clearer explanation of what this change wants to achieve, but that is lacking. No need to grumble about an earlier work was that turned out to be inappropriate for the codebase back then. Repeatedly saying "this is needed" without giving further explaining why it is so, or anything like that, would help readers. Just pretend that the ealier commits and their reversion never happened, and further pretend that we are doing the best thing that should happen to our codebase. How would we explain this change, what the problem it tries to solve and what the solution looks like in the larger picture? When removing a working tree of a submodule (e.g. we may be switching back to an earlier commit in the superproject that did not have the submodule in question yet), we failed to unset core.worktree of the submodule's repository. That caused this and that issues, exhibited by a few new tests this commit adds. Make sure that core.worktree gets unset so that a leftover setting won't cause these issues. or something like that? I am just guessing by looking at the old commit's text, as the above two paragraphs and one sentence does not say much. > diff --git a/submodule.c b/submodule.c > index 6415cc5580..d393e947e6 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -1561,6 +1561,18 @@ 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(); > @@ -1726,6 +1738,8 @@ 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 a680214c01..9e18e9b807 100644 > --- a/submodule.h > +++ b/submodule.h > @@ -131,6 +131,8 @@ 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 016391723c..51d4555549 100755 > --- a/t/lib-submodule-update.sh > +++ b/t/lib-submodule-update.sh > @@ -709,7 +709,8 @@ 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 -e sub1 && > + test_must_fail git config -f .git/modules/sub1/config core.worktree > ) > ' > # ... absorbing a .git directory along the way.
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 [this message] 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 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=xmqqwook7c1k.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 list mirror (unofficial, 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