1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 00:42:40 +01:00
Commit graph

2146 commits

Author SHA1 Message Date
epriestley
ab589ab31d Restore "%d" support to "tsprintf()"
Summary: Ref T13490. I recently made "tsprintf()" more strict, but we do sometimes use "%d" and this is reasonable to support.

Test Plan: Ran "arc branch".

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21098
2020-04-13 04:35:59 -07:00
epriestley
21e80a635d Upgrade "arc download" to Toolsets
Summary:
Ref T13490. This is mostly straightforward.

  - Drop "--show" in favor of "--as -".
  - Drop support for 4+ year old "file.info" API.
  - Use modern stream-to-disk support so we get a real progress bar and don't need to buffer files into memory.

Test Plan: Downloaded various files, including large files.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21097
2020-04-12 16:18:03 -07:00
epriestley
076f7be484 Update "arc call-conduit" for Toolsets
Summary: Ref T13490. Fairly straightforward update.

Test Plan: Made various Conduit calls.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21096
2020-04-12 16:17:47 -07:00
epriestley
0f2e277cd9 Update "arc amend" for Toolsets
Summary: Ref T13490. Moves "arc amend" to Toolsets with modern ref/hardpoint code.

Test Plan: Ran "arc amend --show", "--revision", etc. Hit all the prompts and errors, probably?

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21095
2020-04-12 13:48:26 -07:00
epriestley
ff4c1e7c81 Add a "SymbolEngine" to support top-level ref resolution by symbol
Summary:
Ref T13490. I'm continuing to move toward modernizing "amend", which needs to load some objects by symbol at top level.

Add an API to support this.

Test Plan: Added an API caller to "arc inspect", ran it and hit the new code. Things seemed to work.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21094
2020-04-12 13:48:09 -07:00
epriestley
5fc50c226a Add some support code for printing refs to stdout
Summary: Ref T13490. Make terminal strings work more like HTML does in Phabricator, and make it easier to display refs.

Test Plan: Added some display code, ran `arc inspect` to hit it, saw a nice ref printed.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21093
2020-04-12 13:44:46 -07:00
epriestley
088b157444 Add ref lookup for username symbols
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
2020-04-12 13:42:51 -07:00
epriestley
1f18f25fa5 Add a "RevisionSymbolRef", revision commit messages, and make "--explore" recursive
Summary:
Ref T13490.

  - Support resolution of revision symbols into revision objects.
  - Align commit inspection better against commit symbols.
  - Make "arc inspect --explore" recursively load all object hardpoints.
  - Add a "commit message" hardpoint to "RevisionRef".

Test Plan: Used "arc inspect" to resolve commits and revisions. Used "--explore" to see the whole tree.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21091
2020-04-12 13:24:32 -07:00
epriestley
4cc05c377d Add a "CommitSymbolRef" for resolving symbolic commits into stable commit hashes
Summary:
Ref T13490. Frequently, we know the symbolic name of a commit (like "master") but need the immutable identifier for it (the commit hash).

Provide a Ref and Query for doing this lookup.

Test Plan: Ran `arc inspect symbol(...)` with various symbols, saw appropriate resolutions, nulls, or errors.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21090
2020-04-12 13:19:53 -07:00
epriestley
92be6df0eb Add a mode to "ExecFuture" that makes "resolvex()" semantics the default
Summary:
Ref T13490. "ExecFuture" can raise a "CommandException" when the subprocess fails. In most cases, this is desirable, since we usually do not expect subprocesses to fail.

Currently, "execx()" (which raises these exceptions) is the synchronous default (with the more verbose "exec_manual()" as an alternative if you expect the command may return an error code), but the Future variation has no way to hint that external resolution should raise an exception if the process exits with an error.

Since the "HardpointEngine" yield construct can resolve external futures, add a mode for this to simplify implementing "HardpointQuery" classes a bit. Without this, they either need to retain "$futures" and manually call "resolvex()", or check the "$err" result and throw some other exception, both of which are low-value boilerplate (queries that want to do this still can, of course).

This is basically like providing a hint that the futures should be resolved with "resolvex()" instead of "resolve()".

Also, make this the default behavior of a new "$api->newFuture()" wrapper.

