Maniphest T11940

Commit hooks in hosted Git repositories fail under Git 2.11.0
Closed, ResolvedPublic
Actions

Assigned To

None

Authored By

	Rado
	Dec 2 2016, 12:12 PM

Description

After last phabricator's updates all users recive that error after pushing commits:

$ git push origin dev
Counting objects: 9, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 835 bytes | 0 bytes/s, done.
Total 9 (delta 6), reused 0 (delta 0)
remote: [2016-12-02 15:05:12] EXCEPTION: (CommandException) Command failed with error #128!
remote: COMMAND
remote: git log --format='%H' '603b6909851a07fd8156546c91d490334061acde' --not --all
remote: 
remote: STDOUT
remote: (empty)
remote: 
remote: STDERR
remote: fatal: bad object 603b6909851a07fd8156546c91d490334061acde
remote:  at [<phutil>/src/future/exec/ExecFuture.php:369]
remote: arcanist(head=master, ref.master=fad85844314b), phabricator(head=master, ref.master=99c6b53ab224), phutil(head=master, ref.master=de65ac554ca3)
remote:   #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:565]
remote:   #1 DiffusionCommitHookEngine::findGitContentUpdates(array) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:250]
remote:   #2 DiffusionCommitHookEngine::findContentUpdates(array) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:133]
remote:   #3 DiffusionCommitHookEngine::execute() called at [<phabricator>/scripts/repository/commit_hook.php:186]
To ssh://example.com:22/diffusion/TEST/test.git
 ! [remote rejected] dev -> dev (pre-receive hook declined)
error: failed to push some refs to 'ssh://git@example.com:22/diffusion/TEST/test.git'

phabricator 99c6b53ab2248e5d94a9589398df437a106c9744 (Fri, Dec 2)
arcanist fad85844314b151994769a461825c90f7400c145 (Oct 22 2016)
phutil de65ac554ca3f841d6a48051d6f740bbf7de9629 (Thu, Dec 1)

Revisions and Commits

rP Phabricator
	D16988	rP005d8493b034 Pass GIT_ENVIRONMENTAL_MAGIC through to hook subprocesses to support Git 2.11.0

Related Objects
Search...

		Status	Assigned	Task
		Open	epriestley	T12071 Require "E" be defined in variables_order so $_ENV is correctly populated
		Resolved	None	T11940 Commit hooks in hosted Git repositories fail under Git 2.11.0

Event Timeline

Rado created this task.Dec 2 2016, 12:12 PM

I can't reproduce this. Here's what I did:

Pushed commits to Phabricator.

Next steps:

Provide complete reproduction instructions allowing us to reproduce this issue in a clean environment (see Providing Reproduction Steps).

If we don't receive this information within a few days, we'll close this report. You can always file a new report with complete reproduction steps later.

This happened to me yesterday too. I am using the stable branch. The only thing that changed that I upgraded git to the latest release 2.11.0.
I have tried to change to the master branch which did not solve the problem. The problem was solved then by downgrading git to 2.10.2.

Steps to reproduce:

Install git 2.11.0 on phabricator and try to push.

We have been having a similar issue. Commits to all repositories are being rejected by the EVIL DRAGON BUREAUCRATS. We upgraded both Phabrictaor (Master) and GIT (2.11.0) as part of the maintenance window. In our setup with host repositiores in Diffusion, using SSH.

Following the maintance window all attempted pushes resulted in:

