Support SVN pre-commit hoooks

Summary:
Ref T4189. This adds SVN support, which was a little more messy than I though. Principally, we can not use `PHABRICATOR_USER` for Subversion, because it strips away the entire environment for "security reasons".

Instead, use `--tunnel-user` plus `svnlook author` to figure out the author.

Also fix "ssh://" clone URIs, which needs to be "svn+ssh://".

Test Plan:
  - Made SVN commits through the hook.
  - Made Git commits, too, to make sure I didn't break anything.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4189

Differential Revision: https://secure.phabricator.com/D7683

Author: epriestley

scripts/repository/commit_hook.php

@@ -4,52 +4,75 @@
 $root = dirname(dirname(dirname(__FILE__)));
 require_once $root.'/scripts/__init_script__.php';
 
-$username = getenv('PHABRICATOR_USER');
-if (!$username) {
-  throw new Exception(pht('usage: define PHABRICATOR_USER in environment'));
-}
-
-$user = id(new PhabricatorPeopleQuery())
-  ->setViewer(PhabricatorUser::getOmnipotentUser())
-  ->withUsernames(array($username))
-  ->executeOne();
-if (!$user) {
-  throw new Exception(pht('No such user "%s"!', $username));
-}
-
 if ($argc < 2) {
   throw new Exception(pht('usage: commit-hook <callsign>'));
 }
 
+$engine = new DiffusionCommitHookEngine();
+
 $repository = id(new PhabricatorRepositoryQuery())
-  ->setViewer($user)
+  ->setViewer(PhabricatorUser::getOmnipotentUser())
   ->withCallsigns(array($argv[1]))
-  ->requireCapabilities(
-    array(
-      // This capability check is redundant, but can't hurt.
-      PhabricatorPolicyCapability::CAN_VIEW,
-      DiffusionCapabilityPush::CAPABILITY,
-    ))
   ->executeOne();
 
 if (!$repository) {
   throw new Exception(pht('No such repository "%s"!', $callsign));
 }
 
 if (!$repository->isHosted()) {
-  // This should be redundant too, but double check just in case.
+  // This should be redundant, but double check just in case.
   throw new Exception(pht('Repository "%s" is not hosted!', $callsign));
 }
 
+$engine->setRepository($repository);
+
+
+// Figure out which user is writing the commit.
+
+if ($repository->isGit()) {
+  $username = getenv('PHABRICATOR_USER');
+  if (!strlen($username)) {
+    throw new Exception(pht('usage: PHABRICATOR_USER should be defined!'));
+  }
+} else if ($repository->isSVN()) {
+  // NOTE: In Subversion, the entire environment gets wiped so we can't read
+  // PHABRICATOR_USER. Instead, we've set "--tunnel-user" to specify the
+  // correct user; read this user out of the commit log.
+
+  if ($argc < 4) {
+    throw new Exception(pht('usage: commit-hook <callsign> <repo> <txn>'));
+  }
+
+  $svn_repo = $argv[2];
+  $svn_txn = $argv[3];
+  list($username) = execx('svnlook author -t %s %s', $svn_txn, $svn_repo);
+  $username = rtrim($username, "\n");
+
+  $engine->setSubversionTransactionInfo($svn_txn, $svn_repo);
+} else {
+  throw new Exceptiont(pht('Unknown repository type.'));
+}
+
+$user = id(new PhabricatorPeopleQuery())
+  ->setViewer(PhabricatorUser::getOmnipotentUser())
+  ->withUsernames(array($username))
+  ->executeOne();
+
+if (!$user) {
+  throw new Exception(pht('No such user "%s"!', $username));
+}
+
+$engine->setViewer($user);
+
+
+// Read stdin for the hook engine.
+
 $stdin = @file_get_contents('php://stdin');
 if ($stdin === false) {
   throw new Exception(pht('Failed to read stdin!'));
 }
 
-$engine = id(new DiffusionCommitHookEngine())
-  ->setViewer($user)
-  ->setRepository($repository)
-  ->setStdin($stdin);
+$engine->setStdin($stdin);
 
 $err = $engine->execute();
 

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -173,7 +173,12 @@ private function buildPropertiesTable(PhabricatorRepository $repository) {
       $serve_ssh = $repository->getServeOverSSH();
       if ($serve_ssh !== $serve_off) {
         $uri = new PhutilURI(PhabricatorEnv::getProductionURI($repo_path));
-        $uri->setProtocol('ssh');
+
+        if ($repository->isSVN()) {
+          $uri->setProtocol('svn+ssh');
+        } else {
+          $uri->setProtocol('ssh');
+        }
 
         $ssh_user = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
         if ($ssh_user) {

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

@@ -5,6 +5,15 @@ final class DiffusionCommitHookEngine extends Phobject {
   private $viewer;
   private $repository;
   private $stdin;
+  private $subversionTransaction;
+  private $subversionRepository;
+
+
+  public function setSubversionTransactionInfo($transaction, $repository) {
+    $this->subversionTransaction = $transaction;
+    $this->subversionRepository = $repository;
+    return $this;
+  }
 
   public function setStdin($stdin) {
     $this->stdin = $stdin;
@@ -39,6 +48,9 @@ public function execute() {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
         $err = $this->executeGitHook();
         break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $err = $this->executeSubversionHook();
+        break;
       default:
         throw new Exception(pht('Unsupported repository type "%s"!', $type));
     }
@@ -54,6 +66,13 @@ private function executeGitHook() {
     return 0;
   }
 
+  private function executeSubversionHook() {
+
+    // TODO: Do useful things here, too.
+
+    return 0;
+  }
+
   private function parseGitUpdates($stdin) {
     $updates = array();
 

src/applications/diffusion/ssh/DiffusionSSHSubversionServeWorkflow.php

@@ -38,7 +38,9 @@ protected function executeRepositoryOperations() {
       throw new Exception("Expected `svnserve -t`!");
     }
 
-    $command = csprintf('svnserve -t');
+    $command = csprintf(
+      'svnserve -t --tunnel-user=%s',
+      $this->getUser()->getUsername());
     $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);
 
     $future = new ExecFuture('%C', $command);

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

@@ -85,6 +85,8 @@ public function pullRepository() {
         if ($repository->isHosted()) {
           if ($is_git) {
             $this->installGitHook();
+          } else if ($is_svn) {
+            $this->installSubversionHook();
           } else {
             $this->logPull(
               pht(
@@ -158,7 +160,7 @@ private function installHook($path) {
 
     $root = dirname(phutil_get_library_root('phabricator'));
     $bin = $root.'/bin/commit-hook';
-    $cmd = csprintf('exec -- %s %s', $bin, $callsign);
+    $cmd = csprintf('exec -- %s %s "$@"', $bin, $callsign);
 
     $hook = "#!/bin/sh\n{$cmd}\n";
 
@@ -394,5 +396,15 @@ private function executeSubversionCreate() {
     execx('svnadmin create -- %s', $path);
   }
 
+  /**
+   * @task svn
+   */
+  private function installSubversionHook() {
+    $repository = $this->getRepository();
+    $path = $repository->getLocalPath().'hooks/pre-commit';
+
+    $this->installHook($path);
+  }
+
 
 }