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

Duy Nguyen [off-list ref] writes:

On Fri, Mar 7, 2014 at 1:37 AM, Junio C Hamano [off-list ref] wrote:

quoted

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.

quoted

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.

quoted

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.