Re: [PATCH v5 00/10] fetch --recurse-submodules: fetch unpopulated submodules

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Glen Choo <chooglen@xxxxxxxxxx> writes:
>
>> This uses a "first one wins approach", which obviously doesn't have
>> correctness guarantees. But in practice, I don't think this is likely to
>> cause problems:
>>
>> - As far as I can tell, the only value we read from .gitmodules is
>>   'submodule.<name>.fetchRecurseSubmodules', and this value gets
>>   overridden by two other values: the CLI option, and the config
>>   variable with the same name in .git/config.
>>
>>   During "git submodule init", we copy the config values from
>>   .gitmodules to .git/config. Since we can only fetch init-ed submodules
>>   anyway, it's quite unlikely that we will ever actually make use of the
>>   .gitmodules config.
>
> These are reasonable.
>
>> - Even if we do use the .gitmodules config values, it's unlikely that
>>   the values in .gitmodules will change often, so it _probably_ won't
>>   matter which one we choose.
>
> What bad things would we see if the value changes during the span of
> history of the superproject we fetched?  How often we would see
> broken behaviour is immaterial and breakage being rare is a no excuse
> to import a new code with designed-in flaw.  Unless the "rare" is
> "never", that is.

Makes sense, I'll keep this mind.

> I would think using ANY values from .gitmodules without having the
> end-user agree with the settings and copying the settings to the
> .git/config is a BUG.  So if it mattered from which superproject
> commit we took .gitmodules from, that would mean we already have
> such a bug and it is not a new problem.
>
> That would be a reasonable argument for this topic. Together with
> the previous point, i.e. we do not copy values we see in the in-tree
> .gitmodules file to .git/config anyway, it would make a good enough
> assurance, I would think.

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.

If we still want to consider in-tree .gitmodules e.g. by merging
.git/config and .gitmodules, then we still have the new problem of
choosing the right .gitmodules.

If the answer is "no, we don't even consider in-tree .gitmodules"
(unless we really have to, like cloning a new submodule), that seems
pretty safe and predictable because we wouldn't have to look in two
different places to figure out what the user wants. And more crucially,
we'd never have to guess which .gitmodules to read - which will become
more of an issue as we add more support for init-ed but unpopulated
submodules.

> It leads to a possible #leftoverbit clean-up.  Because we only fetch
> submodules that are initialized, the API functions we are using in
> this series has no reason to require us to feed _a_ commit in the
> superproject to them so that they can find .gitmodules in them.

Hm, this is true; an initialized submodule should already have the
'expected' information in .git/config. And if we no longer have to fret
about whether we're reading the correct .gitmodules, we can revisit the
idea of "init a subrepo using only its name".

> Fixing the API can probably be left outside the scope of the topic,
> to be done soon after the dust from the topic settles, I think, to
> avoid distracting us from the topic.
>
> Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux