@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Add setup checks for the availability of 'which' and 'diff' binaries
Summary:
Spent an hour or two helping a user figure this out. Make sure I never do that again.
If the webserver is configured with an empty or bogus PATH, binaries like 'which' and 'diff' (and 'git', and 'svn', etc.) may not be available. In most cases, this is fine, because we get an error like "sh: whatever-command not found", which is obvious to diagnose.
In the case of 'diff', we don't get this, because 'diff' is expected to exit with a nonzero code for differing files -- so we interpret the "sh: whatever-command not found" as "files differ" and then try to parse the empty output.
Explicitly check for 'which' (on Windows, 'where') and 'diff' during setup (I plan to refine the behavior around 'git', 'svn' and 'hg' at some point, but this is less pressing since the errors are trivial to support).
Test Plan: Faked failures on all modes, verified setup warnings look reasonable.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6008
+101
-5
+2
src/__phutil_library_map__.php
+2
src/__phutil_library_map__.php
···
1392
1392
'PhabricatorSetupCheck' => 'applications/config/check/PhabricatorSetupCheck.php',
1393
1393
'PhabricatorSetupCheckAPC' => 'applications/config/check/PhabricatorSetupCheckAPC.php',
1394
1394
'PhabricatorSetupCheckBaseURI' => 'applications/config/check/PhabricatorSetupCheckBaseURI.php',
1395
+
'PhabricatorSetupCheckBinaries' => 'applications/config/check/PhabricatorSetupCheckBinaries.php',
1395
1396
'PhabricatorSetupCheckDatabase' => 'applications/config/check/PhabricatorSetupCheckDatabase.php',
1396
1397
'PhabricatorSetupCheckExtensions' => 'applications/config/check/PhabricatorSetupCheckExtensions.php',
1397
1398
'PhabricatorSetupCheckExtraConfig' => 'applications/config/check/PhabricatorSetupCheckExtraConfig.php',
···
3147
3148
'PhabricatorSettingsPanelSearchPreferences' => 'PhabricatorSettingsPanel',
3148
3149
'PhabricatorSetupCheckAPC' => 'PhabricatorSetupCheck',
3149
3150
'PhabricatorSetupCheckBaseURI' => 'PhabricatorSetupCheck',
3151
+
'PhabricatorSetupCheckBinaries' => 'PhabricatorSetupCheck',
3150
3152
'PhabricatorSetupCheckDatabase' => 'PhabricatorSetupCheck',
3151
3153
'PhabricatorSetupCheckExtensions' => 'PhabricatorSetupCheck',
3152
3154
'PhabricatorSetupCheckExtraConfig' => 'PhabricatorSetupCheck',
+95
src/applications/config/check/PhabricatorSetupCheckBinaries.php
+95
src/applications/config/check/PhabricatorSetupCheckBinaries.php
···
1
+
<?php
2
+
3
+
final class PhabricatorSetupCheckBinaries extends PhabricatorSetupCheck {
4
+
5
+
6
+
protected function executeChecks() {
7
+
8
+
if (phutil_is_windows()) {
9
+
$bin_name = 'where';
10
+
} else {
11
+
$bin_name = 'which';
12
+
}
13
+
14
+
if (!Filesystem::binaryExists($bin_name)) {
15
+
$message = pht(
16
+
"Without '%s', Phabricator can not test for the availability ".
17
+
"of other binaries.",
18
+
$bin_name);
19
+
$this->raiseWarning($bin_name, $message);
20
+
21
+
// We need to return here if we can't find the 'which' / 'where' binary
22
+
// because the other tests won't be valid.
23
+
return;
24
+
}
25
+
26
+
if (!Filesystem::binaryExists('diff')) {
27
+
$message = pht(
28
+
"Without 'diff', Phabricator will not be able to generate or render ".
29
+
"diffs in multiple applications.");
30
+
$this->raiseWarning('diff', $message);
31
+
} else {
32
+
$tmp_a = new TempFile();
33
+
$tmp_b = new TempFile();
34
+
$tmp_c = new TempFile();
35
+
36
+
Filesystem::writeFile($tmp_a, 'A');
37
+
Filesystem::writeFile($tmp_b, 'A');
38
+
Filesystem::writeFile($tmp_c, 'B');
39
+
40
+
list($err) = exec_manual('diff %s %s', $tmp_a, $tmp_b);
41
+
if ($err) {
42
+
$this->newIssue('bin.diff.same')
43
+
->setName(pht("Unexpected 'diff' Behavior"))
44
+
->setMessage(
45
+
pht(
46
+
"The 'diff' binary on this system has unexpected behavior: ".
47
+
"it was expected to exit without an error code when passed ".
48
+
"identical files, but exited with code %d.",
49
+
$err));
50
+
}
51
+
52
+
list($err) = exec_manual('diff %s %s', $tmp_a, $tmp_c);
53
+
if (!$err) {
54
+
$this->newIssue('bin.diff.diff')
55
+
->setName(pht("Unexpected 'diff' Behavior"))
56
+
->setMessage(
57
+
pht(
58
+
"The 'diff' binary on this system has unexpected behavior: ".
59
+
"it was expected to exit with a nonzero error code when passed ".
60
+
"differing files, but did not."));
61
+
}
62
+
}
63
+
}
64
+
65
+
private function raiseWarning($bin, $message) {
66
+
if (phutil_is_windows()) {
67
+
$preamble = pht(
68
+
"The '%s' binary could not be found. Set the webserver's %s ".
69
+
"environmental variable to include the directory where it resides, or ".
70
+
"add that directory to '%s' in the Phabricator configuration.",
71
+
$bin,
72
+
'PATH',
73
+
'environment.append-paths');
74
+
} else {
75
+
$preamble = pht(
76
+
"The '%s' binary could not be found. Symlink it into '%s', or set the ".
77
+
"webserver's %s environmental variable to include the directory where ".
78
+
"it resides, or add that directory to '%s' in the Phabricator ".
79
+
"configuration.",
80
+
$bin,
81
+
'phabricator/support/bin/',
82
+
'PATH',
83
+
'environment.append-paths');
84
+
}
85
+
86
+
$this->newIssue('bin.'.$bin)
87
+
->setShortName(pht("'%s' Missing", $bin))
88
+
->setName(pht("Missing '%s' Binary", $bin))
89
+
->setSummary(
90
+
pht("The '%s' binary could not be located or excuted.", $bin))
91
+
->setMessage($preamble.' '.$message)
92
+
->addPhabricatorConfig('environment.append-paths');
93
+
}
94
+
95
+
}
+4
-5
src/applications/config/check/PhabricatorSetupCheckImagemagick.php
+4
-5
src/applications/config/check/PhabricatorSetupCheckImagemagick.php
···
5
5
protected function executeChecks() {
6
6
$imagemagick = PhabricatorEnv::getEnvConfig('files.enable-imagemagick');
7
7
if ($imagemagick) {
8
-
list($err) = exec_manual('which convert');
9
-
if ($err) {
8
+
if (!Filesystem::binaryExists('convert')) {
10
9
$message = pht(
11
-
'You have enabled Imagemagick in your config, but the \'convert\''.
12
-
' binary is not in the webserver\'s $PATH. Disable imagemagick'.
13
-
' or make it available to the webserver.');
10
+
'You have enabled Imagemagick in your config, but the \'convert\' '.
11
+
'binary is not in the webserver\'s $PATH. Disable imagemagick '.
12
+
'or make it available to the webserver.');
14
13
15
14
$this->newIssue('files.enable-imagemagick')
16
15
->setName(pht(