Jeff King [off-list ref] writes:
On Sun, Feb 14, 2010 at 09:27:40PM +0000, Michael Lukashov wrote:
quoted
The following functions are duplicated:
verify_remote_names
update_tracking_ref
print_ref_status
status_abbrev
print_ok_ref_status
print_one_push_status
refs_pushed
print_push_status
Signed-off-by: Michael Lukashov <redacted>
---
builtin-send-pack.c | 89 ++++++++++++++----------
send-pack.h | 20 +++++
transport.c | 196 ---------------------------------------------------
I think this is backwards. The versions in send-pack were there first,
and then were ported to transport.c so that other transports could
benefit from them. And that is where they should ultimately be.
I can't remember the exact details of why the originals were not
removed, though (I think I complained about it once before, and there
was some technical reason, but I don't recall now). Daniel (cc'd) might
remember more.
Also the names of these functions probably need to be made more specific
so that people not so familiar with the transport code can tell that they
are from "transport" family. The names didn't matter much while they were
file scope static, but this series changes that.
On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:
quoted
I can't remember the exact details of why the originals were not
removed, though (I think I complained about it once before, and there
was some technical reason, but I don't recall now). Daniel (cc'd) might
remember more.
Also the names of these functions probably need to be made more specific
so that people not so familiar with the transport code can tell that they
are from "transport" family. The names didn't matter much while they were
file scope static, but this series changes that.
Actually, I wonder if we can simply get rid of some of the calls in
send-pack. I think that the code in send-pack isn't even called anymore
via "git push"; it only gets called when you call send-pack directly.
And arguably send-pack as plumbing shouldn't be generating all sorts of
user-facing output. But it is a behavior change. I wonder if anybody
actually calls send-pack directly anymore. It seems like even scripts
use "git push" because of the transport agnosticism.
-Peff
On Mon, Feb 15, 2010 at 02:55:15AM -0500, Jeff King wrote:
On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:
Actually, I wonder if we can simply get rid of some of the calls in
send-pack. I think that the code in send-pack isn't even called anymore
via "git push"; it only gets called when you call send-pack directly.
Actually, its also seemingly called by git-remote-http(s) (at least it
contains references to "stateless RPC", which is related to smart HTTP).
And arguably send-pack as plumbing shouldn't be generating all sorts of
user-facing output. But it is a behavior change. I wonder if anybody
actually calls send-pack directly anymore. It seems like even scripts
use "git push" because of the transport agnosticism.
For non-stateless case, it seems that the only protocols builtin-send-pack
can deal with are ssh://, git:// and file://, it can't deal with any
sort of remote helper, not even one provoding smart transport.
-Ilari
On Mon, 15 Feb 2010, Jeff King wrote:
On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:
quoted
quoted
I can't remember the exact details of why the originals were not
removed, though (I think I complained about it once before, and there
was some technical reason, but I don't recall now). Daniel (cc'd) might
remember more.
Also the names of these functions probably need to be made more specific
so that people not so familiar with the transport code can tell that they
are from "transport" family. The names didn't matter much while they were
file scope static, but this series changes that.
Actually, I wonder if we can simply get rid of some of the calls in
send-pack. I think that the code in send-pack isn't even called anymore
via "git push"; it only gets called when you call send-pack directly.
And arguably send-pack as plumbing shouldn't be generating all sorts of
user-facing output. But it is a behavior change. I wonder if anybody
actually calls send-pack directly anymore. It seems like even scripts
use "git push" because of the transport agnosticism.
I think it would probably be better to get rid of send-pack as a separate
command entirely, rather than changing any of its behavior, and make
remote-curl use a private command that only has the desired behavior,
which is stdio to a local proxy for the remote.
For that matter, it would likely be worthwhile abstracting the packet_line
code such that send-pack (and fetch-pack) could be done in-process without
the messages going over a classic packet_line connection to remote-curl
before being sent over HTTP to the actual server.
-Daniel
*This .sig left intentionally blank*