There is no need to check if the XHPAST binary is available here because this linter does not actually parse PHP, it only considers the library map.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rARC24ab7b7f38e3: Allow `ArcanistPhutilLibraryLinter` to run with an out-of-date XHPAST binary
Removed the XHPAST binary and ran arc lint on a bunch of files.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Oh, the rationale is that we would build XHPAST later anyway. Or may I'm wrong, I'll take a closer look shortly.
Comment Actions
OK, I'm not crazy (I think).
I think that this still makes sense. Specifically, this linter doesn't need to parse PHP at all... it only cares about the library map.
| src/lint/linter/ArcanistPhutilLibraryLinter.php | ||
|---|---|---|
| 74 | Something invokes XHPAST later on, or this doesn't make sense (and I'm like 99% sure the builder invokes it). But if that also builds the binary (which I think it does) then removing this is fine. I think the only reason not to remove this explicit check is if the build failure deep in the stack is much harder to figure out? Does it come out with just some extra stack stuff, or is it like 5 errors wrapping one another as we end up several CommandExceptions deep? | |
| src/lint/linter/ArcanistPhutilLibraryLinter.php | ||
|---|---|---|
| 74 | It's not too bad: Exception
Some linters failed:
- ArcanistPhutilLibraryLinter: CommandException: Command failed with error #1!
COMMAND
'/home/joshua/workspace/github.com/phacility/libphutil/scripts/build_xhpast.sh'
STDOUT
(empty)
STDERR
derp
(Run with `--trace` for a full exception trace.) | |