Re: [PATCH v2 20/25] upload-pack: support define shallow boundary by excluding revisions

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

Eric Sunshine <sunshine@sunshineco.com> writes:

>> @@ -746,6 +757,13 @@ static void receive_needs(void)
>>                         struct object *o = want_obj.objects[i].item;
>>                         argv_array_push(&av, oid_to_hex(&o->oid));
>>                 }
>> +               if (deepen_not.nr) {
>> +                       argv_array_push(&av, "--not");
>> +                       for (i = 0; i < deepen_not.nr; i++) {
>> +                               struct string_list_item *s = deepen_not.items + i;
>> +                               argv_array_push(&av, s->string);
>> +                       }
>
> The documentation for rev-list --not says it "Reverses the meaning ...
> up to the next --not", so would it make sense to add a final:
>
>     argv_array_push(&av, "--not");
>
> here to future-proof against some change pushing more arguments onto
> 'av' following this code?

Yup.

We enumerate the objects that are reachable are by traversing from
"want", but "--max-age" and "--not these refs" are optionally used
to truncate the enumeration, and usually we write "options" before
"parameters", so from that point of view, if this new code comes
before adding want_obj.objects[] (i.e. positive endpoints), that
would even be more readable.  And it would force this new code to
have a trailing "--not" ;-)

>> +               }
>>                 deepen_by_rev_list(av.argc, av.argv, &shallows);
>>                 argv_array_clear(&av);
>>         }