Test Plan: Intentionally broke a command in a HardpointQuery, got a sensible exception automatically during external future resolution.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21089
2020-04-12 13:18:11 -07:00
epriestley
73f48aca74 Allow "loadHardpoints()" to accept a single ref and/or a single hardpoint
Summary: Ref T13490. There's more `array(...)` happening in this API than necessary. Sugar it slightly.

Test Plan: Grepped for "loadHardpoints()", ran a couple workflows.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21088
2020-04-12 13:17:41 -07:00
epriestley
6e24e10bdb Remove obsolete definitions of "defineHardpoints()" in older Ref objects
Summary: Ref T13490. A few older "defineHardpoint()" calls are sticking around. They no longer have callers; get rid of them.

Test Plan: Grepped for this symbol, no hits.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21087
2020-04-12 13:16:22 -07:00
epriestley
9a198ffcc5 Update "feature", "branch", and "bookmark" flows to report properly in "arc help"
Summary:
Ref T13490. These workflows are aliases of one another, which is a little silly, but currently all identify as "arc feature" in "arc help".

Straighten that out, at least.

Test Plan: Ran "arc help", "arc branch".

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21086
2020-04-12 13:16:07 -07:00
epriestley
ccd1ebb256 Port "arc prompts" from wilds and fix a path issue in shell completion
Summary:
Ref T13490. Bring "arc prompts" from "wilds" and hook it into the prompt in "arc shell-complete".

See D21069. Fix an issue where the shell hook tested for a path other than the path it writes to.

Test Plan: Ran "arc shell-complete" with no hook and got a prompt. Shell completed things. Ran "arc prompts shell-complete".

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21085
2020-04-11 10:43:05 -07:00
epriestley
387027eb3a Restore "arc alias" shell aliases
Summary:
See <https://discourse.phabricator-community.org/t/exception-when-trying-to-run-an-arc-alias-undefined-method-arcanistalias-getshellcommand/3731/>.

Shell aliases got lost in the shuffle of porting "arc alias"; restore them.

Test Plan: Bound `arc ls` to `ls -alh`, ran `arc ls`.

Differential Revision: https://secure.phabricator.com/D21084
2020-04-11 09:59:48 -07:00
epriestley
fff2fc8bc9 Remove "RefQuery" and all "HardpointLoader" code
Summary: Ref T11968. "RefQuery" is now "HardpointEngine". "HardpointLoader" is now "HardpointQuery".

Test Plan: Grepped for affected symbols.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21083
2020-04-11 06:46:53 -07:00
epriestley
dc42f51cf7 Reroute all RefQuery callers to HardpointEngine
Summary:
Ref T11968. "arc browse", "arc branch", and "arc diff" currently may execute into the RefQuery engine. Reroute them to the HardpointEngine.

This removes older-generation "Ref" objects and renames the replacement "RefPro" objects to "Ref".

Test Plan: Ran "arc branch", "arc browse <various things>", "arc diff", searched for affected symbols.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21082
2020-04-11 06:46:23 -07:00
epriestley
9e72e4ed1d Bring "pro" browse queries from modern hardpoint code
Summary: Ref T11968. Merge the modern hardpoint queries and refs for the "browse" workflow as "pro" variations.

Test Plan: Ran `arc inspect browse(...)` for objects, paths, commits, and revisions.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21080
2020-04-10 08:05:24 -07:00
epriestley
8bb81217d5 Bring a "pro" WorkingCopyState ref to "master"
Summary:
Ref T11968. Continue bringing modern yield-based hardpoint code into "master" in the parallel "Pro" classtree.

Adds "working-copy(commit-hash)" as an inspectable ref.

Test Plan: Inspected working copy refs, saw them resolve revisions by commit hash and commit message.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21079
2020-04-10 06:16:37 -07:00
epriestley
adea2550f5 Introduce "arc inspect" and some of the new ref/hardpoint classes
Summary:
Ref T11968. Inches toward the new ref/hardpoint code by introducing the modern refs as "RefPro" objects and supporting an "arc inspect <object>" to load objects and hardpoints.

This doesn't impact any existing runtime behavior.

