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 {
/*