Counting objects: 14, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (14/14), done.
Writing objects: 100% (14/14), 2.33 KiB | 0 bytes/s, done.
Total 14 (delta 10), reused 0 (delta 0)
remote: +---------------------------------------------------------------+
remote: |      * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * *     |
remote: +---------------------------------------------------------------+
remote:             \
remote:              \                    ^    /^
remote:               \                  / \  // \
remote:                \   |\___/|      /   \//  .\
remote:                 \  /V  V  \__  /    //  | \ \           *----*
remote:                   /     /  \/_/    //   |  \  \          \   |
remote:                   @___@`    \/_   //    |   \   \         \/\ \
remote:                  0/0/|       \/_ //     |    \    \         \  \
remote:              0/0/0/0/|        \///      |     \     \       |  |
remote:           0/0/0/0/0/_|_ /   (  //       |      \     _\     |  /
remote:        0/0/0/0/0/0/`/,_ _ _/  ) ; -.    |    _ _\.-~       /   /
remote:                    ,-}        _      *-.|.-~-.           .~    ~
remote:   \     \__/        `/\      /                 ~-. _ .-~      /
remote:    \____(Oo)           *.   }            {                   /
remote:    (    (--)          .----~-.\        \-`                 .~
remote:    //__\\  \ DENIED!  ///.----..<        \             _ -~
remote:   //    \\               ///-._ _ _ _ _ _ _{^ - - - - ~
remote:
remote:
remote: DANGEROUS CHANGE: The change you're attempting to push updates the branch 'master' from 'b03ebfe1d069' to 'bd7410ee893c', but this is not a fast-forward. Pushes which rewrite published branch history are dangerous.
remote: Dangerous change protection is enabled for this repository.
remote: Edit the repository configuration before making dangerous changes.
remote:
To ssh://<<redacted>>/diffusion/1/wpta.git
 ! [remote rejected] bd7410ee893c96d4d64369c0e122fa5989be8a47 -> master (pre-receive hook declined)
error: failed to push some refs to 'ssh://<<redacted>>/diffusion/1/wpta.git'
Usage Exception: Push failed! Fix the error and run "arc land" again.

We have downgraded GIT on the Phabricator server to version 2.10.2, and now commits are working as normal.

Steps to Reproduce:

Upgrade GIT to version 2.11.0
Upgrade Phabricator to Master
Run bin/storage --upgrade
Restart daemons and apache
Create a new branch and add a file, commit
arc land after Diff is accepted
Get EVIL DRAGON BUREAUCRATS message

Downgrade back to 2.10.2 on the server and attempt to land the commit and arc land succeeds

phabricator 99c6b53ab2248e5d94a9589398df437a106c9744 (Thu, Dec 1)
arcanist fad85844314b151994769a461825c90f7400c145 (Oct 22 2016)
phutil de65ac554ca3f841d6a48051d6f740bbf7de9629 (Thu, Dec 1)
git 2.11.0

This is likely the relevant change:

* In order for the receiving end of "git push" to inspect the
  received history and decide to reject the push, the objects sent
  from the sending end need to be made available to the hook and
  the mechanism for the connectivity check, and this was done
  traditionally by storing the objects in the receiving repository
  and letting "git gc" expire them.  Instead, store the newly
  received objects in a temporary area, and make them available by
  reusing the alternate object store mechanism to them only while we
  decide if we accept the check, and once we decide, either migrate
  them to the repository or purge them immediately.

However I can't immediately find any documentation on this temporary area and the githooks documentation does not seem to have been updated with any particulars, at least as far as I can tell. It's not clear if we're supposed to change anything about how we perform calls while executing a receive hook or not. Since the documentation does not seem to have been updated, this might be a bug in git.

Since Git 2.11.0 was released four days ago, you should not reasonably expect that Phabricator stable (which was released six days ago, before Git 2.11.0 was available) will necessarily support it.

The Git mailing list archives also aren't currently working for me (no DNS record for blog.gmane.org?) so I can't tell if there's any existing discussion of this.

I noticed that to, they haven't been working for a while. I have been trying to go through the http://marc.info/?l=git mail archive.

epriestley renamed this task from Git push return an error to Commit hooks in hosted Git repositories fail under Git 2.11.0.Dec 2 2016, 1:41 PM

It looks like this is expected to work without changes, but relies on some environmental variables being passed from the hook environment to the child environment.

By default, we pass only whitelisted environmental variables.

The variables appear to be:

