Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

From: Stefan Beller <sbeller@google.com>
Date: Aug 9, 2016, 18:09 UTC
Message-ID: <CAGZ79kY8EiGaugsh4FxKYp1FxqYr10JfGqsrfsnhULB+OBnFXw@mail.gmail.com>
In-Reply-To: <xmqq8tw5bzs1.fsf@gitster.mtv.corp.google.com>

On Tue, Aug 9, 2016 at 10:47 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> "you did ask me to use alternates once and for all when setting up the
>> superproject: now for this added submodule I don't find the alternate;
>> That is strange?"
>
> Absolutely.  I do not think you should expect a user to remember if
> s/he used alternates when getting a copy of the superproject long
> time ago.  Giving "info: using from ..." is good; giving "warning:
> not using from ..." is probably irritating, I would think.
>
> Stepping back a bit, when the user says --reference-if-able, does it
> warrant any warning either way?

Stepping another step back, let's discuss the expectations of the new option "--reference-if-able".

The way I understood and implemented it is

    here is a path, try to use it as an alternate; if that is not
    an alternate, it's fine too; maybe warn about it, but carry
    on with the operation.

With such a semantics you can just add the --reference-if-able to the submodules that try to borrow from the other superprojects submodule.

    I expected the "--reference-if-able" not necessarily be
    used by the end user. It is a convenient way for our scripted
    use, as the -if-able is just a check if the path exists and nothing
    else.

    We could check if the git dir exists inthe helper for the
    submodule command, such that we only pass --reference
    as we are certain the alternate exists; we could have a switch
    in the helper --on-missing-submodule-alternate=[die,info,silent]

The way you write this response I think you have a slightly different understanding of what the -if-able ought to do?

    When --reference is given, only the superproject should
    borrow and the -if-able, may allow submodules to also borrow?

Show 7 quoted lines

> I.e. "we made it borrow from there,
> so be careful not to trash that one" may be just as warning-worthy
> (if not even more) as "you said we can borrow from there if there is
> anything to borrow, but it turns out there isn't any, so the result
> is complete stand-alone."  It feels as if we can go without any
> warning at least from "git clone --reference-if-able", unless "-v"
> option is given.

But when git clone is not issueing a warning/info, who is responsible for that? As you noted the superproject may be setup some time ago and the user forgot they used references and want to use references for this new submodule. So the helper would need to do that?

Show 7 quoted lines

>
> I have a very strong opinion what should happen when the end-user
> facing command is "git submodule", but if I have to choose, I would
> prefer to see "git submodule update" tell what it did with "info:"
> either case, i.e. "info: borrowing from ... because the superproject
> borrows from there", and "info: not borrowing from ... even though
> the superproject borrows from there".

I see. This way the user is most informed. The current implementation of "submodule update --init" for start from scratch yields for example:

Submodule 'foo' (<url>) registered for path 'foo' Submodule 'hooks' (<url>) registered for path 'hooks' Cloning into '/home/sbeller/super/foo'... Cloning into '/home/sbeller/super/hooks'... warning: Not using proposed alternate /home/sbeller/super-reference/.git/modules/hooks/ Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a' Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'

so before this series we had 3 lines per submodule, and with this series we get a 4th line for the unusual case.

You would prefer to see always 4 lines per submodule? Is one extra line (25% more output) a reasonable tradeoff for the reference feature?

I dunno; I guess we could argue either way.