Re: [PATCH 1/2] fetch-pack: remove fetch_if_missing=0
- From
Hariom verma <hariom18599@gmail.com>
- Date
- May 9, 2020, 18:00 UTC
- Message-ID
- <CA+CkUQ8fa=osCunE2Nj1ezzci_qEkv7mRwX=9Cg-kkMYcDHx=w@mail.gmail.com>
- In-Reply-To
- <20200507194354.33347-1-jonathantanmy@google.com>
On Fri, May 8, 2020 at 1:13 AM Jonathan Tan <jonathantanmy@google.com> wrote:
Show 5 quoted lines
> > As Christian said [1], please include tests like in the commit you > mentioned. For a change like this, I think that the test is the most > important part. >
I will definitely add tests.
Show 8 quoted lines
> Also include a justification for why it's safe to remove > fetch_if_missing=0. You can probably cite the aforementioned commit to > say that it covers the fetch_pack() method, and then go through the rest > of the code to see if any may inadvertently fetch an object. > > Also, the fetch-pack and index-pack parts can be sent in separate patch > sets, so you might want to concentrate on one command first. >
Thanks, Will split and concentrate on one at a time.
Show 23 quoted lines
>
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index 1734a573b01..1ca643f6491 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> > @@ -1649,7 +1649,7 @@ static void update_shallow(struct fetch_pack_args *args,
> > struct oid_array extra = OID_ARRAY_INIT;
> > struct object_id *oid = si->shallow->oid;
> > for (i = 0; i < si->shallow->nr; i++)
> > - if (has_object_file(&oid[i]))
> > + if (has_object_file_with_flags(&oid[i], OBJECT_INFO_SKIP_FETCH_OBJECT))
> > oid_array_append(&extra, &oid[i]);
> > if (extra.nr) {
> > setup_alternate_shallow(&shallow_lock,
>
> Hmm...this triggers when the user requests a clone that is both partial
> and shallow, and the server reports a shallow object that it didn't send
> back as a packfile; and it causes another fetch to be sent. This is a
> separate issue, but Hariom, if you'd like to take a look at this, that
> would work out too. You'll need to figure out how to make the server
> send back shallow lines referencing objects that are not in the packfile
> - one way to do it is to use one-time-perl. (Search the codebase to see
> how it is used.) This is probably more complex, though.I'm clueless about "one-time-perl" thing(till now!). Will surely going to learn about that.
Thanks for the nice explanation.
Regards, Hariom