Felipe Contreras [off-list ref] writes:
Of course, transport-helper shouldn't even be specifying the negative
(^) refs, but that's another story.
Hrm, I am not sure I understand what you mean by this.
How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)? Or are you advocating to re-send the entire
history down to the root commit every time?
On Tue, Nov 20, 2012 at 11:43 PM, Junio C Hamano [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
Of course, transport-helper shouldn't even be specifying the negative
(^) refs, but that's another story.
Hrm, I am not sure I understand what you mean by this.
How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)? Or are you advocating to re-send the entire
history down to the root commit every time?
No, it would not re-send the whole history, that's what marks are for.
And right now it doesn't exactly which was the last commit. Let's
suppose the remote helper has a refspec like this:
refs/heads/*:refs/hg/origin/heads/*
1) What happens the first time you push?
5203a268546295ebd895fd87522217ef53bd3313 refs/heads/master
5203a268546295ebd895fd87522217ef53bd3313 refs/remotes/tmp/master
Notice how the remote ref is updated correctly, but it's not the
remote helper refspec, so the next time you push, you will from root.
It's only when you fetch that you get the refspec'ed refs:
5203a268546295ebd895fd87522217ef53bd3313 refs/heads/master
5203a268546295ebd895fd87522217ef53bd3313 refs/hg/tmp/heads/master
5203a268546295ebd895fd87522217ef53bd3313 refs/remotes/tmp/master
So, there's already a mismatch.
2) What happens when you have no marks?
You get something like:
reset refs/heads/heads
from :0
Which is totally useless. Somebody proposed a patch that would replace
the :0 with a git sha-1, but that is equally useless for a remote
helper: we need a hg ref id, or a bzr id, or whatever, and no, there's
mapping between git sha-1's and hg ref ids, there's only git->mark
mark->hg, without marks, there's no way to map the git id to the hg
id.
3) What happens when you have a refspec like this?
*:*
Now nothing works, because we would be requesting ^refs/heads/master
refs/heads/master.
And according to the documentation, this is the default when no
refspec is used, which is not true.
4) What happens when there's no refspec at all.
Now it's even worst; nothing gets done at all:
if (!data->refspecs)
continue;
I documented all this breakages in this patch:
http://article.gmane.org/gmane.comp.version-control.git/209365
not ok 10 - push new branch with old:new refspec # TODO known breakage
ok 11 - cloning without refspec
ok 12 - pulling without refspecs
not ok 13 - pushing without refspecs # TODO known breakage
not ok 14 - pulling with straight refspec # TODO known breakage
not ok 15 - pushing with straight refspec # TODO known breakage
not ok 16 - pulling without marks # TODO known breakage
not ok 17 - pushing without marks # TODO known breakage
And if you apply this patch:
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -750,6 +750,7 @@ static int push_refs_with_export(struct transport
*transport,
struct helper_data *data = transport->data;
struct string_list revlist_args = STRING_LIST_INIT_NODUP;
struct strbuf buf = STRBUF_INIT;
+ struct remote *remote = transport->remote;
helper = get_helper(transport);
@@ -761,22 +762,23 @@ static int push_refs_with_export(struct
transport *transport,
char *private;
unsigned char sha1[20];
- if (!data->refspecs)
+ if (ref->deletion)
+ die("remote-helpers do not support ref deletion");
+
+ if (!ref->peer_ref)
+ continue;
+
+ string_list_append(&revlist_args, ref->peer_ref->name);
+
+ if (!data->import_marks)
continue;
- private = apply_refspecs(data->refspecs,
data->refspec_nr, ref->name);
+
+ private = apply_refspecs(remote->fetch,
remote->fetch_refspec_nr, ref->name);
if (private && !get_sha1(private, sha1)) {
strbuf_addf(&buf, "^%s", private);
string_list_append(&revlist_args,
strbuf_detach(&buf, NULL));
}
free(private);
-
- if (ref->deletion) {
- die("remote-helpers do not support ref deletion");
- }
-
- if (ref->peer_ref)
- string_list_append(&revlist_args, ref->peer_ref->name);
-
}
if (get_exporter(transport, &exporter, &revlist_args))
ok 13 - pushing without refspecs # TODO known breakage
ok 14 - pulling with straight refspec # TODO known breakage
ok 15 - pushing with straight refspec # TODO known breakage
ok 16 - pulling without marks # TODO known breakage
ok 17 - pushing without marks # TODO known breakage
Cheers.
--
Felipe Contreras
Junio C Hamano wrote:
Felipe Contreras [off-list ref] writes:
quoted
Of course, transport-helper shouldn't even be specifying the negative
(^) refs, but that's another story.
Hrm, I am not sure I understand what you mean by this.
How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)? Or are you advocating to re-send the entire
history down to the root commit every time?
I think Felipe has mentioned before that he considers it the remote
helper's responsibility to keep track of what commits have already
been imported, for example using a marks file.
Never mind that others have said that that's not the current interface
(I don't yet see why it would be a good interface after a transition,
but maybe it would be). Still, hopefully that clarifies the intended
meaning.
Hope that helps,
Jonathan
On Wed, Nov 21, 2012 at 5:17 AM, Jonathan Nieder [off-list ref] wrote:
Junio C Hamano wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
Of course, transport-helper shouldn't even be specifying the negative
(^) refs, but that's another story.
Hrm, I am not sure I understand what you mean by this.
How should it be telling the fast-export up to what commit the
receiving end should already have the history for (hence they do not
need to be sent)? Or are you advocating to re-send the entire
history down to the root commit every time?
I think Felipe has mentioned before that he considers it the remote
helper's responsibility to keep track of what commits have already
been imported, for example using a marks file.
It's not the remote helper, fast-export does that.
Never mind that others have said that that's not the current interface
(I don't yet see why it would be a good interface after a transition,
but maybe it would be). Still, hopefully that clarifies the intended
meaning.
The current interface is broken.
not ok 16 - pulling without marks # TODO known breakage
not ok 17 - pushing without marks # TODO known breakage
See? A remote helper without marks doesn't work.
--
Felipe Contreras