Junio C Hamano <[email protected]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
