Currently, we expect this to be built only by Phabricator developers. Are you using it outside of the context of Phabricator itself? I'm hesitant to put developer-only commands as main workflows: it makes arc help harder to deal with for regular users, and over time will give us the git command problem if taken to an extreme.

The error message that arc emits telling you to build xhpast should have the absolute path to deal with the case of multiple libphutils, although this is easy to gloss over.

Maybe we should try to run the build from arc, and only emit the message if it fails?

Correct me if I am wrong here, but it's not true that the build_xhpast.sh script is a developer only feature. Any user of arc who at some stage performs at arc upgrade will possibly have to deal with this script.

I agree with you with regards to arc help which was my motivation for having shouldShellComplete() return false. From memory, this hides the workflow from arc help.

I think that automating the script within arc itself would be a better solution. i.e. instead of instructing users to run build_xhpast.sh, try to do it automatically.

Hmm, arc upgrade should not depend upon xhpast. Do you have the trace where it needed it?

Sorry, I meant that after performing arc upgrade (i.e. after upgrading libphutil), it may be necessary to run build_xhpast.sh, not during arc upgrade. This would be the case if the user wishes to perform arc lint with the XHPAST linter after they have upgraded.

Right -- are you using the xhpast linter for something other than developing Phabricator?

Yes, we use this linter on our own code base, which seems to be a reasonable use case.

To step back, doesn't this solution suffer from the same problem? That is, when we say "run: arc build-xhpast", we have no guarantee that arc (in your path) is the same arc currently running, so it may build a different copy of xhpast. If we emit an absolute path to arc, it's just as easy to miss as the current absolute path to libphutil.

I think the automated solution is better in general, and we should give it a shot and see how it works. Specifically:

• before emitting this message, try to run the command;
• if the command fails, emit the message as we do now (maybe tailoring it more specifically to point out that the absolute path may be important).

...and see how far that gets us?

Yeah, ok... that sounds entirely reasonable. Let me give it a shot.