# send-pack question. 2 messages from 2005-07-31 to 2005-07-31. Participants: Junio C Hamano, Linus Torvalds. Thread: https://gitlist.dev/t/1374 ## Junio C Hamano, 2005-07-31 05:47 Subject: send-pack question. Message-ID: <7v1x5fwmpk.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1x5fwmpk.fsf%40assigned-by-dhcp.cox.net ``` I have been looking at send-pack because some people seem to want to push into a remote repository that names heads differently from local. I have some questions that do not have to do with anything about their request, but about what the current code intends to do. * Right now, "send-pack --all" into an empty repository does not do anything, but "send-pack --all master" into an empty repository pushes all local heads. This is because we do not check "send_all" when deciding if we want to call try_match on local references. I am assuming this is an oversight; am I correct? If so, does the attached patch look OK? * It appears to me that you can say "send-pack net", and depending on how the remote lists its refs, you can end up updating their refs/heads/net or refs/tags/net. More confusingly, you could say "send-pack net net" to update both. More realistically, you could get confused with a remote that has refs/heads/jgarzik/net and refs/heads/dsmiller/net in this way. I think it should detect, stop and warn about the ambiguity and require the user to be more explicit. Am I reading the current code correctly? I've always _hated_ the interface to path_match() which pretends to be just a boolean function but actually has a grave side effect, by the way. --- # - pu: git-fetch-script http fix. # + (working tree) diff --git a/send-pack.c b/send-pack.c --- a/send-pack.c +++ b/send-pack.c @@ -4,7 +4,8 @@ #include "pkt-line.h" static const char send_pack_usage[] = -"git-send-pack [--exec=git-receive-pack] [host:]directory [heads]*"; +"git-send-pack [--all] [--exec=git-receive-pack] [...]\n" +" --all and explicit specification are mutually exclusive."; static const char *exec = "git-receive-pack"; static int send_all = 0; static int force_update = 0; @@ -214,7 +215,7 @@ static int send_pack(int in, int out, in /* * See if we have any refs that the other end didn't have */ - if (nr_match) { + if (nr_match || send_all) { local_ref_nr_match = nr_match; local_ref_match = match; local_ref_list = ref_list; @@ -281,6 +282,8 @@ int main(int argc, char **argv) } if (!dest) usage(send_pack_usage); + if (heads && send_all) + usage(send_pack_usage); pid = git_connect(fd, dest, exec); if (pid < 0) return 1; ``` ## Linus Torvalds, 2005-07-31 06:02 Subject: Re: send-pack question. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0507302257000.29650%40g5.osdl.org In-Reply-To: <7v1x5fwmpk.fsf@assigned-by-dhcp.cox.net> ``` On Sat, 30 Jul 2005, Junio C Hamano wrote: > > * Right now, "send-pack --all" into an empty repository does > not do anything, but "send-pack --all master" into an empty > repository pushes all local heads. This is because we do not > check "send_all" when deciding if we want to call try_match > on local references. I am assuming this is an oversight; am > I correct? If so, does the attached patch look OK? Yeah, that sounds like me just not having taken my 'meds. The patch looks fine. > * It appears to me that you can say "send-pack net", and > depending on how the remote lists its refs, you can end up > updating their refs/heads/net or refs/tags/net. Yeh. I was wanting to sort the refs to make everything be totally repeatable, but that was more of an urge than a real plan. > More > confusingly, you could say "send-pack net net" to update > both. More realistically, you could get confused with a > remote that has refs/heads/jgarzik/net and > refs/heads/dsmiller/net in this way. I think it should > detect, stop and warn about the ambiguity and require the > user to be more explicit. Am I reading the current code > correctly? Yes, warning on ambiguity sounds like a sound plan, and then you don't need to sort. You also probably to come up with a syntax for saying "xyz is the local name, abc is the remote name". That's needed for both the pulling and the pushing side, but I didn't ever do it. > I've always _hated_ the interface to path_match() which > pretends to be just a boolean function but actually has a > grave side effect, by the way. It's "interesting" but useful. But I agree, we've had one bug already due to the interesting part. If you do the ambiguity thing, you might mark them used some separate way, and maybe avoid that side effect (or rather - the side effect would still exist, but instead of removing the entry, it would just mark it as "seen", ie make it less drastic). Linus ```