Skip to content

Commit 972dfa7

Browse files
author
epriestley
committed
Add 'hook.d/' directories to SVN and Git repositories for custom hooks
Summary: Fixes T4189. Ref T4151. Allows repositories to have additional custom hooks for operations which can't be expressed with Herald (one such operation is lint). This adds only local hook directories, since they're easier to use with existing hooks than global directories. I might add global directories eventually. This doesn't support Mercurial since we have no demand for it and it's more complicated (we lose compatibility and power by just dropping a `hooks.d/` somewhere). Test Plan: - Pulled hosted SVN and Git repos to verify the hook directories generate correctly. - Added a variety of hooks to the hook directories (echo + pass, fail). - Pushed commits and verified the hooks fired (output expected info, or failed). - Verified push log reflected the correct error code ("3", external) and detail ("nope.sh") when rejecting. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4151, T4189 Differential Revision: https://secure.phabricator.com/D7884
1 parent 2cfc3ac commit 972dfa7

File tree

4 files changed

+134
-7
lines changed

4 files changed

+134
-7
lines changed

scripts/repository/commit_hook.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
}
8989

9090
$engine->setStdin($stdin);
91+
$engine->setOriginalArgv(array_slice($argv, 2));
9192

9293
$remote_address = getenv(DiffusionCommitHookEngine::ENV_REMOTE_ADDRESS);
9394
if (strlen($remote_address)) {

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ final class DiffusionCommitHookEngine extends Phobject {
1919
private $viewer;
2020
private $repository;
2121
private $stdin;
22+
private $originalArgv;
2223
private $subversionTransaction;
2324
private $subversionRepository;
2425
private $remoteAddress;
@@ -84,6 +85,15 @@ public function getStdin() {
8485
return $this->stdin;
8586
}
8687

88+
public function setOriginalArgv(array $original_argv) {
89+
$this->originalArgv = $original_argv;
90+
return $this;
91+
}
92+
93+
public function getOriginalArgv() {
94+
return $this->originalArgv;
95+
}
96+
8797
public function setRepository(PhabricatorRepository $repository) {
8898
$this->repository = $repository;
8999
return $this;
@@ -141,7 +151,8 @@ public function execute() {
141151

142152
$this->applyHeraldContentRules($content_updates, $all_updates);
143153

144-
// TODO: Fire external hooks.
154+
// Run custom scripts in `hook.d/` directories.
155+
$this->applyCustomHooks($all_updates);
145156

146157
// If we make it this far, we're accepting these changes. Mark all the
147158
// logs as accepted.
@@ -551,6 +562,74 @@ private function findGitContentUpdates(array $ref_updates) {
551562
return $content_updates;
552563
}
553564

565+
/* -( Custom )------------------------------------------------------------- */
566+
567+
private function applyCustomHooks(array $updates) {
568+
$args = $this->getOriginalArgv();
569+
$stdin = $this->getStdin();
570+
$console = PhutilConsole::getConsole();
571+
572+
$env = array(
573+
'PHABRICATOR_REPOSITORY' => $this->getRepository()->getCallsign(),
574+
self::ENV_USER => $this->getViewer()->getUsername(),
575+
self::ENV_REMOTE_PROTOCOL => $this->getRemoteProtocol(),
576+
self::ENV_REMOTE_ADDRESS => $this->getRemoteAddress(),
577+
);
578+
579+
$directories = $this->getRepository()->getHookDirectories();
580+
foreach ($directories as $directory) {
581+
$hooks = $this->getExecutablesInDirectory($directory);
582+
sort($hooks);
583+
foreach ($hooks as $hook) {
584+
// NOTE: We're explicitly running the hooks in sequential order to
585+
// make this more predictable.
586+
$future = id(new ExecFuture('%s %Ls', $hook, $args))
587+
->setEnv($env, $wipe_process_env = false)
588+
->write($stdin);
589+
590+
list($err, $stdout, $stderr) = $future->resolve();
591+
if (!$err) {
592+
// This hook ran OK, but echo its output in case there was something
593+
// informative.
594+
$console->writeOut("%s", $stdout);
595+
$console->writeErr("%s", $stderr);
596+
continue;
597+
}
598+
599+
// Mark everything as rejected by this hook.
600+
foreach ($updates as $update) {
601+
$update->setRejectCode(
602+
PhabricatorRepositoryPushLog::REJECT_EXTERNAL);
603+
$update->setRejectDetails(basename($hook));
604+
}
605+
606+
throw new DiffusionCommitHookRejectException(
607+
pht(
608+
"This push was rejected by custom hook script '%s':\n\n%s%s",
609+
basename($hook),
610+
$stdout,
611+
$stderr));
612+
}
613+
}
614+
}
615+
616+
private function getExecutablesInDirectory($directory) {
617+
$executables = array();
618+
619+
if (!Filesystem::pathExists($directory)) {
620+
return $executables;
621+
}
622+
623+
foreach (Filesystem::listDirectory($directory) as $path) {
624+
$full_path = $directory.DIRECTORY_SEPARATOR.$path;
625+
if (is_executable($full_path)) {
626+
$executables[] = $full_path;
627+
}
628+
}
629+
630+
return $executables;
631+
}
632+
554633

555634
/* -( Mercurial )---------------------------------------------------------- */
556635

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ public function pullRepository() {
103103
} else if ($is_hg) {
104104
$this->installMercurialHook();
105105
}
106+
107+
foreach ($repository->getHookDirectories() as $directory) {
108+
$this->installHookDirectory($directory);
109+
}
106110
}
107111

108112
} catch (Exception $ex) {
@@ -173,6 +177,17 @@ private function installHook($path) {
173177
Filesystem::changePermissions($path, 0755);
174178
}
175179

180+
private function installHookDirectory($path) {
181+
$readme = pht(
182+
"To add custom hook scripts to this repository, add them to this ".
183+
"directory.\n\nPhabricator will run any executables in this directory ".
184+
"after running its own checks, as though they were normal hook ".
185+
"scripts.");
186+
187+
Filesystem::createDirectory($path, 0755);
188+
Filesystem::writeFile($path.'/README', $readme);
189+
}
190+
176191

177192
/* -( Pulling Git Working Copies )----------------------------------------- */
178193

@@ -311,15 +326,15 @@ private function executeGitUpdate() {
311326
*/
312327
private function installGitHook() {
313328
$repository = $this->getRepository();
314-
$path = $repository->getLocalPath();
329+
$root = $repository->getLocalPath();
315330

316331
if ($repository->isWorkingCopyBare()) {
317-
$path .= '/hooks/pre-receive';
332+
$path = '/hooks/pre-receive';
318333
} else {
319-
$path .= '/.git/hooks/pre-receive';
334+
$path = '/.git/hooks/pre-receive';
320335
}
321336

322-
$this->installHook($path);
337+
$this->installHook($root.$path);
323338
}
324339

325340

@@ -438,14 +453,17 @@ private function executeSubversionCreate() {
438453
execx('svnadmin create -- %s', $path);
439454
}
440455

456+
441457
/**
442458
* @task svn
443459
*/
444460
private function installSubversionHook() {
445461
$repository = $this->getRepository();
446-
$path = $repository->getLocalPath().'/hooks/pre-commit';
462+
$root = $repository->getLocalPath();
463+
464+
$path = '/hooks/pre-commit';
447465

448-
$this->installHook($path);
466+
$this->installHook($root.$path);
449467
}
450468

451469

src/applications/repository/storage/PhabricatorRepository.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,35 @@ public function usesLocalWorkingCopy() {
945945
}
946946
}
947947

948+
public function getHookDirectories() {
949+
$directories = array();
950+
if (!$this->isHosted()) {
951+
return $directories;
952+
}
953+
954+
$root = $this->getLocalPath();
955+
956+
switch ($this->getVersionControlSystem()) {
957+
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
958+
if ($this->isWorkingCopyBare()) {
959+
$directories[] = $root.'/hooks/pre-receive-phabricator.d/';
960+
} else {
961+
$directories[] = $root.'/.git/hooks/pre-receive-phabricator.d/';
962+
}
963+
break;
964+
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
965+
$directories[] = $root.'/hooks/pre-commit-phabricator.d/';
966+
break;
967+
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
968+
// NOTE: We don't support custom Mercurial hooks for now because they're
969+
// messy and we can't easily just drop a `hooks.d/` directory next to
970+
// the hooks.
971+
break;
972+
}
973+
974+
return $directories;
975+
}
976+
948977
public function canDestroyWorkingCopy() {
949978
if ($this->isHosted()) {
950979
// Never destroy hosted working copies.

0 commit comments

Comments
 (0)