Re: [PATCH v3] upload-pack: send shallow info over stdin to pack-objects

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

Duy Nguyen <pclouds@gmail.com> writes:

> On Fri, Mar 7, 2014 at 1:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I like what I see in this patch, but I wonder if we can essentially
>> revert that "temporary shallow file" patch and replace it with the
>> same (or a similar) mechanism uniformly?
>
> Using --shallow-file is uniform.  The only downside is temporary file.
> Without it you'll need to think of a way (probably different way for
> each command) to feed shallow info to.

Yes, that is what I meant to say by the "we need a way to tell hooks
in some cases" below; we are in agreement.

>> On the receive-pack side, the comment at the bottom of
>> preprare_shallow_update() makes it clear that, if we wanted to use
>> hooks, we cannot avoid having the proposed new shallow-file in a
>> temporary file, which is unfortunate.  Do we have a similar issue on
>> hooks on the upload-pack side?
>
> No. I don't think we have hooks on upload-pack.

The question was not only about "do we happen to work OK with the
current code?" but about "are we closing the door for the future?"

If we ever need to add hooks to upload-pack, with the updated
approach, its operation will not be affected by the temporary
shallow file tailored for this specific customer.  Which I think is
a good thing in general.

But at the same time, it means that its operation cannot be
customized for the specific customer, taking into account what they
lack (which can be gleaned by looking at the temporary shallow
information).  I do think that it is not a big downside, but that is
merely my knee-jerk reaction.

>> Nothing for Documentation/ anywhere?
>
> Heh git-pack-objects.txt never described stdin format. At least I
> searched for --not in it and found none. So I gladly accepted the
> situation and skipped doc update :D

To pile new technical debt on top of existing ones is to make things
worse, which we would rather not to see.