Re: [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates

[Jacob Keller <jacob.keller@gmail.com>]

On Wed, Aug 24, 2016 at 3:52 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Aug 23, 2016 at 11:29 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Tue, Aug 23, 2016 at 4:03 PM, Stefan Beller <sbeller@google.com> wrote:
>>>>> +
>>>>> +       if (option_recursive) {
>>>>> +               if (option_required_reference.nr &&
>>>>> +                   option_optional_reference.nr)
>>>>> +                       die(_("clone --recursive is not compatible with "
>>>>> +                             "both --reference and --reference-if-able"));
>>>>
>>>> So if you have multiple references that don't all match we basically
>>>> just refuse to allow recursive?
>>>>
>>>> Would it be better to simply assume that we want to die on missing
>>>> references instead of failing the clone here?
>>>
>>> The new config options are per repo (or even set globally), and not
>>> per alternate. And as we communicate the [if-able] part via the config
>>> options to the submodules it is not feasible to transport both
>>> kinds of (reference-or-die and reference-but-ignore-misses).
>>>
>>> That is why I introduced this check in the first place. If we'd go back
>>> to the drawing board and come up with a solution that is on a
>>> "per alternate" basis we could allow such things.
>>>
>>>> That is, treat it so
>>>> that multiple reference and reference-if-able will die, and only info
>>>> if we got only reference-if-able?
>>>>
>>>> Probably what's here is fine, and mixing reference and
>>>> reference-if-able doesn't make much sense.
>>>
>>> I think the reference-if-able doesn't make sense for one project alone
>>> as you can easily script around that, but is only useful if you have
>>> submodules in a partially checked out superproject that you want
>>> to reference to.
>>>
>>> Thanks,
>>> Stefan
>>
>> I'm not sure there is a better design.  How are alternates stored? In
>> a config section?
>
> Alternates are stored in .git/objects/info/alternates
> with each alternate in a new line. On that file (from
> (man gitrepository-layout):
>
> objects/info/alternates
>
> This file records paths to alternate object stores that this object store
> borrows objects from, one pathname per line. Note that not only native
> Git tools use it locally, but the HTTP fetcher also tries to use it remotely;
> this will usually work if you have relative paths (relative to the object
> database, not to the repository!) in your alternates file, but it will not work
> if you use absolute paths unless the absolute path in filesystem and web
> URL is the same. See also objects/info/http-alternates.
>
> So changing that file is out of question.
> Ideally we would have a flag for each path here, though.
>
>> Or is there some way we can store the is-able per
>> alternate and look it up when adding them to submodule?
>
> I guess we could invent a file as alternate-flags that is matches
> line by line to the alternates file.
>
> I don't think we'd want to go that way for now as it would really only
> help in an edge case?
>
> If we later find out we need the flag on a per-alternate basis we can
> still come up with a solution and just not set these config variables,
> so I think we'll be fine for now with this approach.
>

Yes that seems reasonable.

Thanks,
Jake