"fatal error in commit_refs" from pushing to github

"fatal error in commit_refs" from pushing to github

[Duy Nguyen]

I got the message in the subject when pushing to github today. Yes I
know it's github, not git. But according to stackoveflow [1] it's a
local problem. Which makes me think, if we know exactly what this is
(or at least roughly the problem area), maybe we could improve git to
catch it locally in the first place (and because other git servers may
not have the same protection as github).  Jeff maybe you can reveal
something about this "fatal error in commit_refs"? I'm sure it's not
in git code. But I would understand if the answer is "no".

$ git push origin +ZZZ
Counting objects: 95, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (39/39), done.
Writing objects: 100% (95/95), 15.78 KiB | 0 bytes/s, done.
Total 95 (delta 80), reused 69 (delta 56)
remote: Resolving deltas: 100% (80/80), completed with 49 local objects.
remote: fatal error in commit_refs
To github.com:XXX/YYY.git
 ! [remote rejected] ZZZ -> ZZZ (failure)
error: failed to push some refs to 'git@github.com:XXX/YYY.git'


[1] https://stackoverflow.com/questions/37341960/how-do-i-fix-remote-fatal-error-in-commit-refs-errors-trying-to-push-with-git
-- 
Duy

Re: "fatal error in commit_refs" from pushing to github

[Jeff King]

On Thu, Sep 08, 2016 at 07:49:12AM +0700, Duy Nguyen wrote:

> I got the message in the subject when pushing to github today. Yes I
> know it's github, not git. But according to stackoveflow [1] it's a
> local problem. Which makes me think, if we know exactly what this is
> (or at least roughly the problem area), maybe we could improve git to
> catch it locally in the first place (and because other git servers may
> not have the same protection as github).  Jeff maybe you can reveal
> something about this "fatal error in commit_refs"? I'm sure it's not
> in git code. But I would understand if the answer is "no".

The short answer is that it's nothing to do with Git or the client; it's
GitHub-specific code running on the server that is outside of Git
entirely.

The long answer is that pushes to GitHub don't hit Git directly these
days. They hit a proxy layer that speaks just enough of the Git protocol
to relay to N separate receives spread across N replica servers[1]. Those
receive-packs take in the pack and verify it, but don't actually update
any refs[2]. Then the proxy layer runs its own set of policy hooks, and
speaks a commit-protocol to each of the replicas so that they all agree
on the new ref state. That last step is called "commit_refs" internally.

So this is really an internal failure at the ref-update stage. There
_should_ be a reasonable error message, but I think "fatal error in
commit_refs" is the generic last-ditch fallback. I'll pass this along to
people in charge of that code, as we should be generating a more useful
error message.

-Peff

[1] I glossed over a lot of details there. If you're interested:

    http://githubengineering.com/introducing-dgit/

    http://githubengineering.com/building-resilience-in-spokes/

[2] Initially the proxy just fed a set of temporary refs to
    receive-pack, and it was completely stock. These days we do have
    a trivial patch that skips the ref write. I haven't sent it upstream
    because it's useless by itself (but it's below for reference).

    I'm happy to polish it if somebody actually has a use for it.

---
 Documentation/config.txt        |  8 ++++++++
 builtin/receive-pack.c          | 13 +++++++++++--
 t/t9944-receive-pack-nowrite.sh | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100755 t/t9944-receive-pack-nowrite.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f8e6484..38cc1ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2406,6 +2406,14 @@ receive.refUpdateNameLimit::
 	is a hard limit of 65520 bytes due to git's protocol, so this
 	value must be smaller than that.
 
+receive.writeRefs::
+	If set to `false`, `receive-pack` will perform all of the usual
+	ref consistency checks (checking for non-ff, etc), but _not_
+	actually write any ref changes to disk (nor even check that such
+	writes would succeed, as doing so atomically would require
+	taking individual ref locks to be of any value). The default is
+	`true`.
+
 remote.pushDefault::
 	The remote to push to by default.  Overrides
 	`branch.<name>.remote` for all branches, and is overridden by
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 94704e7..4a87365 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -87,6 +87,8 @@ static long nonce_stamp_slop;
 static unsigned long nonce_stamp_slop_limit;
 static struct ref_transaction *transaction;
 
+static int write_refs = 1;
+
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
 	if (value) {
@@ -244,6 +246,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (strcmp(var, "receive.writerefs") == 0) {
+		write_refs = git_config_bool(var, value);
+		return 0;
+	}
+
 	return git_default_config(var, value, cb);
 }
 
@@ -1060,7 +1067,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 				cmd->did_not_exist = 1;
 			}
 		}
