Junio C Hamano <gitster@xxxxxxxxx> writes: > Glen Choo <chooglen@xxxxxxxxxx> writes: > >> Glen Choo <chooglen@xxxxxxxxxx> writes: >> >>> Junio C Hamano <gitster@xxxxxxxxx> writes: >>> >>>> Glen Choo <chooglen@xxxxxxxxxx> writes: >>>> >>>>> To clarify, does this opinion of "don't use config values that aren't >>>>> copied into .git/config" extend to in-tree .gitmodules? Prior to this >>>>> series, we always read the in-tree .gitmodules to get the config - the >>>>> user does not need to copy the settings to .git/config, but we don't >>>>> pick a commit to read .gitmodules from. >>>> >>>> I think we do, but I also think it was a huge mistake to allow >>>> repository data to directly affect the behaviour of local checkout. >>> >>> I'm inclined to agree. >>> >>>> Fixing that is most likely outside the scope of this series, though. >>> >>> Agree. Thanks! >> >> I thought that this would have been the end of the discussion, but after >> reading <xmqqa6dpllmc.fsf@gitster.g>, I guess I had the wrong impression >> (oops). >> >> If I am reading everything correctly, we both agree that it's not >> good to read _any_ config values from .gitmodules (even if it's >> in-tree), and that we should clean it up outside of this topic. So for >> this topic to be merged into 'next', is it enough to say that I will fix >> this behavior in a follow up topic? > > At least we should remember that is something to be fixed. It may > not be you personally who addresses that issue, though ;-) Perhaps squashing in a NEEDSWORK comment into [PATCH v5 09/10] will suffice? I can also resend this series if preferred. ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ---- diff --git a/submodule.c b/submodule.c index 6e6b2d04e4..93c78a4dc3 100644 --- a/submodule.c +++ b/submodule.c @@ -795,6 +795,21 @@ static const char *default_name_or_path(const char *path_or_name) * superproject commit that points to the submodule, but this is * arbitrary - we can choose any (super_oid, path) that matches the * submodule's name. + * + * NEEDSWORK: Storing an arbitrary commit is undesirable because we can't + * guarantee that we're reading the commit that the user would expect. A better + * scheme would be to just fetch a submodule by its name. This requires two + * steps: + * - Create a function that behaves like repo_submodule_init(), but accepts a + * submodule name instead of treeish_name and path. This should be easy + * because repo_submodule_init() internally uses the submodule's name. + * + * - Replace most instances of 'struct submodule' (which is the .gitmodules + * config) with just the submodule name. This is OK because we expect + * submodule settings to be stored in .git/config (via "git submodule init"), + * not .gitmodules. This also lets us delete get_non_gitmodules_submodule(), + * which constructs a bogus 'struct submodule' for the sake of giving a + * placeholder name to a gitlink. */ struct changed_submodule_data { /*