mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-25 16:22:43 +01:00
Config page: add lovely git-related error messages in standard error log
Summary: Premise: the Config page runs git commands. Spoiler: they can fail. Before this change errors were just suppressed and ignored. After this change you get at least a log line. Also, you get a tip for a very specific well-known error affecting recent git. Probably suppressing stuff was fine in the moment git worked here. But nowadays git doesn't work so easily here, since it introduced very weird additional configurations in order for a repository to be indicated as "safe" or not. Error suppression was a problem there, because understanding the error with "future objects" is not trivial for most users. Really. After this change, these errors are beautifully mentioned in the standard log of your webserver, to the best of our communication ability. This is a cute example of a new log line: Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282). Try this system resolution: sudo git config --system --add safe.directory /var/www/phorge Another: Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282). Try this system resolution: sudo git config --system --add safe.directory /var/www/arcanist Incidentally, these specific errors probably afflict your Phorge/Phabricator, and now you have some useful resolution tips. You are welcome! You can also join T15282 to discuss your specific case. Closes T15243 Test Plan: - visit the `/config/` page: does it worked before? it still works now - visit the `/config/` page without `/etc/gitconfig`: you may still see "Unknown" as version - but, you finally get something in the log (instead of nothing) - visit the `/config/` page after following your log messages: now you should see the library versions! yeeh Additional tests: - manually sabotage the command "git log" replacing with "gitfooolog" and visit /config page: see the unexpected 'gitfooolog command not found' log line - manually sabotage the command "git remove" replacing with "gitremotelog" and visit /config/ page: see the unexpected 'gitremotelog command not found' log line Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: avivey, deadalnix, aklapper, speck, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15243 Differential Revision: https://we.phorge.it/D25148
This commit is contained in:
parent
980293b707
commit
83edbffd52
1 changed files with 41 additions and 8 deletions
|
@ -189,9 +189,10 @@ final class PhabricatorConfigConsoleController
|
|||
foreach ($specs as $lib) {
|
||||
$remote_future = $remote_futures[$lib];
|
||||
|
||||
list($err, $stdout) = $remote_future->resolve();
|
||||
if ($err) {
|
||||
// If this fails for whatever reason, just move on.
|
||||
try {
|
||||
list($stdout, $err) = $remote_future->resolvex();
|
||||
} catch (CommandException $e) {
|
||||
$this->logGitErrorWithPotentialTips($e, $lib);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
@ -258,12 +259,13 @@ final class PhabricatorConfigConsoleController
|
|||
|
||||
$results = array();
|
||||
foreach ($log_futures as $lib => $future) {
|
||||
list($err, $stdout) = $future->resolve();
|
||||
if (!$err) {
|
||||
try {
|
||||
list($stdout, $err) = $future->resolvex();
|
||||
list($hash, $epoch) = explode(' ', $stdout);
|
||||
} else {
|
||||
} catch (CommandException $e) {
|
||||
$hash = null;
|
||||
$epoch = null;
|
||||
$this->logGitErrorWithPotentialTips($e, $lib);
|
||||
}
|
||||
|
||||
$result = array(
|
||||
|
@ -275,7 +277,7 @@ final class PhabricatorConfigConsoleController
|
|||
|
||||
$upstream_future = idx($upstream_futures, $lib);
|
||||
if ($upstream_future) {
|
||||
list($err, $stdout) = $upstream_future->resolve();
|
||||
list($stdout, $err) = $upstream_future->resolvex();
|
||||
if (!$err) {
|
||||
$branchpoint = trim($stdout);
|
||||
if (strlen($branchpoint)) {
|
||||
|
@ -340,5 +342,36 @@ final class PhabricatorConfigConsoleController
|
|||
->appendChild($table_view);
|
||||
}
|
||||
|
||||
/**
|
||||
* Help in better troubleshooting git errors.
|
||||
* @param CommandException $e Exception
|
||||
* @param string $lib Library name involved
|
||||
*/
|
||||
private function logGitErrorWithPotentialTips($e, $lib) {
|
||||
|
||||
// First, detect this specific error message related to [safe] stuff.
|
||||
$expected_error_msg_part = 'detected dubious ownership in repository';
|
||||
$stderr = $e->getStderr();
|
||||
if (strpos($stderr, $expected_error_msg_part) !== false) {
|
||||
|
||||
// Found! Let's show a nice resolution tip.
|
||||
|
||||
// Complete path of the problematic repository.
|
||||
$lib_root = dirname(phutil_get_library_root($lib));
|
||||
|
||||
phlog(pht(
|
||||
"Cannot identify the version of the %s repository because ".
|
||||
"the webserver does not trust it (more info on Task %s).\n".
|
||||
"Try this system resolution:\n".
|
||||
"sudo git config --system --add safe.directory %s",
|
||||
$lib,
|
||||
'https://we.phorge.it/T15282',
|
||||
$lib_root));
|
||||
} else {
|
||||
|
||||
// Otherwise show a generic error message
|
||||
phlog($e);
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue