revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-25 16:22:42 +01:00
Code Issues Releases Wiki Activity

Remove --background flag to prevent arc from hanging

Browse source 
Summary:
Ref T4281. A long time ago, we added a `--background` flag to let `arc lint` and `arc unit` run while you're typing a commit message, in some situations.

This code is only moderately beneficial and is way too complicated. Particularly, it has a long history of causing hangs (T4281, T2463), doesn't work on Windows, and is impossible to debug.

It's also running into a serious PHP bug with EAGAIN/EPIPE being indistinguishable that I haven't been able to find a reasonable workaround for in ~3-4 hours of trying.

All the pathways forward that I can see make this already-complex system more complex.

The major reason that this stuff is so complex is that the subprocess may need to prompt the user (notably, to apply patches from lint).

Instead, I'm going to simplify how `arc diff` interacts with `arc lint` and `arc unit`, so we can just fire-and-forget a background process, let it do as much work as it can without needing user input, and then pick up wherever it left off. This will be slightly less cool/magical, but it won't hang bizarrely and I will be able to debug it.

For now, simply remove the `--background` flag and behavior so `arc` works for everyone.

Test Plan: Ran `arc diff` to create this diff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4281

Differential Revision: https://secure.phabricator.com/D10198
This commit is contained in:
epriestley 2014-08-08 16:09:11 -07:00
parent 377eb5d1e0
commit 54bea946c1
1 changed files with 3 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
src/workflow/ArcanistDiffWorkflow.php
View file

@ -364,12 +364,6 @@ EOTEXT
'no-diff' => array( 'no-diff' => array(
'help' => 'Only run lint and unit tests. Intended for internal use.', 'help' => 'Only run lint and unit tests. Intended for internal use.',
), ),
'background' => array(
'param' => 'bool',
'help' =>
'Run lint and unit tests on background. '.
'"0" to disable, "1" to enable (default).',
),
'cache' => array( 'cache' => array(
'param' => 'bool', 'param' => 'bool',
'help' => '0 to disable lint cache, 1 to enable (default).', 'help' => '0 to disable lint cache, 1 to enable (default).',
@ -415,10 +409,6 @@ EOTEXT
) )
); );
if (phutil_is_windows()) {
unset($arguments['background']);
}
return $arguments; return $arguments;
} }
@ -440,43 +430,6 @@ EOTEXT
$this->runDiffSetupBasics(); $this->runDiffSetupBasics();
$background = $this->getArgument('background', true);
if ($this->isRawDiffSource() || phutil_is_windows()) {
$background = false;
}
if ($background) {
$argv = $this->getPassedArguments();
if (!PhutilConsoleFormatter::getDisableANSI()) {
array_unshift($argv, '--ansi');
}
$repo = $this->getRepositoryAPI();
$head_commit = $this->getArgument('head');
if ($head_commit !== null) {
$repo->setHeadCommit($head_commit);
}
if ($repo->supportsCommitRanges()) {
$repo->getBaseCommit();
}
$script = phutil_get_library_root('arcanist').'/../scripts/arcanist.php';
if ($argv) {
$lint_unit = new ExecFuture(
'php %s --recon diff --no-diff %Ls',
$script,
$argv);
} else {
$lint_unit = new ExecFuture(
'php %s --recon diff --no-diff',
$script);
}
$lint_unit->write('', true);
$lint_unit->start();
}
$commit_message = $this->buildCommitMessage(); $commit_message = $this->buildCommitMessage();
$this->dispatchEvent( $this->dispatchEvent(
@ -489,24 +442,10 @@ EOTEXT
$revision = $this->buildRevisionFromCommitMessage($commit_message); $revision = $this->buildRevisionFromCommitMessage($commit_message);
} }
if ($background) { $server = $this->console->getServer();
$server = new PhutilConsoleServer(); $server->setHandler(array($this, 'handleServerMessage'));
$server->addExecFutureClient($lint_unit); $data = $this->runLintUnit();
$server->setHandler(array($this, 'handleServerMessage'));
$server->run();
list($err) = $lint_unit->resolve();
$data = $this->readScratchJSONFile('diff-result.json');
if ($err || !$data) {
throw new Exception(
'Unable to read results from background linting and unit testing. '.
'You can try running arc diff again with --background 0');
}
} else {
$server = $this->console->getServer();
$server->setHandler(array($this, 'handleServerMessage'));
$data = $this->runLintUnit();
}
$lint_result = $data['lintResult']; $lint_result = $data['lintResult'];
$this->unresolvedLint = $data['unresolvedLint']; $this->unresolvedLint = $data['unresolvedLint'];
$this->postponedLinters = $data['postponedLinters']; $this->postponedLinters = $data['postponedLinters'];