From: Karthik Nayak <karthik.188@gmail.com>
Date: Thu, 10 Sep 2026 09:28:20 GMT
Subject: Re: [PATCH v4 2/9] builtin/clone: defer setup of the object database
Message-ID: <CAOLa=ZSH5WtAej++SuWhupLjYv_rLQs8cCcXgETNjLH7_mkZyA@mail.gmail.com>
In-Reply-To: <20260909-pks-odb-write-alternates-at-creation-time-v4-2-d8a78ffc32e4@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> When cloning a repository we defer initialization of the reference
> database. This is because we don't yet know all details required for us
> to initialize the refdb in the first place. Most importantly, what we
> are missing is information about the object hash.
>
> We don't do the same thing for the object database yet, but here we
> essentially have the same problem. While the "files" database does not
> need any information about the object format at creation time, alternate
> backends are likely to require that information so that they can
> properly set up their data structures.
>
> Besides this forward-looking future proofing though, we also have a
> second use case for deferring initialization of the object database,
> namely alternates. When initializing the object database we do not yet
> know whether we'll need alternates or not because this depends on the
> repository we're about to clone from. If it is a local repository and
> the user has passed "--refernce{,-if-able}", then we will end up writing
> alternates into the object database.
>

Nit: s/refernce/reference

> The ugly part though is that we cannot determine where the repository is
> getting cloned from before it has been initialized. While we of course
> already have access to the user-provided URI, that URI can be very well
> rewritten via "url.<base>.insteadOf". We can of course read the global-
> and system-level configuration to resolve it. But we explicitly resolve
> the URI a second time after we have initialized the repository because
> it can happen that we copy a ".git/config" over from our templates, and
> that file may cause us to rewrite the path.
>
> In a subsequent commit though we'll start to write alternates as part of
> the repository initialization, so we'll need to have the URI properly
> resolved before we can initialize the object database. This is ugly, but
> as mentioned above it makes sense for us to defer its initialization
> anyway so that we also know about the object hash already.
>
> Defer creation of the object database until after we have resolved the
> URI.
>
> Note that this also requires us to defer the call to `setup_reference()`
> until after we have created the object database. While you might think
> that this function has something to do with references ("refs/*"), it is
> in fact responsible for setting up alternates. Consequently, we can only
> call it after we have created the object database already.
>

Haha. I like this last para, you kinda explained the thought I was
getting as I was getting it :)

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/clone.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 904d2d859f..bdcbd7aa1b 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1188,7 +1188,6 @@ int cmd_clone(int argc,
>  	create_repository(the_repository, git_dir, real_git_dir, work_tree,
>  			  option_template, GIT_HASH_UNKNOWN, ref_storage_format,
>  			  do_not_override_repo_unix_permissions, NULL);
> -	create_object_database(the_repository);
>
>  	if (real_git_dir) {
>  		free((char *)git_dir);
> @@ -1311,9 +1310,6 @@ int cmd_clone(int argc,
>  		strbuf_reset(&key);
>  	}
>
> -	if (option_required_reference.nr || option_optional_reference.nr)
> -		setup_reference();
> -

So this is the alternate setup, which we move down to after the
`create_object_database()`. Looks good.

>  	remote = remote_get_early(remote_name);
>
>  	if (!option_rev)
> @@ -1342,6 +1338,10 @@ int cmd_clone(int argc,
>  	if (option_local > 0 && !is_local)
>  		warning(_("--local is ignored"));
>
> +	create_object_database(the_repository);
> +	if (option_required_reference.nr || option_optional_reference.nr)
> +		setup_reference();
> +
>  	transport = transport_get(remote, path ? path : remote->url.v[0]);
>  	transport_set_verbosity(transport, option_verbosity, option_progress);
>  	transport->family = family;
>
> --
> 2.55.0.1074.ge7621b4bad.dirty