{"thread":{"id":"26776","subject":"[RFC/PATCH] Documentation/remote-helpers: explain capabilities first","startedAt":"2011-03-18T17:45:04Z","lastAt":"2011-03-19T00:29:05Z","messageCount":6,"participants":["Jonathan Nieder","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"163656","messageId":"20110318174504.GA22332@elie","threadId":"26776","inReplyTo":null,"subject":"[RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-18T17:45:04Z","receivedAt":"2011-03-18T17:45:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The current remote helper documentation is from the perspective of\ngit, so to speak: it presents a full menu of commands for a person\ninvoking a remote helper to choose from.  In practice, that's less\nuseful than it could be, since the daunted novice remote-helper author\nprobably just wanted a list of commands she needs to implement to get\nstarted.  So preface the command list with an overview of each\ncapability, its purpose, and what commands it requires.\n\nAs a side effect, this makes it a little clearer that git doesn't just\nchoose arbitrary commands to run, even if the remote helper advertises\nall capabilities --- instead, there are well defined command sequences\nfor various tasks.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nNo documentation for \"export\" and \"gitdir\" yet (by the way: would it\nmake sense to export the GIT_DIR environment variable so a round trip\nfor the 'gitdir' capability would eventually be unnecessary?).\n\nThe current documentation left me lost, so I wrote this (which still\nprobably leaves one lost).  Thoughts?\n\n Documentation/git-remote-helpers.txt |  101 +++++++++++++++++++++++++--------\n 1 files changed, 76 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 87cd11f..e7a3985 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -29,16 +29,87 @@ commands generally concern facilities like discovering and updating\n remote refs, transporting objects between the object database and\n the remote repository, and updating the local object store.\n \n-Helpers supporting the 'fetch' capability can discover refs from the\n-remote repository and transfer objects reachable from those refs to\n-the local object store. Helpers supporting the 'push' capability can\n-transfer local objects to the remote repository and update remote refs.\n-\n Git comes with a \"curl\" family of remote helpers, that handle various\n transport protocols, such as 'git-remote-http', 'git-remote-https',\n 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities\n 'fetch', 'option', and 'push'.\n \n+CAPABILITIES\n+------------\n+\n+Different remote helpers expect to communicate with git in different\n+ways.  These are declared to git through the helper's response to the\n+`capabilities` command (see COMMANDS, below).\n+\n+'option'::\n+\tCan accept settings like `verbosity` (how much output to write\n+\tto stderr) and `depth` (how much history is wanted in the case\n+\tof a shallow clone) that affect how other commands are carried\n+\tout.\n++\n+Supported commands: 'option'.\n+\n+Capabilities for Pushing\n+~~~~~~~~~~~~~~~~~~~~~~~~\n+'connect'::\n+\tCan attempt to connect to 'git receive-pack' (for pushing),\n+\t'git upload-pack', etc for communication using the\n+\tpackfile protocol.\n++\n+Supported commands: 'connect'.\n+\n+'push'::\n+\tCan discover remote refs and push local commits and the\n+\thistory leading up to them to new or existing remote refs.\n++\n+Supported commands: 'list for-push', 'push'.\n+\n+If a helper advertises both 'connect' and 'push', git will use\n+'connect' if possible and fall back to 'push' if the helper requests\n+so when connecting (see the 'connect' command under COMMANDS).\n+\n+Capabilities for Fetching\n+~~~~~~~~~~~~~~~~~~~~~~~~~\n+'connect'::\n+\tCan try to connect to 'git upload-pack' (for fetching),\n+\t'git receive-pack', etc for communication using the\n+\tpackfile protocol.\n++\n+Supported commands: 'connect'.\n+\n+'fetch'::\n+\tCan discover remote refs and transfer objects reachable from\n+\tthem to the local object store.\n++\n+Supported commands: 'list', 'fetch'.\n+\n+'import'::\n+\tCan discover remote refs and output objects reachable from\n+\tthem as a stream in fast-import format.\n++\n+Supported commands: 'list', 'import'.\n+\n+If a helper advertises 'connect', git will use it if possible and\n+fall back to another capability if the helper requests so when\n+connecting (see the 'connect' command under COMMANDS).\n+When choosing between 'fetch' and 'import', git prefers 'fetch'.\n+Other frontends may have some other order of preference.\n+\n+'refspec' <refspec>::\n+\tThis modifies the 'import' capability.\n++\n+A helper advertising\n+`refspec refs/heads/{asterisk}:refs/svn/origin/branches/{asterisk}`\n+in its capabilities is saying that, when it handles\n+`import refs/heads/topic`, the stream it outputs will update the\n+`refs/svn/origin/branches/topic` ref.\n++\n+This capability can be advertised multiple times.  The first\n+applicable refspec takes precedence.  The left-hand of refspecs\n+advertised with this capability must cover all refs reported by\n+the list command.  If no 'refspec' capability is advertised,\n+there is an implied `refspec {asterisk}:{asterisk}`.\n+\n INVOCATION\n ----------\n \n@@ -167,26 +238,6 @@ completing a valid response for the current command.\n Additional commands may be supported, as may be determined from\n capabilities reported by the helper.\n \n-CAPABILITIES\n-------------\n-\n-'fetch'::\n-'option'::\n-'push'::\n-'import'::\n-'connect'::\n-\tThis helper supports the corresponding command with the same name.\n-\n-'refspec' 'spec'::\n-\tWhen using the import command, expect the source ref to have\n-\tbeen written to the destination ref. The earliest applicable\n-\trefspec takes precedence. For example\n-\t\"refs/heads/*:refs/svn/origin/branches/*\" means that, after an\n-\t\"import refs/heads/name\", the script has written to\n-\trefs/svn/origin/branches/name. If this capability is used at\n-\tall, it must cover all refs reported by the list command; if\n-\tit is not used, it is effectively \"*:*\"\n-\n REF LIST ATTRIBUTES\n -------------------\n \n-- \n1.7.4.1\n"},{"id":"163659","messageId":"AANLkTimfZM6muiU3vPMgx3NnRdb4H0t4E2DMXt1233LP@mail.gmail.com","threadId":"26776","inReplyTo":"20110318174504.GA22332@elie","subject":"Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-03-18T18:07:35Z","receivedAt":"2011-03-18T18:07:35Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Mar 18, 2011 at 18:45, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> No documentation for \"export\" ... yet\n\nWhich is for the better, since I think the current interface is\nbroken. It'll be easier to justify changing it if it's not documented\n;).\n\n> (by the way: would it\n> make sense to export the GIT_DIR environment variable so a round trip\n> for the 'gitdir' capability would eventually be unnecessary?).\n\nYes, that makes a lot of sense.\n\n> The current documentation left me lost, so I wrote this (which still\n> probably leaves one lost).  Thoughts?\n\nI think it makes sense.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"163684","messageId":"20110318222107.GD23407@elie","threadId":"26776","inReplyTo":"AANLkTimfZM6muiU3vPMgx3NnRdb4H0t4E2DMXt1233LP@mail.gmail.com","subject":"Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-18T22:21:07Z","receivedAt":"2011-03-18T22:21:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Fri, Mar 18, 2011 at 18:45, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> No documentation for \"export\" ... yet\n>\n> Which is for the better, since I think the current interface is\n> broken. It'll be easier to justify changing it if it's not documented\n> ;).\n\nThanks for explaining.  FWIW I'd be interested in your thoughts on this\nsometime (not now necessarily, just some time before the svn push\nsupport project is on the front burner :)).\n\n>> (by the way: would it\n>> make sense to export the GIT_DIR environment variable so a round trip\n>> for the 'gitdir' capability would eventually be unnecessary?).\n>\n> Yes, that makes a lot of sense.\n>\n>> The current documentation left me lost, so I wrote this (which still\n>> probably leaves one lost).  Thoughts?\n>\n> I think it makes sense.\n\nGlad to hear it.  (The GIT_DIR idea is Tom's iirc.)\n"},{"id":"163689","messageId":"AANLkTiksEgm+n6nLQ=HYjdE1PV4-XfmJhwj39Wp1crmg@mail.gmail.com","threadId":"26776","inReplyTo":"20110318222107.GD23407@elie","subject":"Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-03-18T22:34:37Z","receivedAt":"2011-03-18T22:34:37Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Mar 18, 2011 at 23:21, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Thanks for explaining.  FWIW I'd be interested in your thoughts on this\n> sometime (not now necessarily, just some time before the svn push\n> support project is on the front burner :)).\n\nAny idea on when that will be? I've been wanting to finish push\nsupport for git-remote-hg for ages now, but haven't been able to make\nthe time :(.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"163691","messageId":"20110318224817.GA25026@elie","threadId":"26776","inReplyTo":"AANLkTiksEgm+n6nLQ=HYjdE1PV4-XfmJhwj39Wp1crmg@mail.gmail.com","subject":"Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-18T22:48:39Z","receivedAt":"2011-03-18T22:48:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n> On Fri, Mar 18, 2011 at 23:21, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> Thanks for explaining.  FWIW I'd be interested in your thoughts on this\n>> sometime (not now necessarily, just some time before the svn push\n>> support project is on the front burner :)).\n>\n> Any idea on when that will be?\n\nNo, not sure.  My current priorities are\n\n - getting \"git clone svndump-alpha::/home/jrn/dump/svn-kde0:1200000\"\n   to work;\n - mustering up the courage to clean up glibc vfprintf\n\nAfter that, who knows?\n\nBut I hear that Ram has done some work on a fast-import to dumpfile\nconverter, so push support is definitely simmering even if there is\nnot much momentum yet.\n"},{"id":"163694","messageId":"20110319002904.GA25155@elie","threadId":"26776","inReplyTo":"20110318174504.GA22332@elie","subject":"Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-03-19T00:29:05Z","receivedAt":"2011-03-19T00:29:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nSorry for the spam.  Just ran into a quick puzzle:\n\nJonathan Nieder wrote:\n\n> +'push'::\n> +\tCan discover remote refs and push local commits and the\n> +\thistory leading up to them to new or existing remote refs.\n> ++\n> +Supported commands: 'list for-push', 'push'.\n[...]\n> +'fetch'::\n> +\tCan discover remote refs and transfer objects reachable from\n> +\tthem to the local object store.\n> ++\n> +Supported commands: 'list', 'fetch'.\n> +\n> +'import'::\n> +\tCan discover remote refs and output objects reachable from\n> +\tthem as a stream in fast-import format.\n> ++\n> +Supported commands: 'list', 'import'.\n\nIn the actual code:\n\n * \"list for-push\" is used if:\n    - we are scouting out for \"refs in common\" in preparation for a push;\n    - the \"push\" capability has been advertised.\n * \"list\" is used in all other cases.\n * in particular, even if no capabilities are advertised, the \"list\"\n   command will be used.\n\nIt's been this way since v1.6.6-rc0~22^2~26 (2009-10-30).  Anyway, the\nrule doesn't sound very principled --- if the \"push\" capability is not\nadvertised, isn't the push going to error out anyway?  Why fall back\nto a normal \"list\" which is likely not to work, either?\n\nOne might suggest:\n\nA.\n - if scouting for \"refs in common\", use \"list for-push\";\n - otherwise, use \"list\" without for-push;\n - in all cases, check for an appropriate capability first\n   (push/export in the for-push case, fetch/import otherwise)\n\nAt first it sounds ok but this one makes for a lousy story when new\ncapabilities are invented.  If \"git remote-helper\" only supports\nfetching with a \"better-import\" capability that my copy of git does\nnot support, I will still try \"git ls-remote helper::url\" to learn\nwhat I am missing.\n\nB.\n - if scouting for \"refs in common\", use \"list for-push\";\n - otherwise, use \"list\" without for-push;\n - all remote helpers must implement both \"list\" and \"list for-push\".\n\nThis rule sounds better, and it doesn't seem to break remote-testgit\n(which currently only receives \"list\" commands without \"for-push\"\nbecause it only advertises \"export\" and not \"push\").  What do you\nthink?\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nProbably I confused myself too far after reading in the manpage that\nfor-push is an attribute.  Note to self: it isn't.\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 0c5b1bd..6d3d15e 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -802,7 +802,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \t\treturn transport->get_refs_list(transport, for_push);\n \t}\n \n-\tif (data->push && for_push)\n+\tif (for_push)\n \t\twrite_str_in_full(helper->in, \"list for-push\\n\");\n \telse\n \t\twrite_str_in_full(helper->in, \"list\\n\");\n-- \n1.7.4.1\n"}]}