* send-pack question.
@ 2005-07-31 5:47 Junio C Hamano
2005-07-31 6:02 ` Linus Torvalds
0 siblings, 1 reply; 2+ messages in thread
From: Junio C Hamano @ 2005-07-31 5:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
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] <remote> [<head>...]\n"
+" --all and explicit <head> 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;
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: send-pack question.
2005-07-31 5:47 send-pack question Junio C Hamano
@ 2005-07-31 6:02 ` Linus Torvalds
0 siblings, 0 replies; 2+ messages in thread
From: Linus Torvalds @ 2005-07-31 6:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2005-07-31 6:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-31 5:47 send-pack question Junio C Hamano
2005-07-31 6:02 ` Linus Torvalds
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox