[RFC] On the --depth argument when fetching with submodules

[RFC] On the --depth argument when fetching with submodules

[Stefan Beller]

Currently when cloning a project, including submodules, the --depth argument
is passed on recursively, i.e. when cloning with "--depth 2", both the
superproject as well as the submodule will have a depth of 2.  It is not
garantueed that the commits as specified by the superproject are included
in these 2 commits of the submodule.

Illustration:
(superproject with depth 2, so A would have more parents, not shown)

superproject/master: A <- B
                    /      \
submodule/master:  C <- D <- E <- F <- G

(Current behavior is to fetch G and F)

The submodule is referenced at C and E from the two superproject commits.
So it is a reasonable expectation to have C and E included when fetching
the superproject with depth of 2.

So to fetch the correct submodule commits, we need to
* traverse the superproject and list all submodule commits.
* fetch these submodule commits (C and E) by sha1
* in case of shallow clones, I'd propose to have the submodules be shallowed
  with depth 1, i.e. to fetch C and E and no parents thereof
* we need to think of a way to preserve the commits in the submodule for
  later use (i.e. gc must not delete those commits)

For the later I propose to use a ref in the submodule (refs/meta/superproject ?)
which will be an evil merge of all relevant commits to be preserved for the
superproject. It is an evil merge commit as we do not need to store any worktree
at all, but we only care about the parents relationship preventing the gc to
collect data required from the superproject. The parents should contain all
branches or in case of disjoint shallow histories (like in the example above)
all the relevant commits.

Using these ideas we can go a step further in submodules and not need branches,
but only detached heads, which are directed from the superproject.

As for the implementation of these ideas, whenever you commit in the
superproject
with modification to a submodule, the superproject would need to modify the
refs/meta/superproject submodule branch to make sure the submodule is
safe against
garbage collection.

Thanks,
Stefan

Re: [RFC] On the --depth argument when fetching with submodules

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Currently when cloning a project, including submodules, the --depth argument
> is passed on recursively, i.e. when cloning with "--depth 2", both the
> superproject as well as the submodule will have a depth of 2.  It is not
> garantueed that the commits as specified by the superproject are included
> in these 2 commits of the submodule.
>
> Illustration:
> (superproject with depth 2, so A would have more parents, not shown)
>
> superproject/master: A <- B
>                     /      \
> submodule/master:  C <- D <- E <- F <- G
>
> (Current behavior is to fetch G and F)

I think the issue is deeper than merely "--depth 2", and you would
be better off stepping back and think about various use cases to
make sure that we know what kind of behaviour we want to support
before delving into one particular corner case.  We currently pass
the depth recursively, and I do not think it makes much sense, but I
view it as a secondary question "among the behaviours we want to
support, which one should be the default?"  It may turn out that not
passing it recursively at all, or even passing a different depth, is
a better default, but we wouldn't know until we know what are the
desirable behaviour in various workflows.

If you are actively working on the superproject plus some submodules
but you are merely using the submodule you depicted above, not
working on changing it, even when you want the full history of the
superproject (i.e. no "--depth 2"), you may not want history of the
submodule.  Even though we have a way to say "I am not interested in
this submodule AT ALL" by not doing "submodule init", not having
anything at all at the path submodule/ may not allow you to build
the whole thing, and we currently lack a way to express "I am not
interested in the history of this thing, but I need at least the
tree that matches the commit referred to by the superproject".

If you are working on a single submodule, trying to fix a bug in the
context of the whole project, you might want to have a single-depth
clone of the superproject and all other submodules, plus the whole
history of the single submodule.

In either of these examples, the top-level "--depth" does not have
much to do with what depth the user wants to use when cloning or
fetching the submodule repositories.

I have a feeling (but I would not be surprised if somebody who uses
submodules heavily has a counter-example from real life) that
regardless of "--depth" or full clone, fetching the tip of matching
branch is not a good default behaviour.  In your picture, even when
depth is not given at all, there isn't much point fetching F or G.

> So to fetch the correct submodule commits, we need to
> * traverse the superproject and list all submodule commits.
> * fetch these submodule commits (C and E) by sha1

I do not think requiring that C to be fetched when the superproject
is cloned with --depth=2 (hence A and B are present in the result)
is a good definition of "correct submodule commits".  The initial
clone could be "superproject follows --depth, all submodules are
cloned with --depth=1 at the commits referenced by the superproject
tree"--by that definition, you need E but you do not want C.

