Re: [PATCH v3 2/9] builtin/clone: defer setup of the object database

[Justin Tobler <jltobler@gmail.com>]

On 26/09/07 10:25AM, Patrick Steinhardt wrote:
> 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.

> 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