{"thread":{"id":"20507","subject":"[PATCH 5/8] Add a config option for remotes to specify a foreign vcs","startedAt":"2009-08-09T19:28:19Z","lastAt":"2009-08-13T22:00:42Z","messageCount":35,"participants":["Daniel Barkalow","Johannes Schindelin","Junio C Hamano","Johan Herland","Bert Wesarg","Jeff King","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"120082","messageId":"alpine.LNX.2.00.0908091526060.27553@iabervon.org","threadId":"20507","inReplyTo":null,"subject":"[PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-09T19:28:19Z","receivedAt":"2009-08-09T19:28:19Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"If this is set, the url is not required, and the transport always uses\na helper named \"git-remote-<value>\".\n\nIt is a separate configuration option in order to allow a sensible\nconfiguration for foreign systems which either have no meaningful urls\nfor repositories or which require urls that do not specify the system\nused by the repository at that location. However, this only affects\nhow the name of the helper is determined, not anything about the\ninteraction with the helper, and the contruction is such that, if the\nforeign scm does happen to use a co-named url method, a url with that\nmethod may be used directly.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n Documentation/config.txt |    4 ++++\n remote.c                 |    4 +++-\n remote.h                 |    2 ++\n transport-helper.c       |   14 ++++++++++----\n transport.c              |    5 +++++\n 5 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bcaec0d..f0144b1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1380,6 +1380,10 @@ remote.<name>.tagopt::\n \tSetting this value to \\--no-tags disables automatic tag following when\n \tfetching from remote <name>\n \n+remote.<name>.vcs::\n+\tSetting this to a value <vcs> will cause git to interact with\n+\tthe remote with the git-remote-<vcs> helper.\n+\n remotes.<group>::\n \tThe list of remotes which are fetched by \"git remote update\n \t<group>\".  See linkgit:git-remote[1].\ndiff --git a/remote.c b/remote.c\nindex e6f5cd2..057ac02 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;\n+\treturn remote->url || remote->foreign_vcs;\n }\n \n static const char *alias_url(const char *url)\n@@ -427,6 +427,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t} else if (!strcmp(subkey, \".proxy\")) {\n \t\treturn git_config_string((const char **)&remote->http_proxy,\n \t\t\t\t\t key, value);\n+\t} else if (!strcmp(subkey, \".vcs\")) {\n+\t\treturn git_config_string(&remote->foreign_vcs, key, value);\n \t}\n \treturn 0;\n }\ndiff --git a/remote.h b/remote.h\nindex 5db8420..ac0ce2f 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -11,6 +11,8 @@ struct remote {\n \tconst char *name;\n \tint origin;\n \n+\tconst char *foreign_vcs;\n+\n \tconst char **url;\n \tint url_nr;\n \tint url_alloc;\ndiff --git a/transport-helper.c b/transport-helper.c\nindex a5ac575..536dd06 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -162,10 +162,16 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n int transport_helper_init(struct transport *transport)\n {\n \tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n-\tchar *eom = strchr(transport->url, ':');\n-\tif (!eom)\n-\t\treturn -1;\n-\tdata->name = xstrndup(transport->url, eom - transport->url);\n+\n+\tif (transport->remote->foreign_vcs) {\n+\t\tdata->name = xstrdup(transport->remote->foreign_vcs);\n+\t\ttransport->url = transport->remote->foreign_vcs;\n+\t} else {\n+\t\tchar *eom = strchr(transport->url, ':');\n+\t\tif (!eom)\n+\t\t\treturn -1;\n+\t\tdata->name = xstrndup(transport->url, eom - transport->url);\n+\t}\n \n \ttransport->data = data;\n \ttransport->get_refs_list = get_refs_list;\ndiff --git a/transport.c b/transport.c\nindex b21e82e..19f330a 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -815,6 +815,11 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\turl = remote->url[0];\n \tret->url = url;\n \n+\tif (remote && remote->foreign_vcs) {\n+\t\ttransport_helper_init(ret);\n+\t\treturn ret;\n+\t}\n+\n \tif (!prefixcmp(url, \"rsync:\")) {\n \t\tret->get_refs_list = get_refs_via_rsync;\n \t\tret->fetch = fetch_objs_via_rsync;\n-- \n1.6.4.183.g77eb9.dirty\n"},{"id":"120087","messageId":"alpine.DEB.1.00.0908092238290.8306@pacific.mpi-cbg.de","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908091526060.27553@iabervon.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-09T20:38:57Z","receivedAt":"2009-08-09T20:38:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 9 Aug 2009, Daniel Barkalow wrote:\n\n> If this is set, the url is not required, and the transport always uses\n> a helper named \"git-remote-<value>\".\n> \n> It is a separate configuration option in order to allow a sensible\n> configuration for foreign systems which either have no meaningful urls\n> for repositories or which require urls that do not specify the system\n> used by the repository at that location. However, this only affects\n> how the name of the helper is determined, not anything about the\n> interaction with the helper, and the contruction is such that, if the\n> foreign scm does happen to use a co-named url method, a url with that\n> method may be used directly.\n> \n> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n> ---\n\nOh well, just try it often enough.  I give up.\n\nCiao,\nDscho\n"},{"id":"120100","messageId":"7v1vnk79lt.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908091526060.27553@iabervon.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-10T01:15:10Z","receivedAt":"2009-08-10T01:15:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> If this is set, the url is not required, and the transport always uses\n> a helper named \"git-remote-<value>\".\n>\n> It is a separate configuration option in order to allow a sensible\n> configuration for foreign systems which either have no meaningful urls\n> for repositories or which require urls that do not specify the system\n> used by the repository at that location. However, this only affects\n> how the name of the helper is determined, not anything about the\n> interaction with the helper, and the contruction is such that, if the\n> foreign scm does happen to use a co-named url method, a url with that\n> method may be used directly.\n\nPersonally, I do not like this.\n\nWhy isn't it enough to define the canonical remote name git takes as\n\"<name of the helper>:<whatever string the helper understands>\"?\n\nThen <whatever string the helper understands> part does not have to\nresemble URL at all, if the foreign system does not have such a concept\n(i.e. \"have no meaning urls for repositories\").  \n\nYour \"let's eject curl based transport out of core\" helper (already in\n'next') will become something like these in the canonical form:\n\n\tcurl:http://git.kernel.org/pub/scm/git/git.git\n\tcurl:ftp://git.kernel.org/pub/scm/git/git.git\n\tcurl:https://git.kernel.org/pub/scm/git/git.git\n\nthat are handled by a single helper binary git-remote-curl, but nobody has\nto use these canonical forms for well-known transports, because we can\nhave an obvious set of backward-compatible synonyms that are understood by\nthe transport layer to choose that helper program, so that usual\n\n\thttp://git.kernel.org/pub/scm/git/git.git\n\nwill be understood to choose git-remote-curl backend.  We do not need to\nhave three git-remote-{http,https,ftp} helpers at all.\n\nThat way, a Subversion repository people may want to interact with would\nbe spelled, if the helper is \"git-remote-svn\", like this:\n\n\tsvn:https://scummvm.svn.sourceforge.net/svnroot/scummvm/\n\nand it would be crystal clear that it is not a git native repository that\nis accessed over curl based walkers, and also the folks who still have not\nmigrated to git can simply drop the leading \"svn:\" and learn the name of\nthe repository they could access natively with Subversion.  Nobody on the\nSubversion side would think svn:https://... is the URL to use with\nSubversion (after all they will see that on the communication in the git\ncircle).\n\nAnd from the git side, we can tell users: \"if you want to interact with\nSubversion repositories, you can use traditional git-svn, or you can use\nthe unified remote mechanism.  To use the latter, just prefix 'svn:' in\nfront of the URL used to refer to the repository in the Subversion world\".\n\nIf you spell the \"URL\" as https://scummvm.svn.sourceforge.net/svnroot/scummvm/\nthen you wouldn't be able to give that directly to git from the command\nline, without using this new configuration.  I do not quite understand why\nthis indirection is desired.  I think it only confuses users.\n\nI recall somebody earlier mentioned a possibility to have more than one\nhelper that deals with one type of foreign system.  In such a case, a pair\nof URL and vcs configuration can be used to identify what helper to use on\nwhat foreign \"repository\":\n\n\t(https://scummvm.svn.sourceforge.net/svnroot/scummvm/, svn)\n\t(https://scummvm.svn.sourceforge.net/svnroot/scummvm/, svn-ng)\n\nand it might become easier to switch the helper without changing the URL\npart.  But I do not particularly think that would be a practical\nadvantage, unless we can assume that the next-generation version of the\nhelper can somehow reuse the metadata the old version of the helper left\nin the repository and incrementally operate on the repository.  Even in\nthat case, I think spelling everything out in a single configuration\n(i.e.. remote.$name.url) would make it clearer to see what is going on.\n"},{"id":"120106","messageId":"alpine.LNX.2.00.0908092153520.27553@iabervon.org","threadId":"20507","inReplyTo":"7v1vnk79lt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-10T04:30:29Z","receivedAt":"2009-08-10T04:30:29Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 9 Aug 2009, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > If this is set, the url is not required, and the transport always uses\n> > a helper named \"git-remote-<value>\".\n> >\n> > It is a separate configuration option in order to allow a sensible\n> > configuration for foreign systems which either have no meaningful urls\n> > for repositories or which require urls that do not specify the system\n> > used by the repository at that location. However, this only affects\n> > how the name of the helper is determined, not anything about the\n> > interaction with the helper, and the contruction is such that, if the\n> > foreign scm does happen to use a co-named url method, a url with that\n> > method may be used directly.\n> \n> Personally, I do not like this.\n> \n> Why isn't it enough to define the canonical remote name git takes as\n> \"<name of the helper>:<whatever string the helper understands>\"?\n\nI think that users should never need to know the names of the helpers. I \nmean, the native protocol uses helpers that most users are completely \nunaware of. The users do need to know what the \n\n> Then <whatever string the helper understands> part does not have to\n> resemble URL at all, if the foreign system does not have such a concept\n> (i.e. \"have no meaning urls for repositories\").  \n\nThe problem is not that foreign systems don't have \"urls\" in particular. \nThe problem is that there are foreign systems, like perforce, where the \nlines between different repositories in the git sense are drawn in very \ndifferent ways. In perforce, for example, there is a single namespace for \nall branches of all projects hosted on the same server. It's like if \nkernel.org had branches:\n\n - git-master\n - linux-master\n - git-next\n - linux-next\n - linux-rt\n - git-pu\n - linux-stable\n - sparse-master\n(and hundreds more, mixing all the branches of all of the repositories \nfor all of the projects, without any conventions beyond what the \ncommunity on the particular server made up)\n\nYou really need a different sort of configuration option to specify what \nwe think of as \"the git.git repository\", which includes certain branches \nand doesn't include other projects hosted on kernel.org. And in order to \nfind the server, it uses an arbitrary shell command line that acts like \nrsh (and often consists of \"ssh\" as the command and a bunch of complicated \noptions to connect to the right port of the right host to get the right \nrestricted shell with the right preset environment). So there's nothing \nyou can put in the <whatever string the helper understands> part, because \nthe helper really has to figure out what it's doing from a bunch of \noptions.\n\nFurthermore, I don't want to just use \"p4:\" as what people should use in \nremote.*.url in order to select the right helper, because then people are \ngoing to have different remotes with the same url option value in order to \naccess entirely different data.\n\nThe only way I've been able to come up with to support this at all \nusefully is to have a bunch of helper-specific options that specify what \nthe helper needs to know about the locations you consider to be part of \nthe project and an option that tells git that this remote uses the p4 \nhelper. I'm not sure what makes sense for other helpers, but the case I \nactually use needs something like what's in this patch.\n\nI think it makes sense for svn access to support just having a url \noption like \"svn://something (svn native protocol)\", or \n\"svn+ssh://something (svn protocol over ssh)\" or \"svn+https://something \n(https access to a svn repo)\", or some other similar syntax, but this is a \npoor fit for p4.\n\nIn order to support this, there just needs to be a call to check whether \n\"remote-<something>\" is an available git command (without running it or \ngiving an error), and the helper code should be used if it is. This is \nactually required so that people with workstations whose domain is \n.kernel.org and who have cloned \"master:/home/linus/something.git\" don't \nstart getting \"remote-master isn't a git command\" errors (that is, \nmisinterpreting ssh-format location hostnames as helper names. Johan, \nperhaps you could write that for your CVS helper?\n\n> Your \"let's eject curl based transport out of core\" helper (already in\n> 'next') will become something like these in the canonical form:\n> \n> \tcurl:http://git.kernel.org/pub/scm/git/git.git\n> \tcurl:ftp://git.kernel.org/pub/scm/git/git.git\n> \tcurl:https://git.kernel.org/pub/scm/git/git.git\n> \n> that are handled by a single helper binary git-remote-curl, but nobody has\n> to use these canonical forms for well-known transports, because we can\n> have an obvious set of backward-compatible synonyms that are understood by\n> the transport layer to choose that helper program, so that usual\n> \n> \thttp://git.kernel.org/pub/scm/git/git.git\n> \n> will be understood to choose git-remote-curl backend.  We do not need to\n> have three git-remote-{http,https,ftp} helpers at all.\n> \n> That way, a Subversion repository people may want to interact with would\n> be spelled, if the helper is \"git-remote-svn\", like this:\n> \n> \tsvn:https://scummvm.svn.sourceforge.net/svnroot/scummvm/\n> \n> and it would be crystal clear that it is not a git native repository that\n> is accessed over curl based walkers, and also the folks who still have not\n> migrated to git can simply drop the leading \"svn:\" and learn the name of\n> the repository they could access natively with Subversion.  Nobody on the\n> Subversion side would think svn:https://... is the URL to use with\n> Subversion (after all they will see that on the communication in the git\n> circle).\n> \n> And from the git side, we can tell users: \"if you want to interact with\n> Subversion repositories, you can use traditional git-svn, or you can use\n> the unified remote mechanism.  To use the latter, just prefix 'svn:' in\n> front of the URL used to refer to the repository in the Subversion world\".\n> \n> If you spell the \"URL\" as https://scummvm.svn.sourceforge.net/svnroot/scummvm/\n> then you wouldn't be able to give that directly to git from the command\n> line, without using this new configuration.  I do not quite understand why\n> this indirection is desired.  I think it only confuses users.\n\nAs I said above, I think that just picking the start off of the URL is a \ngood solution for systems where the remote repository has a location, but \nthat's not every system, and not the one that's my personal itch.\n\n> I recall somebody earlier mentioned a possibility to have more than one\n> helper that deals with one type of foreign system.  In such a case, a pair\n> of URL and vcs configuration can be used to identify what helper to use on\n> what foreign \"repository\":\n> \n> \t(https://scummvm.svn.sourceforge.net/svnroot/scummvm/, svn)\n> \t(https://scummvm.svn.sourceforge.net/svnroot/scummvm/, svn-ng)\n> \n> and it might become easier to switch the helper without changing the URL\n> part.  But I do not particularly think that would be a practical\n> advantage, unless we can assume that the next-generation version of the\n> helper can somehow reuse the metadata the old version of the helper left\n> in the repository and incrementally operate on the repository.  Even in\n> that case, I think spelling everything out in a single configuration\n> (i.e.. remote.$name.url) would make it clearer to see what is going on.\n\nI think that, ideally, helpers for foreign systems would be portable \nacross multiple native systems. The svn helper could be a program \n\"svn-remote-access-helper\", and anything that speaks fast-import (e.g., \nbzr or hg) would be able to use it. When installing it for git, you'd \nsymlink it to git-remote-svn; if you decided to install \n\"svn-remote-access-helper-ng\", you'd change the symlink.\n\nOf course, there's a long way to go before helpers can be portable in this \ndirection, but I think it's mostly tractable and good for making the code \nclean.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"120112","messageId":"200908101032.12835.johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908092153520.27553@iabervon.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-10T08:32:12Z","receivedAt":"2009-08-10T08:32:12Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 10 August 2009, Daniel Barkalow wrote:\n> On Sun, 9 Aug 2009, Junio C Hamano wrote:\n> > Daniel Barkalow <barkalow@iabervon.org> writes:\n> > > If this is set, the url is not required, and the transport always\n> > > uses a helper named \"git-remote-<value>\".\n> > >\n> > > It is a separate configuration option in order to allow a sensible\n> > > configuration for foreign systems which either have no meaningful\n> > > urls for repositories or which require urls that do not specify the\n> > > system used by the repository at that location. However, this only\n> > > affects how the name of the helper is determined, not anything about\n> > > the interaction with the helper, and the contruction is such that, if\n> > > the foreign scm does happen to use a co-named url method, a url with\n> > > that method may be used directly.\n> >\n> > Personally, I do not like this.\n> >\n> > Why isn't it enough to define the canonical remote name git takes as\n> > \"<name of the helper>:<whatever string the helper understands>\"?\n>\n> [...]\n>\n> The only way I've been able to come up with to support this at all\n> usefully is to have a bunch of helper-specific options that specify what\n> the helper needs to know about the locations you consider to be part of\n> the project and an option that tells git that this remote uses the p4\n> helper. I'm not sure what makes sense for other helpers, but the case I\n> actually use needs something like what's in this patch.\n\nI'm somewhat agnostic on this issue. At the moment, I follow the P4 cues,\nand use a config like this:\n\n    [remote \"foo\"]\n        vcs = cvs\n        cvsRoot = \":pserver:user@cvs.server.example.com/var/cvs/cvsroot\"\n        cvsModule = \"bar\"\n        ...\n\nBut I could just as well use a config like this instead:\n\n    [remote \"foo\"]\n        url = \"cvs::pserver:user@cvs.server.example.com/var/cvs/cvsroot#bar\"\n        ...\n\nEither is fine with me, although I suspect users might find the\ncurrent/first alternative easier to parse.\n\n> I think it makes sense for svn access to support just having a url\n> option like \"svn://something (svn native protocol)\", or\n> \"svn+ssh://something (svn protocol over ssh)\" or \"svn+https://something\n> (https access to a svn repo)\", or some other similar syntax, but this is\n> a poor fit for p4.\n>\n> In order to support this, there just needs to be a call to check whether\n> \"remote-<something>\" is an available git command (without running it or\n> giving an error), and the helper code should be used if it is. This is\n> actually required so that people with workstations whose domain is\n> .kernel.org and who have cloned \"master:/home/linus/something.git\" don't\n> start getting \"remote-master isn't a git command\" errors (that is,\n> misinterpreting ssh-format location hostnames as helper names. Johan,\n> perhaps you could write that for your CVS helper?\n\nSorry, not following you here. Write exactly what?\n\n- The code in the transport layer for checking if \"remote-<something>\"\n  is an available git command?\n\n- The code in my CVS helper for handling the ssh-format misinterpretation,\n  i.e. the case where someone has a git/ssh server called \"cvs\"? If so,\n  how should this be handled?\n\n> I think that, ideally, helpers for foreign systems would be portable\n> across multiple native systems. The svn helper could be a program\n> \"svn-remote-access-helper\", and anything that speaks fast-import (e.g.,\n> bzr or hg) would be able to use it. When installing it for git, you'd\n> symlink it to git-remote-svn; if you decided to install\n> \"svn-remote-access-helper-ng\", you'd change the symlink.\n\nIn that case, helpers must keep their metadata in a repo-independent\nformat. Currently that is outside the scope of my CVS helper, since I'm\nleveraging git-notes for most of the CVS helper's metadata.\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"120117","messageId":"7v63cw3vjv.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908092153520.27553@iabervon.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-10T08:47:00Z","receivedAt":"2009-08-10T08:47:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> I think that users should never need to know the names of the helpers. I \n> mean, the native protocol uses helpers that most users are completely \n> unaware of. The users do need to know what the \n\n... heck ;-)?\n\nBut seriously, why not?\n\n> The problem is not that foreign systems don't have \"urls\" in particular. \n> The problem is that there are foreign systems, like perforce, where the \n> lines between different repositories in the git sense are drawn in very \n> different ways. In perforce, for example, there is a single namespace for \n> all branches of all projects hosted on the same server. It's like... \n> ...\n> (and hundreds more, mixing all the branches of all of the repositories \n> for all of the projects, without any conventions beyond what the \n> community on the particular server made up)\n\nThat looks similar to a subversion server with multiple projects with\nmultiple branches, and they seem to be able to use\n\n    \"svn://<whatever svn uses as its own convention>\"\n\njust fine.\n\nWhy p4 cannot do what svn can?  Doesn't p4 ever \"name\" a repository/branch\ninside that \"single namespace\" with a simple string that is easy to\nremember for users?  And if there is one, I do not see what prevents you\nfrom using \"p4://<whatever information p4 users use>\"?\n\n> You really need a different sort of configuration option to specify what \n> we think of as \"the git.git repository\", which includes certain branches \n> and doesn't include other projects hosted on kernel.org. And in order to \n> find the server, it uses an arbitrary shell command line that acts like \n> rsh (and often consists of \"ssh\" as the command and a bunch of complicated \n> options to connect to the right port of the right host to get the right \n> restricted shell with the right preset environment). So there's nothing \n> you can put in the <whatever string the helper understands> part, because \n> the helper really has to figure out what it's doing from a bunch of \n> options.\n>\n> Furthermore, I don't want to just use \"p4:\" as what people should use in \n> remote.*.url in order to select the right helper, because then people are \n> going to have different remotes with the same url option value in order to \n> access entirely different data.\n\nMaybe it is the _primary_ itch you would want to scratch, but this does\nsound like a very special case that wants to have multiple pieces of\npre-parsed information to identify a \"repository\".\n\nPerhaps in such a case it would be equally valid if you said \"p4:hello\"\nand \"p4:goodbye\" to identify two \"repositories\", with p4-helper specific\nconfiguration option that is keyed off of of p4.hello.* and p4.goodbye.*\nnamespace, e.g. (without knowing p4 at all)\n\n\t[remote \"origin\"]\n        \turl = p4:hello\n\t[p4 \"hello\"]\n        \tfind-server-command = ssh bunch of complicated options\n\t\trandom-p4-option = ...\n\t\trandom-other-p4-option = ...\n\nOf course it does not have to be called \"url\", and you could do the same\nthing with:\n\n\t[remote \"origin\"]\n        \tvcs = p4\n\t[p4 \"origin\"]\n        \tfind-server-command = ssh bunch of complicated options\n\t\trandom-p4-option = ...\n\t\trandom-other-p4-option = ...\n\nAnd probably there aren't much difference either way between these two\nappraoches.  The former gives an extra name (\"hello\") that may simply be\nredundant, or perhaps adds value by descriptive, depending on your\nviewpoint.\n"},{"id":"120205","messageId":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","threadId":"20507","inReplyTo":"200908101032.12835.johan@herland.net","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-10T19:30:44Z","receivedAt":"2009-08-10T19:30:44Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 10 Aug 2009, Johan Herland wrote:\n\n> On Monday 10 August 2009, Daniel Barkalow wrote:\n> > On Sun, 9 Aug 2009, Junio C Hamano wrote:\n> > > Daniel Barkalow <barkalow@iabervon.org> writes:\n> > > > If this is set, the url is not required, and the transport always\n> > > > uses a helper named \"git-remote-<value>\".\n> > > >\n> > > > It is a separate configuration option in order to allow a sensible\n> > > > configuration for foreign systems which either have no meaningful\n> > > > urls for repositories or which require urls that do not specify the\n> > > > system used by the repository at that location. However, this only\n> > > > affects how the name of the helper is determined, not anything about\n> > > > the interaction with the helper, and the contruction is such that, if\n> > > > the foreign scm does happen to use a co-named url method, a url with\n> > > > that method may be used directly.\n> > >\n> > > Personally, I do not like this.\n> > >\n> > > Why isn't it enough to define the canonical remote name git takes as\n> > > \"<name of the helper>:<whatever string the helper understands>\"?\n> >\n> > [...]\n> >\n> > The only way I've been able to come up with to support this at all\n> > usefully is to have a bunch of helper-specific options that specify what\n> > the helper needs to know about the locations you consider to be part of\n> > the project and an option that tells git that this remote uses the p4\n> > helper. I'm not sure what makes sense for other helpers, but the case I\n> > actually use needs something like what's in this patch.\n> \n> I'm somewhat agnostic on this issue. At the moment, I follow the P4 cues,\n> and use a config like this:\n> \n>     [remote \"foo\"]\n>         vcs = cvs\n>         cvsRoot = \":pserver:user@cvs.server.example.com/var/cvs/cvsroot\"\n>         cvsModule = \"bar\"\n>         ...\n> \n> But I could just as well use a config like this instead:\n> \n>     [remote \"foo\"]\n>         url = \"cvs::pserver:user@cvs.server.example.com/var/cvs/cvsroot#bar\"\n>         ...\n> \n> Either is fine with me, although I suspect users might find the\n> current/first alternative easier to parse.\n\nI suspect there will be some users who want (1) and some who want (2), and \nboth ought to work.\n\n> > I think it makes sense for svn access to support just having a url\n> > option like \"svn://something (svn native protocol)\", or\n> > \"svn+ssh://something (svn protocol over ssh)\" or \"svn+https://something\n> > (https access to a svn repo)\", or some other similar syntax, but this is\n> > a poor fit for p4.\n> >\n> > In order to support this, there just needs to be a call to check whether\n> > \"remote-<something>\" is an available git command (without running it or\n> > giving an error), and the helper code should be used if it is. This is\n> > actually required so that people with workstations whose domain is\n> > .kernel.org and who have cloned \"master:/home/linus/something.git\" don't\n> > start getting \"remote-master isn't a git command\" errors (that is,\n> > misinterpreting ssh-format location hostnames as helper names. Johan,\n> > perhaps you could write that for your CVS helper?\n> \n> Sorry, not following you here. Write exactly what?\n> \n> - The code in the transport layer for checking if \"remote-<something>\"\n>   is an available git command?\n\nThat's what I was thinking.\n\n> - The code in my CVS helper for handling the ssh-format misinterpretation,\n>   i.e. the case where someone has a git/ssh server called \"cvs\"? If so,\n>   how should this be handled?\n\nI'd been thinking that people's servers wouldn't be named \"cvs\", but they \nmight be named things that aren't clearly not plausible names for helpers. \nSo I don't think there should be a need to deal with misdirected names \ngoing to actual helpers, just names for helpers that don't exist.\n\n> > I think that, ideally, helpers for foreign systems would be portable\n> > across multiple native systems. The svn helper could be a program\n> > \"svn-remote-access-helper\", and anything that speaks fast-import (e.g.,\n> > bzr or hg) would be able to use it. When installing it for git, you'd\n> > symlink it to git-remote-svn; if you decided to install\n> > \"svn-remote-access-helper-ng\", you'd change the symlink.\n> \n> In that case, helpers must keep their metadata in a repo-independent\n> format. Currently that is outside the scope of my CVS helper, since I'm\n> leveraging git-notes for most of the CVS helper's metadata.\n\nYeah, that's one of several tricky issues. I don't think there's enough \nexperience yet to design something to support portable helpers, but I \nthink it's worth thinking about.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"120220","messageId":"7vmy67orwr.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"200908101032.12835.johan@herland.net","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-11T05:12:20Z","receivedAt":"2009-08-11T05:12:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> On Monday 10 August 2009, Daniel Barkalow wrote:\n> ...\n>> The only way I've been able to come up with to support this at all\n>> usefully is to have a bunch of helper-specific options that specify what\n>> the helper needs to know about the locations you consider to be part of\n>> the project and an option that tells git that this remote uses the p4\n>> helper. I'm not sure what makes sense for other helpers, but the case I\n>> actually use needs something like what's in this patch.\n>\n> I'm somewhat agnostic on this issue. At the moment, I follow the P4 cues,\n> and use a config like this:\n>\n>     [remote \"foo\"]\n>         vcs = cvs\n>         cvsRoot = \":pserver:user@cvs.server.example.com/var/cvs/cvsroot\"\n>         cvsModule = \"bar\"\n>         ...\n>\n> But I could just as well use a config like this instead:\n>\n>     [remote \"foo\"]\n>         url = \"cvs::pserver:user@cvs.server.example.com/var/cvs/cvsroot#bar\"\n>         ...\n>\n> Either is fine with me, although I suspect users might find the\n> current/first alternative easier to parse.\n\nAh, ok, that is a much better (rather, easier to understand for _me_)\nexample to illustrate what Daniel meant, and I can well imagine P4\ncounterpart of cvsRoot may resemble an URL even less than your cvsRoot\nexample does.\n\nIf the foreign system uses a single string as \"the string to identify\nlocation\", like the latter (which is a good example, even though I do not\nthink a CVS folks write a reference to a module like you wrote), then I\nthink it makes sense to use that form as remote.$name.url.  But if naming\na location with a single simple string is an alien notion to the foreign\nsystem, I agree with Daniel that we do not gain much by trying to shoehorn\nthem into a single remote.$name.url configuration.\n"},{"id":"120246","messageId":"alpine.DEB.1.00.0908111033430.4638@intel-tinevez-2-302","threadId":"20507","inReplyTo":"7vmy67orwr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T08:39:49Z","receivedAt":"2009-08-11T08:39:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 10 Aug 2009, Junio C Hamano wrote:\n\n> Johan Herland <johan@herland.net> writes:\n> \n> > I'm somewhat agnostic on this issue. At the moment, I follow the P4 \n> > cues, and use a config like this:\n> >\n> >     [remote \"foo\"]\n> >         vcs = cvs\n> >         cvsRoot = \":pserver:user@cvs.server.example.com/var/cvs/cvsroot\"\n> >         cvsModule = \"bar\"\n> >         ...\n> >\n> > But I could just as well use a config like this instead:\n> >\n> >     [remote \"foo\"]\n> >         url = \"cvs::pserver:user@cvs.server.example.com/var/cvs/cvsroot#bar\"\n> >         ...\n> >\n> > Either is fine with me, although I suspect users might find the\n> > current/first alternative easier to parse.\n> \n> Ah, ok, that is a much better (rather, easier to understand for _me_) \n> example to illustrate what Daniel meant, and I can well imagine P4 \n> counterpart of cvsRoot may resemble an URL even less than your cvsRoot \n> example does.\n> \n> If the foreign system uses a single string as \"the string to identify\n> location\", like the latter (which is a good example, even though I do not\n> think a CVS folks write a reference to a module like you wrote), then I\n> think it makes sense to use that form as remote.$name.url.  But if naming\n> a location with a single simple string is an alien notion to the foreign\n> system, I agree with Daniel that we do not gain much by trying to shoehorn\n> them into a single remote.$name.url configuration.\n\nOf course, it is always nice to be able to tell people: just clone the \nrepository with\n\n\tgit clone cvs::pserver:anonymous@cool.haxx.se:/cvsroot/curl#curl\n\nRather than telling them: \"you know, it is really trivial once you \nunderstand the inner workings of Git.  Just follow this recipe for the \nmoment, and you are all set up:\n\n\t1) mkdir curl\n\t2) cd curl\n\t3) git init\n\t4) git remote.origin.vcs cvs\n\t5) git remote.origin.cvsRoot :pserver:anonymous@cool.haxx.se:/cvsroot/curl\n\t6) git remote.origin.cvsModule curl\n\t7) git fetch origin\n\t8) git checkout --track origin/master\n\nOh, and please ignore that warning in the last step telling you that you \nare already on branch 'master', that is perfectly normal.\"\n\nI don't know, but this sounds more and more like the complicator's glove \nto me (with the same reactions).\n\nCiao,\nDscho\n"},{"id":"120258","messageId":"1249985426-13726-1-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 0/6] Graceful handling of missing remote helpers","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:20Z","receivedAt":"2009-08-11T10:10:20Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Mon, 10 Aug 2009, Daniel Barkalow wrote:\n> > > In order to support this, there just needs to be a call to check whether\n> > > \"remote-<something>\" is an available git command (without running it or\n> > > giving an error), and the helper code should be used if it is. This is\n> > > actually required so that people with workstations whose domain is\n> > > .kernel.org and who have cloned \"master:/home/linus/something.git\" don't\n> > > start getting \"remote-master isn't a git command\" errors (that is,\n> > > misinterpreting ssh-format location hostnames as helper names. Johan,\n> > > perhaps you could write that for your CVS helper?\n> >\n> > Sorry, not following you here. Write exactly what?\n> >\n> > - The code in the transport layer for checking if \"remote-<something>\"\n> >   is an available git command?\n>\n> That's what I was thinking.\n\nOk, I've spent way too much time on this, so if this doesn't cut it, I leave it\nup to someone else to pick up the pieces.\n\nThe following patch series implements what I understand to be the current\nconsensus (or at least a good compromise) in this thread:\n\n- There are two ways to indicate that a remote helper should be used:\n  1. remote.foo.vcs = <something>\n  2. remote.foo.url starts with \"<something>:\"\n  If both are present, remote.foo.vcs is preferred.\n\n- Teach transport_helper_init() to check if \"remote-<something>\" is an\n  available git command. If so, use it. If not, gracefully abort the\n  transport-helper setup, and fall back to the \"native\" transport code.\n\n- Skip the transport-helper code is the remote is obviously \"native\" (i.e.\n  URL starts with \"git:\"/\"file:\"/\"ssh:\"/\"rsync:\")\n\nAFAICS this should make Daniel happy because his P4 setup will still work, and\nit should also make Dscho happy, since he can tell people to\n  \"git clone cvs::pserver:anonymous@cool.haxx.se:/cvsroot/curl#curl\"\n(of course, this depends on the CVS helper supporting remote.foo.url, which it\nwill, in a future iteration)\n\nThe non-trivial part of the series is in patch #5.\n\nImplementing the rsync handling as another remote helper program is left as an\nexercise for the reader...\n\n\nHave fun! :)\n\n...Johan\n\n\nJohan Herland (6):\n  Minor unrelated fixes\n  transport_get(): Don't SEGFAULT on missing url\n  Move setup of curl remote helper from transport.c to transport-helper.c\n  Add is_git_command_or_alias() for checking availability of a given git command\n  Let transport_helper_init() decide if a remote helper program can be used\n  Add testcase to verify handling of missing remote helper programs\n\n help.c                           |   23 ++++++++++++-\n help.h                           |    2 +\n t/t5590-remote-helper-missing.sh |   56 +++++++++++++++++++++++++++++++\n transport-helper.c               |   67 +++++++++++++++++++++++++++++++++++--\n transport.c                      |   56 ++++++-------------------------\n 5 files changed, 154 insertions(+), 50 deletions(-)\n create mode 100755 t/t5590-remote-helper-missing.sh\n"},{"id":"120256","messageId":"1249985426-13726-2-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 1/6] Minor unrelated fixes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:21Z","receivedAt":"2009-08-11T10:10:21Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Signed-off-by: Johan Herland <johan@herland.net>\n---\n help.c             |    2 +-\n transport-helper.c |    6 ++++--\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 6c46d8b..994561d 100644\n--- a/help.c\n+++ b/help.c\n@@ -302,7 +302,7 @@ const char *help_unknown_cmd(const char *cmd)\n \tstruct cmdnames main_cmds, other_cmds;\n \n \tmemset(&main_cmds, 0, sizeof(main_cmds));\n-\tmemset(&other_cmds, 0, sizeof(main_cmds));\n+\tmemset(&other_cmds, 0, sizeof(other_cmds));\n \tmemset(&aliases, 0, sizeof(aliases));\n \n \tgit_config(git_unknown_cmd_config, NULL);\ndiff --git a/transport-helper.c b/transport-helper.c\nindex cc99368..a901630 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -8,7 +8,7 @@\n \n struct helper_data\n {\n-\tconst char *name;\n+\tchar *name;\n \tstruct child_process *helper;\n \n \tchar *marks_filename;\n@@ -258,8 +258,10 @@ int transport_helper_init(struct transport *transport)\n \t\ttransport->url = transport->remote->foreign_vcs;\n \t} else {\n \t\tchar *eom = strchr(transport->url, ':');\n-\t\tif (!eom)\n+\t\tif (!eom) {\n+\t\t\tfree(data);\n \t\t\treturn -1;\n+\t\t}\n \t\tdata->name = xstrndup(transport->url, eom - transport->url);\n \t}\n \n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120254","messageId":"1249985426-13726-3-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 2/6] transport_get(): Don't SEGFAULT on missing url","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:22Z","receivedAt":"2009-08-11T10:10:22Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Signed-off-by: Johan Herland <johan@herland.net>\n---\n transport.c |   11 ++++++-----\n 1 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 19f330a..26d9999 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -820,14 +820,15 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\treturn ret;\n \t}\n \n-\tif (!prefixcmp(url, \"rsync:\")) {\n+\tif (url && !prefixcmp(url, \"rsync:\")) {\n \t\tret->get_refs_list = get_refs_via_rsync;\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n \n-\t} else if (!prefixcmp(url, \"http://\")\n-\t        || !prefixcmp(url, \"https://\")\n-\t        || !prefixcmp(url, \"ftp://\")) {\n+\t} else if (url\n+\t        && (!prefixcmp(url, \"http://\")\n+\t         || !prefixcmp(url, \"https://\")\n+\t         || !prefixcmp(url, \"ftp://\"))) {\n \t\ttransport_helper_init(ret);\n #ifdef NO_CURL\n \t\terror(\"git was compiled without libcurl support.\");\n@@ -835,7 +836,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->push = curl_transport_push;\n #endif\n \n-\t} else if (is_local(url) && is_file(url)) {\n+\t} else if (url && is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->get_refs_list = get_refs_from_bundle;\n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120259","messageId":"1249985426-13726-4-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 3/6] Move setup of curl remote helper from transport.c to transport-helper.c","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:23Z","receivedAt":"2009-08-11T10:10:23Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Since the curl transport is now launched by the transport-helper mechanism,\nit makes sense to move the remaining curl-related code (i.e.\ncurl_transport_push()) from transport.c into transport-helper.c.\n\nThe patch also consolidates the two transport_helper_init() call sites\n(\"foreign\" helper and curl helper).\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n transport-helper.c |   39 +++++++++++++++++++++++++++++++++++++++\n transport.c        |   48 ++++++------------------------------------------\n 2 files changed, 45 insertions(+), 42 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex a901630..d3ce984 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -249,6 +249,34 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \treturn ret;\n }\n \n+#ifndef NO_CURL\n+static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)\n+{\n+\tconst char **argv;\n+\tint argc;\n+\n+\tif (flags & TRANSPORT_PUSH_MIRROR)\n+\t\treturn error(\"http transport does not support mirror mode\");\n+\n+\targv = xmalloc((refspec_nr + 12) * sizeof(char *));\n+\targv[0] = \"http-push\";\n+\targc = 1;\n+\tif (flags & TRANSPORT_PUSH_ALL)\n+\t\targv[argc++] = \"--all\";\n+\tif (flags & TRANSPORT_PUSH_FORCE)\n+\t\targv[argc++] = \"--force\";\n+\tif (flags & TRANSPORT_PUSH_DRY_RUN)\n+\t\targv[argc++] = \"--dry-run\";\n+\tif (flags & TRANSPORT_PUSH_VERBOSE)\n+\t\targv[argc++] = \"--verbose\";\n+\targv[argc++] = transport->url;\n+\twhile (refspec_nr--)\n+\t\targv[argc++] = *refspec++;\n+\targv[argc] = NULL;\n+\treturn !!run_command_v_opt(argv, RUN_GIT_CMD);\n+}\n+#endif\n+\n int transport_helper_init(struct transport *transport)\n {\n \tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n@@ -269,5 +297,16 @@ int transport_helper_init(struct transport *transport)\n \ttransport->get_refs_list = get_refs_list;\n \ttransport->fetch = fetch;\n \ttransport->disconnect = disconnect_helper;\n+\n+\tif (!strcmp(data->name, \"http\")\n+\t || !strcmp(data->name, \"https\")\n+\t || !strcmp(data->name, \"ftp\")) {\n+#ifdef NO_CURL\n+\t\terror(\"git was compiled without libcurl support.\");\n+#else\n+\t\ttransport->push = curl_transport_push;\n+#endif\n+\t}\n+\n \treturn 0;\n }\ndiff --git a/transport.c b/transport.c\nindex 26d9999..81a28bc 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -349,35 +349,6 @@ static int rsync_transport_push(struct transport *transport,\n \treturn result;\n }\n \n-#ifndef NO_CURL\n-static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)\n-{\n-\tconst char **argv;\n-\tint argc;\n-\n-\tif (flags & TRANSPORT_PUSH_MIRROR)\n-\t\treturn error(\"http transport does not support mirror mode\");\n-\n-\targv = xmalloc((refspec_nr + 12) * sizeof(char *));\n-\targv[0] = \"http-push\";\n-\targc = 1;\n-\tif (flags & TRANSPORT_PUSH_ALL)\n-\t\targv[argc++] = \"--all\";\n-\tif (flags & TRANSPORT_PUSH_FORCE)\n-\t\targv[argc++] = \"--force\";\n-\tif (flags & TRANSPORT_PUSH_DRY_RUN)\n-\t\targv[argc++] = \"--dry-run\";\n-\tif (flags & TRANSPORT_PUSH_VERBOSE)\n-\t\targv[argc++] = \"--verbose\";\n-\targv[argc++] = transport->url;\n-\twhile (refspec_nr--)\n-\t\targv[argc++] = *refspec++;\n-\targv[argc] = NULL;\n-\treturn !!run_command_v_opt(argv, RUN_GIT_CMD);\n-}\n-\n-#endif\n-\n struct bundle_transport_data {\n \tint fd;\n \tstruct bundle_header header;\n@@ -815,26 +786,19 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\turl = remote->url[0];\n \tret->url = url;\n \n-\tif (remote && remote->foreign_vcs) {\n-\t\ttransport_helper_init(ret);\n-\t\treturn ret;\n-\t}\n+\tif (remote && remote->foreign_vcs)\n+\t\turl = NULL;\n \n \tif (url && !prefixcmp(url, \"rsync:\")) {\n \t\tret->get_refs_list = get_refs_via_rsync;\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n \n-\t} else if (url\n-\t        && (!prefixcmp(url, \"http://\")\n-\t         || !prefixcmp(url, \"https://\")\n-\t         || !prefixcmp(url, \"ftp://\"))) {\n+\t} else if (!url\n+\t        || !prefixcmp(url, \"http://\")\n+\t        || !prefixcmp(url, \"https://\")\n+\t        || !prefixcmp(url, \"ftp://\")) {\n \t\ttransport_helper_init(ret);\n-#ifdef NO_CURL\n-\t\terror(\"git was compiled without libcurl support.\");\n-#else\n-\t\tret->push = curl_transport_push;\n-#endif\n \n \t} else if (url && is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120260","messageId":"1249985426-13726-5-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 4/6] Add is_git_command_or_alias() for checking availability of a given git command","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:24Z","receivedAt":"2009-08-11T10:10:24Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"The transport-helper mechanism requires us to gracefully handle the case\nwhere a git command is not available for some reason.\n\nThis patch introduces a simple function for querying the availability of a\ngit command, without attempting to execute said command.\n\nThe new function is very similar to the static is_git_command() function in\nbuiltin-help.c, except that this function also accepts a matching alias.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n help.c |   21 +++++++++++++++++++++\n help.h |    2 ++\n 2 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 994561d..a616277 100644\n--- a/help.c\n+++ b/help.c\n@@ -296,6 +296,27 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)\n \told->names = NULL;\n }\n \n+int is_git_command_or_alias(const char *cmd)\n+{\n+\tstruct cmdnames main_cmds, other_cmds;\n+\n+\tmemset(&main_cmds, 0, sizeof(main_cmds));\n+\tmemset(&other_cmds, 0, sizeof(other_cmds));\n+\tmemset(&aliases, 0, sizeof(aliases));\n+\n+\tgit_config(git_unknown_cmd_config, NULL);\n+\n+\tload_command_list(\"git-\", &main_cmds, &other_cmds);\n+\n+\tadd_cmd_list(&main_cmds, &aliases);\n+\tadd_cmd_list(&main_cmds, &other_cmds);\n+\tqsort(main_cmds.names, main_cmds.cnt,\n+\t      sizeof(main_cmds.names), cmdname_compare);\n+\tuniq(&main_cmds);\n+\n+\treturn is_in_cmdlist(&main_cmds, cmd);\n+}\n+\n const char *help_unknown_cmd(const char *cmd)\n {\n \tint i, n, best_similarity = 0;\ndiff --git a/help.h b/help.h\nindex 56bc154..6c43452 100644\n--- a/help.h\n+++ b/help.h\n@@ -26,4 +26,6 @@ int is_in_cmdlist(struct cmdnames *c, const char *s);\n void list_commands(const char *title, struct cmdnames *main_cmds,\n \t\t   struct cmdnames *other_cmds);\n \n+int is_git_command_or_alias(const char *cmd);\n+\n #endif /* HELP_H */\n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120255","messageId":"1249985426-13726-6-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:25Z","receivedAt":"2009-08-11T10:10:25Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Teach the transport-helper mechanism to verify that the remote helper\nprogram is available, before finalizing the transport setup.\nIf the remote helper is NOT available, transport_helper_init() returns -1,\nand transport_get() falls back to trying a native transport.\n\nThis patch introduces a subtle, but important change in the handling of\nremotes and their URLs:\n\nBefore this patch, we only invoke transport_helper_init() _after_ we find\nthat the transport-helper mechanism is appropriate for this remote\n(i.e. the remote is \"foreign\", or is handled by the curl helper).\n\nThis patch moves the decision point into transport_helper_init(): If the\nremote is not obviously using a native transport (URL starts with \"file:\",\n\"git:\" or \"ssh:\"), then we first call transport_helper_init(), and only if\nit returns error (meaning that no appropriate remote helper program was\nfound) do we fall back to native transport handling.\n\nSigned-off-by: Johan Herland <johan@herland.net>\n---\n transport-helper.c |   22 ++++++++++++++++++++--\n transport.c        |   11 ++++++-----\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex d3ce984..de30727 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -5,6 +5,7 @@\n #include \"commit.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n+#include \"help.h\"\n \n struct helper_data\n {\n@@ -279,11 +280,18 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons\n \n int transport_helper_init(struct transport *transport)\n {\n-\tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n+\tstruct helper_data *data;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *cmd;\n+\n+\tif (!transport->remote)\n+\t\treturn -1;\n \n+\tdata = xcalloc(sizeof(*data), 1);\n \tif (transport->remote->foreign_vcs) {\n \t\tdata->name = xstrdup(transport->remote->foreign_vcs);\n-\t\ttransport->url = transport->remote->foreign_vcs;\n+\t\tif (!transport->url)\n+\t\t\ttransport->url = transport->remote->foreign_vcs;\n \t} else {\n \t\tchar *eom = strchr(transport->url, ':');\n \t\tif (!eom) {\n@@ -293,6 +301,16 @@ int transport_helper_init(struct transport *transport)\n \t\tdata->name = xstrndup(transport->url, eom - transport->url);\n \t}\n \n+\tstrbuf_addf(&buf, \"remote-%s\", data->name);\n+\tcmd = strbuf_detach(&buf, NULL);\n+\tif (!is_git_command_or_alias(cmd)) {\n+\t\twarning(\"Could not find remote helper command \\\"git %s\\\". Assuming native remote.\", cmd);\n+\t\tfree(cmd);\n+\t\tfree(data->name);\n+\t\tfree(data);\n+\t\treturn -1;\n+\t}\n+\n \ttransport->data = data;\n \ttransport->get_refs_list = get_refs_list;\n \ttransport->fetch = fetch;\ndiff --git a/transport.c b/transport.c\nindex 81a28bc..b7033eb 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -794,11 +794,12 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n \n-\t} else if (!url\n-\t        || !prefixcmp(url, \"http://\")\n-\t        || !prefixcmp(url, \"https://\")\n-\t        || !prefixcmp(url, \"ftp://\")) {\n-\t\ttransport_helper_init(ret);\n+\t} else if ((!url\n+\t         || (prefixcmp(url, \"git:\")\n+\t          && prefixcmp(url, \"ssh:\")\n+\t          && prefixcmp(url, \"file:\")))\n+\t        && !transport_helper_init(ret)) {\n+\t\t/* no-op, ret is initialized by transport_helper_init() */\n \n \t} else if (url && is_local(url) && is_file(url)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120257","messageId":"1249985426-13726-7-git-send-email-johan@herland.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908101205120.27553@iabervon.org","subject":"[RFC/PATCH 6/6] Add testcase to verify handling of missing remote helper programs","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-11T10:10:26Z","receivedAt":"2009-08-11T10:10:26Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"Signed-off-by: Johan Herland <johan@herland.net>\n---\n t/t5590-remote-helper-missing.sh |   56 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 56 insertions(+), 0 deletions(-)\n create mode 100755 t/t5590-remote-helper-missing.sh\n\ndiff --git a/t/t5590-remote-helper-missing.sh b/t/t5590-remote-helper-missing.sh\nnew file mode 100755\nindex 0000000..3bc2bc2\n--- /dev/null\n+++ b/t/t5590-remote-helper-missing.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+\n+test_description='test graceful failure when missing remote helper program'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup repository' '\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -m one &&\n+\tgit config remote.missing.vcs foo &&\n+\tgit remote add missing2 foo:/nonexisting/path\n+'\n+\n+test_expect_success 'fetch changes from \"missing\" remote' '\n+\tcat <<EOF >expect &&\n+warning: Could not find remote helper command \"git remote-foo\". Assuming native remote.\n+EOF\n+\t! git fetch missing >actual 2>&1 &&\n+\thead -n1 actual >actual.first &&\n+\ttest_cmp expect actual.first\n+'\n+\n+test_expect_success 'fetch changes from \"missing2\" remote' '\n+\tcat <<EOF >expect &&\n+warning: Could not find remote helper command \"git remote-foo\". Assuming native remote.\n+EOF\n+\t! git fetch missing2 >actual 2>&1 &&\n+\thead -n1 actual >actual.first &&\n+\ttest_cmp expect actual.first\n+'\n+\n+test_expect_success 'push changes to \"missing\" remote' '\n+\techo \"more content\" >>file &&\n+\tgit add file &&\n+\tgit commit -m two &&\n+\tcat <<EOF >expect &&\n+warning: Could not find remote helper command \"git remote-foo\". Assuming native remote.\n+EOF\n+\t! git push --all missing >actual 2>&1 &&\n+\thead -n1 actual >actual.first &&\n+\ttest_cmp expect actual.first\n+'\n+\n+test_expect_success 'push changes to \"missing2\" remote' '\n+\techo \"even more content\" >>file &&\n+\tgit add file &&\n+\tgit commit -m three &&\n+\tcat <<EOF >expect &&\n+warning: Could not find remote helper command \"git remote-foo\". Assuming native remote.\n+EOF\n+\t! git push --all missing2 >actual 2>&1 &&\n+\thead -n1 actual >actual.first &&\n+\ttest_cmp expect actual.first\n+'\n+\n+test_done\n-- \n1.6.4.rc3.138.ga6b98.dirty\n"},{"id":"120240","messageId":"alpine.DEB.1.00.0908111403500.4638@intel-tinevez-2-302","threadId":"20507","inReplyTo":"1249985426-13726-6-git-send-email-johan@herland.net","subject":"Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T12:21:02Z","receivedAt":"2009-08-11T12:21:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Johan Herland wrote:\n\n> diff --git a/transport-helper.c b/transport-helper.c\n> index d3ce984..de30727 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -5,6 +5,7 @@\n>  #include \"commit.h\"\n>  #include \"diff.h\"\n>  #include \"revision.h\"\n> +#include \"help.h\"\n>  \n>  struct helper_data\n>  {\n> @@ -279,11 +280,18 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons\n>  \n>  int transport_helper_init(struct transport *transport)\n>  {\n> -\tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n> +\tstruct helper_data *data;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tchar *cmd;\n> +\n> +\tif (!transport->remote)\n> +\t\treturn -1;\n>  \n> +\tdata = xcalloc(sizeof(*data), 1);\n>  \tif (transport->remote->foreign_vcs) {\n>  \t\tdata->name = xstrdup(transport->remote->foreign_vcs);\n> -\t\ttransport->url = transport->remote->foreign_vcs;\n> +\t\tif (!transport->url)\n> +\t\t\ttransport->url = transport->remote->foreign_vcs;\n>  \t} else {\n>  \t\tchar *eom = strchr(transport->url, ':');\n>  \t\tif (!eom) {\n\nIMHO it would be better to decide early if there is no vcs helper, rather \nthan doing all kinds of set up, only to free() the data we worked so hard \nin setting up later.  Something like\n\n\tif (!transport->remote->foreign_vcs) {\n\t\tconst char *colon = transport->url ?\n\t\t\tstrchr(transport->url, ':') : NULL;\n\n\t\tif (!colon)\n\t\t\treturn -1;\n\n\t\ttransport->remote->foreign_vcs =\n\t\t\txstrndup(transport->url, colon - transport->url);\n\t}\n\n\tstrbuf_addf(&buf, \"remote-%s\", transport->remote->foreign_vcs);\n\tif (!is_git_command_or_alias(buf.buf)) {\n\t\terror(\"Could not find remote helper '%s'\", buf.buf);\n\t\tstrbuf_release(&buf);\n\t\treturn -1;\n\t}\n\n> diff --git a/transport.c b/transport.c\n> index 81a28bc..b7033eb 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -794,11 +794,12 @@ struct transport *transport_get(struct remote *remote, const char *url)\n>  \t\tret->fetch = fetch_objs_via_rsync;\n>  \t\tret->push = rsync_transport_push;\n>  \n> -\t} else if (!url\n> -\t        || !prefixcmp(url, \"http://\")\n> -\t        || !prefixcmp(url, \"https://\")\n> -\t        || !prefixcmp(url, \"ftp://\")) {\n> -\t\ttransport_helper_init(ret);\n> +\t} else if ((!url\n> +\t         || (prefixcmp(url, \"git:\")\n> +\t          && prefixcmp(url, \"ssh:\")\n> +\t          && prefixcmp(url, \"file:\")))\n> +\t        && !transport_helper_init(ret)) {\n> +\t\t/* no-op, ret is initialized by transport_helper_init() */\n>  \n>  \t} else if (url && is_local(url) && is_file(url)) {\n\nConfusing...\n\nWhen is transport_helper_init(ret) called now?  What is done in the code \nblock?  Ah, nothing is done, but we usually write that this way:\n\n\t\t; /* do nothing */\n\nAnd a comment would have been in order to say that we fall back to native \nGit transport, which will probably silently fail when there is no URL (I \n_know_ that allowing two different ways to specify the same thing are not \nonly inconsistent and confusing, they lead to errors -- if not here then \nsomewhere else).\n\nAlso, you missed two cases mentioned in connect.c: \"git+ssh\" and \n\"ssh+git\".  Which brings me to another thing: I'd rather handle the known \nprotocols and fall back to a remote helper.  The other way round seems \npretty backwards.\n\nSidenote: I have to admit that I find it distracting that \"ret\" is passed \nto transport_helper_init(), either, it's probably not an \"int\" but an \ninstance of struct transport.)\n\nCiao,\nDscho\n"},{"id":"120238","messageId":"alpine.DEB.1.00.0908111358060.4638@intel-tinevez-2-302","threadId":"20507","inReplyTo":"1249985426-13726-5-git-send-email-johan@herland.net","subject":"Re: [RFC/PATCH 4/6] Add is_git_command_or_alias() for checking availability of a given git command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T12:21:10Z","receivedAt":"2009-08-11T12:21:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Johan Herland wrote:\n\n> diff --git a/help.c b/help.c\n> index 994561d..a616277 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -296,6 +296,27 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)\n>  \told->names = NULL;\n>  }\n>  \n> +int is_git_command_or_alias(const char *cmd)\n> +{\n> +\tstruct cmdnames main_cmds, other_cmds;\n> +\n> +\tmemset(&main_cmds, 0, sizeof(main_cmds));\n> +\tmemset(&other_cmds, 0, sizeof(other_cmds));\n> +\tmemset(&aliases, 0, sizeof(aliases));\n> +\n> +\tgit_config(git_unknown_cmd_config, NULL);\n> +\n> +\tload_command_list(\"git-\", &main_cmds, &other_cmds);\n> +\n> +\tadd_cmd_list(&main_cmds, &aliases);\n> +\tadd_cmd_list(&main_cmds, &other_cmds);\n> +\tqsort(main_cmds.names, main_cmds.cnt,\n> +\t      sizeof(main_cmds.names), cmdname_compare);\n> +\tuniq(&main_cmds);\n> +\n> +\treturn is_in_cmdlist(&main_cmds, cmd);\n> +}\n\nSorting a list is an O(n log n) operation, searching through a list \nlinearly is O(n).  You throw away main_cmds (without freeing) after that, \nso I think it is not a good trade-off.\n\nCiao,\nDscho\n"},{"id":"120241","messageId":"alpine.DEB.1.00.0908111421290.4638@intel-tinevez-2-302","threadId":"20507","inReplyTo":"1249985426-13726-4-git-send-email-johan@herland.net","subject":"Re: [RFC/PATCH 3/6] Move setup of curl remote helper from transport.c to transport-helper.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T12:23:32Z","receivedAt":"2009-08-11T12:23:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Johan Herland wrote:\n\n> diff --git a/transport-helper.c b/transport-helper.c\n> index a901630..d3ce984 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -249,6 +249,34 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n>  \treturn ret;\n>  }\n>  \n> +#ifndef NO_CURL\n> +static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)\n> +{\n> +\tconst char **argv;\n> +\tint argc;\n> +\n> +\tif (flags & TRANSPORT_PUSH_MIRROR)\n> +\t\treturn error(\"http transport does not support mirror mode\");\n> +\n> +\targv = xmalloc((refspec_nr + 12) * sizeof(char *));\n> +\targv[0] = \"http-push\";\n> +\targc = 1;\n> +\tif (flags & TRANSPORT_PUSH_ALL)\n> +\t\targv[argc++] = \"--all\";\n> +\tif (flags & TRANSPORT_PUSH_FORCE)\n> +\t\targv[argc++] = \"--force\";\n> +\tif (flags & TRANSPORT_PUSH_DRY_RUN)\n> +\t\targv[argc++] = \"--dry-run\";\n> +\tif (flags & TRANSPORT_PUSH_VERBOSE)\n> +\t\targv[argc++] = \"--verbose\";\n> +\targv[argc++] = transport->url;\n> +\twhile (refspec_nr--)\n> +\t\targv[argc++] = *refspec++;\n> +\targv[argc] = NULL;\n> +\treturn !!run_command_v_opt(argv, RUN_GIT_CMD);\n> +}\n> +#endif\n> +\n>  int transport_helper_init(struct transport *transport)\n>  {\n>  \tstruct helper_data *data = xcalloc(sizeof(*data), 1);\n> @@ -269,5 +297,16 @@ int transport_helper_init(struct transport *transport)\n>  \ttransport->get_refs_list = get_refs_list;\n>  \ttransport->fetch = fetch;\n>  \ttransport->disconnect = disconnect_helper;\n> +\n> +\tif (!strcmp(data->name, \"http\")\n> +\t || !strcmp(data->name, \"https\")\n> +\t || !strcmp(data->name, \"ftp\")) {\n> +#ifdef NO_CURL\n> +\t\terror(\"git was compiled without libcurl support.\");\n> +#else\n> +\t\ttransport->push = curl_transport_push;\n> +#endif\n> +\t}\n> +\n>  \treturn 0;\n>  }\n\nI still find it rather, uhm, confusing that copying git-http-push and/or \ngit-remote-http into place will still give you the error message \"Git was \ncompiled without libcurl support\".\n\nIt's just as well that I do not need to maintain RPMs or DEBs.\n\nCiao,\nDscho\n"},{"id":"120280","messageId":"36ca99e90908110831g2ad52a5ar4a755723a6682f77@mail.gmail.com","threadId":"20507","inReplyTo":"7v1vnk79lt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-08-11T15:31:50Z","receivedAt":"2009-08-11T15:31:50Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, Aug 10, 2009 at 03:15, Junio C Hamano<gitster@pobox.com> wrote:\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n>\n>> If this is set, the url is not required, and the transport always uses\n>> a helper named \"git-remote-<value>\".\n>>\n>> It is a separate configuration option in order to allow a sensible\n>> configuration for foreign systems which either have no meaningful urls\n>> for repositories or which require urls that do not specify the system\n>> used by the repository at that location. However, this only affects\n>> how the name of the helper is determined, not anything about the\n>> interaction with the helper, and the contruction is such that, if the\n>> foreign scm does happen to use a co-named url method, a url with that\n>> method may be used directly.\n>\n> Personally, I do not like this.\n>\n> Why isn't it enough to define the canonical remote name git takes as\n> \"<name of the helper>:<whatever string the helper understands>\"?\n\nMay I ask what will happen to these supported URL notations:\n\n\n       o   [user@]host.xz:/path/to/repo.git/\n\n       o   [user@]host.xz:~user/path/to/repo.git/\n\n       o   [user@]host.xz:path/to/repo.git\n\nthis will bite you, if you have an ssh host alias named \"<your\nfavorite helper name>\".\n\nBert\n"},{"id":"120291","messageId":"alpine.DEB.1.00.0908111817490.4638@intel-tinevez-2-302","threadId":"20507","inReplyTo":"36ca99e90908110831g2ad52a5ar4a755723a6682f77@mail.gmail.com","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T16:20:54Z","receivedAt":"2009-08-11T16:20:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Bert Wesarg wrote:\n\n> On Mon, Aug 10, 2009 at 03:15, Junio C Hamano<gitster@pobox.com> wrote:\n> > Daniel Barkalow <barkalow@iabervon.org> writes:\n> >\n> >> If this is set, the url is not required, and the transport always uses\n> >> a helper named \"git-remote-<value>\".\n> >>\n> >> It is a separate configuration option in order to allow a sensible\n> >> configuration for foreign systems which either have no meaningful urls\n> >> for repositories or which require urls that do not specify the system\n> >> used by the repository at that location. However, this only affects\n> >> how the name of the helper is determined, not anything about the\n> >> interaction with the helper, and the contruction is such that, if the\n> >> foreign scm does happen to use a co-named url method, a url with that\n> >> method may be used directly.\n> >\n> > Personally, I do not like this.\n> >\n> > Why isn't it enough to define the canonical remote name git takes as\n> > \"<name of the helper>:<whatever string the helper understands>\"?\n> \n> May I ask what will happen to these supported URL notations:\n> \n> \n>        o   [user@]host.xz:/path/to/repo.git/\n> \n>        o   [user@]host.xz:~user/path/to/repo.git/\n> \n>        o   [user@]host.xz:path/to/repo.git\n> \n> this will bite you, if you have an ssh host alias named \"<your\n> favorite helper name>\".\n\nThat is a valid concern.  I'd suggest to disallow @ and . in the protocol \nname (maybe even everything non-alnum).  For shortcuts that really read \nlike \"svn\", I think it is not too much asked for to require adjusting the \nURL (by prefixing ssh:// and adjusting the path).\n\nIf there is really anybody who uses Git via ssh to access a server called \n\"svn\", we could just as well have a little fun with them, don't you agree?\n\nCiao,\nDscho\n"},{"id":"120313","messageId":"20090811214849.GA3868@sigill.intra.peff.net","threadId":"20507","inReplyTo":"alpine.DEB.1.00.0908111817490.4638@intel-tinevez-2-302","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-11T21:48:49Z","receivedAt":"2009-08-11T21:48:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 11, 2009 at 06:20:54PM +0200, Johannes Schindelin wrote:\n\n> >        o   [user@]host.xz:/path/to/repo.git/\n> \n> That is a valid concern.  I'd suggest to disallow @ and . in the protocol \n> name (maybe even everything non-alnum).  For shortcuts that really read \n> like \"svn\", I think it is not too much asked for to require adjusting the \n> URL (by prefixing ssh:// and adjusting the path).\n> \n> If there is really anybody who uses Git via ssh to access a server called \n> \"svn\", we could just as well have a little fun with them, don't you agree?\n\nIt is not actually that unreasonable. I have remotes which point to:\n\n  vcs:git/foo.git\n\nwhere \"vcs\" is a shorthand for vcs.gtisc.gatech.edu, defined in my\n.ssh/config (it's a machine which hosts several different version\ncontrol systems). So I could see somebody having something like \"svn\" if\nthey were, e.g., hosting a git-to-svn conversion on their company's svn\nserver.\n\nAnd as far as asking people in such a situation to change, consider:\n\n  1. No matter how small a change, asking for a change means you are\n     breaking people's current config.\n\n  2. ssh:// really is inferior, especially if you are typing it on the\n     command-line as part of a one-off pull. It is really convenient to\n     assume you start in your home directory (as above), and to be able\n     to use tilde-notation to point to the home directories of others\n     (e.g., \"git pull vcs:~mike/foo.git\").\n\n-Peff\n"},{"id":"120322","messageId":"20090811230233.GA25642@coredump.intra.peff.net","threadId":"20507","inReplyTo":"20090812075914.6117@nanako3.lavabit.com","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-11T23:02:33Z","receivedAt":"2009-08-11T23:02:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 12, 2009 at 07:59:14AM +0900, Nanako Shiraishi wrote:\n\n> Then how about using a prefix that has been invalid, \"vcs::$string\",\n> for example?\n\nI have no problem with that, and I think it makes it even more visually\nobvious what is going. For example:\n\n  svn::http://server/path/to/repo\n\nmakes the \"svn\" prefix jump out a bit more.\n\n-Peff\n"},{"id":"120328","messageId":"alpine.LNX.2.00.0908111915100.27553@iabervon.org","threadId":"20507","inReplyTo":"1249985426-13726-6-git-send-email-johan@herland.net","subject":"Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-11T23:28:19Z","receivedAt":"2009-08-11T23:28:19Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 11 Aug 2009, Johan Herland wrote:\n\n> @@ -293,6 +301,16 @@ int transport_helper_init(struct transport *transport)\n>  \t\tdata->name = xstrndup(transport->url, eom - transport->url);\n>  \t}\n>  \n> +\tstrbuf_addf(&buf, \"remote-%s\", data->name);\n> +\tcmd = strbuf_detach(&buf, NULL);\n> +\tif (!is_git_command_or_alias(cmd)) {\n> +\t\twarning(\"Could not find remote helper command \\\"git %s\\\". Assuming native remote.\", cmd);\n\nWon't this give you a warning for:\n\n\turl = master.kernel.org:something\n\nof 'Could not find (...) \"git remote-master.kernel.org\" (...)'? That \nwould be certain to upset some people. I think we can assume that people's \nscheme parts of their URLs that are actually URLs don't contain '.', and \nthat people with:\n\n\turl = master:something\n\nwill append their domains if the warning gets annoying.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"120331","messageId":"alpine.DEB.1.00.0908120128120.8306@pacific.mpi-cbg.de","threadId":"20507","inReplyTo":"20090811214849.GA3868@sigill.intra.peff.net","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-11T23:53:38Z","receivedAt":"2009-08-11T23:53:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Jeff King wrote:\n\n> On Tue, Aug 11, 2009 at 06:20:54PM +0200, Johannes Schindelin wrote:\n> \n> > >        o   [user@]host.xz:/path/to/repo.git/\n> > \n> > That is a valid concern.  I'd suggest to disallow @ and . in the protocol \n> > name (maybe even everything non-alnum).  For shortcuts that really read \n> > like \"svn\", I think it is not too much asked for to require adjusting the \n> > URL (by prefixing ssh:// and adjusting the path).\n> > \n> > If there is really anybody who uses Git via ssh to access a server called \n> > \"svn\", we could just as well have a little fun with them, don't you agree?\n> \n> It is not actually that unreasonable. I have remotes which point to:\n> \n>   vcs:git/foo.git\n\nThat is still not \"svn\".\n\n> where \"vcs\" is a shorthand for vcs.gtisc.gatech.edu, defined in my \n> .ssh/config (it's a machine which hosts several different version \n> control systems). So I could see somebody having something like \"svn\" if \n> they were, e.g., hosting a git-to-svn conversion on their company's svn \n> server.\n\nIf _I_ were to judge whether to make it convenient for computer-savvy \npeople like you who would have _no_ problem diagnosing the problem (_if_ \nthey have the problem, having edited .ssh/config themselves!), who would \ncurse briefly, and then go on fixing the problem, or in the alternative \nmake it convenient for people who do not know their way around .ssh/config \nas well as you (and who happen to make up the _vast_ majority of Git users \nby now [*1*]), and who would really prefer to have an easy way to clone \n\"foreign\" repositories, I have _no_ problem deciding which way to go.\n\nSo I'm a bastard.  Big news.  But I'm a pragmatic one.\n\nCiao,\nDscho\n\nFootnote [*1*]: Maybe I was helping to increase the number of Git users \nwho do not know .ssh/config with my work on Git for Windows, but that was \nnot my original intention.  Maybe I should start to feel better about \nit...\n"},{"id":"120332","messageId":"alpine.DEB.1.00.0908120212500.8306@pacific.mpi-cbg.de","threadId":"20507","inReplyTo":"20090811230233.GA25642@coredump.intra.peff.net","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-12T00:14:12Z","receivedAt":"2009-08-12T00:14:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Aug 2009, Jeff King wrote:\n\n> On Wed, Aug 12, 2009 at 07:59:14AM +0900, Nanako Shiraishi wrote:\n> \n> > Then how about using a prefix that has been invalid, \"vcs::$string\",\n> > for example?\n> \n> I have no problem with that, and I think it makes it even more visually\n> obvious what is going. For example:\n> \n>   svn::http://server/path/to/repo\n> \n> makes the \"svn\" prefix jump out a bit more.\n\n... and http:://repo.or.cz/r/git.git ?  Thanks.  But no, thanks.\n\nCiao,\nDscho\n"},{"id":"120359","messageId":"7vljlpaf0d.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"alpine.DEB.1.00.0908120212500.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-12T03:26:58Z","receivedAt":"2009-08-12T03:26:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> I have no problem with that, and I think it makes it even more visually\n>> obvious what is going. For example:\n>> \n>>   svn::http://server/path/to/repo\n>> \n>> makes the \"svn\" prefix jump out a bit more.\n>\n> ... and http:://repo.or.cz/r/git.git ?  Thanks.  But no, thanks.\n\nHuh?\n\nIf you meant a \"canonical format that always spells out the name of the\nhelper, and then whatever string the chosen helper uses to identify the\nrepository\", that would be spelled as:\n\n\tlibcurl::http://repo.or.cz/r/git.git/\n\nand will be handled by a single helper, git-remote-libcurl, that is\nessentially what Linus and Daniel ejected from the builtin.\n\nAnd in fact, that would be vastly more sensible than \"we have one helper\nthat uses libcurl, but we hide the implementation detail and call the\nhelper with three names git-remote-{http,https,ftp}, so you would spell\nthe repository http://repo.or.cz/r/git.git/\", which is what we have queued\nin 'next/pu'.\n\nAnd of course the use of \"canonical format\" for transports that git\ntraditionally has known about is only for consistency; we would want to\ngive shortcut for them.  Obviously we would want \"http://<anything>\" to be\na short-hand for \"curl::http://<anything>\".\n"},{"id":"120385","messageId":"20090812074521.GD15152@coredump.intra.peff.net","threadId":"20507","inReplyTo":"alpine.DEB.1.00.0908120128120.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-12T07:45:21Z","receivedAt":"2009-08-12T07:45:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 12, 2009 at 01:53:38AM +0200, Johannes Schindelin wrote:\n\n> > It is not actually that unreasonable. I have remotes which point to:\n> > \n> >   vcs:git/foo.git\n> \n> That is still not \"svn\".\n\nNo, but you snipped the part where I explain how that leads me to\nbelieve \"svn\" is plausible. Remember that you and I are just a\nrepresentative sample of a much larger userbase.\n\nThere is also a related question: should the the meaning of the URL rely\npurely on _syntax_, or must we understand the _semantics_ of the\nindividual tokens? That is, given $X:$Y, does that syntactically mean\nthat $X _must_ be a remote helper, or must I understand what helpers git\nknows about to know what it is?\n\nI tend to think purely syntactic systems are more robust and easier to\nunderstand. The downside is that it's less DWIM, which can often mean\nmore typing.\n\n> If _I_ were to judge whether to make it convenient for computer-savvy \n> people like you who would have _no_ problem diagnosing the problem (_if_ \n> they have the problem, having edited .ssh/config themselves!), who would \n> curse briefly, and then go on fixing the problem, or in the alternative \n> make it convenient for people who do not know their way around .ssh/config \n> as well as you (and who happen to make up the _vast_ majority of Git users \n> by now [*1*]), and who would really prefer to have an easy way to clone \n> \"foreign\" repositories, I have _no_ problem deciding which way to go.\n> \n> So I'm a bastard.  Big news.  But I'm a pragmatic one.\n\nYou didn't quote the part of my email about how ssh:// sucks. It is not\njust about having my config break, figuring it out, and fixing it. You\nare losing a useful construct that I might be using on the command line.\n\nThat being said, I am not 100% opposed to the proposal. I just think it\nis worth considering this breakage as a downside, and considering\n\n  1. Is there some other syntax that _doesn't_ have this breakage\n     but that similarly helps the \"vast majority of Git users\".\n\n  2. Should such a breakage follow a deprecation schedule, and if so,\n     what schedule?\n\n-Peff\n"},{"id":"120386","messageId":"20090812074658.GE15152@coredump.intra.peff.net","threadId":"20507","inReplyTo":"alpine.LNX.2.00.0908111915100.27553@iabervon.org","subject":"Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-12T07:46:59Z","receivedAt":"2009-08-12T07:46:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 11, 2009 at 07:28:19PM -0400, Daniel Barkalow wrote:\n\n> of 'Could not find (...) \"git remote-master.kernel.org\" (...)'? That \n> would be certain to upset some people. I think we can assume that people's \n> scheme parts of their URLs that are actually URLs don't contain '.', and \n> that people with:\n> \n> \turl = master:something\n> \n> will append their domains if the warning gets annoying.\n\nKeep in mind that these URLs should be usable from the command-line,\ntoo. So it is not just appending the domain in the config, but appending\nit every time you want to do a one-off pull.\n\n-Peff\n"},{"id":"120394","messageId":"m363ctpedr.fsf@localhost.localdomain","threadId":"20507","inReplyTo":"20090812074521.GD15152@coredump.intra.peff.net","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-12T09:33:08Z","receivedAt":"2009-08-12T09:33:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   1. Is there some other syntax that _doesn't_ have this breakage\n>      but that similarly helps the \"vast majority of Git users\".\n\nWell, proposed possible syntax was:\n\n1. <vcs>:<repository location>\n\n   e.g.:\n\n     svn:http://svn.example.com/project\n\n   but\n\n     host:path/to/repo\n\n2. <vcs>::<repository location>\n\n   e.g.\n\n     svn::http://svn.example.com/project\n\n3. <vcs>+<repository location>\n\n   e.g.\n\n     svn+http://svn.example.com/project\n\n   but\n\n     http+svn://svn.example.com/project\n     svn+path/to/repo\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"120422","messageId":"alpine.LNX.2.00.0908121216570.27553@iabervon.org","threadId":"20507","inReplyTo":"20090812074658.GE15152@coredump.intra.peff.net","subject":"Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-08-12T16:21:40Z","receivedAt":"2009-08-12T16:21:40Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 12 Aug 2009, Jeff King wrote:\n\n> On Tue, Aug 11, 2009 at 07:28:19PM -0400, Daniel Barkalow wrote:\n> \n> > of 'Could not find (...) \"git remote-master.kernel.org\" (...)'? That \n> > would be certain to upset some people. I think we can assume that people's \n> > scheme parts of their URLs that are actually URLs don't contain '.', and \n> > that people with:\n> > \n> > \turl = master:something\n> > \n> > will append their domains if the warning gets annoying.\n> \n> Keep in mind that these URLs should be usable from the command-line,\n> too. So it is not just appending the domain in the config, but appending\n> it every time you want to do a one-off pull.\n\nFor that sort of thing, I think that url.*.insteadOf in your global config \nis the way to go. It applies before anything else, and can supply parts of \nthe path as well as the hostname.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"120447","messageId":"7vocqkydum.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"m363ctpedr.fsf@localhost.localdomain","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-12T20:30:25Z","receivedAt":"2009-08-12T20:30:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>>   1. Is there some other syntax that _doesn't_ have this breakage\n>>      but that similarly helps the \"vast majority of Git users\".\n>\n> Well, proposed possible syntax was:\n> 1. <vcs>:<repository location>\n> ...\n> 2. <vcs>::<repository location>\n> ...\n> 3. <vcs>+<repository location>\n>\n>    e.g.\n>\n>      svn+http://svn.example.com/project\n>\n>    but\n>\n>      http+svn://svn.example.com/project\n>      svn+path/to/repo\n\nI do not think these are valid examples to demonstrate that 3 is bad.\n\nWe do not have (and we will not create) \"http+svn://\" native transport, so\nthe former can only mean \"Feed 'svn://svn.example.com/project' to the vcs\nhelper whose name is 'http'\".  Similarly I do not see any way to read the\nlatter other than \"Feed 'path/to/repo' to 'svn' vcs helper\".\n\nWe do have a pair of synonyms \"git+ssh://foo\" and \"ssh+git://bar\" that\ncould make the use of '+' ambiguous.  They could be feeding 'ssh://foo'\nand 'git://bar' to 'git' and 'ssh' vcs helpers respectively, but\n\n (1) they are not even advertised in Documentation/ anywhere as far as I\n     can see; and\n\n (2) these are the only two existing ones that are misdesigned, and we can\n     easily special case them to keep backward compatibility.\n\nDouble-colon (your 2) is also workable.  It probably is slightly better\nthan plus because it does not have to grandfather \"git+ssh\" and \"ssh+git\"\nand that would be beneficial for requiring less complexity in both code\n(i.e. special case logic) and more importantly in mental burden to the end\nusers (i.e. '::' would stand out more than '+' and clearly different from\ntraditional git URLs in all cases).\n\nAs Jeff said (your 1.), a single colon ':' has a rather bad ambiguity\nbetween <vcs> and hostname part in the existing scp-style repository\nnaming.\n"},{"id":"120459","messageId":"7vab24wtzo.fsf@alter.siamese.dyndns.org","threadId":"20507","inReplyTo":"1249985426-13726-3-git-send-email-johan@herland.net","subject":"Re: [RFC/PATCH 2/6] transport_get(): Don't SEGFAULT on missing url","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-12T22:24:43Z","receivedAt":"2009-08-12T22:24:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n\n> Signed-off-by: Johan Herland <johan@herland.net>\n\nHow does url end up to be NULL?  At the beginning of transport_get(), you\ndo this:\n\n\tret->remote = remote;\n\tif (!url && remote && remote->url)\n\t\turl = remote->url[0];\n\tret->url = url;\n\tif (remote && remote->foreign_vcs) {\n\t\ttransport_helper_init(ret);\n\t\treturn ret;\n\t}\n\nand the case where remote.$name.vcs is defined, we do not need\nremote.$name.url.\n\nWhen (!url && remote && remote->url), is remote->url[0] allowed to be\nNULL?  I am guessing it would be a bug in whoever prepared the remote, and\nif that is indeed the case, the patch shifts the symptoms without fixing\nthe cause.\n\nWhen (remote && remote->foreign_vcs) does not hold, iow, if no remote is\ndefined or the remote is defined but lacks remote.$name.url, you will go\nto the last else clause in the function that sets up a git_transport_data\nfor the native transport, but it has ret->url == NULL.  \n\nWhom does that transport talk with?  Is such a transport of any use, or\ndoes it cause a segfault downstream in the call chain?\n\nIn other words, I am wondering if this patch should just diagnose the case\nas an error, instead of pretending all is well.\n"},{"id":"120464","messageId":"200908130139.10876.johan@herland.net","threadId":"20507","inReplyTo":"7vab24wtzo.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 2/6] transport_get(): Don't SEGFAULT on missing url","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-12T23:39:10Z","receivedAt":"2009-08-12T23:39:10Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 13 August 2009, Junio C Hamano wrote:\n> Johan Herland <johan@herland.net> writes:\n> > Signed-off-by: Johan Herland <johan@herland.net>\n>\n> How does url end up to be NULL?  At the beginning of transport_get(), you\n> do this:\n>\n> \tret->remote = remote;\n> \tif (!url && remote && remote->url)\n> \t\turl = remote->url[0];\n> \tret->url = url;\n> \tif (remote && remote->foreign_vcs) {\n> \t\ttransport_helper_init(ret);\n> \t\treturn ret;\n> \t}\n>\n> and the case where remote.$name.vcs is defined, we do not need\n> remote.$name.url.\n>\n> When (!url && remote && remote->url), is remote->url[0] allowed to be\n> NULL?  I am guessing it would be a bug in whoever prepared the remote,\n> and if that is indeed the case, the patch shifts the symptoms without\n> fixing the cause.\n>\n> When (remote && remote->foreign_vcs) does not hold, iow, if no remote is\n> defined or the remote is defined but lacks remote.$name.url, you will go\n> to the last else clause in the function that sets up a git_transport_data\n> for the native transport, but it has ret->url == NULL.\n>\n> Whom does that transport talk with?  Is such a transport of any use, or\n> does it cause a segfault downstream in the call chain?\n>\n> In other words, I am wondering if this patch should just diagnose the\n> case as an error, instead of pretending all is well.\n\nYou are right. Instead of pushing the segfault downstream, we should verify \nthat the url is non-NULL before returning it as part of ret (modulo the \nforeign case, of course).\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"120591","messageId":"200908140000.43185.jnareb@gmail.com","threadId":"20507","inReplyTo":"7vocqkydum.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-08-13T22:00:42Z","receivedAt":"2009-08-13T22:00:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 12 August 2009, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> Jeff King <peff@peff.net> writes:\n>>\n>>>   1. Is there some other syntax that _doesn't_ have this breakage\n>>>      but that similarly helps the \"vast majority of Git users\".\n>>\n>> Well, proposed possible syntax was:\n>> 1. <vcs>:<repository location>\n>> ...\n>> 2. <vcs>::<repository location>\n>> ...\n>> 3. <vcs>+<repository location>\n>>\n>>    e.g.\n>>\n>>      svn+http://svn.example.com/project\n>>\n>>    but\n>>\n>>      http+svn://svn.example.com/project\n>>      svn+path/to/repo\n> \n> I do not think these are valid examples to demonstrate that 3 is bad.\n> \n> We do not have (and we will not create) \"http+svn://\" native transport, so\n> the former can only mean \"Feed 'svn://svn.example.com/project' to the vcs\n> helper whose name is 'http'\".  Similarly I do not see any way to read the\n> latter other than \"Feed 'path/to/repo' to 'svn' vcs helper\".\n\nAnd not \"Use 'svn+path/to/repo' as local filesystem path to repository?\nOn the other hand you can always use here './svn+path/to/repo'\n\n> Double-colon (your 2) is also workable.  It probably is slightly better\n> than plus because it does not have to grandfather \"git+ssh\" and \"ssh+git\"\n> and that would be beneficial for requiring less complexity in both code\n> (i.e. special case logic) and more importantly in mental burden to the end\n> users (i.e. '::' would stand out more than '+' and clearly different from\n> traditional git URLs in all cases).\n> \n> As Jeff said (your 1.), a single colon ':' has a rather bad ambiguity\n> between <vcs> and hostname part in the existing scp-style repository\n> naming.\n\nAlso double colon is better for scp-like repository location, as e.g.\n\n  svn+example.com:path/to/repo\n\nmight be 'path/to/repo' on 'svn+example.com' host; there is no \"escape\"\nmechanism like for './svn+path/to/repo' relative path.  On the other \nhand unescaped ':' cannot be present in hostname, therefore the following\nis unambiguous:\n\n  svn::example.com:path/to/repo\n\nBut on another hand svn+http://svn.example.com/project/trunk/ looks IMVHO\nbetter that svn::http://svn.example.com/project/trunk/\n\n-- \nJakub Narebski\nPoland\n"}]}