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

[Stefan Beller <sbeller@google.com>]

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.

Thanks,
Stefan