As a specification of the behaviour, the above two might work, but I
do not think that should be the implementation.  In other words,
"The implementation should behave as if it did the above two" is OK,
and it is also OK to qualify with further conditions to help the
implementation.  For example, the current structure assumes that E
and C are reachable from "some" ref in submodule, so that at least a
whole clone of the submodule would give them to you--otherwise you
would not be able to even build the superproject at A or B.  Perhaps
it is OK to further require that, when you are working in a single
branch mode and working on 'master', you are required to have
commits C and E reachable on the 'master' branch in the submodule,
and that may lets you limit the need for such scanning of the
history?

Re: [RFC] On the --depth argument when fetching with submodules

[Fredrik Gustafsson]

On Fri, Feb 05, 2016 at 04:05:01PM -0800, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
> 
> > Currently when cloning a project, including submodules, the --depth argument
> > is passed on recursively, i.e. when cloning with "--depth 2", both the
> > superproject as well as the submodule will have a depth of 2.  It is not
> > garantueed that the commits as specified by the superproject are included
> > in these 2 commits of the submodule.
> >
> > Illustration:
> > (superproject with depth 2, so A would have more parents, not shown)
> >
> > superproject/master: A <- B
> >                     /      \
> > submodule/master:  C <- D <- E <- F <- G
> >
> > (Current behavior is to fetch G and F)
> 

The --depth option to git submodule is something I've wondered for some
time if it was correct to implement it or not. It's clearly not a
complete feature yet and it has some weaknesses, the most obvious one
stated above.

The reason for implementing --depth in submodules was for people having
huge (or many) submodules, that they aren't interested in developing in,
but need to download to  build their project. So they want to save time
and bandwidth.

My first thought was to implement depth from the sha1 the superproject
refered sha1. However, when implementing this, git didn't support
fetching a sha1 from a remote repository for security reasons (you
should never be allowed to fetch a commit that is not reachable from a
branch or tag).

Waiting for this to be supported (an (expensive) check could be done if
the sha1 asked for exists in any branch or tag), the --depth was added
and it's a guessing game on how deep you should fetch.
-- 
Fredrik Gustafsson

phone: +46 733-608274
e-mail: iveqy@iveqy.com
website: http://www.iveqy.com

Re: [RFC] On the --depth argument when fetching with submodules

[Lars Schneider]

On 06 Feb 2016, at 01:05, Junio C Hamano <gitster@pobox.com> wrote:

> Stefan Beller <sbeller@google.com> writes:
> 
>> Currently when cloning a project, including submodules, the --depth argument
>> is passed on recursively, i.e. when cloning with "--depth 2", both the
>> superproject as well as the submodule will have a depth of 2.  It is not
>> garantueed that the commits as specified by the superproject are included
>> in these 2 commits of the submodule.
>> 
>> Illustration:
>> (superproject with depth 2, so A would have more parents, not shown)
>> 
>> superproject/master: A <- B
>>                    /      \
>> submodule/master:  C <- D <- E <- F <- G
>> 
>> (Current behavior is to fetch G and F)
> 
> I think the issue is deeper than merely "--depth 2", and you would
> be better off stepping back and think about various use cases to
> make sure that we know what kind of behaviour we want to support
> before delving into one particular corner case.  We currently pass
> the depth recursively, and I do not think it makes much sense, but I
> view it as a secondary question "among the behaviours we want to
> support, which one should be the default?"  It may turn out that not
> passing it recursively at all, or even passing a different depth, is
> a better default, but we wouldn't know until we know what are the
> desirable behaviour in various workflows.
> 
> If you are actively working on the superproject plus some submodules
> but you are merely using the submodule you depicted above, not
> working on changing it, even when you want the full history of the
> superproject (i.e. no "--depth 2"), you may not want history of the
> submodule.  Even though we have a way to say "I am not interested in
> this submodule AT ALL" by not doing "submodule init", not having
> anything at all at the path submodule/ may not allow you to build
> the whole thing, and we currently lack a way to express "I am not
> interested in the history of this thing, but I need at least the
> tree that matches the commit referred to by the superproject".
> 
> If you are working on a single submodule, trying to fix a bug in the
> context of the whole project, you might want to have a single-depth
> clone of the superproject and all other submodules, plus the whole
> history of the single submodule.
> 
> In either of these examples, the top-level "--depth" does not have
> much to do with what depth the user wants to use when cloning or
> fetching the submodule repositories.
> 
> I have a feeling (but I would not be surprised if somebody who uses
> submodules heavily has a counter-example from real life) that
> regardless of "--depth" or full clone, fetching the tip of matching
> branch is not a good default behaviour.  In your picture, even when
> depth is not given at all, there isn't much point fetching F or G.

