Re: [PATCH v3 1/2] list-objects-filter: only parse sparse OID when 'have_git_dir'

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Jeff Hostetler <jeffhost@microsoft.com>,
	Jon Simons <jon@jonsimons.org>,
	git@vger.kernel.org, me@ttaylorr.com, sunshine@sunshineco.com,
	stolee@gmail.com
Subject: Re: [PATCH v3 1/2] list-objects-filter: only parse sparse OID when 'have_git_dir'
Date: Thu, 05 Sep 2019 11:57:32 -0700	[thread overview]
Message-ID: <xmqqv9u6po4j.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190904045424.GA6488@sigill.intra.peff.net> (Jeff King's message of "Wed, 4 Sep 2019 00:54:24 -0400")

Jeff King <peff@peff.net> writes:

> So these patches are punting on the greater question of why we want to
> parse so early, and are not making anything worse. AFAICT, "clone
> --filter=sparse:oid" has never worked (even though our tests did cover
> the underlying rev-list and pack-objects code paths).
> ...
> TBH, I'm not sure why the original is so eager to parse early. I guess
> it allows:
>
>   - a dual use of the options parser; we can use it both to sanity-check
>     the options before sending them to a server, and to actually use the
>     filter ourselves.
>
>   - earlier detection maybe gives us a cleaner error path (e.g.,
>     rev-list can do its own error handling). But I'd think doing it when
>     we actually initialize the filter would be enough.
>
> I.e., if we want to go all the way, I think this two-patch series could
> basically be replaced with something like the (totally untested)
> approach below, which just pushes the parsing closer to the
> point-of-use.
>
> Adding Jeff Hostetler to the cc, in case he recalls any reason not to
> use that approach.

Thanks.