Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones

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

Junio C Hamano <gitster@pobox.com> writes:

> This change affects non-clone/fetch uses of object listing depending
> on the shallowness of the repository, and does not even care if it
> is driven as part of the pack-object codepath, if I am reading it
> correctly.  It smells wrong.
>
> The problematic fbd4a70 already had unintended fallout that needed
> to be corrected with 200abe74 (list-objects: only look at cmdline
> trees with edge_hint, 2014-01-20).  The current code with the fix,
> the decision to use the more expensive marking is tied to
> "edge_hint". I notice that edge_hint is turned on only if the caller
> of rev-list passes the "--objects-edge" option, and currently that
> only happens in the pack-objects codepath when "thin" is given.
> Perhaps that part should decide if it really wants to do edge_hint
> depending on the shallowness of the repository perhaps?
>
> That is, something like this instead?

Eh, perhaps not like that, as that would disable milder use of
"thin" when fetching into non-shallow repository.

The right approach would be more like allocating one more bit in
struct rev_info (call that edge_hint_aggressive), give a new option
"--objects-edge-aggressive", and do something like

	if (thin) {
        	use_internal_rev_list = 1;
		argv_array_push(&rp, is_repository_shallow()
                	? "--objects-edge-aggressive"
                        : "--objects-edge");
	}

in this codepath?  I'd actually suggest is_repository_shallow()
detection to happen one level even higher (i.e. make decision at the
caller of pack-objects) and decide to pass either "--thin" or
"--thin-aggressive", so that we can make sure that the damage caused
by fbd4a70 to be limited only to fetches into shallow repository
with stronger confidence.


>
>  builtin/pack-objects.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 3f9f5c7..a9ebf56 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -2709,7 +2709,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>  		usage_with_options(pack_usage, pack_objects_options);
>  
>  	argv_array_push(&rp, "pack-objects");
> -	if (thin) {
> +	if (thin && is_repository_shallow()) {
>  		use_internal_rev_list = 1;
>  		argv_array_push(&rp, "--objects-edge");
>  	} else