mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-24 07:42:39 +01:00
Fix ArcanistExternalLinter on Windows
Summary: When using `proc_open()` with `'bypass_shell' => true` on Windows, file extensions other than .exe will not be resolved. Various linters therefore don't work, such as `jshint`, which is actually `jshint.cmd`. The problem was already observed and fixed in some places (e.g. ArcanistGitAPI trying to run `git`), but not in ArcanistExternalLinter. Changes: * Fix `Filesystem::resolveBinary()` to actually only resolve executable files on Windows, and not any other files with no extension or with an extension listed in %PATHEXT%. Those files can be executed by typing their name in the cmd.exe shell, but not directly by low-level Windows functions, and we're using `'bypass_shell'` to bypass the shell. * Fix `ArcanistExternalLinter::getBinary()` to call `Filesystem::resolveBinary()` on Windows. Test Plan: Run `arc lint` on the Phorge repository while on Windows. Observe no errors related to jshint. Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: aklapper, avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15544 Differential Revision: https://we.phorge.it/D25341
This commit is contained in:
parent
25611ba24a
commit
16a412b108
2 changed files with 27 additions and 4 deletions
|
@ -1103,13 +1103,24 @@ final class Filesystem extends Phobject {
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
$stdout = head($stdout);
|
|
||||||
|
// These are the only file extensions that can be executed directly
|
||||||
|
// when using proc_open() with 'bypass_shell'.
|
||||||
|
$executable_extensions = ['exe', 'bat', 'cmd', 'com'];
|
||||||
|
|
||||||
|
foreach ($stdout as $line) {
|
||||||
|
$path = trim($line);
|
||||||
|
$ext = pathinfo($path, PATHINFO_EXTENSION);
|
||||||
|
if (in_array($ext, $executable_extensions)) {
|
||||||
|
return $path;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
} else {
|
} else {
|
||||||
list($err, $stdout) = exec_manual('which %s', $binary);
|
list($err, $stdout) = exec_manual('which %s', $binary);
|
||||||
}
|
|
||||||
|
|
||||||
return $err === 0 ? trim($stdout) : null;
|
return $err === 0 ? trim($stdout) : null;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -133,7 +133,19 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
|
||||||
* @task bin
|
* @task bin
|
||||||
*/
|
*/
|
||||||
final public function getBinary() {
|
final public function getBinary() {
|
||||||
return coalesce($this->bin, $this->getDefaultBinary());
|
$bin = coalesce($this->bin, $this->getDefaultBinary());
|
||||||
|
if (phutil_is_windows()) {
|
||||||
|
// On Windows, we use proc_open with 'bypass_shell' option, which will
|
||||||
|
// resolve %PATH%, but not %PATHEXT% (unless the extension is .exe).
|
||||||
|
// Therefore find the right binary ourselves.
|
||||||
|
// If we can't find it, leave it unresolved, as this string will be
|
||||||
|
// used in some error messages elsewhere.
|
||||||
|
$resolved = Filesystem::resolveBinary($bin);
|
||||||
|
if ($resolved) {
|
||||||
|
return $resolved;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return $bin;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
Loading…
Reference in a new issue