I really wonder in what cases people use the "--depth" option, too. 
For instance I have never used it in either one of the two cases you
described above. I don't worry about a long running "clone" as it 
usually is a one-time operation.

However, in case of a continuous integration system that starts with
a clean state in the beginning of every run (e.g. Travis CI) a
"clone" operation is no one-time operation anymore. In this case the
"--depth 1" option makes very much sense to me. This was the situation
where I realized the problem that Stefan wants to tackle here and I
tried to make it tangible with a test case [1]. 

On top of that I think Git's error message is really confusing if
you clone a repo with "--depth" that has submodules and Git is not
fetching the necessary submodule commits:

Unable to checkout '$SHA' in submodule path 'path/to/submodule'

I tried to tackle that with [2] which would detect this case and
print the following error instead (slightly changed from the patch):

Unable to checkout '$SHA' in submodule path '/path/to/commit'.
Try to remove the '--depth' argument on clone!

[1] https://www.mail-archive.com/git%40vger.kernel.org/msg82614.html
[2] https://www.mail-archive.com/git%40vger.kernel.org/msg82613.html


> 
>> So to fetch the correct submodule commits, we need to
>> * traverse the superproject and list all submodule commits.
>> * fetch these submodule commits (C and E) by sha1
> 
> I do not think requiring that C to be fetched when the superproject
> is cloned with --depth=2 (hence A and B are present in the result)
> is a good definition of "correct submodule commits".  The initial
> clone could be "superproject follows --depth, all submodules are
> cloned with --depth=1 at the commits referenced by the superproject
> tree"--by that definition, you need E but you do not want C.
> 
> As a specification of the behaviour, the above two might work, but I
> do not think that should be the implementation.  In other words,
> "The implementation should behave as if it did the above two" is OK,
> and it is also OK to qualify with further conditions to help the
> implementation.  For example, the current structure assumes that E
> and C are reachable from "some" ref in submodule, so that at least a
> whole clone of the submodule would give them to you--otherwise you
> would not be able to even build the superproject at A or B.  Perhaps
> it is OK to further require that, when you are working in a single
> branch mode and working on 'master', you are required to have
> commits C and E reachable on the 'master' branch in the submodule,
> and that may lets you limit the need for such scanning of the
> history?
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] On the --depth argument when fetching with submodules

[Stefan Beller]

On Sun, Feb 7, 2016 at 5:32 AM, Lars Schneider <larsxschneider@gmail.com> wrote:
>
> On 06 Feb 2016, at 01:05, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> Currently when cloning a project, including submodules, the --depth argument
>>> is passed on recursively, i.e. when cloning with "--depth 2", both the
>>> superproject as well as the submodule will have a depth of 2.  It is not
>>> garantueed that the commits as specified by the superproject are included
>>> in these 2 commits of the submodule.
>>>
>>> Illustration:
>>> (superproject with depth 2, so A would have more parents, not shown)
>>>
>>> superproject/master: A <- B
>>>                    /      \
>>> submodule/master:  C <- D <- E <- F <- G
>>>
>>> (Current behavior is to fetch G and F)
>>
>> I think the issue is deeper than merely "--depth 2", and you would
>> be better off stepping back and think about various use cases to
>> make sure that we know what kind of behaviour we want to support
>> before delving into one particular corner case.  We currently pass
>> the depth recursively, and I do not think it makes much sense, but I
>> view it as a secondary question "among the behaviours we want to
>> support, which one should be the default?"  It may turn out that not
>> passing it recursively at all, or even passing a different depth, is
>> a better default, but we wouldn't know until we know what are the
>> desirable behaviour in various workflows.
>>
>> If you are actively working on the superproject plus some submodules
>> but you are merely using the submodule you depicted above, not
>> working on changing it, even when you want the full history of the
>> superproject (i.e. no "--depth 2"), you may not want history of the
>> submodule.  Even though we have a way to say "I am not interested in
>> this submodule AT ALL" by not doing "submodule init", not having
>> anything at all at the path submodule/ may not allow you to build
>> the whole thing, and we currently lack a way to express "I am not
>> interested in the history of this thing, but I need at least the
>> tree that matches the commit referred to by the superproject".
>>
>> If you are working on a single submodule, trying to fix a bug in the
>> context of the whole project, you might want to have a single-depth
>> clone of the superproject and all other submodules, plus the whole
>> history of the single submodule.
>>
>> In either of these examples, the top-level "--depth" does not have
>> much to do with what depth the user wants to use when cloning or
>> fetching the submodule repositories.
>>
>> I have a feeling (but I would not be surprised if somebody who uses
>> submodules heavily has a counter-example from real life) that
>> regardless of "--depth" or full clone, fetching the tip of matching
>> branch is not a good default behaviour.  In your picture, even when
>> depth is not given at all, there isn't much point fetching F or G.
>
> I really wonder in what cases people use the "--depth" option, too.
> For instance I have never used it in either one of the two cases you
> described above. I don't worry about a long running "clone" as it
> usually is a one-time operation.

