Re: [PATCH 2/2] repack: repack promisor objects if -a or -A is set

[Junio C Hamano <gitster@pobox.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

> @@ -293,6 +346,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	if (pack_everything & ALL_INTO_ONE) {
>  		get_non_kept_pack_filenames(&existing_packs, &keep_pack_list);
>  
> +		if (repository_format_partial_clone)
> +			repack_promisor_objects(&po_args, &names);
> +
>  		if (existing_packs.nr && delete_redundant) {
>  			if (unpack_unreachable) {
>  				argv_array_pushf(&cmd.args,

Just a note (and a request-for-sanity-check) and not meant to be a
request to update the code, but with a still-in-pu 4b757a40 ("Use
remote_odb_get_direct() and has_remote_odb()", 2018-08-02) in
flight, repository_format_partial_clone is now gone.

I've tentatively resolved the above to read like so:

		if (has_remote_odb())
			repack_promisor_objects(&po_args, &names);

but I am not sure if it makes sense to always require odb helper to
be present for any promisor.  As long as you do not have need to
actually access these missing objects, you do not need any remote
odb access at all, in which case requiring has_remote_odb() as a
precondition to concatenate the promisor packs to coalesce them into
one pack does not make sense---you only want to know if there are
any .promisor packs.

In other words, I suspect that the world is not black (i.e. partial
clone, which always has remote-odb) and white (i.e. full repository,
without remote-odb).  4b757a40 makes it impossible to have a gray
(i.e. partial clone, but no access to remote-odb), which I am not
sure if it is a good thing.