git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Jonathan TanNext: Kousik Sanagavarapu
Message 8 of 19 in “[WIP] removed fetch_if_missing global”
  1. 0/2 [WIP] removed fetch_if_missing globalHariom Verma via GitGitGadget, Apr 20, 2020
  2. 1/2 fetch-pack: remove fetch_if_missing=0Hariom Verma via GitGitGadget, Apr 20, 2020
  3. Christian CouderMay 7, 2020
  4. Junio C HamanoMay 7, 2020
  5. Hariom vermaMay 9, 2020
  6. Hariom vermaMay 9, 2020
  7. Jonathan TanMay 7, 2020
  8. Hariom vermaMay 9, 2020
  9. Kousik SanagavarapuFeb 20, 2023
  10. Jonathan TanFeb 22, 2023
  11. Kousik SanagavarapuFeb 22, 2023
  12. 2/2 index-pack: remove fetch_if_missing=0Hariom Verma via GitGitGadget, Apr 20, 2020
  13. Christian CouderMay 7, 2020
  14. Hariom vermaMay 9, 2020
  15. Kousik SanagavarapuFeb 17, 2023
  16. Christian CouderFeb 18, 2023
  17. Kousik SanagavarapuFeb 19, 2023
  18. Christian CouderFeb 19, 2023
  19. Hariom vermaFeb 19, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.