I think there are 3 use cases.

1) You work on the superproject and don't care about the submodules.
In this case you want the superproject non-shallow and the submodules
may be just fine with depth 1. (Think of libraries pulled in via Git instead
of via the build system)

2) The superproject is a collection of submodules, i.e. not much content
in the superproject except for the submodules. You want to work
in the submodules, i.e. you want the suberproject shallow, and all
submodules deep.

3) same as 2, but you're interested in only one (or a few) submodules,
which means you want superproject and most of the other submodules
shallow, but one submodule needs to be deep.

So covering 1 and 2 is easy, 3 is complicated.
For 1) we can make it so, that the depth argument is not passed on,
but only covers the referenced submodule commits, and then we
introduce another switch "--submodule-depth" to cover 2).

For 3 we don't know which submodules the user is interested in,
so the user needs to unshallow the interesting submodules themselves
after doing a "--depth 1 --submodule--depth 1" clone. "--depth 1" sort of
implies "--submodule--depth 1", though.

>
> However, in case of a continuous integration system that starts with
> a clean state in the beginning of every run (e.g. Travis CI) a
> "clone" operation is no one-time operation anymore. In this case the
> "--depth 1" option makes very much sense to me. This was the situation
> where I realized the problem that Stefan wants to tackle here and I
> tried to make it tangible with a test case [1].

Thanks for the test! The problem to make it work is in making it working
in a backwards compatible way. Instead of the branch, you can just pass
a sha1 to git fetch and it sometimes works (if the server permits fetching
arbitrary or hidden sha1s. Though Jonathan noted this check may be in
the client only and the server trusts the client on not wanting arbitrary
sha1s?)

So for fetching I think we need to have a "--try-to-get-commit <sha1>"
argument for fetch, which depending on the server capabilities and
the history obtained otherwise may try again to fetch the exact sha1.


>
> On top of that I think Git's error message is really confusing if
> you clone a repo with "--depth" that has submodules and Git is not
> fetching the necessary submodule commits:
>
> Unable to checkout '$SHA' in submodule path 'path/to/submodule'
>
> I tried to tackle that with [2] which would detect this case and
> print the following error instead (slightly changed from the patch):
>
> Unable to checkout '$SHA' in submodule path '/path/to/commit'.
> Try to remove the '--depth' argument on clone!
>
> [1] https://www.mail-archive.com/git%40vger.kernel.org/msg82614.html
> [2] https://www.mail-archive.com/git%40vger.kernel.org/msg82613.html
>
>
>>
>>> So to fetch the correct submodule commits, we need to
>>> * traverse the superproject and list all submodule commits.
>>> * fetch these submodule commits (C and E) by sha1
>>
>> I do not think requiring that C to be fetched when the superproject
>> is cloned with --depth=2 (hence A and B are present in the result)
>> is a good definition of "correct submodule commits".  The initial
>> clone could be "superproject follows --depth, all submodules are
>> cloned with --depth=1 at the commits referenced by the superproject
>> tree"--by that definition, you need E but you do not want C.
>>
>> As a specification of the behaviour, the above two might work, but I
>> do not think that should be the implementation.  In other words,
>> "The implementation should behave as if it did the above two" is OK,
>> and it is also OK to qualify with further conditions to help the
>> implementation.  For example, the current structure assumes that E
>> and C are reachable from "some" ref in submodule, so that at least a
>> whole clone of the submodule would give them to you--otherwise you
>> would not be able to even build the superproject at A or B.  Perhaps
>> it is OK to further require that, when you are working in a single
>> branch mode and working on 'master', you are required to have
>> commits C and E reachable on the 'master' branch in the submodule,
>> and that may lets you limit the need for such scanning of the
>> history?
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [RFC] On the --depth argument when fetching with submodules

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> ... Instead of the branch, you can just pass
> a sha1 to git fetch and it sometimes works (if the server permits fetching
> arbitrary or hidden sha1s. Though Jonathan noted this check may be in
> the client only and the server trusts the client on not wanting arbitrary
> sha1s?)

