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

Fix an obscure dependency issue in Arcanist + libphutil

Browse source 
Summary:
The module analyzer reads "phutil_require_module" in the source of a module as a
dependency, and tries to regenerate __init__.php if symbols from that module
aren't actually used. This creates patches which don't actually resolve the
problem, since changing __init__.php won't change the dependency.

Instead, trust that anyone using phutil_require_module in the source of a module
knows what they're doing and don't mark it as a dependency.

We currently have an issue with this in phabricator's Setup process since I load
some other libraries' modules just to test if they can be loaded

@lesha, this might be the issue you reported a while ago.

Test Plan: Ran "arc lint" on a module which pulls in another module explicitly
in the source, didn't get a no-op lint error.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, lesha
CC: aran, epriestley, jungejason
Differential Revision: 770
This commit is contained in:
epriestley 2011-07-29 09:12:24 -07:00
parent 17c82ff03c
commit 39e4656278
2 changed files with 11 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
scripts/phutil_analyzer.php
View file

@ -122,7 +122,7 @@ foreach (Futures($futures) as $file => $future) {
} }
$requirements->addSourceDependency($name, $value); $requirements->addSourceDependency($name, $value);
} else if ($call_name == 'phutil_require_module') { } else if ($call_name == 'phutil_require_module') {
analyze_phutil_require_module($call, $requirements); analyze_phutil_require_module($call, $requirements, true);
} }
} }
} else { } else {
@ -154,7 +154,7 @@ foreach (Futures($futures) as $file => $future) {
$call_name = $name->getConcreteString(); $call_name = $name->getConcreteString();
if ($call_name == 'phutil_require_module') { if ($call_name == 'phutil_require_module') {
analyze_phutil_require_module($call, $requirements); analyze_phutil_require_module($call, $requirements, false);
} else if ($call_name == 'call_user_func' || } else if ($call_name == 'call_user_func' ||
$call_name == 'call_user_func_array') { $call_name == 'call_user_func_array') {
$params = $call->getChildByIndex(1)->getChildren(); $params = $call->getChildByIndex(1)->getChildren();
@ -327,7 +327,8 @@ echo json_encode($requirements->toDictionary());
*/ */
function analyze_phutil_require_module( function analyze_phutil_require_module(
XHPASTNode $call, XHPASTNode $call,
PhutilModuleRequirements $requirements) { PhutilModuleRequirements $requirements,
$create_dependency) {
$name = $call->getChildByIndex(0); $name = $call->getChildByIndex(0);
$params = $call->getChildByIndex(1)->getChildren(); $params = $call->getChildByIndex(1)->getChildren();
@ -363,7 +364,9 @@ function analyze_phutil_require_module(
return; return;
} }
$requirements->addModuleDependency( if ($create_dependency) {
$name, $requirements->addModuleDependency(
$library_value.':'.$module_value); $name,
$library_value.':'.$module_value);
}
} }

3
src/lint/message/ArcanistLintMessage.php
View file

@ -168,7 +168,8 @@ class ArcanistLintMessage {
} }
public function isPatchable() { public function isPatchable() {
return ($this->getReplacementText() !== null); return ($this->getReplacementText() !== null) &&
($this->getReplacementText() !== $this->getOriginalText());
} }
public function didApplyPatch() { public function didApplyPatch() {