Summary:
Ref T13658. Remove all product name literals from "pht()" strings, by replacing them with generic text where that feels reasonably natural, or "PlatformSymbols" calls elsewhere.
These calls were identified with `arc lint --everything` after enabling the lint rule in D21763.
Test Plan: Read strings, ran "arc".
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21764
Summary: Ref T13582. When this code is reached with a raw HTTP exception, it currently fatals.
Test Plan:
- Ran `arc branches --conduit-uri=http://example.org` (a bad Conduit URI).
- Before: hard fatal with a bad method call.
- After: non-Conduit exception raised to user. Not ideal, but a step forward.
Maniphest Tasks: T13582
Differential Revision: https://secure.phabricator.com/D21467
Summary: Ref T13581. This message can be slightly more helpful in some cases by showing which method call failed.
Test Plan: Ran `arc branches` under the error condition in D21462, got a more useful error.
Maniphest Tasks: T13581
Differential Revision: https://secure.phabricator.com/D21463
Summary:
Correct two minor Conduit future issues:
- FutureAgent shows up in the trace log as "<mystery>". Since it isn't doing anything useful, solve the mystery and drop it from the log.
- Simply the ConduitFuture code for interacting with the service profiler now that a more structured integration is available.
Test Plan: Ran "arc branches --trace", saw conduit calls and no more "<mystery>" clutter.
Differential Revision: https://secure.phabricator.com/D21388
Summary:
See PHI1802. After D21384, "arc land" and similar with no credentials now properly raise a useful exception, but it isn't formatted readably.
Update the display code to make it look prettier.
Test Plan: Ran "arc land" with no and invalid credentials, got properly formatted output.
Differential Revision: https://secure.phabricator.com/D21387
Summary:
See PHI1802. Currently, we can't raise a "you must login" error in a generic way at the beginning of a workflow because we don't know if a workflow needs credentials or not.
For example, "arc help" does not need credentials but "arc diff" does.
Additionally, some actual Conduit calls do not need credentials ("conduit.ping", "conduit.getcapabilities") and others do.
Although I'd like to simplify this eventually and move away from anonymous/unauthenticated "arc", this isn't trivial today. It's also possible for third-party code to add authenticated calls to "arc help", etc., so even if we could execute these tests upfront it's not obvious we'd want to.
So, for now, we raise "you must login" at runtime, when we receive an authentication error from Conduit.
This got implemented for Toolsets in a well-intentioned but not-so-great way somewhere in wilds/experimental, with an "ArcanistConduitCall" that behaves a bit like a future but is not really a future. This implementation made more sense when ConduitEngine was serving as a future engine, and FutureProxy could not rewrite exceptions.
After the Toolsets code was first written, ConduitEngine has stopped serving as a future engine (this is now in "HardpointEngine"). Since HardpointEngine needs a real future, this "show the user a login message" code gets bypassed. This results in user-visible raw authentication exceptions on some workflows:
```
[2020-06-30 21:39:53] EXCEPTION: (ConduitClientException) ERR-INVALID-SESSION: Session key is not present. at [<arcanist>/src/conduit/ConduitFuture.php:76]
```
To fix this:
- Allow FutureProxy to rewrite exceptions (see D21383).
- Implement "ArcanistConduitCall" as a FutureProxy, not a future-like object.
- Collapse the mixed-mode future/not-quite-a-future APIs into a single "real future" API.
Test Plan:
- Created a paste with "echo hi | arc paste --".
- Uploaded a file with "arc upload".
- Called a raw method with "echo {} | arc call-conduit conduit.ping --".
- Invoked hardpoint behavior with "arc branches".
- Grepped for calls to either "resolveCall()" method, found none.
- Grepped for calls to "newCall()", found none.
- Grepped for "ArcanistConduitCall", found no references.
Then:
- Removed my "~/.arcrc", ran "arc land", got a sensible and human-readable (but currently ugly) exception instead of a raw authentication stack trace.
Differential Revision: https://secure.phabricator.com/D21384
Summary:
See PHI1735. "ConduitEngine" was once a future pool, but this has moved to "HardpointEngine". This class may no longer make much sense.
In Phacility code, "bin/host upload" depends on using the Uploader, which needs a "ConduitEngine", not a "ConduitClient". This workflow may use asymmetric key signing, which "ConduitEngine" does not support.
To unblock PHI1735, provide glue code between "Client" and "Engine". But a "more correct" change is probably removal of "Engine".
Test Plan:
- Ran `bin/host upload`, uploaded files (with additional changes to wrap the Client).
- Created this revision.
Differential Revision: https://secure.phabricator.com/D21260
Summary:
Ref T13507. For various messy reasons we can't blindly assume the server supports "gzip" -- but if the server tells us it does, we're on firmer ground.
If the server returns an "X-Conduit-Capabilities: gzip" header and we have compression support locally, compress subsequent requests.
This restores D21073, which was reverted by D21076.
Test Plan: With a gzip-asserting server, added debugging code and ran various "arc" commands. Saw the 2nd..Nth calls hit compression code.
Maniphest Tasks: T13507
Differential Revision: https://secure.phabricator.com/D21119
Summary: Ref T13490. More-or-less straightforward upgrade to modern calls.
Test Plan: Created and viewed pastes.
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21104
Summary:
Ref T13490. I'm attempting to update "arc amend", but it needs to fetch revision authors to raise an "amending a revision you don't own" error.
Support user-symbol resolution.
Along the way, this introduces some infrastructure for abstracting away iteration over a multi-page Conduit result set.
Test Plan:
- Ran "arc inspect ..." for various "user(...)" queries.
- Set page size to 3 and issued a general query, saw the future page it away properly.
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21092
Summary:
This reverts commit 7e25288f49. See T13507 for discussion, shortly.
This needs to be switched to capability detection because some server configurations can not actually accept these requests.
Test Plan: Straight revert.
Differential Revision: https://secure.phabricator.com/D21076
Summary: Ref T13507. Enable compression in the body of Conduit requests if we have client support for it; we expect the server should always support it.
Test Plan: Created this revision, ran random "arc" commands that use Conduit. Added debugging code, saw payload size drop.
Maniphest Tasks: T13507
Differential Revision: https://secure.phabricator.com/D21073
Summary:
Depends on D21071. Ref T11968. Currently, "ConduitEngine" tries to lightly parallelize futures. This was a compromise when the initial "hardpoint" change didn't plan to pursue real request paralleization.
Now that the newer hardpoint change does, we don't need onboard resolution in ConduitEngine. Throw it away.
When the engine is supposed to resolve a future, it now just resolves that future on its own. This should be functionally identical to the previous behavior, except that it may be slower.
(In practice, because HTTP futures are backed by an internal cURL request pool, this proably has little effect anywhere. Moving to modern hardpoints will make performance no worse than it was prior to this change, in any case.)
Test Plan: Ran various modern "arc" commands.
Maniphest Tasks: T11968
Differential Revision: https://secure.phabricator.com/D21072
Summary: Ref T13490. This largely makes "arc upload" work, although the Future stuff is still a bit wonky. See T11968.
Test Plan: Ran "arc upload README.md", got an upload.
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21030
Summary:
Depends on D20988. Ref T13395. Ref T13098. Ref T13203.
This brings all the "toolsets" code into "master". We try to run commands as toolsets commands first, then fall back to older code.
Since the "toolsets" class tree is mostly parallel to the older class tree, this isn't completely broken. Currently, all commands fall back.
Test Plan: Created this diff, ran various other commands. But this is probably a long shot from finished.
Maniphest Tasks: T13395, T13203, T13098
Differential Revision: https://secure.phabricator.com/D20990
Summary:
Depends on D20986. Ref T13395. This //mostly// collapses the entire "experimental" branch into "master".
I plan to change the "Ref/Hardpoint" pattern to become future oriented, but this is more steps forward than sideways.
Test Plan: Ran various `arc` workflows.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20987
Summary: Ref T13395. Moves all remaining code in "libphutil/" into "arcanist/".
Test Plan: Ran various arc workflows, although this probably has some remaining rough edges.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20980