allowtipsha1inwant and allowreachablesha1inwant are the server side
controls that are by default off.

> So for fetching I think we need to have a "--try-to-get-commit <sha1>"
> argument for fetch, which depending on the server capabilities and
> the history obtained otherwise may try again to fetch the exact sha1.

Hmph, "try"?  t5516 seems to show how to do this.

I however have to wonder how we disambiguate a 40-hex that is an
object name and a branch whose name happens to be 40-hex.

Re: [RFC] On the --depth argument when fetching with submodules

[Jonathan Nieder]

Hi,

Stefan Beller wrote:

> 1) You work on the superproject and don't care about the submodules.
> In this case you want the superproject non-shallow and the submodules
> may be just fine with depth 1. (Think of libraries pulled in via Git instead
> of via the build system)
>
> 2) The superproject is a collection of submodules, i.e. not much content
> in the superproject except for the submodules. You want to work
> in the submodules, i.e. you want the suberproject shallow, and all
> submodules deep.
>
> 3) same as 2, but you're interested in only one (or a few) submodules,
> which means you want superproject and most of the other submodules
> shallow, but one submodule needs to be deep.
>
> So covering 1 and 2 is easy, 3 is complicated.
> For 1) we can make it so, that the depth argument is not passed on,
> but only covers the referenced submodule commits, and then we
> introduce another switch "--submodule-depth" to cover 2).
>
> For 3 we don't know which submodules the user is interested in,
> so the user needs to unshallow the interesting submodules themselves
> after doing a "--depth 1 --submodule--depth 1" clone. "--depth 1" sort of
> implies "--submodule--depth 1", though.

What does a --submodule-depth other than 1 mean?  For example, if I pass 2
there, does that mean I want depth 2 in all sub-sub-submodules, and
sub-sub-sub-submodules, etc?  This has similar problems to the existing
"--depth=2 --recurse-submodules" options.

If I understood the use cases you are describing correctly, you'd want two
relevant options when cloning:

 --depth: how deep of a history to fetch in the superproject
 --shallow-submodules: whether the history in submodules is interesting

If the history in a submodule is uninteresting and the submodule
supports fetch-by-sha1, then a fetch-by-sha1 with depth 1 for any
commits needed by the superproject would get all objects needed.

--depth could imply --shallow-submodules in the same way as it implies
--single-branch because when people pass --depth they are usually
trying to avoid transferring too much data (for disk space or clone
time reasons).  Then someone wanting non-shallow submodules can
explicitly pass --no-shallow-submodules with --depth.

[...]
> So for fetching I think we need to have a "--try-to-get-commit <sha1>"
> argument for fetch, which depending on the server capabilities and
> the history obtained otherwise may try again to fetch the exact sha1.

The case of servers without fetch-by-sha1 is a hard one.

 1. One approach would be to use the ls-remote response to see if
    there is a ref pointing to the particular SHA-1 needed.  If there
    is one, we're all set.

 2. If there is no ref pointing to the desired SHA-1, fetching HEAD
    (or the branch named in the submodule.<name>.branch configuration)
    with full history is possible to work as a fallback.

 3. If the history of HEAD also doesn't contain the desired SHA-1,
    fetching the refs named in the default refspec (refs/heads/*,
    refs/tags/*) might work.

 4. If even then the desired SHA-1 is unreachable, then there's
    nothing left to try --- it's time to error out and have the user
    contact their server admin (as they'd already be doing today
    without --depth).

Perhaps some people would want to error out earlier.  Some
configuration or a --strict-shallow-submodules option could be useful
to error out if step (1) fails.  That would allow the user to be sure
that the submodules are actually fetched without history.

The missing functionality from fetch/ls-remote/etc is a way to read
the server capabilities that were sent with the ref listing to tell
whether the server supports fetch-by-sha1.

> On Sun, Feb 7, 2016 at 5:32 AM, Lars Schneider <larsxschneider@gmail.com> wrote:

>> On top of that I think Git's error message is really confusing if
>> you clone a repo with "--depth" that has submodules and Git is not
>> fetching the necessary submodule commits:
>>
>> Unable to checkout '$SHA' in submodule path 'path/to/submodule'

Yep.  Thanks for working on it.

This confusing error can appear without --depth, too (if the commit
named in the submodule is not an ancestor of anything in refs/heads/*
or refs/tags/*).  For example, this happens in gerrit when people try
to check out a change under review in a parent project that imports a
change in a subproject that has not been accepted yet (it is reachable
from refs/changes/* but not refs/heads/*).

Thanks,
Jonathan