Test Plan: Ran "arc inspect [--all] commit(...)", got hardpoint queries and yield-based data fetching.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21078
2020-04-10 05:01:25 -07:00
epriestley
92b29f53f3 Rename "getWorkingCopy()" to "getWorkingCopyIdentity()" in Arcanist
Summary:
Depends on D21074. Ref T13490. Ref T11968. Before toolsets, Arcanist has a "WorkingCopyIdentity" object. After toolsets, it has a "WorkingCopy" object.

Most workflows don't access either one, so make a slightly-breaking API change to simplify the transition.

  - "getWorkingCopy()" now returns a modern object (a "WorkingCopy").
  - "getWorkingCopyIdentity()" now returns an older object (a "WorkingCopyIdentity").

This isn't backward-compatible, but out-of-date code should get an explicit failure with clear resolution steps.

Test Plan: Ran "arc lint", "arc branch", and a few other commands. Grepped for "getWorkingCopy()".

Maniphest Tasks: T13490, T11968

Differential Revision: https://secure.phabricator.com/D21075
2020-04-10 04:24:29 -07:00
epriestley
391c164313 Trivially update "arc branch/feature" and "arc browse" for Toolsets
Summary:
Ref T11968. Ref T13490. These are the workflows which currently use the intermediate-level hardpoint code (which made hardpoints formal, but didn't do the yield stuff).

Move toward updating them by doing some basic bookkeeping, with a few compatibility adjustments to the parent Workflow class.

Test Plan: Ran "arc branch" and "arc browse" with various arguments.

Maniphest Tasks: T13490, T11968

Differential Revision: https://secure.phabricator.com/D21074
2020-04-10 04:23:03 -07:00
epriestley
f56c6bde2b Revert "Compress requests from the Conduit client to Phabricator"
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
2020-04-09 12:25:18 -07:00
epriestley
7e25288f49 Compress requests from the Conduit client to Phabricator
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
2020-04-08 09:24:54 -07:00
epriestley
deb72c37db Remove onboard future bulk-resolution from ConduitEngine
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
2020-04-08 09:24:37 -07:00
epriestley
85141c4d90 Add new "Hardpoint" classes to support request parallelization
Summary:
Depends on D21070. Ref T11968. Adds "yield"-aware query classes for parallelizing service calls.

These will replace the similar (but not yield-aware) "Hardpoint" classes introduced previously. This is an API change but most of the old classes still exist and still do the same thing, just with more "yield" statements.

This just adds a bunch of new code with no callers and no API updates.

Test Plan: See future changes.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21071
2020-04-08 09:23:50 -07:00
epriestley
0b3cd39230 Update the "WorkingCopy" API and create a fallback "Filesystem" working copy
Summary:
Ref T11968.

  - Allow "WorkingCopy" objects to maintain an API object and update callers ("get...()" instead of "new...()").
  - Always generate a WorkingCopy object and a RepositoryAPI object.

Currently, code has to look like this:

```
$working_copy = ...
if ($working_copy) {
  $repository_api = ...
  if ($repository_api [instanceof ... ]) {
```

This is clunky. There's also no reason some "arc" commands can't run outside a VCS working directory without special-casing how they interact with the filesystem.

Conceptually, model the filesystem as a trivial VCS (which stores exactly one commit, always amends onto it, and discards history). Provide a trivial WorkingCopy and API for it.

(This change isn't terribly interesting on its own, but chips away at landing the new Hardpoint infrastructure.)

Test Plan: Ran `arc version`, `arc upgrade`.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21070
2020-04-08 09:22:43 -07:00
epriestley
a1ee2ab931 Fix improper XHPAST parsing of namespace grammar like "use x as private;"
Summary:
Depends on D21067. Ref T13492. Converting unit tests to be readable exposed this error in the grammar.

Normally, "grammar_rule" rules emit a standalone Node. In this case, the bottom-level grammar rule is a collection of trivial rules and the callers configure the Node. Wrap the bottom-level rule in a configuration rule so the node is configured correctly and consistently, in exactly one place.

Test Plan: Ran unit tests.

Maniphest Tasks: T13492

Differential Revision: https://secure.phabricator.com/D21068
2020-04-07 14:35:42 -07:00
epriestley
e03431def8 Fix XHPAST parsing of variadic calls
Summary:
Depends on D21066. Ref T13492. The switch of unit test data to stable/readable output exposed this bug in parsing of variadic calls: some nodes are not given types properly.

Fix the parser and update the test.

Test Plan: Ran the test, which now works.

Maniphest Tasks: T13492

Differential Revision: https://secure.phabricator.com/D21067
2020-04-07 14:33:37 -07:00
epriestley
a689dee228 Update XHPAST "expect" test blocks to the new stable, human-readable format
Summary:
Depends on D21065. Ref T13492. Swap existing "expect" blocks from unstable, unreadable JSON to readable, stable trees.

(There are two "INVALID TYPE" outputs which this update effectively detects and which future changes correct.)

Test Plan: Ran "arc unit --everything", got a clean build.

Maniphest Tasks: T13492

Differential Revision: https://secure.phabricator.com/D21066
2020-04-07 14:32:48 -07:00
epriestley
8a7ce97b51 Make XHPAST unit test "expect" blocks stable and human-readable
Summary:
Depends on D21064. Ref T13492. Earlier, see D17819. This is essentially the same change, although I inlined the token stream into the node list.

This intentionally breaks most tests since it just has the new "expect" generator; the next change will fix them by swapping the test bodies.

Test Plan: Ran "arc unit --everything" after the next change (which fixes all the tests), got a clean build. This change on its own fails all existing XHPAST tests since the block formats don't match.

Maniphest Tasks: T13492

Differential Revision: https://secure.phabricator.com/D21065
2020-04-07 14:31:56 -07:00
epriestley
6d15c6ea48 Remove duplication of XHPAST version in PHP and C code
Summary:
Depends on D21063. Ref T13492. Currently, XHPAST defines a version in both PHP code and C code, and they must be kept in sync.

Switch to a single definition in PHP, then carry it through the build pipeline into C.

Test Plan: Did a clean rebuild of XHPAST, saw a version number carried in from PHP. Ran "xhpast --version".

Maniphest Tasks: T13492

Differential Revision: https://secure.phabricator.com/D21064
2020-04-07 14:31:11 -07:00
epriestley
763ac445dc Revert xhpast changes that impacted builds under Bison 2.3
Summary:
Fixes T9753. Changes some time ago (in D13970 + D13974) improved XHPAST build compile-time warning behavior under Bison 3.

However, macOS still ships with Bison 2.3 and these changes prevent XHPAST from building with Bison 2.3. The changes didn't introduce version detection, so Bison 2.3 builds fail somewhat mysteriously without obvious next steps.

It's relatively easy to install Bison 3 on macOS via Homebrew, but the Bison 3 changes aren't terribly substantive and XHPAST doesn't actually depend on any Bison 3 features, so just return to Bison 2.3 for now.

It would be reasonable to undo this again and retarget Bison 3 in the future, but ideally we should wait until macOS ships with Bison 3 or we have a specific reason to bump the minimum required version to 3. If/when we do, we should version-detect Bison and raise a clear error message.

Test Plan: Built xhpast under Bison 2.3 on a default macOS install using "make cleanall && make install".

Maniphest Tasks: T9753

Differential Revision: https://secure.phabricator.com/D21063
2020-04-07 14:29:19 -07:00
epriestley
33dc2fe819 Allow "phage" to print execution status on SIGINT
Summary: Ref T13490. The new Arcanist runtime supports workflow signal handling, but Phage isn't quite able to make use of it. Clean up the last few pieces so it can work.

Test Plan: Ran "phage", hit ^C, got status information.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21060
2020-04-06 11:09:43 -07:00
epriestley
32005f26a4 Move Phage to FuturePool
Summary: Ref T11968. Phage has another "sustained pool of Futures" use case, and needs some slight adjustments after Future API changes.

Test Plan: Ran `bin/phage status ...`, got a clean result instead of a JSON decoding failure.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21058
2020-04-05 05:56:21 -07:00
epriestley
099c2ae648 Introduce "FuturePool" to make it easier to manage an ongoing pool of futures
Summary:
Ref T11968. "FutureIterator" recently became non-rewindable, and starting a Future twice is now an error.

This complicates a handful of use cases where a mostly-constant pool of futures is maintained over a long period of time, notably in daemon overseers and repository pull daemons.

They previously relied on being able to do "new FutureIterator($futures)" to continue resolution of a list of futures from any state. This no longer works quite like it used to, since Futures generally may not belong to more than one iterator now (and this property is desirable).

Introduce "FuturePool", which maintains exactly one iterator but manages the small amount of glue around adding and removing Futures from it, destroying it if the pool empties, and rebuilding it if the pool fills.

Test Plan: See next change, which makes use of this.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21053
2020-04-03 12:05:09 -07:00
epriestley
368aec16a1 Update some ancient "set X=Y" environment code for new Windows execution without a shell
Summary:
Depends on D21051. Ref T13504. Ref T13209. This very old Mercurial code uses "X=Y hg ..." on Linux and "set X=Y & hg ..." on Windows.

The latter construct no longer works because we bypass the shell. The former construct is obsolete.

Additionaly, delete some ancient "branch merge" code which has no callers.

Test Plan: Created a diff in a Mercurial repository on Linux. I minimally vetted this on Windows since I don't have a "hg + Windows" environment at the moment.

Maniphest Tasks: T13504, T13209

Differential Revision: https://secure.phabricator.com/D21052
2020-04-02 13:50:57 -07:00
epriestley
d4d095dbf6 Make Windows escaping preserve "%" symbols in arguments
Summary:
Ref T13504. Ref T13209. We currently use "escapeshellarg()" under Windows, which destructively replaces "%" symbols with spaces. This is wrong, breaks general behavior with "git log --format=...", and has led to a lot of weird workarounds in `arc` code where we don't escape arguments we should be escaping to tiptoe around issues with "%".

Now that execution occurs via "bypass_shell", we can safely (probably?) escape things, although programs are still apparently free to parse the command string however they want.

Test Plan: Added unit tests, ran them under Mac and Windows, got clean results. Ran "arc version" on Mac and Windows.

Maniphest Tasks: T13504, T13209

Differential Revision: https://secure.phabricator.com/D21051
2020-04-02 13:44:13 -07:00
epriestley
5ce1d79717 Fix error behavior of "arc version" when it encounters a library which is not a working copy
Summary:
Ref T13504. The API has changed here slightly, and if you run "arc version" without "arcanist/" being a Git working copy, it currently fatals in a misleading way.

Instead, reach the error properly.

Test Plan: Ran "arc version" after moving aside ".git/", got a helpful error message instead of a confusing "call on null" exception.

Maniphest Tasks: T13504

Differential Revision: https://secure.phabricator.com/D21050
2020-04-02 08:24:22 -07:00
epriestley
63276697eb Fix three Windows subprocess issues
Summary:
Fixes T13504. This fixes three issues:

  # In ExecFuture, "proc_open()" on an invalid binary could fail with an unconditional exception.
  # In ExecPassthru, "proc_open()" on an invalid binary could fail with an unconditional exception.
  # In "arc browse", "start <url>" does not work when the shell is bypassed.

In (1) and (2), the desired behavior is to fail with an exit code which is sometimes upgraded to an exception depending on calling convention.

Issue (1) most commonly manifested as "find" failing when run via "cmd.exe".

Issue (2) most commonly manifested as "arc browse" failing.

Issue (3) was entangled with issue (2).

In cases (1) and (2), assume "proc_open()" failures under Windows are because of bad binaries and treat them like bogus commands on Linux/Mac.

In case (3), use "cmd /c start" instead of "start" as a default browser on Windows.

Test Plan:
  - On Windows, did mime type detection in cmd.exe. Before patch: proc_open() exception in "find". After patch: clean (albeit not terribly useful) detection.
  - On Windows, did "arc browse ...". Before patch: proc_open() exception in "start". After patch: clean browser execution.

Maniphest Tasks: T13504

Differential Revision: https://secure.phabricator.com/D21047
2020-04-01 16:11:05 -07:00
epriestley
492113370a Fix two issues with Future key selection inside FutureIterator
Summary:
Ref T11968.

  - Rekeying natural lists of futures creates more problems than it solves. For now, don't try to deal with this.
  - The scope of the future autokey was incorrect (per subclass, not per process).

Test Plan: Ran various future-oriented flows on new HardpointEngine code.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21046
2020-04-01 10:36:44 -07:00
epriestley
b1a712add8 Integrate "ServiceProfiler" into the base "Future"
Summary:
Depends on D21036. Ref T11968. Ref T13177. Currently, each Future integrates separately with ServiceProfiler, but much of the code is similar.

Move integration to the base class and lift up most of the implementation details.

Test Plan: Ran `arc diff --trace`, saw sensible output.

Maniphest Tasks: T13177, T11968

Differential Revision: https://secure.phabricator.com/D21038
2020-03-30 07:42:40 -07:00
epriestley
cb80f69715 Make "FutureIterator" queue management more formal
Summary: Depends on D21035. Ref T11968. This allows a "FutureIterator" to hold futures which are blocked because of unresolved dependencies, and makes the resolution process more structured.

Test Plan: Ran unit tests, created this revision.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21036
2020-03-30 07:42:09 -07:00
epriestley
6b75562c3e Make "exception" on Future a private property
Summary: Depends on D21034. Ref T11968. Continue modernizing Future.

Test Plan: Ran tests, created a revision.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21035
2020-03-30 07:34:51 -07:00
epriestley
4d55067fd8 Make the "result" property on Future private
Summary: Depends on D21033. Ref T11968. This is just an incremental step in modernizing Future and making it more robust. Currently, subclasses are expected to write directly to `$this->result`, but this isn't consistent with how modern classes generally work.

Test Plan: Ran unit tests, created this revision.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21034
2020-03-29 10:18:26 -07:00
epriestley
e20dce875c Resolve all futures inside FutureIterator
Summary:
Depends on D21032. Ref T11968. Currently, "Future" and "FutureIterator" can both resolve futures.

Treat "Future->resolve()" as sugar on resolving an iterator of size 1.

Test Plan: Ran tests, created this revision.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21033
2020-03-29 10:18:06 -07:00
epriestley
3df48c9257 Remove the "timeout" parameter from "Future->resolve()"
Summary:
Ref T11968. This future-level parameter has no nontrivial callers and makes the "fate of FutureGraph" changes more difficult.

Callers that are genuinely interested in this behavior can wrap the Future in a FutureIterator and use "setUpdateInterval()" to get the same behavior.

Test Plan: Grepped for "resolve()" and "resolvex()", updated callers.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21031
2020-03-29 10:13:59 -07:00
Paul Tarjan
33b9728b5f Run ls-files from the root of the directory
Summary:
One of our users ran into an issue where they were running `arc diff` in
a subdirectory and it so happened that in that directoy there wasn't any `lfs`
files so the repo was marked as `$is_lfs == false`. Lets allow `arc` to be run
from anywhere.

Test Plan: Internally this patch worked

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D21045
2020-03-25 14:23:53 -07:00
epriestley
4c12c3119b Treat all PHP language-level errors as exceptions by default
Summary:
Ref T13499. Currently, we throw most language errors as exceptions, but this list isn't exhaustive and errors we don't specifically make more severe are allowed to slip through.

This is generally undesirable, particularly in the case of "Undefined index:" errors. See T13499 for a specific case where this caused behavior to be more difficult to understand and diagnose than it should have been.

Make this the default behavior instead, except for "E_USER" errors, which we never expect to arise from first-party code.

This may be slightly too aggressive, but future changes can selectively reduce the severity of some types of errors if problems arise.

Test Plan:
  - Executed code which intentionally accessed an undefined index, got an exception.
  - Poked around Phabricator and Arcanist without any further issues cropping up, but I don't have a good way to develop confidence that the "reduced severity" list should genuinely be empty.

Maniphest Tasks: T13499

Differential Revision: https://secure.phabricator.com/D21044
2020-03-22 12:41:05 -07:00
epriestley
18799c1829 Switch file uploader in "arc diff" to use ConduitEngine
Summary: Fixes T13498. Currently, `arc diff` may invoke the file upload flow using an older variation of the API. Both the modern and fallback clients build an Engine, so switching to the modern API is trivial.

Test Plan:
  - Created a local commit with a 1MB binary file.
  - Ran `arc diff --only`.
  - Before patch: fatal on old API call.
  - After patch: clean upload.

Maniphest Tasks: T13498

Differential Revision: https://secure.phabricator.com/D21043
2020-03-20 12:19:17 -07:00