{"thread":{"id":"20842","subject":"[PATCH 4/8] Allow fetch to modify refs","startedAt":"2009-09-04T02:13:55Z","lastAt":"2009-09-04T16:29:53Z","messageCount":3,"participants":["Daniel Barkalow","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"122380","messageId":"alpine.LNX.2.00.0909032213260.28290@iabervon.org","threadId":"20842","inReplyTo":null,"subject":"[PATCH 4/8] Allow fetch to modify refs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-04T02:13:55Z","receivedAt":"2009-09-04T02:13:55Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This allows the transport to use the null sha1 for a ref reported to\nbe present in the remote repository to indicate that a ref exists but\nits actual value is presently unknown and will be set if the objects\nare fetched.\n\nAlso adds documentation to the API to specify exactly what the methods\nshould do and how they should interpret arguments.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n builtin-clone.c    |    6 ++++--\n transport-helper.c |    4 ++--\n transport.c        |   13 +++++++------\n transport.h        |   41 +++++++++++++++++++++++++++++++++++++++--\n 4 files changed, 52 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex ad04808..deef435 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -520,8 +520,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t     option_upload_pack);\n \n \t\trefs = transport_get_remote_refs(transport);\n-\t\tif (refs)\n-\t\t\ttransport_fetch_refs(transport, refs);\n+\t\tif (refs) {\n+\t\t\tstruct ref *ref_cpy = copy_ref_list(refs);\n+\t\t\ttransport_fetch_refs(transport, ref_cpy);\n+\t\t}\n \t}\n \n \tif (refs) {\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b1ea7e6..e2b5270 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -70,7 +70,7 @@ static int disconnect_helper(struct transport *transport)\n }\n \n static int fetch_with_fetch(struct transport *transport,\n-\t\t\t    int nr_heads, const struct ref **to_fetch)\n+\t\t\t    int nr_heads, struct ref **to_fetch)\n {\n \tstruct child_process *helper = get_helper(transport);\n \tFILE *file = fdopen(helper->out, \"r\");\n@@ -94,7 +94,7 @@ static int fetch_with_fetch(struct transport *transport,\n }\n \n static int fetch(struct transport *transport,\n-\t\t int nr_heads, const struct ref **to_fetch)\n+\t\t int nr_heads, struct ref **to_fetch)\n {\n \tstruct helper_data *data = transport->data;\n \tint i, count;\ndiff --git a/transport.c b/transport.c\nindex 4cb8077..93430fa 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -204,7 +204,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)\n }\n \n static int fetch_objs_via_rsync(struct transport *transport,\n-\t\t\t\tint nr_objs, const struct ref **to_fetch)\n+\t\t\t\tint nr_objs, struct ref **to_fetch)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct child_process rsync;\n@@ -408,7 +408,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_from_bundle(struct transport *transport,\n-\t\t\t       int nr_heads, const struct ref **to_fetch)\n+\t\t\t       int nr_heads, struct ref **to_fetch)\n {\n \tstruct bundle_transport_data *data = transport->data;\n \treturn unbundle(&data->header, data->fd);\n@@ -486,7 +486,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n }\n \n static int fetch_refs_via_pack(struct transport *transport,\n-\t\t\t       int nr_heads, const struct ref **to_fetch)\n+\t\t\t       int nr_heads, struct ref **to_fetch)\n {\n \tstruct git_transport_data *data = transport->data;\n \tchar **heads = xmalloc(nr_heads * sizeof(*heads));\n@@ -922,16 +922,17 @@ const struct ref *transport_get_remote_refs(struct transport *transport)\n \treturn transport->remote_refs;\n }\n \n-int transport_fetch_refs(struct transport *transport, const struct ref *refs)\n+int transport_fetch_refs(struct transport *transport, struct ref *refs)\n {\n \tint rc;\n \tint nr_heads = 0, nr_alloc = 0, nr_refs = 0;\n-\tconst struct ref **heads = NULL;\n-\tconst struct ref *rm;\n+\tstruct ref **heads = NULL;\n+\tstruct ref *rm;\n \n \tfor (rm = refs; rm; rm = rm->next) {\n \t\tnr_refs++;\n \t\tif (rm->peer_ref &&\n+\t\t    !is_null_sha1(rm->old_sha1) &&\n \t\t    !hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))\n \t\t\tcontinue;\n \t\tALLOC_GROW(heads, nr_heads + 1, nr_alloc);\ndiff --git a/transport.h b/transport.h\nindex c14da6f..503db11 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -18,11 +18,48 @@ 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-\tint (*fetch)(struct transport *transport, int refs_nr, const struct ref **refs);\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, for any refs where peer_ref is set and\n+\t * peer_ref->new_sha1 is different from old_sha1, tell the\n+\t * remote side to update each ref in the list from old_sha1 to\n+\t * peer_ref->new_sha1.\n+\t *\n+\t * Where possible, set the status for each ref appropriately.\n+\t *\n+\t * The transport must modify new_sha1 in the ref to the new\n+\t * value if the remote accepted the change. Note that this\n+\t * could be a different value from peer_ref->new_sha1 if the\n+\t * process involved generating new commits.\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@@ -74,7 +111,7 @@ int transport_push(struct transport *connection,\n \n const struct ref *transport_get_remote_refs(struct transport *transport);\n \n-int transport_fetch_refs(struct transport *transport, const struct ref *refs);\n+int transport_fetch_refs(struct transport *transport, struct ref *refs);\n void transport_unlock_pack(struct transport *transport);\n int transport_disconnect(struct transport *transport);\n char *transport_anonymize_url(const char *url);\n-- \n1.6.4.2.419.gc86f8\n"},{"id":"122410","messageId":"alpine.DEB.1.00.0909041243420.4605@intel-tinevez-2-302","threadId":"20842","inReplyTo":"alpine.LNX.2.00.0909032213260.28290@iabervon.org","subject":"Re: [PATCH 4/8] Allow fetch to modify refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-09-04T10:46:53Z","receivedAt":"2009-09-04T10:46:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 3 Sep 2009, Daniel Barkalow wrote:\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\nThis is not clear at all.  You should rather say \"[...] and should not \nlook at, or set, the 'next' member of the refs\".\n\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\nNot the \"new_sha1\"?\n\n> +\t **/\n> +\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n> +\n> [...]\n> +\t/** get_refs_list(), fetch(), and push_refs() can keep\n\nThe \"/**\" wants to have a line to itself.\n\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\nCiao,\nDscho\n"},{"id":"122448","messageId":"alpine.LNX.2.00.0909041225080.28290@iabervon.org","threadId":"20842","inReplyTo":"alpine.DEB.1.00.0909041243420.4605@intel-tinevez-2-302","subject":"Re: [PATCH 4/8] Allow fetch to modify refs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-04T16:29:53Z","receivedAt":"2009-09-04T16:29:53Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 4 Sep 2009, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Thu, 3 Sep 2009, Daniel Barkalow wrote:\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> \n> This is not clear at all.  You should rather say \"[...] and should not \n> look at, or set, the 'next' member of the refs\".\n\nThat is a better wording, yes.\n\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> \n> Not the \"new_sha1\"?\n\nNo, because get_refs_list() sets the old_sha1, and this isn't indicating \nanything different. The old/new thing is to indicate that the ref is \nchanging value. What's happening here is that the ref isn't changing value \nbut we didn't know what value it always (effectively) had until now.\n\n> > +\t **/\n> > +\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n> > +\n> > [...]\n> > +\t/** get_refs_list(), fetch(), and push_refs() can keep\n> \n> The \"/**\" wants to have a line to itself.\n\nGood point, thanks.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}