All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Richard Oliver <roliver@roku.com>, Taylor Blau <me@ttaylorr.com>,
	Derrick Stolee <derrickstolee@github.com>,
	git@vger.kernel.org, jonathantanmy@google.com
Subject: Re: [PATCH] mktree: learn about promised objects
Date: Thu, 16 Jun 2022 01:02:44 -0400	[thread overview]
Message-ID: <Yqq5dLagSfVMzyQH@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqa6adzln6.fsf@gitster.g>

On Wed, Jun 15, 2022 at 02:01:33PM -0700, Junio C Hamano wrote:

> Richard Oliver <roliver@roku.com> writes:
> 
> > Meanwhile, is it worth considering a '--promised-as-missing' option
> > (or a config option) for invocations such as 'mktree --missing' that
> > prevents promised objects being faulted-in? Currently, the only
> > reliable way that I've found to prevent 'mktree --missing' faulting-in
> > promised objects is to remove the remote. Such an option could either
> > set the global variable 'fetch_if_missing' to '0' or could ensure
> > 'OBJECT_INFO_SKIP_FETCH_OBJECT' is passed appropriately.
> 
> I didn't spend too much time on thinking this one through myself,
> but do we really need a separte option?
> [...]
> We by grave mistake at 31c8221a (mktree: validate entry type in
> input, 2009-05-14) started insisting on inspecting objects even when
> allow-mising was given.  I do not think it was sensible, given why
> we had "--missing" as an option to allow users to say "you do not
> have to be too paranoid".
> 
> The codebase is so distant but I think we should probably do a moral
> revert/reconstruct of that commit so that the extra paranoia of the
> said commit applies only when "--missing" is not in effect, or
> something like that.

FWIW, I had the same reaction. I think fixing "--missing" should be the
first step, and would unstick Richard's use case, as I understand it.

There is some value to improving the promisor case, since using
"--missing" is a pretty broad stroke (i.e., you'd fail to actual
corruption of missing non-promisor objects). That could either be
checking the promisor object and type without faulting it in, or just
skipping the type-check for objects after confirming that they're
promisors.

But that can come on top, I think. The use case there is also a bit
narrower.  The local repository does not know about all promised
objects. It can only see the boundaries of objects it doesn't have (so
with --filter=tree:0, for example, a partial clone of a repo with path
"a/b/c" would know about "b" but not "c"). So in the most general case
you'd still have to resort to "--missing", but I suspect in practice
you'd always feed things at that boundary to mktree (otherwise, how
would you even know the oid of "c").

-Peff

  reply	other threads:[~2022-06-16  5:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-14 13:36 [PATCH] mktree: learn about promised objects Richard Oliver
2022-06-14 14:14 ` Derrick Stolee
2022-06-14 16:33   ` Richard Oliver
2022-06-14 17:27     ` Derrick Stolee
2022-06-15  0:35       ` Taylor Blau
2022-06-15  4:00         ` Jeff King
2022-06-15 17:40           ` Richard Oliver
2022-06-15 18:17             ` Derrick Stolee
2022-06-16  6:07               ` Jeff King
2022-06-16  6:54                 ` [PATCH] is_promisor_object(): walk promisor packs in pack-order Jeff King
2022-06-16 14:00                   ` Derrick Stolee
2022-06-17 19:50                   ` Jonathan Tan
2022-06-16 13:59                 ` [PATCH] mktree: learn about promised objects Derrick Stolee
2022-06-15 21:01             ` Junio C Hamano
2022-06-16  5:02               ` Jeff King [this message]
2022-06-16 15:46               ` [PATCH] mktree: Make '--missing' behave as documented Richard Oliver
2022-06-16 17:44                 ` Junio C Hamano
2022-06-21 13:59                   ` [PATCH] mktree: do not check type of remote objects Richard Oliver
2022-06-21 16:51                     ` Junio C Hamano
2022-06-21 17:48                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yqq5dLagSfVMzyQH@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=me@ttaylorr.com \
    --cc=roliver@roku.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.