-		if (ref_transaction_delete(transaction,
+		if (write_refs &&
+		    ref_transaction_delete(transaction,
 					   namespaced_name,
 					   old_sha1,
 					   flags, "push", &err)) {
@@ -1077,7 +1085,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		    update_shallow_ref(cmd, si))
 			return "shallow error";
 
-		if (ref_transaction_update(transaction,
+		if (write_refs &&
+		    ref_transaction_update(transaction,
 					   namespaced_name,
 					   new_sha1, old_sha1,
 					   flags, "push",
diff --git a/t/t9944-receive-pack-nowrite.sh b/t/t9944-receive-pack-nowrite.sh
new file mode 100755
index 0000000..7b27bc1
--- /dev/null
+++ b/t/t9944-receive-pack-nowrite.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='test no-write tweak to receive-pack'
+. ./test-lib.sh
+
+test_expect_success 'create a few commits' '
+	test_commit one &&
+	git update-ref refs/heads/a HEAD &&
+	test_commit two &&
+	git update-ref refs/heads/b HEAD &&
+	test_commit three &&
+	git update-ref refs/heads/c HEAD
+'
+
+# push just two; hold back "c" so we can push a creation later
+test_expect_success 'create destination repo' '
+	git init --bare dst.git &&
+	git for-each-ref refs/heads/a refs/heads/b >expect &&
+	git push dst.git a b &&
+	git -C dst.git for-each-ref >actual &&
+	test_cmp expect actual
+'
+
+# push an update, a deletion, and a creation
+test_expect_success 'push with no-write config set' '
+	git push --receive-pack="git -c \
+			receive.writeRefs=false \
+			receive-pack" \
+		dst.git b:a :b c:c
+'
+
+test_expect_success 'push did not touch real refs' '
+	git -C dst.git for-each-ref >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
2.10.0.rc2.154.gb4a4b8b

Re: "fatal error in commit_refs" from pushing to github

[Duy Nguyen]

On Thu, Sep 8, 2016 at 8:25 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Sep 08, 2016 at 07:49:12AM +0700, Duy Nguyen wrote:
>
>> I got the message in the subject when pushing to github today. Yes I
>> know it's github, not git. But according to stackoveflow [1] it's a
>> local problem. Which makes me think, if we know exactly what this is
>> (or at least roughly the problem area), maybe we could improve git to
>> catch it locally in the first place (and because other git servers may
>> not have the same protection as github).  Jeff maybe you can reveal
>> something about this "fatal error in commit_refs"? I'm sure it's not
>> in git code. But I would understand if the answer is "no".
>
> The short answer is that it's nothing to do with Git or the client; it's
> GitHub-specific code running on the server that is outside of Git
> entirely.
>
> The long answer is that pushes to GitHub don't hit Git directly these
> days. They hit a proxy layer that speaks just enough of the Git protocol
> to relay to N separate receives spread across N replica servers[1]. Those
> receive-packs take in the pack and verify it, but don't actually update
> any refs[2]. Then the proxy layer runs its own set of policy hooks, and
> speaks a commit-protocol to each of the replicas so that they all agree
> on the new ref state. That last step is called "commit_refs" internally.
>
> So this is really an internal failure at the ref-update stage. There
> _should_ be a reasonable error message, but I think "fatal error in
> commit_refs" is the generic last-ditch fallback. I'll pass this along to
> people in charge of that code, as we should be generating a more useful
> error message.

Hmm.. I'm interested in this because the "fix" is from client side. I
did "git gc" and "git fetch" and the problem was gone. From this
description, I suppose C Git sends a good pack (phew!), but probably
with some stale ref or something that upsets this this last stage.
It's hard to make a connection back to either gc or fetch. Maybe gc
does ref trimming or something (that should probably be done by
git-push as well). Oh well.. maybe next time I see it, I'll get a nice
and clear message :)
-- 
Duy

Re: "fatal error in commit_refs" from pushing to github

[Jeff King]

On Thu, Sep 08, 2016 at 06:03:33PM +0700, Duy Nguyen wrote:

> > So this is really an internal failure at the ref-update stage. There
> > _should_ be a reasonable error message, but I think "fatal error in
> > commit_refs" is the generic last-ditch fallback. I'll pass this along to
> > people in charge of that code, as we should be generating a more useful
> > error message.
> 
> Hmm.. I'm interested in this because the "fix" is from client side. I
> did "git gc" and "git fetch" and the problem was gone.

It may also have been a transient error inside GitHub that resolved
itself between your two pushes.

-Peff