Re: [PATCH/RFC] remote-helper: allow fetch to discover sha1 later
Devin J. Pohly djpo...@gmail.com writes: From: Devin J. Pohly djpo...@gmail.com Allow a fetch-style remote helper to issue the notification ref sha1 name to specify a ref's value during the fetch operation. Currently, a remote helper with the fetch capability cannot fetch a ref unless its sha1 was known when the list command was executed. This limitation is arbitrary, as the helper may eventually be able to determine the correct sha1 as it fetches objects. While I am not fundamentally against supporting a remote helper that is not capable of saying what the object names of the tip of its histories are before doing a lot of work, I do not think it is a good idea to allow such a helper to claim that it supports fetch capability, for at least two reasons: * Being able to list is essential for fetch based helpers, I think, far from arbitrary. When running a git fetch against such a remote, we can first issue list to see what it has and avoid asking for the refs that point at the histories we already have (or the refs that are the same as the last time we fetched from the remote). I do not offhand know if we do this kind of optimization, but if we don't, we should. * Existing versions of git that can drive remote helpers that claim to have fetch capability are not prepared to accept an unknown refs protocol message from the helper, so a helper written for your new semantics of fetch capability will not work with them. The point of capability mechanism is to prevent such a compatibility issue from breaking the system, and this patch goes against that spirit. Such a remote helper should not advertise to support fetch capability, because it does not support it. It can advertise that it supports something else and it is OK to make an updated Git that knows how to drive a remote helper that lack fetch but support that something else work with it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] remote-helper: allow fetch to discover sha1 later
On Thu, Sep 13, 2012 at 11:10:26PM -0700, Junio C Hamano wrote: I do not think it is a good idea to allow such a helper to claim that it supports fetch capability, for at least two reasons: * Being able to list is essential for fetch based helpers. think, far from arbitrary. ... Oh, I don't mean to change the semantics of the list command at all. What I thought seemed arbitrary was limiting the *fetch* command to refs with pre-known sha1s. Any list-time optimization in place or possible for such refs wouldn't be affected. (In light of this, specifying a new sha1 for a ref which already had one in the list should probably be an error rather than a warning. I'll update this in the next version and make it clear that a ref message must only be issued for refs reported without a sha1.) * Existing versions of git that can drive remote helpers that claim to have fetch capability are not prepared to accept an unknown refs protocol message from the helper, so a helper written for your new semantics of fetch capability will not work with them. ... I see. I didn't realize that new helper + old Git client is a supported combination. Still, hear me out. 1. A new helper must only send a ref message when git asks for a ref without a known sha1. 2. Asking for a ref without a known sha1 is unsupported, according to git-remote-helpers.txt: Only objects which were reported in the ref list with a sha1 may be fetched [with fetch]. 3. Furthermore, asking for a ref without a known sha1 *already* breaks in existing versions of git with current handlers: $ git fetch testfetch::../git1 foo fatal: bad object error: testfetch::../git1 did not send all necessary objects Compare - existing version of git, with a new handler: $ git fetch testref::../git1 foo warning: testref unexpectedly said: 'ref 0f31snip refs/heads/foo' fatal: bad object error: testref::../git1 did not send all necessary objects So the proposed change doesn't break anything that isn't already broken. :) That said, if you're still not sold, a new capability is fine. Though I think it can be done without it, I'm certainly not opposed to adding one. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] remote-helper: allow fetch to discover sha1 later
From: Devin J. Pohly djpo...@gmail.com
Allow a fetch-style remote helper to issue the notification
ref sha1 name
to specify a ref's value during the fetch operation.
Currently, a remote helper with the fetch capability cannot fetch a
ref unless its sha1 was known when the list command was executed.
This limitation is arbitrary, as the helper may eventually be able to
determine the correct sha1 as it fetches objects.
Signed-off-by: Devin J. Pohly djpo...@gmail.com
---
Soliciting general opinions - first git patch. :)
The fetch command can be a lot more convenient than import if you're working
with a remote that doesn't give you a commit and the associated objects at the
same time. And there's no apparent reason that something like this isn't
possible.
How it works: the old_sha1 field is currently set when the output from list
is parsed, then not used again until after the fetch command completes, just
before updating refs. Changing it during the fetch only affects the final
value of the ref. (Forgetting to resolve a ref will result in exactly the
same behavior as before: an error from check_everything_connected.)
Not sure if either or both of the warning()s should be a die() instead.
Documentation/git-remote-helpers.txt | 8 ++--
transport-helper.c | 24
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index f5836e4..fb4240f 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -229,8 +229,12 @@ Supported if the helper has the option capability.
to the database. Fetch commands are sent in a batch, one
per line, terminated with a blank line.
Outputs a single blank line when all fetch commands in the
- same batch are complete. Only objects which were reported
- in the ref list with a sha1 may be fetched this way.
+ same batch are complete.
++
+If the named ref was reported in the ref list without a sha1, must
+output a 'ref sha1 name' line once the sha1 is known. This is
+also required for a symref if its target did not have a sha1 in the
+list.
+
Optionally may output a 'lock file' line indicating a file under
GIT_DIR/objects/pack which is keeping a pack until refs can be
diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..6fce419 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -363,6 +363,30 @@ static int fetch_with_fetch(struct transport *transport,
warning(%s also locked %s, data-name, name);
else
transport-pack_lockfile = xstrdup(name);
+ } else if (!prefixcmp(buf.buf, ref )) {
+ const char *rest = buf.buf + 4;
+ char *eov, *eon;
+ struct ref *posn;
+
+ eov = strchr(rest, ' ');
+ if (!eov)
+ die(Malformed ref command: %s, buf.buf);
+ eon = strchr(eov + 1, ' ');
+ *eov = '\0';
+ if (eon)
+ *eon = '\0';
+ for (i = 0; i nr_heads; i++) {
+ posn = to_fetch[i];
+ if (!strcmp(eov + 1, posn-name))
+ break;
+ }
+ if (i == nr_heads || (posn-status
REF_STATUS_UPTODATE)) {
+ warning(Ref %s is not being fetched, eov + 1);
+ continue;
+ }
+ if (!is_null_sha1(posn-old_sha1))
+ warning(Ref %s is being overwritten, eov + 1);
+ get_sha1_hex(rest, posn-old_sha1);
}
else if (!buf.len)
break;
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
