Skip to content

Commit 973b8ac

Browse files
author
epriestley
committedFeb 18, 2016
Remove dependence on callsigns from bin/commit-hook
Summary: Ref T4245. Two effects: - First, let hooks work for future repositories without callsigns. - Second, provide a better error when users push directly to hosted repositories. Test Plan: Ran `bin/commit-hook PHID-REPO-xxx`. Reviewers: chad, avivey Reviewed By: avivey Maniphest Tasks: T4245 Differential Revision: https://secure.phabricator.com/D15293
1 parent f5e2f95 commit 973b8ac

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed
 

‎scripts/repository/commit_hook.php

+6-5
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@
3232
require_once $root.'/scripts/__init_script__.php';
3333

3434
if ($argc < 2) {
35-
throw new Exception(pht('usage: commit-hook <callsign>'));
35+
throw new Exception(pht('usage: commit-hook <repository>'));
3636
}
3737

3838
$engine = new DiffusionCommitHookEngine();
3939

4040
$repository = id(new PhabricatorRepositoryQuery())
4141
->setViewer(PhabricatorUser::getOmnipotentUser())
42-
->withCallsigns(array($argv[1]))
42+
->withIdentifiers(array($argv[1]))
4343
->needProjectPHIDs(true)
4444
->executeOne();
4545

@@ -62,8 +62,9 @@
6262
if (!strlen($username)) {
6363
throw new Exception(
6464
pht(
65-
'Usage: %s should be defined!',
66-
DiffusionCommitHookEngine::ENV_USER));
65+
'No Direct Pushes: You are pushing directly to a repository hosted '.
66+
'by Phabricator. This will not work. See "No Direct Pushes" in the '.
67+
'documentation for more information.'));
6768
}
6869

6970
if ($repository->isHg()) {
@@ -77,7 +78,7 @@
7778
// specify the correct user; read this user out of the commit log.
7879

7980
if ($argc < 4) {
80-
throw new Exception(pht('usage: commit-hook <callsign> <repo> <txn>'));
81+
throw new Exception(pht('usage: commit-hook <repository> <repo> <txn>'));
8182
}
8283

8384
$svn_repo = $argv[2];

‎src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ private function installHookDirectory($path) {
192192
}
193193

194194
private function getHookContextIdentifier(PhabricatorRepository $repository) {
195-
$identifier = $repository->getCallsign();
195+
$identifier = $repository->getPHID();
196196

197197
$instance = PhabricatorEnv::getEnvConfig('cluster.instance');
198198
if (strlen($instance)) {

‎src/docs/user/userguide/diffusion_hosting.diviner

+35
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,41 @@ with `sudoers` configuration.
371371
is caused by SVN wiping the environment (including PATH) when invoking
372372
commit hooks.
373373

374+
No Direct Pushes
375+
================
376+
377+
You may get an error about "No Direct Pushes" when trying to push. This means
378+
you are pushing directly to the repository instead of pushing through
379+
Phabricator. This is not supported: writes to hosted repositories must go
380+
through Phabricator so it can perform authentication, enforce permissions,
381+
write logs, proxy requests, apply rewriting, etc.
382+
383+
One way to do a direct push by mistake is to use a `file:///` URI to interact
384+
with the repository from the same machine. This is not supported. Instead, use
385+
one of the repository URIs provided in the web interface, even if you're
386+
working on the same machine.
387+
388+
Another way to do a direct push is to misconfigure SSH (or not configure it at
389+
all) so that none of the logic described above runs and you just connect
390+
normally as a system user. In this case, the `ssh` test described above will
391+
fail (you'll get a command prompt when you connect, instead of the message you
392+
are supposed to get, as described above).
393+
394+
If you encounter this error: make sure you're using a remote URI given to
395+
you by Diffusion in the web interface, then run through the troubleshooting
396+
steps above carefully.
397+
398+
Sometimes users encounter this problem because they skip this whole document
399+
assuming they don't need to configure anything. This will not work, and you
400+
MUST configure things as described above for hosted repositories to work.
401+
402+
The technical reason this error occurs is that the `PHABRICATOR_USER` variable
403+
is not defined in the environment when commit hooks run. This variable is set
404+
by Phabricator when a request passes through the authentication layer that this
405+
document provides instructions for configuring. Its absence indicates that the
406+
request did not pass through Phabricator.
407+
408+
374409
= Next Steps =
375410

376411
Once hosted repositories are set up:

0 commit comments

Comments
 (0)
Failed to load comments.