Re: [PATCH v3 2/9] builtin/clone: defer setup of the object database
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Sep 8, 2026, 22:20 UTC
- Message-ID
- <aqCI5pfM3AGtfQsJ@denethor>
- In-Reply-To
- <20260907-pks-odb-write-alternates-at-creation-time-v3-2-735d0b5b3e00@pks.im>
On 26/09/07 10:25AM, Patrick Steinhardt wrote:
Show 42 quoted lines
> 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.
>
> 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.Ah ok, so now that we are deferring ODB creation a bit further, we need to also move ref DB creation to afterwards too. Makes sense.
Show 34 quoted lines
> 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();
> -
> 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();Ok so the move here is really just in preparation for the alternates setup being handled as part of ODB creation in a subsequent patch and purely just structural. Looks good.
-Justin