Re: should git maintenance prefetch be taught to honor remote.fetch refspec?

From

TSTom Saeger <tom.saeger@oracle.com>

Date

Apr 5, 2021, 20:38 UTC

Message-ID

<20210405203832.x275fqoinfsw5gik@dhcp-10-154-153-195.vpn.oracle.com>

In-Reply-To

<xmqqr1jo5zzb.fsf@gitster.g>

On Mon, Apr 05, 2021 at 11:48:56AM -0700, Junio C Hamano wrote:

> Derrick Stolee <stolee@gmail.com> writes:
> 
> > ... but it becomes difficult
> > when we intend to make a distinction between "refs/heads/" and
> > other sub-areas within "refs/". Finally, I punted on that
> > conversion and made the logic in 'prefetch' extremely obvious:
> >
> > 1. If the refspec's 'dst' starts with "refs/", then replace
> >    that prefix with "refs/prefetch/".
> >
> > 2. If the refspec's 'dst' does not start with "refs/", then
> >    concatenate "refs/prefetch/" and 'dst'.
> >
> > This will keep a roughly-equivalent partition of refs (some
> > might have previously collided that will not any more).
> 
> Makes sense.  Do we need to add another rule?
> 
> 3. If the refspec does not have 'dst', ignore it.

I just tried what I think you're saying here.

Consider this fetch config:

Seems legal, fetch even works

$ git fetch pr-924
Fetching pr-924
From https://github.com/gitgitgadget/git
 * tag                         pr-924/derrickstolee/maintenance/refspec-v1 -> FETCH_HEAD

Prefetch segfaults

refspec which isn't a 'negative' with a src, but no dst.

Quick hack:

diff --git a/builtin/gc.c b/builtin/gc.c
index 92cb8b4e0bfa..2f4b8f2375c3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -899,6 +899,10 @@ static int fetch_remote(struct remote *remote, void *cbdata)
                        continue;
                }

+               if (!rsi->dst) {
+                       continue;
+               }
+
                refspec_item_init(&replace, remote->fetch.raw[i], 1);

                /*


Better, but now a different issue:

$ git maintenance run --task prefetch
fatal: --refmap option is only meaningful with command-line refspec(s).
error: failed to prefetch remotes
error: task 'prefetch' failed

So a "remote" where the only fetch url is skipped.


> 
> > I have posted my patch series [1], so please take a look. It
> > builds up the infrastructure to properly test such a refspec
> > expansion, if we wish to do so.
> >
> > [1] https://lore.kernel.org/git/pull.924.git.1617627856.gitgitgadget@gmail.com/
> >
> > Thanks,
> > -Stolee
> 
> Thanks.