{"thread":{"id":"41289","subject":"clones over rsync broken?","startedAt":"2016-01-30T05:11:33Z","lastAt":"2016-01-30T08:04:52Z","messageCount":6,"participants":["Eric Wong","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"277070","messageId":"20160130051133.GA21973@dcvr.yhbt.net","threadId":"41289","inReplyTo":null,"subject":"clones over rsync broken?","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2016-01-30T05:11:33Z","receivedAt":"2016-01-30T05:11:33Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"I have not used rsync remotes in ages, but I was working on the\npatch for -4/-6 support and decided to test it against rsync.kernel.org\n\nCloning git.git takes forever and failed with:\n\n$ git clone rsync://rsync.kernel.org/pub/scm/git/git.git\nChecking connectivity... fatal: bad object ecdc6d8612df80e871ed34bb6c3b01b20b0b82e6\nfatal: remote did not send all necessary objects\n\ngit.git is gigantic and I haven't looked closely, and that may not be\na git bug...\n\nHowever, trying to clone a smaller repo like pahole.git via rsync fails\ndifferently; this looks more like a git bug:\n\n$ git clone rsync://rsync.kernel.org/pub/scm/devel/pahole/pahole.git\nfatal: Multiple updates for ref 'refs/remotes/origin/master' not allowed.\n\nUsing rsync(1) manually to grab pahole.git and inspecting the bare\nrepo with yields no anomalies with \"git fsck --full\".\n$GIT_DIR/info/refs and $GIT_DIR/packed-refs both look fine, but\nperhaps it's confused by the existence of $GIT_DIR/refs/heads/master\nas a loose ref?\n\nI tried dumping the refnames with the following patch and can see\nrefs/remotes/origin/master is the only duplicated ref:\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c648b5e..e0cb0ab 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3112,6 +3112,8 @@ static int ref_update_reject_duplicates(struct string_list *refnames,\n \tint i, n = refnames->nr;\n \n \tassert(err);\n+\tfor (i = 0; i < n; i++)\n+\t\tfprintf(stderr, \"ref: %s\\n\", refnames->items[i].string);\n \n \tfor (i = 1; i < n; i++)\n \t\tif (!strcmp(refnames->items[i - 1].string, refnames->items[i].string)) {\n\nAnd the output is:\n\nref: refs/remotes/origin/master\nref: refs/remotes/origin/master\nref: refs/tags/v1.0\nref: refs/tags/v1.1\nref: refs/tags/v1.10\nref: refs/tags/v1.2\nref: refs/tags/v1.3\nref: refs/tags/v1.4\nref: refs/tags/v1.5\nref: refs/tags/v1.6\nref: refs/tags/v1.7\nref: refs/tags/v1.8\nref: refs/tags/v1.8pre1\nref: refs/tags/v1.9\n\nNot sure what exactly is going on, my git internal API knowledge is not\ngreat.  Anyways, this is without any of my pending patches, and I was\nalso able to reproduce a slightly different error with the Debian wheezy\nversion (1.7.10.4) with:\n\nfatal: Duplicated ref, and SHA1s don't match: refs/remotes/origin/master\n"},{"id":"277072","messageId":"20160130054141.GB1677@sigill.intra.peff.net","threadId":"41289","inReplyTo":"20160130051133.GA21973@dcvr.yhbt.net","subject":"Re: clones over rsync broken?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-30T05:41:41Z","receivedAt":"2016-01-30T05:41:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 30, 2016 at 05:11:33AM +0000, Eric Wong wrote:\n\n> I have not used rsync remotes in ages, but I was working on the\n> patch for -4/-6 support and decided to test it against rsync.kernel.org\n> \n> Cloning git.git takes forever and failed with:\n\nNo kidding. There are over 95,000 unreachable loose objects consuming a\ngigabyte. The rsync transport blindly pulls all of the data over, with\nno idea that it doesn't need most of it.\n\n> $ git clone rsync://rsync.kernel.org/pub/scm/git/git.git\n> Checking connectivity... fatal: bad object ecdc6d8612df80e871ed34bb6c3b01b20b0b82e6\n> fatal: remote did not send all necessary objects\n\nAll those objects, and we still manage to miss one. :)\n\nInterestingly, that object does not seem to exist at all on the remote!\nI think this is the same bug as the one below. Read on...\n\n> However, trying to clone a smaller repo like pahole.git via rsync fails\n> differently; this looks more like a git bug:\n> \n> $ git clone rsync://rsync.kernel.org/pub/scm/devel/pahole/pahole.git\n> fatal: Multiple updates for ref 'refs/remotes/origin/master' not allowed.\n> \n> Using rsync(1) manually to grab pahole.git and inspecting the bare\n> repo with yields no anomalies with \"git fsck --full\".\n> $GIT_DIR/info/refs and $GIT_DIR/packed-refs both look fine, but\n> perhaps it's confused by the existence of $GIT_DIR/refs/heads/master\n> as a loose ref?\n\nYes, that's exactly what's going on. In get_refs_via_rsync, we blindly\nconcatenate the list of loose refs and packed refs. But that's not\nright, and never has been. If the same ref exists in both stores, the\nloose ref takes precedence (that is how we can write new refs without\nhaving to rewrite the whole packed-refs file).\n\nSo we erroneously believe that refs/heads/master exists _twice_ on the\nremote, with two different values (and try to store it twice as\nrefs/remotes/origin/master). But we should be accepting only the loose\nvalue.\n\nThis explains the git.git problem, too. There are two entries for\nrefs/heads/pu: one loose and one in packed-refs. The latter is a stale,\nolder value, and should never be looked at. But because pu gets rewound,\nits older values are not necessarily reachable and may even have been\npruned!\n\nSo no, we do not have ecdc6d86, but neither does the upstream, and\nnothing is referencing it.\n\nIt looks like this has been broken since cd547b4 (fetch/push: readd\nrsync support, 2007-10-01). The fix is just to ignore packed-refs\nentries which duplicate loose ones. But given the length of time this\nhas been broken with nobody complaining, I have to wonder if it is\nsimply time to retire the rsync protocol. Even if was made to work, it\nis a horribly inefficient protocol.\n\n-Peff\n"},{"id":"277073","messageId":"20160130063036.GC1677@sigill.intra.peff.net","threadId":"41289","inReplyTo":"20160130054141.GB1677@sigill.intra.peff.net","subject":"Re: clones over rsync broken?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-30T06:30:36Z","receivedAt":"2016-01-30T06:30:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 30, 2016 at 12:41:41AM -0500, Jeff King wrote:\n\n> It looks like this has been broken since cd547b4 (fetch/push: readd\n> rsync support, 2007-10-01). The fix is just to ignore packed-refs\n> entries which duplicate loose ones. But given the length of time this\n> has been broken with nobody complaining, I have to wonder if it is\n> simply time to retire the rsync protocol. Even if was made to work, it\n> is a horribly inefficient protocol.\n\nI took a look at whether there would be an easy fix. There are three\nobvious ways to go about this:\n\n  1. Use the loose/packed reading code from refs/files-backend.c.\n\n     This would require some refactoring, as we currently assume we are\n     either reading the refs for _this_ repository, or for a submodule.\n     This is sort-of like reading a submodule, but I think there are a\n     few rough edges.\n\n     Worse, though, is that the upcoming pluggable refs work will\n     probably require that submodules and the main repo have the same\n     ref backend. I'm a little dubious of that requirement in general,\n     but certainly it would be a show-stopper here.\n\n  2. Create a \"struct transport\" for the tempdir holding the data we\n     rsynced from the other side, and just treat it like a local repo.\n     We already do something like this to handle object \"alternates\"\n     repositories (and we run \"upload-pack\" on the other directory and\n     parse it just like a real remote).\n\n     Unfortunately, what we bring over in get_refs_via_pack is not\n     enough for this to work. It's _just_ the refs/ directories. We can\n     use \"git init\" to make it more like a real repo, but ultimately we\n     don't have any objects, so upload-pack will complain.\n\n     We could fix that by just rsyncing the objects down at this stage,\n     too. It's not like git is careful enough to do a real \"what do we\n     need\" walk like it does for dumb-http. But we would end up rsyncing\n     even in cases where we didn't need _any_ objects, though that is\n     probably a vast minority case.\n\n  3. Just teach the local ad-hoc loose and packed readers to do the\n     proper deduplication. I started on this, but then realized that we\n     really do implement a from-scratch packed-refs reader here. And\n     it's missing some features, like parsing peeled tags.\n\n     So it really would want to call into the regular packed-refs\n     parsing code, which requires more refactoring as in (1).\n\nOf all of these, I think (2) is the closest to sane, because it lets\nupload-pack do the heavy-lifting, meaning we can understand whatever\nformats we rsync from the other side. But given that rsync is already\nnaive about what objects it pulls (i.e., it gets everything), I have to\nreally question whether there is any value in using git-over-rsync\nversus just:\n\n  rsync $src tmp/\n  git clone tmp my-repo ;# will hard-link, no extra space needed!\n  rm -rf $tmp\n\nI guess that doesn't handle subsequent fetches. But\nreally...git-over-rsync is just an awful protocol. Nobody should be\nusing it. Having looked at it in more detail, I'm more in favor than\never of removing it.\n\n-Peff\n"},{"id":"277074","messageId":"20160130072126.GA14696@sigill.intra.peff.net","threadId":"41289","inReplyTo":"20160130063036.GC1677@sigill.intra.peff.net","subject":"[PATCH] transport: drop support for git-over-rsync","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-30T07:21:26Z","receivedAt":"2016-01-30T07:21:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 30, 2016 at 01:30:36AM -0500, Jeff King wrote:\n\n> I guess that doesn't handle subsequent fetches. But\n> really...git-over-rsync is just an awful protocol. Nobody should be\n> using it. Having looked at it in more detail, I'm more in favor than\n> ever of removing it.\n\nSo here it is. I think this hasn't happened before simply because nobody\nbothered. I tried to dig up relevant discussions; see below.\n\nAlso, it's Friday night and I'm reading (and enjoying!) 10-year-old\ndiscussions on the git mailing list. That's got to be worth some kind of\nnerd cred.\n\n-- >8 --\nSubject: transport: drop support for git-over-rsync\n\nThe git-over-rsync protocol is inefficient and broken, and\nhas been for a long time. It transfers way more objects than\nit needs (grabbing all of the remote's \"objects/\",\nregardless of which objects we need). It does its own ad-hoc\nparsing of loose and packed refs from the remote, but\ndoesn't properly override packed refs with loose ones,\nleading to garbage results (e.g., expecting the other side\nto have an object pointed to by a stale packed-refs entry,\nor complaining that the other side has two copies of the\nrefs[1]).\n\nThis latter breakage means that nobody could have\nsuccessfully pulled from a moderately active repository\nsince cd547b4 (fetch/push: readd rsync support, 2007-10-01).\n\nWe never made an official deprecation notice in the release\nnotes for git's rsync protocol, but the tutorial has marked\nit as such since 914328a (Update tutorial., 2005-08-30).\nAnd on the mailing list as far back as Oct 2005, we can find\nJunio mentioning it as having \"been deprecated for quite\nsome time.\"[2,3,4]. So it was old news then; cogito had\ndeprecated the transport in July of 2005[5] (though it did\ncome back briefly when Linus broke git-http-pull!).\n\nOf course some people professed their love of rsync through\n2006, but Linus clarified in his usual gentle manner[6]:\n\n  > Thanks!  This is why I still use rsync, even though\n  > everybody and their mother tells me \"Linus says rsync is\n  > deprecated.\"\n\n  No. You're using rsync because you're actively doing\n  something _wrong_.\n\nThe deprecation sentiment was reinforced in 2008, with a\nmention that cloning via rsync is broken (with no fix)[7].\n\nEven the commit porting rsync over to C from shell (cd547b4)\nlists it as deprecated! So between the 10 years of informal\nwarnings, and the fact that it has been severely broken\nsince 2007, it's probably safe to simply remove it without\nfurther deprecation warnings.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/285101\n[2] http://article.gmane.org/gmane.comp.version-control.git/10093\n[3] http://article.gmane.org/gmane.comp.version-control.git/17734\n[4] http://article.gmane.org/gmane.comp.version-control.git/18911\n[5] http://article.gmane.org/gmane.comp.version-control.git/5617\n[6] http://article.gmane.org/gmane.comp.version-control.git/19354\n[7] http://article.gmane.org/gmane.comp.version-control.git/103635\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/config.txt           |   2 +-\n Documentation/git-bundle.txt       |   2 +-\n Documentation/git-clone.txt        |   3 +-\n Documentation/git-repack.txt       |   2 +-\n Documentation/git.txt              |   2 -\n Documentation/gitcore-tutorial.txt |  18 +-\n Documentation/gittutorial.txt      |   2 +-\n Documentation/urls.txt             |   6 +-\n t/t5510-fetch.sh                   |  36 ----\n transport.c                        | 332 +------------------------------------\n 10 files changed, 10 insertions(+), 395 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 877cbc8..65fe684 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2088,7 +2088,7 @@ pack.indexVersion::\n \tlarger than 2 GB.\n +\n If you have an old Git that does not understand the version 2 `*.idx` file,\n-cloning or fetching over a non native protocol (e.g. \"http\" and \"rsync\")\n+cloning or fetching over a non native protocol (e.g. \"http\")\n that will copy both `*.pack` file and corresponding `*.idx` file from the\n other side may give you a repository that cannot be accessed with your\n older version of Git. If the `*.pack` file is smaller than 2 GB, however,\ndiff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\nindex 0417562..3a8120c 100644\n--- a/Documentation/git-bundle.txt\n+++ b/Documentation/git-bundle.txt\n@@ -20,7 +20,7 @@ DESCRIPTION\n Some workflows require that one or more branches of development on one\n machine be replicated on another machine, but the two machines cannot\n be directly connected, and therefore the interactive Git protocols (git,\n-ssh, rsync, http) cannot be used.  This command provides support for\n+ssh, http) cannot be used.  This command provides support for\n 'git fetch' and 'git pull' to operate by packaging objects and references\n in an archive at the originating machine, then importing those into\n another repository using 'git fetch' and 'git pull'\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 789b668..b7c467a 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -115,8 +115,7 @@ objects from the source repository into a pack in the cloned repository.\n --quiet::\n -q::\n \tOperate quietly.  Progress is not reported to the standard\n-\terror stream. This flag is also passed to the `rsync'\n-\tcommand when given.\n+\terror stream.\n \n --verbose::\n -v::\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex 0e0bd36..af230d0 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -133,7 +133,7 @@ By default, the command passes `--delta-base-offset` option to\n 'git pack-objects'; this typically results in slightly smaller packs,\n but the generated packs are incompatible with versions of Git older than\n version 1.4.4. If you need to share your repository with such ancient Git\n-versions, either directly or via the dumb http or rsync protocol, then you\n+versions, either directly or via the dumb http protocol, then you\n need to set the configuration variable `repack.UseDeltaBaseOffset` to\n \"false\" and repack. Access from old Git versions over the native protocol\n is unaffected by this option as the conversion is performed on the fly\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex bff6302..6524f4a 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -1123,8 +1123,6 @@ of clones and fetches.\n \t  - `ssh`: git over ssh (including `host:path` syntax,\n \t    `git+ssh://`, etc).\n \n-\t  - `rsync`: git over rsync\n-\n \t  - `http`: git over http, both \"smart http\" and \"dumb http\".\n \t    Note that this does _not_ include `https`; if you want both,\n \t    you should specify both as `http:https`.\ndiff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt\nindex 36e9ab3..15b3bfa 100644\n--- a/Documentation/gitcore-tutorial.txt\n+++ b/Documentation/gitcore-tutorial.txt\n@@ -710,7 +710,7 @@ files).\n Again, this can all be simplified with\n \n ----------------\n-$ git clone rsync://rsync.kernel.org/pub/scm/git/git.git/ my-git\n+$ git clone git://git.kernel.org/pub/scm/git/git.git/ my-git\n $ cd my-git\n $ git checkout\n ----------------\n@@ -1011,20 +1011,6 @@ $ git fetch <remote-repository>\n One of the following transports can be used to name the\n repository to download from:\n \n-Rsync::\n-\t`rsync://remote.machine/path/to/repo.git/`\n-+\n-Rsync transport is usable for both uploading and downloading,\n-but is completely unaware of what git does, and can produce\n-unexpected results when you download from the public repository\n-while the repository owner is uploading into it via `rsync`\n-transport.  Most notably, it could update the files under\n-`refs/` which holds the object name of the topmost commits\n-before uploading the files in `objects/` -- the downloader would\n-obtain head commit object name while that object itself is still\n-not available in the repository.  For this reason, it is\n-considered deprecated.\n-\n SSH::\n \t`remote.machine:/path/to/repo.git/` or\n +\n@@ -1430,7 +1416,7 @@ while, depending on how active your project is.\n \n When a repository is synchronized via `git push` and `git pull`\n objects packed in the source repository are usually stored\n-unpacked in the destination, unless rsync transport is used.\n+unpacked in the destination.\n While this allows you to use different packing strategies on\n both ends, it also means you may need to repack both\n repositories every once in a while.\ndiff --git a/Documentation/gittutorial.txt b/Documentation/gittutorial.txt\nindex b00c67d..b3b58d3 100644\n--- a/Documentation/gittutorial.txt\n+++ b/Documentation/gittutorial.txt\n@@ -451,7 +451,7 @@ perform clones and pulls using the ssh protocol:\n bob$ git clone alice.org:/home/alice/project myrepo\n -------------------------------------\n \n-Alternatively, Git has a native protocol, or can use rsync or http;\n+Alternatively, Git has a native protocol, or can use http;\n see linkgit:git-pull[1] for details.\n \n Git can also be used in a CVS-like mode, with a central repository\ndiff --git a/Documentation/urls.txt b/Documentation/urls.txt\nindex 9ccb246..b05da95 100644\n--- a/Documentation/urls.txt\n+++ b/Documentation/urls.txt\n@@ -7,9 +7,8 @@ Depending on the transport protocol, some of this information may be\n absent.\n \n Git supports ssh, git, http, and https protocols (in addition, ftp,\n-and ftps can be used for fetching and rsync can be used for fetching\n-and pushing, but these are inefficient and deprecated; do not use\n-them).\n+and ftps can be used for fetching, but this is inefficient and\n+deprecated; do not use it).\n \n The native transport (i.e. git:// URL) does no authentication and\n should be used with caution on unsecured networks.\n@@ -20,7 +19,6 @@ The following syntaxes may be used with them:\n - git://host.xz{startsb}:port{endsb}/path/to/repo.git/\n - http{startsb}s{endsb}://host.xz{startsb}:port{endsb}/path/to/repo.git/\n - ftp{startsb}s{endsb}://host.xz{startsb}:port{endsb}/path/to/repo.git/\n-- rsync://host.xz/path/to/repo.git/\n \n An alternative scp-like syntax may also be used with the ssh protocol:\n \ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 9203a65..0c10c85 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -314,42 +314,6 @@ test_expect_success 'bundle should be able to create a full history' '\n \n '\n \n-! rsync --help > /dev/null 2> /dev/null &&\n-say 'Skipping rsync tests because rsync was not found' || {\n-test_expect_success 'fetch via rsync' '\n-\tgit pack-refs &&\n-\tmkdir rsynced &&\n-\t(cd rsynced &&\n-\t git init --bare &&\n-\t git fetch \"rsync:../.git\" master:refs/heads/master &&\n-\t git gc --prune &&\n-\t test $(git rev-parse master) = $(cd .. && git rev-parse master) &&\n-\t git fsck --full)\n-'\n-\n-test_expect_success 'push via rsync' '\n-\tmkdir rsynced2 &&\n-\t(cd rsynced2 &&\n-\t git init) &&\n-\t(cd rsynced &&\n-\t git push \"rsync:../rsynced2/.git\" master) &&\n-\t(cd rsynced2 &&\n-\t git gc --prune &&\n-\t test $(git rev-parse master) = $(cd .. && git rev-parse master) &&\n-\t git fsck --full)\n-'\n-\n-test_expect_success 'push via rsync' '\n-\tmkdir rsynced3 &&\n-\t(cd rsynced3 &&\n-\t git init) &&\n-\tgit push --all \"rsync:rsynced3/.git\" &&\n-\t(cd rsynced3 &&\n-\t test $(git rev-parse master) = $(cd .. && git rev-parse master) &&\n-\t git fsck --full)\n-'\n-}\n-\n test_expect_success 'fetch with a non-applying branch.<name>.merge' '\n \tgit config branch.master.remote yeti &&\n \tgit config branch.master.merge refs/heads/bigfoot &&\ndiff --git a/transport.c b/transport.c\nindex 67f3666..9da4e89 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -17,133 +17,6 @@\n #include \"sha1-array.h\"\n #include \"sigchain.h\"\n \n-/* rsync support */\n-\n-/*\n- * We copy packed-refs and refs/ into a temporary file, then read the\n- * loose refs recursively (sorting whenever possible), and then inserting\n- * those packed refs that are not yet in the list (not validating, but\n- * assuming that the file is sorted).\n- *\n- * Appears refactoring this from refs.c is too cumbersome.\n- */\n-\n-static int str_cmp(const void *a, const void *b)\n-{\n-\tconst char *s1 = a;\n-\tconst char *s2 = b;\n-\n-\treturn strcmp(s1, s2);\n-}\n-\n-/* path->buf + name_offset is expected to point to \"refs/\" */\n-\n-static int read_loose_refs(struct strbuf *path, int name_offset,\n-\t\tstruct ref **tail)\n-{\n-\tDIR *dir = opendir(path->buf);\n-\tstruct dirent *de;\n-\tstruct {\n-\t\tchar **entries;\n-\t\tint nr, alloc;\n-\t} list;\n-\tint i, pathlen;\n-\n-\tif (!dir)\n-\t\treturn -1;\n-\n-\tmemset (&list, 0, sizeof(list));\n-\n-\twhile ((de = readdir(dir))) {\n-\t\tif (is_dot_or_dotdot(de->d_name))\n-\t\t\tcontinue;\n-\t\tALLOC_GROW(list.entries, list.nr + 1, list.alloc);\n-\t\tlist.entries[list.nr++] = xstrdup(de->d_name);\n-\t}\n-\tclosedir(dir);\n-\n-\t/* sort the list */\n-\n-\tqsort(list.entries, list.nr, sizeof(char *), str_cmp);\n-\n-\tpathlen = path->len;\n-\tstrbuf_addch(path, '/');\n-\n-\tfor (i = 0; i < list.nr; i++, strbuf_setlen(path, pathlen + 1)) {\n-\t\tstrbuf_addstr(path, list.entries[i]);\n-\t\tif (read_loose_refs(path, name_offset, tail)) {\n-\t\t\tint fd = open(path->buf, O_RDONLY);\n-\t\t\tchar buffer[40];\n-\t\t\tstruct ref *next;\n-\n-\t\t\tif (fd < 0)\n-\t\t\t\tcontinue;\n-\t\t\tnext = alloc_ref(path->buf + name_offset);\n-\t\t\tif (read_in_full(fd, buffer, 40) != 40 ||\n-\t\t\t\t\tget_oid_hex(buffer, &next->old_oid)) {\n-\t\t\t\tclose(fd);\n-\t\t\t\tfree(next);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tclose(fd);\n-\t\t\t(*tail)->next = next;\n-\t\t\t*tail = next;\n-\t\t}\n-\t}\n-\tstrbuf_setlen(path, pathlen);\n-\n-\tfor (i = 0; i < list.nr; i++)\n-\t\tfree(list.entries[i]);\n-\tfree(list.entries);\n-\n-\treturn 0;\n-}\n-\n-/* insert the packed refs for which no loose refs were found */\n-\n-static void insert_packed_refs(const char *packed_refs, struct ref **list)\n-{\n-\tFILE *f = fopen(packed_refs, \"r\");\n-\tstatic char buffer[PATH_MAX];\n-\n-\tif (!f)\n-\t\treturn;\n-\n-\tfor (;;) {\n-\t\tint cmp = 0; /* assigned before used */\n-\t\tint len;\n-\n-\t\tif (!fgets(buffer, sizeof(buffer), f)) {\n-\t\t\tfclose(f);\n-\t\t\treturn;\n-\t\t}\n-\n-\t\tif (!isxdigit(buffer[0]))\n-\t\t\tcontinue;\n-\t\tlen = strlen(buffer);\n-\t\tif (len && buffer[len - 1] == '\\n')\n-\t\t\tbuffer[--len] = '\\0';\n-\t\tif (len < 41)\n-\t\t\tcontinue;\n-\t\twhile ((*list)->next &&\n-\t\t\t\t(cmp = strcmp(buffer + 41,\n-\t\t\t\t      (*list)->next->name)) > 0)\n-\t\t\tlist = &(*list)->next;\n-\t\tif (!(*list)->next || cmp < 0) {\n-\t\t\tstruct ref *next = alloc_ref(buffer + 41);\n-\t\t\tbuffer[40] = '\\0';\n-\t\t\tif (get_oid_hex(buffer, &next->old_oid)) {\n-\t\t\t\twarning (\"invalid SHA-1: %s\", buffer);\n-\t\t\t\tfree(next);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tnext->next = (*list)->next;\n-\t\t\t(*list)->next = next;\n-\t\t\tlist = &(*list)->next;\n-\t\t}\n-\t}\n-}\n-\n static void set_upstreams(struct transport *transport, struct ref *refs,\n \tint pretend)\n {\n@@ -192,205 +65,6 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \t}\n }\n \n-static const char *rsync_url(const char *url)\n-{\n-\tif (!starts_with(url, \"rsync://\"))\n-\t\tskip_prefix(url, \"rsync:\", &url);\n-\treturn url;\n-}\n-\n-static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)\n-{\n-\tstruct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;\n-\tstruct ref dummy = {NULL}, *tail = &dummy;\n-\tstruct child_process rsync = CHILD_PROCESS_INIT;\n-\tconst char *args[5];\n-\tint temp_dir_len;\n-\n-\tif (for_push)\n-\t\treturn NULL;\n-\n-\t/* copy the refs to the temporary directory */\n-\n-\tstrbuf_addstr(&temp_dir, git_path(\"rsync-refs-XXXXXX\"));\n-\tif (!mkdtemp(temp_dir.buf))\n-\t\tdie_errno (\"Could not make temporary directory\");\n-\ttemp_dir_len = temp_dir.len;\n-\n-\tstrbuf_addstr(&buf, rsync_url(transport->url));\n-\tstrbuf_addstr(&buf, \"/refs\");\n-\n-\trsync.argv = args;\n-\trsync.stdout_to_stderr = 1;\n-\targs[0] = \"rsync\";\n-\targs[1] = (transport->verbose > 1) ? \"-rv\" : \"-r\";\n-\targs[2] = buf.buf;\n-\targs[3] = temp_dir.buf;\n-\targs[4] = NULL;\n-\n-\tif (run_command(&rsync))\n-\t\tdie (\"Could not run rsync to get refs\");\n-\n-\tstrbuf_reset(&buf);\n-\tstrbuf_addstr(&buf, rsync_url(transport->url));\n-\tstrbuf_addstr(&buf, \"/packed-refs\");\n-\n-\targs[2] = buf.buf;\n-\n-\tif (run_command(&rsync))\n-\t\tdie (\"Could not run rsync to get refs\");\n-\n-\t/* read the copied refs */\n-\n-\tstrbuf_addstr(&temp_dir, \"/refs\");\n-\tread_loose_refs(&temp_dir, temp_dir_len + 1, &tail);\n-\tstrbuf_setlen(&temp_dir, temp_dir_len);\n-\n-\ttail = &dummy;\n-\tstrbuf_addstr(&temp_dir, \"/packed-refs\");\n-\tinsert_packed_refs(temp_dir.buf, &tail);\n-\tstrbuf_setlen(&temp_dir, temp_dir_len);\n-\n-\tif (remove_dir_recursively(&temp_dir, 0))\n-\t\twarning (\"Error removing temporary directory %s.\",\n-\t\t\t\ttemp_dir.buf);\n-\n-\tstrbuf_release(&buf);\n-\tstrbuf_release(&temp_dir);\n-\n-\treturn dummy.next;\n-}\n-\n-static int fetch_objs_via_rsync(struct transport *transport,\n-\t\t\t\tint nr_objs, struct ref **to_fetch)\n-{\n-\tstruct child_process rsync = CHILD_PROCESS_INIT;\n-\n-\trsync.stdout_to_stderr = 1;\n-\targv_array_push(&rsync.args, \"rsync\");\n-\targv_array_push(&rsync.args, (transport->verbose > 1) ? \"-rv\" : \"-r\");\n-\targv_array_push(&rsync.args, \"--ignore-existing\");\n-\targv_array_push(&rsync.args, \"--exclude\");\n-\targv_array_push(&rsync.args, \"info\");\n-\targv_array_pushf(&rsync.args, \"%s/objects/\", rsync_url(transport->url));\n-\targv_array_push(&rsync.args, get_object_directory());\n-\n-\t/* NEEDSWORK: handle one level of alternates */\n-\treturn run_command(&rsync);\n-}\n-\n-static int write_one_ref(const char *name, const struct object_id *oid,\n-\t\t\t int flags, void *data)\n-{\n-\tstruct strbuf *buf = data;\n-\tint len = buf->len;\n-\n-\t/* when called via for_each_ref(), flags is non-zero */\n-\tif (flags && !starts_with(name, \"refs/heads/\") &&\n-\t\t\t!starts_with(name, \"refs/tags/\"))\n-\t\treturn 0;\n-\n-\tstrbuf_addstr(buf, name);\n-\tif (safe_create_leading_directories(buf->buf) ||\n-\t    write_file_gently(buf->buf, \"%s\", oid_to_hex(oid)))\n-\t\treturn error(\"problems writing temporary file %s: %s\",\n-\t\t\t     buf->buf, strerror(errno));\n-\tstrbuf_setlen(buf, len);\n-\treturn 0;\n-}\n-\n-static int write_refs_to_temp_dir(struct strbuf *temp_dir,\n-\t\t\t\t  int refspec_nr, const char **refspec)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < refspec_nr; i++) {\n-\t\tstruct object_id oid;\n-\t\tchar *ref;\n-\n-\t\tif (dwim_ref(refspec[i], strlen(refspec[i]), oid.hash, &ref) != 1)\n-\t\t\treturn error(\"Could not get ref %s\", refspec[i]);\n-\n-\t\tif (write_one_ref(ref, &oid, 0, temp_dir)) {\n-\t\t\tfree(ref);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tfree(ref);\n-\t}\n-\treturn 0;\n-}\n-\n-static int rsync_transport_push(struct transport *transport,\n-\t\tint refspec_nr, const char **refspec, int flags)\n-{\n-\tstruct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;\n-\tint result = 0, i;\n-\tstruct child_process rsync = CHILD_PROCESS_INIT;\n-\tconst char *args[10];\n-\n-\tif (flags & TRANSPORT_PUSH_MIRROR)\n-\t\treturn error(\"rsync transport does not support mirror mode\");\n-\n-\t/* first push the objects */\n-\n-\tstrbuf_addstr(&buf, rsync_url(transport->url));\n-\tstrbuf_addch(&buf, '/');\n-\n-\trsync.argv = args;\n-\trsync.stdout_to_stderr = 1;\n-\ti = 0;\n-\targs[i++] = \"rsync\";\n-\targs[i++] = \"-a\";\n-\tif (flags & TRANSPORT_PUSH_DRY_RUN)\n-\t\targs[i++] = \"--dry-run\";\n-\tif (transport->verbose > 1)\n-\t\targs[i++] = \"-v\";\n-\targs[i++] = \"--ignore-existing\";\n-\targs[i++] = \"--exclude\";\n-\targs[i++] = \"info\";\n-\targs[i++] = get_object_directory();\n-\targs[i++] = buf.buf;\n-\targs[i++] = NULL;\n-\n-\tif (run_command(&rsync))\n-\t\treturn error(\"Could not push objects to %s\",\n-\t\t\t\trsync_url(transport->url));\n-\n-\t/* copy the refs to the temporary directory; they could be packed. */\n-\n-\tstrbuf_addstr(&temp_dir, git_path(\"rsync-refs-XXXXXX\"));\n-\tif (!mkdtemp(temp_dir.buf))\n-\t\tdie_errno (\"Could not make temporary directory\");\n-\tstrbuf_addch(&temp_dir, '/');\n-\n-\tif (flags & TRANSPORT_PUSH_ALL) {\n-\t\tif (for_each_ref(write_one_ref, &temp_dir))\n-\t\t\treturn -1;\n-\t} else if (write_refs_to_temp_dir(&temp_dir, refspec_nr, refspec))\n-\t\treturn -1;\n-\n-\ti = 2;\n-\tif (flags & TRANSPORT_PUSH_DRY_RUN)\n-\t\targs[i++] = \"--dry-run\";\n-\tif (!(flags & TRANSPORT_PUSH_FORCE))\n-\t\targs[i++] = \"--ignore-existing\";\n-\targs[i++] = temp_dir.buf;\n-\targs[i++] = rsync_url(transport->url);\n-\targs[i++] = NULL;\n-\tif (run_command(&rsync))\n-\t\tresult = error(\"Could not push to %s\",\n-\t\t\t\trsync_url(transport->url));\n-\n-\tif (remove_dir_recursively(&temp_dir, 0))\n-\t\twarning (\"Could not remove temporary directory %s.\",\n-\t\t\t\ttemp_dir.buf);\n-\n-\tstrbuf_release(&buf);\n-\tstrbuf_release(&temp_dir);\n-\n-\treturn result;\n-}\n-\n struct bundle_transport_data {\n \tint fd;\n \tstruct bundle_header header;\n@@ -984,11 +658,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \tif (helper) {\n \t\ttransport_helper_init(ret, helper);\n \t} else if (starts_with(url, \"rsync:\")) {\n-\t\ttransport_check_allowed(\"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-\t\tret->smart_options = NULL;\n+\t\tdie(\"git-over-rsync is no longer supported\");\n \t} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\ttransport_check_allowed(\"file\");\n-- \n2.7.0.485.g87653c0\n"},{"id":"277075","messageId":"20160130072813.GB14696@sigill.intra.peff.net","threadId":"41289","inReplyTo":"20160130072126.GA14696@sigill.intra.peff.net","subject":"Re: [PATCH] transport: drop support for git-over-rsync","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-30T07:28:13Z","receivedAt":"2016-01-30T07:28:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 30, 2016 at 02:21:26AM -0500, Jeff King wrote:\n\n> Even the commit porting rsync over to C from shell (cd547b4)\n> lists it as deprecated! So between the 10 years of informal\n> warnings, and the fact that it has been severely broken\n> since 2007, it's probably safe to simply remove it without\n> further deprecation warnings.\n\nObviously it would not be the end of the world to start with a\nwarning(\"git-over-rsync is going away!\") patch, or to mention it in the\nrelease notes and hold this patch back for a version or two. But after\nseeing the extent of the breakage, I can't believe anybody has used it\nfor years. But I'm also open to erring on the conservative side.\n\n> ---\n>  Documentation/config.txt           |   2 +-\n>  Documentation/git-bundle.txt       |   2 +-\n>  Documentation/git-clone.txt        |   3 +-\n>  Documentation/git-repack.txt       |   2 +-\n>  Documentation/git.txt              |   2 -\n>  Documentation/gitcore-tutorial.txt |  18 +-\n>  Documentation/gittutorial.txt      |   2 +-\n>  Documentation/urls.txt             |   6 +-\n>  t/t5510-fetch.sh                   |  36 ----\n>  transport.c                        | 332 +------------------------------------\n>  10 files changed, 10 insertions(+), 395 deletions(-)\n\nI cleaned up all of the documentation references I could find, except\none: the git-svn manual notes that because SVN metadata is kept outside\nof refs, you should use rsync for cloning. I'm not sure what to\nrecommend there. I don't eve nthink that \"git clone rsync://\" would copy\nthat metadata. So perhaps it just meant \"rsync the whole thing yourself\"\n(in which case it is OK to leave it).\n\n> @@ -984,11 +658,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n>  \tif (helper) {\n>  \t\ttransport_helper_init(ret, helper);\n>  \t} else if (starts_with(url, \"rsync:\")) {\n> -\t\ttransport_check_allowed(\"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> -\t\tret->smart_options = NULL;\n> +\t\tdie(\"git-over-rsync is no longer supported\");\n\nI added this as a convenience to anybody who does try to use it\n(otherwise they get a more confusing \"eh, what is rsync\" message).\n\nBut if we drop this \"if\" entirely, then somebody can ship\ngit-remote-rsync, if they really wanted to (you can use it either way as\n\"rsync::whatever\" but this blocks the fallback of \"rsync:whatever\").\n\n-Peff\n"},{"id":"277076","messageId":"20160130080452.GA2851@dcvr.yhbt.net","threadId":"41289","inReplyTo":"20160130072813.GB14696@sigill.intra.peff.net","subject":"Re: [PATCH] transport: drop support for git-over-rsync","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2016-01-30T08:04:52Z","receivedAt":"2016-01-30T08:04:52Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> On Sat, Jan 30, 2016 at 02:21:26AM -0500, Jeff King wrote:\n> > Even the commit porting rsync over to C from shell (cd547b4)\n> > lists it as deprecated! So between the 10 years of informal\n> > warnings, and the fact that it has been severely broken\n> > since 2007, it's probably safe to simply remove it without\n> > further deprecation warnings.\n\nI suppose there's a chance some projects are so small they never\ntrigger ref packing; especially if some people are serving .git\nout of a working directory... *shrug*\n\nBut yeah, plain rsync remains usable, of course.\n\n> I cleaned up all of the documentation references I could find, except\n> one: the git-svn manual notes that because SVN metadata is kept outside\n> of refs, you should use rsync for cloning. I'm not sure what to\n> recommend there. I don't eve nthink that \"git clone rsync://\" would copy\n> that metadata. So perhaps it just meant \"rsync the whole thing yourself\"\n> (in which case it is OK to leave it).\n\nYes, probably \"rsync the whole thing yourself\".  And I should\nupdate that since \"clone --mirror\" supports remotes, nowadays.\ngit-svn metadata is rebuildable anyways (unless somebody made\nthe mistake of disabling it).\n"}]}