{"thread":{"id":"18521","subject":"[PATCH 2/5] Document details of transport function APIs","startedAt":"2009-03-25T03:04:10Z","lastAt":"2009-03-25T18:42:05Z","messageCount":5,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"109300","messageId":"alpine.LNX.1.00.0903242303250.19665@iabervon.org","threadId":"18521","inReplyTo":null,"subject":"[PATCH 2/5] Document details of transport function APIs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-25T03:04:10Z","receivedAt":"2009-03-25T03:04:10Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"In particular, explain which of the fields of struct ref is used for\nwhat purpose in the input to and output from each function.\n\nSigned-off-by: Daniel Barkalow \n---\n transport.h | 38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/transport.h b/transport.h\nindex 489e96a..2e1650a 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -18,11 +18,49 @@ struct transport {\n \tint (*set_option)(struct transport *connection, const char *name,\n \t\t\t const char *value);\n \n+\t/**\n+\t * Returns a list of the remote side's refs. In order to allow\n+\t * the transport to try to share connections, for_push is a\n+\t * hint as to whether the ultimate operation is a push or a fetch.\n+\t *\n+\t * If the transport is able to determine the remote hash for\n+\t * the ref without a huge amount of effort, it should store it\n+\t * in the ref's old_sha1 field; otherwise it should be all 0.\n+\t **/\n \tstruct ref *(*get_refs_list)(struct transport *transport, int for_push);\n+\n+\t/**\n+\t * Fetch the objects for the given refs. Note that this gets\n+\t * an array, and should ignore the list structure.\n+\t *\n+\t * If the transport did not get hashes for refs in\n+\t * get_refs_list(), it should set the old_sha1 fields in the\n+\t * provided refs now.\n+\t **/\n \tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n+\n+\t/**\n+\t * Push the objects and refs. Send the necessary objects, and\n+\t * then tell the remote side to update each ref in the list\n+\t * from old_sha1 to new_sha1.\n+\t *\n+\t * Where possible, set the status for each ref appropriately.\n+\t *\n+\t * If, in the process, the transport determines that the\n+\t * remote side actually responded to the push by updating the\n+\t * ref to a different value, the transport should modify the\n+\t * new_sha1 in the ref. (Note that this is a matter of the\n+\t * remote accepting but rewriting the change, not rejecting it\n+\t * and reporting that a different update had already taken\n+\t * place)\n+\t **/\n \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n \tint (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);\n \n+\t/** get_refs_list(), fetch(), and push_refs() can keep\n+\t * resources (such as a connection) reserved for futher\n+\t * use. disconnect() releases these resources.\n+\t **/\n \tint (*disconnect)(struct transport *connection);\n \tchar *pack_lockfile;\n \tsigned verbose : 2;\n-- \n1.6.2.1.476.g9bf04b\n"},{"id":"109322","messageId":"7v63hy9k0l.fsf@gitster.siamese.dyndns.org","threadId":"18521","inReplyTo":"alpine.LNX.1.00.0903242303250.19665@iabervon.org","subject":"Re: [PATCH 2/5] Document details of transport function APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-25T06:47:06Z","receivedAt":"2009-03-25T06:47:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> In particular, explain which of the fields of struct ref is used for\n> what purpose in the input to and output from each function.\n>\n> Signed-off-by: Daniel Barkalow \n> ---\n> transport.h | 38 ++++++++++++++++++++++++++++++++++++++\n> 1 files changed, 38 insertions(+), 0 deletions(-)\n>\n> diff --git a/transport.h b/transport.h\n> index 489e96a..2e1650a 100644\n> --- a/transport.h\n> +++ b/transport.h\n> @@ -18,11 +18,49 @@ struct transport {\n> \tint (*set_option)(struct transport *connection, const char *name,\n> \t\t\t const char *value);\n> \n> +\t/**\n> +\t * Returns a list of the remote side's refs. In order to allow\n> +\t * the transport to try to share connections, for_push is a\n> +\t * hint as to whether the ultimate operation is a push or a fetch.\n> +\t *\n\nIt is unclear what this \"hint\" is intended to be used for, what the\ntransport is and isn't allowed to use it for, and what existing transports\ntypically use it for.\n\n> +\t/**\n> +\t * Push the objects and refs. Send the necessary objects, and\n> +\t * then tell the remote side to update each ref in the list\n> +\t * from old_sha1 to new_sha1.\n> +\t *\n> +\t * Where possible, set the status for each ref appropriately.\n> +\t *\n> +\t * If, in the process, the transport determines that the\n> +\t * remote side actually responded to the push by updating the\n> +\t * ref to a different value, the transport should modify the\n> +\t * new_sha1 in the ref. (Note that this is a matter of the\n> +\t * remote accepting but rewriting the change, not rejecting it\n> +\t * and reporting that a different update had already taken\n> +\t * place)\n> +\t **/\n\nIt this even a sane thing to allow? How would it interact with the\n\"pretend we immediately turned around and fetched them into the remote\ntracking branches\" local updates we usually do?\n\n> \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n> \tint (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);\n> \n> +\t/** get_refs_list(), fetch(), and push_refs() can keep\n> +\t * resources (such as a connection) reserved for futher\n> +\t * use. disconnect() releases these resources.\n> +\t **/\n> \tint (*disconnect)(struct transport *connection);\n> \tchar *pack_lockfile;\n> \tsigned verbose : 2;\n\nIt is just a style thing, but all of our multi-line comments are\n\n /*\n * of\n * this\n * form\n */\n\nand these new comments are formatted slightly differently with double\nasterisks on only the first and the last lines. In addition, th last\ncomment block uses a yet another different style, which is a bit of\neyesore.\n"},{"id":"109367","messageId":"alpine.LNX.1.00.0903251142470.19665@iabervon.org","threadId":"18521","inReplyTo":"7v63hy9k0l.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] Document details of transport function APIs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-25T16:19:56Z","receivedAt":"2009-03-25T16:19:56Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 24 Mar 2009, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > In particular, explain which of the fields of struct ref is used for\n> > what purpose in the input to and output from each function.\n> >\n> > Signed-off-by: Daniel Barkalow \n> > ---\n> > transport.h | 38 ++++++++++++++++++++++++++++++++++++++\n> > 1 files changed, 38 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/transport.h b/transport.h\n> > index 489e96a..2e1650a 100644\n> > --- a/transport.h\n> > +++ b/transport.h\n> > @@ -18,11 +18,49 @@ struct transport {\n> > \tint (*set_option)(struct transport *connection, const char *name,\n> > \t\t\t const char *value);\n> > \n> > +\t/**\n> > +\t * Returns a list of the remote side's refs. In order to allow\n> > +\t * the transport to try to share connections, for_push is a\n> > +\t * hint as to whether the ultimate operation is a push or a fetch.\n> > +\t *\n> \n> It is unclear what this \"hint\" is intended to be used for, what the\n> transport is and isn't allowed to use it for, and what existing transports\n> typically use it for.\n\nI don't think there's anything the transport isn't allowed to use the hint \nfor (of course, it shouldn't go and do things that haven't been requested \nat all yet, but that's true with or without a hint). It's intended to be \nused for things like the git native protocol, where the client side has to \ndeclare up front whether it wants to talk to receive-pack or upload-pack, \nand it would have to disconnect and reconnect to get the other one.\n\nIt's probably also legitimate for the http transport to guess that, if \n\"for_push\" is true, that it's worth trying to list directories with \nwebdav, although perhaps it should try both in any case.\n\n> > +\t/**\n> > +\t * Push the objects and refs. Send the necessary objects, and\n> > +\t * then tell the remote side to update each ref in the list\n> > +\t * from old_sha1 to new_sha1.\n> > +\t *\n> > +\t * Where possible, set the status for each ref appropriately.\n> > +\t *\n> > +\t * If, in the process, the transport determines that the\n> > +\t * remote side actually responded to the push by updating the\n> > +\t * ref to a different value, the transport should modify the\n> > +\t * new_sha1 in the ref. (Note that this is a matter of the\n> > +\t * remote accepting but rewriting the change, not rejecting it\n> > +\t * and reporting that a different update had already taken\n> > +\t * place)\n> > +\t **/\n> \n> It this even a sane thing to allow? How would it interact with the\n> \"pretend we immediately turned around and fetched them into the remote\n> tracking branches\" local updates we usually do?\n\nWe already allow a git server to rewrite refs with a hook when it gets \nthem, and we record the pre-rewriting value. This allows the transport to \npropagate the post-rewriting value back (if it can get it), and we'd \nupdate the tracking branches with what the server actually did instead of \nwhat we asked it to (i.e., we do what we would do if we really did turn \naround and fetch them immediately).\n\nOf course, I primarily want this for the case where it's a foreign system \nand we don't get to control everything in the foreign history, even when \nit is accepted (e.g., it applies its own timestamp and attributes it to a \ndifferent username), and we want to avoid simply losing the information as \nto what became of the commit.\n\n> > \tint (*push_refs)(struct transport *transport, struct ref *refs, int flags);\n> > \tint (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);\n> > \n> > +\t/** get_refs_list(), fetch(), and push_refs() can keep\n> > +\t * resources (such as a connection) reserved for futher\n> > +\t * use. disconnect() releases these resources.\n> > +\t **/\n> > \tint (*disconnect)(struct transport *connection);\n> > \tchar *pack_lockfile;\n> > \tsigned verbose : 2;\n> \n> It is just a style thing, but all of our multi-line comments are\n> \n> /*\n> * of\n> * this\n> * form\n> */\n> \n> and these new comments are formatted slightly differently with double\n> asterisks on only the first and the last lines. In addition, th last\n> comment block uses a yet another different style, which is a bit of\n> eyesore.\n\nAh, yes, I'll correct that.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"109384","messageId":"7vmyb95vky.fsf@gitster.siamese.dyndns.org","threadId":"18521","inReplyTo":"alpine.LNX.1.00.0903251142470.19665@iabervon.org","subject":"Re: [PATCH 2/5] Document details of transport function APIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-25T18:03:09Z","receivedAt":"2009-03-25T18:03:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n>> > +\t * If, in the process, the transport determines that the\n>> > +\t * remote side actually responded to the push by updating the\n>> > +\t * ref to a different value, the transport should modify the\n>> > +\t * new_sha1 in the ref. (Note that this is a matter of the\n>> > +\t * remote accepting but rewriting the change, not rejecting it\n>> > +\t * and reporting that a different update had already taken\n>> > +\t * place)\n>> > +\t **/\n>> \n>> It this even a sane thing to allow? How would it interact with the\n>> \"pretend we immediately turned around and fetched them into the remote\n>> tracking branches\" local updates we usually do?\n>\n> We already allow a git server to rewrite refs with a hook when it gets \n> them, and we record the pre-rewriting value. This allows the transport to \n> propagate the post-rewriting value back (if it can get it), and we'd \n> update the tracking branches with what the server actually did instead of \n> what we asked it to (i.e., we do what we would do if we really did turn \n> around and fetch them immediately).\n\nBut how are you guaranteeing that objects necessary to complete the\nhistory the remote end re-written are already available on the local end?\nDo you have a reverse object transfer phase now in send-pack protocol?\n\nOtherwise I am afraid that you are corrupting the local repository.\n"},{"id":"109397","messageId":"alpine.LNX.1.00.0903251435480.19665@iabervon.org","threadId":"18521","inReplyTo":"7vmyb95vky.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] Document details of transport function APIs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-25T18:42:05Z","receivedAt":"2009-03-25T18:42:05Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 25 Mar 2009, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> >> > +\t * If, in the process, the transport determines that the\n> >> > +\t * remote side actually responded to the push by updating the\n> >> > +\t * ref to a different value, the transport should modify the\n> >> > +\t * new_sha1 in the ref. (Note that this is a matter of the\n> >> > +\t * remote accepting but rewriting the change, not rejecting it\n> >> > +\t * and reporting that a different update had already taken\n> >> > +\t * place)\n> >> > +\t **/\n> >> \n> >> It this even a sane thing to allow? How would it interact with the\n> >> \"pretend we immediately turned around and fetched them into the remote\n> >> tracking branches\" local updates we usually do?\n> >\n> > We already allow a git server to rewrite refs with a hook when it gets \n> > them, and we record the pre-rewriting value. This allows the transport to \n> > propagate the post-rewriting value back (if it can get it), and we'd \n> > update the tracking branches with what the server actually did instead of \n> > what we asked it to (i.e., we do what we would do if we really did turn \n> > around and fetch them immediately).\n> \n> But how are you guaranteeing that objects necessary to complete the\n> history the remote end re-written are already available on the local end?\n> Do you have a reverse object transfer phase now in send-pack protocol?\n\nThe current send-pack protocol wouldn't be able to use this feature (I \ndon't think it can even report the replacement hashes, so there's little \ndanger of that).\n\nThe foreign VCS transport would actually reimport the objects (in the \nprocess of discovering the hash), so that's fine.\n\n> Otherwise I am afraid that you are corrupting the local repository.\n\nIt wouldn't be any different from fetch() reporting success when it hadn't \nactually succeeded, but I think it would be worth being explicit here that \nyou can't simply report the other end's hash if you may not have it.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}