ALTERNATE_DB_ENVIRONMENT (actually named "GIT_ALTERNATE_OBJECT_DIRECTORIES")
DB_ENVIRONMENT (actually named "GIT_OBJECT_DIRECTORY")
GIT_QUARANTINE_ENVIRONMENT (actually named "GIT_QUARANTINE_PATH")

There's also a bunch of GIT_PUSH_OPTION stuff that seems to have shown up fairly recently, so I'm probably just going to let anything matching GIT_* pass through the environment.

If anyone has a reproduction case that they can test without undue disruption, try this patch against phabricator/?

diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
index 2813927..7f09584 100644
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -610,7 +610,11 @@ final class DiffusionCommitHookEngine extends Phobject {
       self::ENV_REMOTE_ADDRESS => $this->getRemoteAddress(),
     );
 
-    $directories = $this->getRepository()->getHookDirectories();
+    $repository = $this->getRepository();
+
+    $env += $repository->getPassthroughEnvironmentalVariables($_ENV);
+
+    $directories = $repository->getHookDirectories();
     foreach ($directories as $directory) {
       $hooks = $this->getExecutablesInDirectory($directory);
       sort($hooks);
diff --git a/src/applications/diffusion/protocol/DiffusionCommandEngine.php b/src/applications/diffusion/protocol/DiffusionCommandEngine.php
index 033f151..b4d3719 100644
--- a/src/applications/diffusion/protocol/DiffusionCommandEngine.php
+++ b/src/applications/diffusion/protocol/DiffusionCommandEngine.php
@@ -209,6 +209,8 @@ abstract class DiffusionCommandEngine extends Phobject {
       }
     }
 
+    $env += $repository->getPassthroughEnvironmentalVariables($_ENV);
+
     return $env;
   }
 
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index 5667dc4..6c7d5ed 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -2015,6 +2015,23 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return $client;
   }
 
+  public function getPassthroughEnvironmentalVariables(array $env) {
+    $result = array();
+
+    foreach ($env as $key => $value) {
+      // In Git, pass anything matching "GIT_*" though. Some of these variables
+      // need to be preserved to allow `git` operations to work properly when
+      // running from commit hooks.
+      if ($this->isGit()) {
+        if (preg_match('/^GIT_/', $key)) {
+          $result[$key] = $value;
+        }
+      }
+    }
+
+    return $result;
+  }
+
 /* -(  Repository URIs  )---------------------------------------------------- */

It doesn't break older Git, but I haven't yet set up a newer Git to see if it fixes the issue there.

The patch didn't appear to work for us.

We have:

Applied the above patch
Restarted Daemons and Apache
Upgraded back to GIT 2.11.0 and checked server git version is 2.11.0
Ran arc land on an accepted commit

Unfortunately, we still get the EVIL DRAGON message which includes:

remote: DANGEROUS CHANGE: The change you're attempting to push updates the branch 'master' from '45936140fc3a' to 'e032e3606960', but this is not a fast-forward. Pushes which rewrite published branch history are dangerous.
remote: Dangerous change protection is enabled for this repository.
remote: Edit the repository configuration before making dangerous changes.

Hope this helps.

Just to double check, does git merge-base e032e3606960 45936140fc3a in the working copy that you're trying to push print 45936140fc3a?

Just tried another and included the full output from arc land and the git merge-base:

$ arc land
Landing current branch '<<redacted>>'.
 TARGET  Landing onto "master", the default target under git.
 REMOTE  Using remote "origin", the default remote under git.
 FETCH  Fetching origin/master...
This commit will be landed:

      - 834835b <<redacted>>

Landing revision <<redacted>>..
 BUILDS PASSED  Harbormaster builds for the active diff completed successfully.
 PUSHING  Pushing changes to "origin/master".
