{"thread":{"id":"21414","subject":"[PATCH 2/7] .gitignore: add git-remote-cvs","startedAt":"2009-10-29T06:40:19Z","lastAt":"2009-10-29T18:47:39Z","messageCount":17,"participants":["Sverre Rabbelier","Johan Herland","Shawn O. Pearce","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"126243","messageId":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":null,"subject":"[PATCH 0/7] Prepare git-remote-helpers series for hg support","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:19Z","receivedAt":"2009-10-29T06:40:19Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This series does not add hg support, but it does do all the\nrefactoring that is needed to make it possible. With this series\napplied actually adding full-fledged hg support is a matter of adding\nthe git-remote-hg file.\nThe reason the git-remote-hg file is not included in this series is\nthat while I have finished support for 'git clone hg::/path/to/hg' as\nwell as 'git fetch hg::/path/to/hg' and 'git fetch hgremote',\ncurrently it does a full export on each run (that is, incremental\nfetch is not supported).\nNevertheless, I think it is vital that this is included in the series\nbefore we do anything serious with it. I hope to have the\ngit-remote-hg bit done tomorrow before I get on my airplane, but no\npromises there.\n\nJunio, I can try to re-roll the entire series after I finish\ngit-remote-hg, or I can just send it in on top of this series.\n\nBased on abdcdc5c [More fixes to the git-remote-cvs...].\n\nJohannes Schindelin (1):\n      Finally make remote helper support useful\n\nSverre Rabbelier (5):\n      .gitignore: add git-remote-cvs\n      Add notify and warn to util.py\n      Allow both url and foreign vcs in remote config\n      When updating refs after a fetch, resolve the symref if set\n      Factor ref updating out of fetch_with_import\n\n .gitignore                 |    1 +\n builtin-clone.c            |    7 +++++++\n builtin-fetch.c            |    4 ++++\n git_remote_helpers/util.py |    8 ++++++++\n remote.c                   |    2 +-\n transport-helper.c         |   22 +++++++++++++++-------\n transport.c                |   10 ++++++++++\n transport.h                |    7 +++++--\n 8 files changed, 51 insertions(+), 10 deletions(-)\n"},{"id":"126237","messageId":"1256798426-21816-3-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 2/7] .gitignore: add git-remote-cvs","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:21Z","receivedAt":"2009-10-29T06:40:21Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n .gitignore |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 46c26cd..b8afdf4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -105,6 +105,7 @@ git-reflog\n git-relink\n git-remote\n git-remote-curl\n+git-remote-cvs\n git-repack\n git-repo-config\n git-request-pull\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126244","messageId":"1256798426-21816-4-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 3/7] Add notify and warn to util.py","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:22Z","receivedAt":"2009-10-29T06:40:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tThese turned out to be quite useful for my git-remote-hg, if\n\tdesired this patch can move to later in the series, but I\n\tfigured that it might be useful to someone else already.\n\n git_remote_helpers/util.py |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py\nindex 7d6adb4..d3ca487 100644\n--- a/git_remote_helpers/util.py\n+++ b/git_remote_helpers/util.py\n@@ -15,6 +15,10 @@ import subprocess\n # Whether or not to show debug messages\n DEBUG = False\n \n+def notify(msg, *args):\n+\t\"\"\"Print a message to stderr.\"\"\"\n+\tprint >> sys.stderr, msg % args\n+\n def debug (msg, *args):\n     \"\"\"Print a debug message to stderr when DEBUG is enabled.\"\"\"\n     if DEBUG:\n@@ -24,6 +28,10 @@ def error (msg, *args):\n     \"\"\"Print an error message to stderr.\"\"\"\n     print >> sys.stderr, \"ERROR:\", msg % args\n \n+def warn(msg, *args):\n+\t\"\"\"Print a warning message to stderr.\"\"\"\n+\tprint >> sys.stderr, \"warning:\", msg % args\n+\n def die (msg, *args):\n     \"\"\"Print as error message to stderr and exit the program.\"\"\"\n     error(msg, *args)\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126242","messageId":"1256798426-21816-5-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 4/7] Allow both url and foreign vcs in remote config","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:23Z","receivedAt":"2009-10-29T06:40:23Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"The current logic prevents both 'remote.url' and 'remote.vcs' from\nbeing set, while that might be useful.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tThis way I can do:\n\n\tremote.hgremote.url = /path/to/hg/repo\n\tremote.hgremote.vcs = hg\n\n\tAs well as:\n\n\tremote.hgremote.url = hg::/path/to/hg/repo\n\n remote.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex f0441c4..29365de 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -50,7 +50,7 @@ static char buffer[BUF_SIZE];\n \n static int valid_remote(const struct remote *remote)\n {\n-\treturn !remote->url != !remote->foreign_vcs;\n+\treturn (!!remote->url) || (!!remote->foreign_vcs);\n }\n \n static const char *alias_url(const char *url)\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126238","messageId":"1256798426-21816-6-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 5/7] Finally make remote helper support useful","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:24Z","receivedAt":"2009-10-29T06:40:24Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe common case for remote helpers will be to import some repository which\ncan be specified by a single URL.  Rather than supporting those who say\nthat Git is really complicated, let's be nice to users so that they will\nbe able to say:\n\n\tgit clone hg::https://soc.googlecode.com/hg/ soc\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n--\n\n\tDscho wrote this a while back and it really does make the\n\tseries so much more useful.\n\n transport.c |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex ef33404..a682c75 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -813,6 +813,16 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\turl = remote->url[0];\n \tret->url = url;\n \n+\t/* maybe it is a foreign URL? */\n+\tif (url) {\n+\t\tconst char *p = url;\n+\n+\t\twhile (isalnum(*p))\n+\t\t\tp++;\n+\t\tif (!prefixcmp(p, \"::\"))\n+\t\t\tremote->foreign_vcs = xstrndup(url, p - url);\n+\t}\n+\n \tif (remote && remote->foreign_vcs) {\n \t\ttransport_helper_init(ret, remote->foreign_vcs);\n \t\treturn ret;\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126241","messageId":"1256798426-21816-7-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 6/7] When updating refs after a fetch, resolve the symref if set","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:25Z","receivedAt":"2009-10-29T06:40:25Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This makes it possible to return in response to 'list':\n\t@refs/hg/origin/tip refs/heads/tip\nAnd have HEAD resolve properly after the import completes.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tWithout this it is very hard to support 'git clone' on a\n\tnon-git repository, which I think is a very important thing\n\tto have.\n\n transport-helper.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 639f08c..a361366 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -159,7 +159,7 @@ static int fetch_with_import(struct transport *transport,\n \t\tposn = to_fetch[i];\n \t\tif (posn->status & REF_STATUS_UPTODATE)\n \t\t\tcontinue;\n-\t\tread_ref(posn->name, posn->old_sha1);\n+\t\tread_ref(posn->symref ? posn->symref : posn->name, posn->old_sha1);\n \t}\n \treturn 0;\n }\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126239","messageId":"1256798426-21816-8-git-send-email-srabbelier@gmail.com","threadId":"21414","inReplyTo":"1256798426-21816-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T06:40:26Z","receivedAt":"2009-10-29T06:40:26Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Also allow the new update_refs to actually update the refs set, this\nway the remote helper can set the value of previously unknown refs.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tThis is the most important patch of the series, without it it\n\tis truly impossible to 'git clone hg::/path/to/hg/repo' since\n\twhen we try to guess remote_head, we use\n\tfind_ref_by_name(refs, \"HEAD\"), which will not find anything\n\tunless we set refs to the updated refs _after_ doing the\n\timport. This is a result of the fact that we do not know the\n\tvalue of the remote HEAD until after the import is complete.\n\n builtin-clone.c    |    7 +++++++\n builtin-fetch.c    |    4 ++++\n transport-helper.c |   22 +++++++++++++++-------\n transport.h        |    7 +++++--\n 4 files changed, 31 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex f281756..768668d 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -512,6 +512,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\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\tif (transport->update_refs)\n+\t\t\t{\n+\t\t\t\tref_cpy = copy_ref_list(refs);\n+\t\t\t\ttransport->update_refs(transport, ref_cpy);\n+\t\t\t\trefs = ref_cpy;\n+\t\t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n+\t\t\t}\n \t\t}\n \t}\n \ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 63a4ff0..3aaac47 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -479,7 +479,11 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n {\n \tint ret = quickfetch(ref_map);\n \tif (ret)\n+\t{\n \t\tret = transport_fetch_refs(transport, ref_map);\n+\t\tif (transport->update_refs)\n+\t\t\ttransport->update_refs(transport, ref_map);\n+\t}\n \tif (!ret)\n \t\tret |= store_updated_refs(transport->url,\n \t\t\t\ttransport->remote->name,\ndiff --git a/transport-helper.c b/transport-helper.c\nindex a361366..9a98fae 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -154,13 +154,6 @@ static int fetch_with_import(struct transport *transport,\n \tfinish_command(&fastimport);\n \tfree((char *) fastimport.argv[2]);\n \tfree((char *) fastimport.argv[3]);\n-\n-\tfor (i = 0; i < nr_heads; i++) {\n-\t\tposn = to_fetch[i];\n-\t\tif (posn->status & REF_STATUS_UPTODATE)\n-\t\t\tcontinue;\n-\t\tread_ref(posn->symref ? posn->symref : posn->name, posn->old_sha1);\n-\t}\n \treturn 0;\n }\n \n@@ -255,6 +248,20 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \treturn ret;\n }\n \n+static int update_refs(struct transport *transport, struct ref *refs)\n+{\n+\tstruct ref *ref = refs;\n+\n+\twhile (ref) {\n+\t\tif (ref->status & REF_STATUS_UPTODATE)\n+\t\t\tcontinue;\n+\t\tread_ref(ref->symref ? ref->symref : ref->name, ref->old_sha1);\n+\t\tref = ref->next;\n+\t}\n+\n+\treturn 0;\n+}\n+\n int transport_helper_init(struct transport *transport, const char *name)\n {\n \tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n@@ -264,5 +271,6 @@ int transport_helper_init(struct transport *transport, const char *name)\n \ttransport->get_refs_list = get_refs_list;\n \ttransport->fetch = fetch;\n \ttransport->disconnect = release_helper;\n+\ttransport->update_refs = update_refs;\n \treturn 0;\n }\ndiff --git a/transport.h b/transport.h\nindex 40dfe64..6d31df3 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -32,12 +32,15 @@ struct transport {\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 **/\n+\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **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 \t **/\n-\tint (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);\n+\tint (*update_refs)(struct transport *transport, struct ref *refs);\n \n \t/**\n \t * Push the objects and refs. Send the necessary objects, and\n-- \n1.6.5.2.291.gf76a3\n"},{"id":"126264","messageId":"200910291305.26523.johan@herland.net","threadId":"21414","inReplyTo":"1256798426-21816-2-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 1/7] Refactor git_remote_cvs to a more generic git_remote_helpers","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-10-29T12:05:26Z","receivedAt":"2009-10-29T12:05:26Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 29 October 2009, Sverre Rabbelier wrote:\n> This in an effort to allow future remote helpers written in python to\n> re-use the non-cvs-specific code.\n> \n> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n> CC: Johan Herland <johan@herland.net>\n> ---\n> \n> \tAs discussed with Johan Herland, refactored git_remote_cvs into a\n> \tmore reusable git_remote_helpers module.\n\nIt's hard to review this patch for a couple of reasons:\n\n1. It's over 200K large, which causes it to be blocked by the Git mailing \nlist (100K limit, I believe).\n\n2. The patch renames some files, but instead of simply stating the rename, \nthe patch lists the entire file twice (deletion + creation)\n\nFortunately, you can easily solve both problems by rerolling the patch with \nthe -M flag to git-format-patch.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"126271","messageId":"20091029142232.GS10505@spearce.org","threadId":"21414","inReplyTo":"1256798426-21816-8-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-10-29T14:22:32Z","receivedAt":"2009-10-29T14:22:32Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> Also allow the new update_refs to actually update the refs set, this\n> way the remote helper can set the value of previously unknown refs.\n...\n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index f281756..768668d 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -512,6 +512,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\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\tif (transport->update_refs)\n> +\t\t\t{\n> +\t\t\t\tref_cpy = copy_ref_list(refs);\n> +\t\t\t\ttransport->update_refs(transport, ref_cpy);\n\nPlease define a transport_update_refs wrapper function to implement\nthis method invocation on the transport instance.  Callers should\nnot be reaching into the struct transport directly.\n\n> diff --git a/builtin-fetch.c b/builtin-fetch.c\n> index 63a4ff0..3aaac47 100644\n> --- a/builtin-fetch.c\n> +++ b/builtin-fetch.c\n> @@ -479,7 +479,11 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n>  {\n>  \tint ret = quickfetch(ref_map);\n>  \tif (ret)\n> +\t{\n>  \t\tret = transport_fetch_refs(transport, ref_map);\n> +\t\tif (transport->update_refs)\n> +\t\t\ttransport->update_refs(transport, ref_map);\n\nI certainly have to wonder... if this is done in both fetch and\nclone, why isn't it just part of fetch_refs?\n\n> diff --git a/transport-helper.c b/transport-helper.c\n> index a361366..9a98fae 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -264,5 +271,6 @@ int transport_helper_init(struct transport *transport, const char *name)\n>  \ttransport->get_refs_list = get_refs_list;\n>  \ttransport->fetch = fetch;\n>  \ttransport->disconnect = release_helper;\n> +\ttransport->update_refs = update_refs;\n\nPlease set this before the disconnect method (move it up one line).\n\n-- \nShawn.\n"},{"id":"126282","messageId":"fabb9a1e0910290848h3fe03a63r15f461d70ec6d6f3@mail.gmail.com","threadId":"21414","inReplyTo":"200910291305.26523.johan@herland.net","subject":"Re: [PATCH 1/7] Refactor git_remote_cvs to a more generic git_remote_helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T15:48:35Z","receivedAt":"2009-10-29T15:48:35Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Oct 29, 2009 at 05:05, Johan Herland <johan@herland.net> wrote:\n> Fortunately, you can easily solve both problems by rerolling the patch with\n> the -M flag to git-format-patch.\n\nWhow, I feel really silly now, thanks for pointing that out, proper patch sent!\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"126283","messageId":"fabb9a1e0910290853p49070443v6d6bf2bf75faf80@mail.gmail.com","threadId":"21414","inReplyTo":"20091029142232.GS10505@spearce.org","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T15:53:59Z","receivedAt":"2009-10-29T15:53:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Oct 29, 2009 at 07:22, Shawn O. Pearce <spearce@spearce.org> wrote:\n> Please define a transport_update_refs wrapper function to implement\n> this method invocation on the transport instance.  Callers should\n> not be reaching into the struct transport directly.\n\nWith pleasure, should the transport_update_refs wrapper simply 'return\n1' without doing anything if transport->update_refs is not set?\n\n> I certainly have to wonder... if this is done in both fetch and\n> clone, why isn't it just part of fetch_refs?\n\nBecause clone does not use fetch_refs, or am I missing something?\n\n> Please set this before the disconnect method (move it up one line).\n\nWill do.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"126284","messageId":"20091029155607.GA10505@spearce.org","threadId":"21414","inReplyTo":"fabb9a1e0910290853p49070443v6d6bf2bf75faf80@mail.gmail.com","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-10-29T15:56:07Z","receivedAt":"2009-10-29T15:56:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> On Thu, Oct 29, 2009 at 07:22, Shawn O. Pearce <spearce@spearce.org> wrote:\n> > Please define a transport_update_refs wrapper function to implement\n> > this method invocation on the transport instance. ?Callers should\n> > not be reaching into the struct transport directly.\n> \n> With pleasure, should the transport_update_refs wrapper simply 'return\n> 1' without doing anything if transport->update_refs is not set?\n\nIf the function isn't defined, it should behave as though it were\ndefined and successfully completed.\n \n> > I certainly have to wonder... if this is done in both fetch and\n> > clone, why isn't it just part of fetch_refs?\n> \n> Because clone does not use fetch_refs, or am I missing something?\n\nHmmph.  Weird.  Its been a while since I last looked at this code,\nmaybe I misunderstood it.\n \n-- \nShawn.\n"},{"id":"126290","messageId":"fabb9a1e0910290932u45c9c416m4d0ba0a8b2f5b01d@mail.gmail.com","threadId":"21414","inReplyTo":"20091029155607.GA10505@spearce.org","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T16:32:38Z","receivedAt":"2009-10-29T16:32:38Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Oct 29, 2009 at 08:56, Shawn O. Pearce <spearce@spearce.org> wrote:\n>> > I certainly have to wonder... if this is done in both fetch and\n>> > clone, why isn't it just part of fetch_refs?\n>>\n>> Because clone does not use fetch_refs, or am I missing something?\n>\n> Hmmph.  Weird.  Its been a while since I last looked at this code,\n> maybe I misunderstood it.\n\nUnless you mean transport_fetch_refs? It can't be done in\ntransport_fetch_refs because the foreign helper transport needs to\nupdate all refs, including those that wanted_peer_refs decided not to\nfetch. Otherwise the 'HEAD' ref will not be updated, and it is left at\nall zeros. It is not as obvious that that is a problem in this patch,\nbut when Junio merges with Nico's [5bdc32d3e \"make 'git clone' ask the\nremote only for objects it cares about\"] the code looks like this:\n\n\t\tif (refs) {\n\t\t\tstruct ref *ref_cpy;\n\t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n\t\t\tref_cpy = copy_ref_list(mapped_refs);\n\t\t\ttransport_fetch_refs(transport, ref_cpy);\n\t\t\tif (transport->update_refs)\n\t\t\t{\n\t\t\t\tref_cpy = copy_ref_list(refs);\n\t\t\t\ttransport->update_refs(transport, ref_cpy);\n\t\t\t\trefs = ref_cpy;\n\t\t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n\t\t\t}\n\t\t}\n\nDoes it make more sense now? transport_fetch_refs gets only a limited\nview of the refs, so it cannot pass all the refs to\ntransport_update_refs as needed.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"126295","messageId":"alpine.LNX.2.00.0910291311570.14365@iabervon.org","threadId":"21414","inReplyTo":"fabb9a1e0910290932u45c9c416m4d0ba0a8b2f5b01d@mail.gmail.com","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-29T17:22:30Z","receivedAt":"2009-10-29T17:22:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 29 Oct 2009, Sverre Rabbelier wrote:\n\n> Heya,\n> \n> On Thu, Oct 29, 2009 at 08:56, Shawn O. Pearce <spearce@spearce.org> wrote:\n> >> > I certainly have to wonder... if this is done in both fetch and\n> >> > clone, why isn't it just part of fetch_refs?\n> >>\n> >> Because clone does not use fetch_refs, or am I missing something?\n> >\n> > Hmmph.  Weird.  Its been a while since I last looked at this code,\n> > maybe I misunderstood it.\n> \n> Unless you mean transport_fetch_refs? It can't be done in\n> transport_fetch_refs because the foreign helper transport needs to\n> update all refs, including those that wanted_peer_refs decided not to\n> fetch. Otherwise the 'HEAD' ref will not be updated, and it is left at\n> all zeros. It is not as obvious that that is a problem in this patch,\n> but when Junio merges with Nico's [5bdc32d3e \"make 'git clone' ask the\n> remote only for objects it cares about\"] the code looks like this:\n> \n> \t\tif (refs) {\n> \t\t\tstruct ref *ref_cpy;\n> \t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n> \t\t\tref_cpy = copy_ref_list(mapped_refs);\n> \t\t\ttransport_fetch_refs(transport, ref_cpy);\n> \t\t\tif (transport->update_refs)\n> \t\t\t{\n> \t\t\t\tref_cpy = copy_ref_list(refs);\n> \t\t\t\ttransport->update_refs(transport, ref_cpy);\n> \t\t\t\trefs = ref_cpy;\n> \t\t\t\tmapped_refs = wanted_peer_refs(refs, refspec);\n> \t\t\t}\n> \t\t}\n> \n> Does it make more sense now? transport_fetch_refs gets only a limited\n> view of the refs, so it cannot pass all the refs to\n> transport_update_refs as needed.\n\nI think there are a bunch of bugs that you're working around:\n\n(a) if HEAD is determined to be a symref, and we care about HEAD, we care \nabout whatever HEAD is a symref to; wanted_peer_refs() shouldn't be \nfiltering such things out.\n\n(b) we don't want to make a whole bunch of copies of the ref list to avoid \nupdating the mutable ref list that we will look for updated values in; the \nmerge of my series with Nico's should replace my copy_ref_list with his \nwanted_peer_refs, not include the copy afterwards. Correcting this should \nlead to the value that matters in the list that will be used having been \nupdated directly by transport_fetch_refs().\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"126297","messageId":"fabb9a1e0910291039u3c9d7eb6qe36e25ea76cdddc9@mail.gmail.com","threadId":"21414","inReplyTo":"alpine.LNX.2.00.0910291311570.14365@iabervon.org","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T17:39:31Z","receivedAt":"2009-10-29T17:39:31Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Oct 29, 2009 at 10:22, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> (a) if HEAD is determined to be a symref, and we care about HEAD, we care\n> about whatever HEAD is a symref to; wanted_peer_refs() shouldn't be\n> filtering such things out.\n\nIt seems that wanted_peer_refs filters out HEAD no matter what though?\n\n> (b) we don't want to make a whole bunch of copies of the ref list to avoid\n> updating the mutable ref list that we will look for updated values in; the\n> merge of my series with Nico's should replace my copy_ref_list with his\n> wanted_peer_refs, not include the copy afterwards. Correcting this should\n> lead to the value that matters in the list that will be used having been\n> updated directly by transport_fetch_refs().\n\nThis I can and will fix. Patch-bomb incoming.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"126323","messageId":"alpine.LNX.2.00.0910291435180.14365@iabervon.org","threadId":"21414","inReplyTo":"fabb9a1e0910291039u3c9d7eb6qe36e25ea76cdddc9@mail.gmail.com","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-29T18:43:41Z","receivedAt":"2009-10-29T18:43:41Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 29 Oct 2009, Sverre Rabbelier wrote:\n\n> Heya,\n> \n> On Thu, Oct 29, 2009 at 10:22, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> > (a) if HEAD is determined to be a symref, and we care about HEAD, we care\n> > about whatever HEAD is a symref to; wanted_peer_refs() shouldn't be\n> > filtering such things out.\n> \n> It seems that wanted_peer_refs filters out HEAD no matter what though?\n\nIt probably shouldn't. Actually, I bet Nico's clone will get extremely \nconfused if the remote's HEAD isn't the same as any of its branches. I'll \ntry to take a look tonight.\n\n> > (b) we don't want to make a whole bunch of copies of the ref list to avoid\n> > updating the mutable ref list that we will look for updated values in; the\n> > merge of my series with Nico's should replace my copy_ref_list with his\n> > wanted_peer_refs, not include the copy afterwards. Correcting this should\n> > lead to the value that matters in the list that will be used having been\n> > updated directly by transport_fetch_refs().\n> \n> This I can and will fix. Patch-bomb incoming.\n\nI'll look over it later, thanks.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"126324","messageId":"fabb9a1e0910291147o78fef01g3f91f4a9e85cacc9@mail.gmail.com","threadId":"21414","inReplyTo":"alpine.LNX.2.00.0910291435180.14365@iabervon.org","subject":"Re: [PATCH 7/7] Factor ref updating out of fetch_with_import","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-10-29T18:47:39Z","receivedAt":"2009-10-29T18:47:39Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Oct 29, 2009 at 11:43, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> It probably shouldn't. Actually, I bet Nico's clone will get extremely\n> confused if the remote's HEAD isn't the same as any of its branches. I'll\n> try to take a look tonight.\n>\n> I'll look over it later, thanks.\n\nThanks!\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}