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

Re: [PATCH v2] fetch: delay fetch_if_missing=0 until after config

From
Emily Shaffer <emilyshaffer@google.com>
Date
Oct 23, 2019, 23:30 UTC
Message-ID
<20191023233026.GC124161@google.com>
In-Reply-To
<20191023214428.129593-1-jonathantanmy@google.com>
On Wed, Oct 23, 2019 at 02:44:28PM -0700, Jonathan Tan wrote:
Show 22 quoted lines
> Suppose, from a repository that has ".gitmodules", we clone with
> --filter=blob:none:
> 
>   git clone --filter=blob:none --no-checkout \
>     https://kernel.googlesource.com/pub/scm/git/git
> 
> Then we fetch:
> 
>   git -C git fetch
> 
> This will cause a "unable to load config blob object", because the
> fetch_config_from_gitmodules() invocation in cmd_fetch() will attempt to
> load ".gitmodules" (which Git knows to exist because the client has the
> tree of HEAD) while fetch_if_missing is set to 0.
> 
> fetch_if_missing is set to 0 too early - ".gitmodules" here should be
> lazily fetched.  Git must set fetch_if_missing to 0 before the fetch
> because as part of the fetch, packfile negotiation happens (and we do
> not want to fetch any missing objects when checking existence of
> objects), but we do not need to set it so early. Move the setting of
> fetch_if_missing to the earliest possible point in cmd_fetch(), right
> before any fetching happens.
I think your sign-off is missing from the new commit message, right?
Otherwise it looks fine to me.
Show 25 quoted lines
> ---
> No changes from v1 except that I improved the commit message.
> 
> Thanks, Emily, for taking a look.
> 
> > I'm having some trouble figuring out which object is actually missing.
> > Is this the .git/config object? (That doesn't make much sense to me...)
> > Is it .gitmodules?
> 
> Yes, it is indeed .gitmodules. I improved the commit message to further
> explain what is going on.
> 
> > By the way, I think I understand that this is OK to go in
> > unconditionally because:
> >  - In the full clone case, it's a no-op; we haven't got anything that's
> >    missing, so who cares.
> >  - In the filter case, it's as you said - we don't want to
> >    fetch_if_missing because that will turn someone's partial clone into
> >    a a full clone.
> >    - This probably applies to bare checkout, too.
> 
> Yes, that is correct. What do you mean by bare checkout? If you mean the
> checkout that happens after clone (that we can suppress with
> --no-checkout), that indeed happens after fetch_if_missing=0, so we
> shouldn't have a problem there.

I meant bare clone, not checkout, my apologies, but as I understand it better, they're completely separate concepts - that is, you can certainly have a bare clone which is also a full clone. So, please disregard this comment.

 - Emily
Previous: Jonathan TanNext: Jonathan Tan
Message 4 of 11 in “fetch: delay fetch_if_missing=0 until after config”
  1. fetch: delay fetch_if_missing=0 until after configJonathan Tan, Oct 7, 2019
  2. Emily ShafferOct 23, 2019
  3. fetch: delay fetch_if_missing=0 until after configJonathan Tan, Oct 23, 2019
  4. Emily ShafferOct 23, 2019
  5. fetch: delay fetch_if_missing=0 until after configJonathan Tan, Oct 23, 2019
  6. Junio C HamanoOct 24, 2019
  7. Jonathan TanOct 24, 2019
  8. Junio C HamanoOct 25, 2019
  9. Jonathan TanOct 25, 2019
  10. Junio C HamanoOct 29, 2019
  11. Jonathan TanNov 1, 2019

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.