Re: [PATCH] mktree: learn about promised objects

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 15, 2022, 21:01 UTC
Message-ID: <xmqqa6adzln6.fsf@gitster.g>
In-Reply-To: <ad9b5ec9-14fd-cd66-be87-2fe1eb24296a@roku.com>

Richard Oliver <roliver@roku.com> writes:

Show 7 quoted lines

> 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?

As far as I remember, I wrote the original behaviour implemented in 1c64e79a (mktree --missing: allow missing objects, 2009-05-10) when the command is run without --missing primarily to be extra paranoid to detect broken repository with every chance we have to avoid spreading existing breakage to new objects we create. With the input "mktree" gets from the end user, we have no need to learn anything from existing objects in order to create the tree object at all.

The original 1c64e79a even carefully made sure that with --missing in effect, we do not ask the object store about an object:

	/* It is perfectly normal if we do not have a commit from a submodule */
	if (S_ISGITLINK(mode))
		allow_missing = 1;

	if (!allow_missing)
		type = sha1_object_info(sha1, NULL);
	else
		type = object_type(mode);

	if (type < 0)
		die("object %s unavailable", sha1_to_hex(sha1));

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.