Re: [PATCH v3 1/2] list-objects-filter: only parse sparse OID when 'have_git_dir'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 30, 2019, 18:08 UTC
- Message-ID
- <xmqqr252y199.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190829231925.15223-2-jon@jonsimons.org>
Jon Simons <jon@jonsimons.org> writes:
Show 14 quoted lines
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c > index 1cb20c659c..aaba312edb 100644 > --- a/list-objects-filter-options.c > +++ b/list-objects-filter-options.c > @@ -71,7 +71,8 @@ static int gently_parse_list_objects_filter( > * command, but DO NOT complain if we don't have the blob or > * ref locally. > */ > - if (!get_oid_with_context(the_repository, v0, GET_OID_BLOB, > + if (have_git_dir() && > + !get_oid_with_context(the_repository, v0, GET_OID_BLOB, > &sparse_oid, &oc)) > filter_options->sparse_oid_value = oiddup(&sparse_oid); > filter_options->choice = LOFC_SPARSE_OID;
Sorry, I do not quite understand what this wants to do. We say "we parsed this correctly, this filter is sparse:oid=<blob>" without filling sparse_oid_value field at all. What do the consumers of such a filter_options instance do to such a half-read option?
If they say "ah, the parser wanted to do sparse:oid but we couldn't really figure the <blob> part out, so let's ignore it", that might be the best they could do anyway, but it somewhat feels iffy. I wonder if we are better off inventing a new "we tried to parse but we lack sufficient info to make it useful" value to use in .choice field and return it instead.
In the longer term, what do we want to happen in such a case where "read this blob to figure out what I want you to do" cannot be satisfied due to chicken-and-egg situation like this? Can we postpone fetching or cloning that *wants* to use the named <blob> when we discover that the <blob> is not available (of which, your "!have_git_dir()" is a subset), grab that single <blob> first from the other side before doing the main transfer, and then resume the transfer that wants to use the <blob> after we successfully do so, or something?