Counting objects: 8, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 915 bytes | 0 bytes/s, done.
Total 8 (delta 6), reused 0 (delta 0)
remote: +---------------------------------------------------------------+
remote: |      * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * *     |
remote: +---------------------------------------------------------------+
remote:             \
remote:              \                    ^    /^
remote:               \                  / \  // \
remote:                \   |\___/|      /   \//  .\
remote:                 \  /V  V  \__  /    //  | \ \           *----*
remote:                   /     /  \/_/    //   |  \  \          \   |
remote:                   @___@`    \/_   //    |   \   \         \/\ \
remote:                  0/0/|       \/_ //     |    \    \         \  \
remote:              0/0/0/0/|        \///      |     \     \       |  |
remote:           0/0/0/0/0/_|_ /   (  //       |      \     _\     |  /
remote:        0/0/0/0/0/0/`/,_ _ _/  ) ; -.    |    _ _\.-~       /   /
remote:                    ,-}        _      *-.|.-~-.           .~    ~
remote:   \     \__/        `/\      /                 ~-. _ .-~      /
remote:    \____(Oo)           *.   }            {                   /
remote:    (    (--)          .----~-.\        \-`                 .~
remote:    //__\\  \ DENIED!  ///.----..<        \             _ -~
remote:   //    \\               ///-._ _ _ _ _ _ _{^ - - - - ~
remote:
remote:
remote: DANGEROUS CHANGE: The change you're attempting to push updates the branch 'master' from '429361e0fc3a' to '168d7671e085', but this is not a fast-forward. Pushes which rewrite published branch history are dangerous.
remote: Dangerous change protection is enabled for this repository.
remote: Edit the repository configuration before making dangerous changes.
remote:
To ssh://<<redacted>>/diffusion/1/wpta.git
 ! [remote rejected] 168d7671e0854b0c36acdff2b07e2e21b87f88a1 -> master (pre-receive hook declined)
error: failed to push some refs to 'ssh://<<redacted>>/diffusion/1/wpta.git'
Usage Exception: Push failed! Fix the error and run "arc land" again.

$ git merge-base 168d7671e085 429361e0fc3a
429361e0fc3aabea0ba0a1a061726dc56b6e0e99

Alright, thanks. I'll bring up a test system with a more modern git.

kislinsk added a subscriber: kislinsk.Dec 3 2016, 7:41 PM

epriestley mentioned this in T11947: Unable to push commits to hosted repositories.Dec 4 2016, 8:36 PM

jasonrumney added a subscriber: jasonrumney.Dec 5 2016, 3:42 AM

mbishopim3 added a subscriber: mbishopim3.Dec 5 2016, 2:03 PM

epriestley added a revision: D16988: Pass GIT_ENVIRONMENTAL_MAGIC through to hook subprocesses to support Git 2.11.0.Dec 5 2016, 8:13 PM

epriestley closed this task as Resolved by committing rP005d8493b034: Pass GIT_ENVIRONMENTAL_MAGIC through to hook subprocesses to support Git 2.11.0.Dec 5 2016, 8:45 PM

epriestley added a commit: rP005d8493b034: Pass GIT_ENVIRONMENTAL_MAGIC through to hook subprocesses to support Git 2.11.0.

I believe this is now resolved at HEAD of master. Let us know if you're still seeing issues after updating.

(I was able to reproduce the issue locally by upgrading to Git 2.11.0, and could push and pull over HTTP and SSH after the patch.)

We have updated to Git 2.11 and updated to master pull/push works for us. Thank you as always. :)

epriestley mentioned this in Z1336: General Chat.Dec 12 2016, 4:36 PM

epriestley added a parent task: T12071: Require "E" be defined in variables_order so $_ENV is correctly populated.Jan 5 2017, 10:30 PM

Commit hooks in hosted Git repositories fail under Git 2.11.0Closed, ResolvedPublicActions

Description

Revisions and Commits

Related ObjectsSearch...

Event Timeline

Commit hooks in hosted Git repositories fail under Git 2.11.0
Closed, ResolvedPublic
Actions

Related Objects
Search...