Re: [RFC PATCH] promisor-remote: always JIT fetch with --refetch

>
> Emily Shaffer <emilyshaffer@google.com> writes:
>
> > By the time we decide we need to do a partial clone fetch, we already
> > know the object is missing, even if the_repository->parsed_objects
> > thinks it exists. But --refetch bypasses the local object check, so we
> > can guarantee that a JIT fetch will fix incorrect local caching.
> > ...
> > The culprit is that we're assuming all local refs already must have
> > objects in place. Using --refetch means we ignore that assumption during
> > JIT fetch.
>
> Hmph.  The whole lazy fetch business looks more and more broken X-<.

> There is a comment in the refetch code path that tells us to "perform
> a full refetch ignoring existing objects", but if an object truly
> exists, there should be no need to refetch, and it starts to smell
> more like "ignoring somebody who gives us an incorrect information
> that these objects exist".
>
> But a ref that points at a missing commit is "somebody giving a
> false information" and an option to ignore such misinformation would
> be a perfect tool fit to sweep such a breakage under the rug.
>
> But is this sufficient?  Looking at how check_exist_and_connected()
> does its work, I am not sure how it would cope with a case where an
> object that is pointed by a ref does happen to exist, but the commit
> that is referred to by the commit is missing, as it only checks the
> existence of the tips.

>
> > diff --git a/promisor-remote.c b/promisor-remote.c
> > index 9345ae3db2..cf00e31d3b 100644
> > --- a/promisor-remote.c
> > +++ b/promisor-remote.c
> > @@ -43,7 +43,7 @@ static int fetch_objects(struct repository *repo,
> >       strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
> >                    "fetch", remote_name, "--no-tags",
> >                    "--no-write-fetch-head", "--recurse-submodules=no",
> > -                  "--filter=blob:none", "--stdin", NULL);
> > +                  "--filter=blob:none", "--refetch", "--stdin", NULL);
> >       if (!git_config_get_bool("promisor.quiet", &quiet) && quiet)
> >               strvec_push(&child.args, "--quiet");
> >       if (start_command(&child))
>
> The documentation for "git fetch --refetch" says that this grabs
> everything as if we are making a fresh clone, ignoring everything we
> already have.  Which makes the change in this patch prohibitively
> expensive for asking each single object lazily from the promisor
> remote, but is that really the case?  If there is a reasonable
> safety that prevents us from doing something silly like transferring
> one clone worth of data for every single object we lazily fetch,
> perhaps this would be a workable solution (but if that is the case,
> perhaps "git fetch --refetch" documentation needs to be rephrased,
> to avoid such an impression).