{"thread":{"id":"44034","subject":"\"fatal error in commit_refs\" from pushing to github","startedAt":"2016-09-08T00:49:51Z","lastAt":"2016-09-08T18:40:22Z","messageCount":4,"participants":["Duy Nguyen","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"301350","messageId":"CACsJy8BF_mnSUcEeH=RBMyfh8RrQGJ4dwm_svsC4TciJtECn_w@mail.gmail.com","threadId":"44034","inReplyTo":null,"subject":"\"fatal error in commit_refs\" from pushing to github","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T00:49:12Z","receivedAt":"2016-09-08T00:49:51Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I got the message in the subject when pushing to github today. Yes I\nknow it's github, not git. But according to stackoveflow [1] it's a\nlocal problem. Which makes me think, if we know exactly what this is\n(or at least roughly the problem area), maybe we could improve git to\ncatch it locally in the first place (and because other git servers may\nnot have the same protection as github).  Jeff maybe you can reveal\nsomething about this \"fatal error in commit_refs\"? I'm sure it's not\nin git code. But I would understand if the answer is \"no\".\n\n$ git push origin +ZZZ\nCounting objects: 95, done.\nDelta compression using up to 4 threads.\nCompressing objects: 100% (39/39), done.\nWriting objects: 100% (95/95), 15.78 KiB | 0 bytes/s, done.\nTotal 95 (delta 80), reused 69 (delta 56)\nremote: Resolving deltas: 100% (80/80), completed with 49 local objects.\nremote: fatal error in commit_refs\nTo github.com:XXX/YYY.git\n ! [remote rejected] ZZZ -> ZZZ (failure)\nerror: failed to push some refs to 'git@github.com:XXX/YYY.git'\n\n\n[1] https://stackoverflow.com/questions/37341960/how-do-i-fix-remote-fatal-error-in-commit-refs-errors-trying-to-push-with-git\n-- \nDuy\n"},{"id":"301353","messageId":"20160908012553.q2aubze4qggfzjxz@sigill.intra.peff.net","threadId":"44034","inReplyTo":"CACsJy8BF_mnSUcEeH=RBMyfh8RrQGJ4dwm_svsC4TciJtECn_w@mail.gmail.com","subject":"Re: \"fatal error in commit_refs\" from pushing to github","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-08T01:25:54Z","receivedAt":"2016-09-08T01:26:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2016 at 07:49:12AM +0700, Duy Nguyen wrote:\n\n> I got the message in the subject when pushing to github today. Yes I\n> know it's github, not git. But according to stackoveflow [1] it's a\n> local problem. Which makes me think, if we know exactly what this is\n> (or at least roughly the problem area), maybe we could improve git to\n> catch it locally in the first place (and because other git servers may\n> not have the same protection as github).  Jeff maybe you can reveal\n> something about this \"fatal error in commit_refs\"? I'm sure it's not\n> in git code. But I would understand if the answer is \"no\".\n\nThe short answer is that it's nothing to do with Git or the client; it's\nGitHub-specific code running on the server that is outside of Git\nentirely.\n\nThe long answer is that pushes to GitHub don't hit Git directly these\ndays. They hit a proxy layer that speaks just enough of the Git protocol\nto relay to N separate receives spread across N replica servers[1]. Those\nreceive-packs take in the pack and verify it, but don't actually update\nany refs[2]. Then the proxy layer runs its own set of policy hooks, and\nspeaks a commit-protocol to each of the replicas so that they all agree\non the new ref state. That last step is called \"commit_refs\" internally.\n\nSo this is really an internal failure at the ref-update stage. There\n_should_ be a reasonable error message, but I think \"fatal error in\ncommit_refs\" is the generic last-ditch fallback. I'll pass this along to\npeople in charge of that code, as we should be generating a more useful\nerror message.\n\n-Peff\n\n[1] I glossed over a lot of details there. If you're interested:\n\n    http://githubengineering.com/introducing-dgit/\n\n    http://githubengineering.com/building-resilience-in-spokes/\n\n[2] Initially the proxy just fed a set of temporary refs to\n    receive-pack, and it was completely stock. These days we do have\n    a trivial patch that skips the ref write. I haven't sent it upstream\n    because it's useless by itself (but it's below for reference).\n\n    I'm happy to polish it if somebody actually has a use for it.\n\n---\n Documentation/config.txt        |  8 ++++++++\n builtin/receive-pack.c          | 13 +++++++++++--\n t/t9944-receive-pack-nowrite.sh | 37 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 56 insertions(+), 2 deletions(-)\n create mode 100755 t/t9944-receive-pack-nowrite.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f8e6484..38cc1ac 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2406,6 +2406,14 @@ receive.refUpdateNameLimit::\n \tis a hard limit of 65520 bytes due to git's protocol, so this\n \tvalue must be smaller than that.\n \n+receive.writeRefs::\n+\tIf set to `false`, `receive-pack` will perform all of the usual\n+\tref consistency checks (checking for non-ff, etc), but _not_\n+\tactually write any ref changes to disk (nor even check that such\n+\twrites would succeed, as doing so atomically would require\n+\ttaking individual ref locks to be of any value). The default is\n+\t`true`.\n+\n remote.pushDefault::\n \tThe remote to push to by default.  Overrides\n \t`branch.<name>.remote` for all branches, and is overridden by\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 94704e7..4a87365 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -87,6 +87,8 @@ static long nonce_stamp_slop;\n static unsigned long nonce_stamp_slop_limit;\n static struct ref_transaction *transaction;\n \n+static int write_refs = 1;\n+\n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n \tif (value) {\n@@ -244,6 +246,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.writerefs\") == 0) {\n+\t\twrite_refs = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -1060,7 +1067,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\tcmd->did_not_exist = 1;\n \t\t\t}\n \t\t}\n-\t\tif (ref_transaction_delete(transaction,\n+\t\tif (write_refs &&\n+\t\t    ref_transaction_delete(transaction,\n \t\t\t\t\t   namespaced_name,\n \t\t\t\t\t   old_sha1,\n \t\t\t\t\t   flags, \"push\", &err)) {\n@@ -1077,7 +1085,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t    update_shallow_ref(cmd, si))\n \t\t\treturn \"shallow error\";\n \n-\t\tif (ref_transaction_update(transaction,\n+\t\tif (write_refs &&\n+\t\t    ref_transaction_update(transaction,\n \t\t\t\t\t   namespaced_name,\n \t\t\t\t\t   new_sha1, old_sha1,\n \t\t\t\t\t   flags, \"push\",\ndiff --git a/t/t9944-receive-pack-nowrite.sh b/t/t9944-receive-pack-nowrite.sh\nnew file mode 100755\nindex 0000000..7b27bc1\n--- /dev/null\n+++ b/t/t9944-receive-pack-nowrite.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='test no-write tweak to receive-pack'\n+. ./test-lib.sh\n+\n+test_expect_success 'create a few commits' '\n+\ttest_commit one &&\n+\tgit update-ref refs/heads/a HEAD &&\n+\ttest_commit two &&\n+\tgit update-ref refs/heads/b HEAD &&\n+\ttest_commit three &&\n+\tgit update-ref refs/heads/c HEAD\n+'\n+\n+# push just two; hold back \"c\" so we can push a creation later\n+test_expect_success 'create destination repo' '\n+\tgit init --bare dst.git &&\n+\tgit for-each-ref refs/heads/a refs/heads/b >expect &&\n+\tgit push dst.git a b &&\n+\tgit -C dst.git for-each-ref >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+# push an update, a deletion, and a creation\n+test_expect_success 'push with no-write config set' '\n+\tgit push --receive-pack=\"git -c \\\n+\t\t\treceive.writeRefs=false \\\n+\t\t\treceive-pack\" \\\n+\t\tdst.git b:a :b c:c\n+'\n+\n+test_expect_success 'push did not touch real refs' '\n+\tgit -C dst.git for-each-ref >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.10.0.rc2.154.gb4a4b8b\n\n"},{"id":"301398","messageId":"CACsJy8DW1-EsfgGkbnD3LgiygAu2JV9vs_8e+oQbenXERUA6hw@mail.gmail.com","threadId":"44034","inReplyTo":"20160908012553.q2aubze4qggfzjxz@sigill.intra.peff.net","subject":"Re: \"fatal error in commit_refs\" from pushing to github","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T11:03:33Z","receivedAt":"2016-09-08T11:04:21Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 8, 2016 at 8:25 AM, Jeff King <peff@peff.net> wrote:\n> On Thu, Sep 08, 2016 at 07:49:12AM +0700, Duy Nguyen wrote:\n>\n>> I got the message in the subject when pushing to github today. Yes I\n>> know it's github, not git. But according to stackoveflow [1] it's a\n>> local problem. Which makes me think, if we know exactly what this is\n>> (or at least roughly the problem area), maybe we could improve git to\n>> catch it locally in the first place (and because other git servers may\n>> not have the same protection as github).  Jeff maybe you can reveal\n>> something about this \"fatal error in commit_refs\"? I'm sure it's not\n>> in git code. But I would understand if the answer is \"no\".\n>\n> The short answer is that it's nothing to do with Git or the client; it's\n> GitHub-specific code running on the server that is outside of Git\n> entirely.\n>\n> The long answer is that pushes to GitHub don't hit Git directly these\n> days. They hit a proxy layer that speaks just enough of the Git protocol\n> to relay to N separate receives spread across N replica servers[1]. Those\n> receive-packs take in the pack and verify it, but don't actually update\n> any refs[2]. Then the proxy layer runs its own set of policy hooks, and\n> speaks a commit-protocol to each of the replicas so that they all agree\n> on the new ref state. That last step is called \"commit_refs\" internally.\n>\n> So this is really an internal failure at the ref-update stage. There\n> _should_ be a reasonable error message, but I think \"fatal error in\n> commit_refs\" is the generic last-ditch fallback. I'll pass this along to\n> people in charge of that code, as we should be generating a more useful\n> error message.\n\nHmm.. I'm interested in this because the \"fix\" is from client side. I\ndid \"git gc\" and \"git fetch\" and the problem was gone. From this\ndescription, I suppose C Git sends a good pack (phew!), but probably\nwith some stale ref or something that upsets this this last stage.\nIt's hard to make a connection back to either gc or fetch. Maybe gc\ndoes ref trimming or something (that should probably be done by\ngit-push as well). Oh well.. maybe next time I see it, I'll get a nice\nand clear message :)\n-- \nDuy\n"},{"id":"301429","messageId":"20160908184014.l4g6if2gke6i3yjj@sigill.intra.peff.net","threadId":"44034","inReplyTo":"CACsJy8DW1-EsfgGkbnD3LgiygAu2JV9vs_8e+oQbenXERUA6hw@mail.gmail.com","subject":"Re: \"fatal error in commit_refs\" from pushing to github","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-08T18:40:14Z","receivedAt":"2016-09-08T18:40:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2016 at 06:03:33PM +0700, Duy Nguyen wrote:\n\n> > So this is really an internal failure at the ref-update stage. There\n> > _should_ be a reasonable error message, but I think \"fatal error in\n> > commit_refs\" is the generic last-ditch fallback. I'll pass this along to\n> > people in charge of that code, as we should be generating a more useful\n> > error message.\n> \n> Hmm.. I'm interested in this because the \"fix\" is from client side. I\n> did \"git gc\" and \"git fetch\" and the problem was gone.\n\nIt may also have been a transient error inside GitHub that resolved\nitself between your two pushes.\n\n-Peff\n"}]}