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

12 commits

Author SHA1 Message Date
epriestley
df2c1ba912 [Wilds] Provide a skeleton for prompt behaviors
Summary:
Ref T13098. Ref T13198. Ref T12996. The major ideas here are:

Workflows must define a list of all the prompts they can raise, so that these prompts can be enumerated with `arc prompts <workflow>`.

Prompts themselves should respond properly to ^C (abort immediately) and we should be able to make them nonblocking in the future (particularly, we'd like to be able to continue reading bytes from subprocess buffers while the prompt is shown on screen).

This doesn't have a lot of fancy features yet (non-confirm prompts, default yes, prompts which don't abort on "N", etc) but those should be easy to add later.

In the future, you'll be able to configure a default answer to prompts either in a config file or at runtime with `--config prompts=x.y.z=N` or similar.

This removes the history/readline mode for prompts, where you could use the up arrow to cycle through older responses. I believe this was only really useful for "excuse" prompts and intend to remove those.

Test Plan:
Forced `arc shell-complete` to always prompt, then:

  - Got prompted, answered "y", "n", "N", "", "quack". Got sensible behavior in all cases.
  - Ran `echo | arc shell-complete`, got a TTY error.
  - Ran `arc prompts`, `arc prompts shell-complete`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098, T13198, T12996

Differential Revision: https://secure.phabricator.com/D19706
2018-09-26 14:08:49 -07:00
epriestley
a62c1d70db [Wilds] Handle SIGINT (^C) in ArcanistRuntime in a more formal way
Summary:
Ref T13098. Add ^C handling and some small bits:

  - Update `arc weld`.
  - Test that `arc weld filen<tab>` completes `filename` (it does).
  - Add a "workflow stack" -- I plan to make it easier for `arc diff` to call `arc unit` / `arc lint` as formal sub-workflows, etc., and make "workflow X delegates to workflow Y" a more formal thing.

On interrupts:

  - Workflows can do something when you press ^C.
  - If they do, press ^C twice quickly to exit.
  - Otherwise, we exit on the first ^C.

The major thing I'd like to do in the short-ish term is to make `phage` report status on interrupt, but some other workflows might make sense to have interrupt handlers (maybe long-running stuff like `arc upload` / `arc download`) and third parties may find creative uses for them.

Test Plan:
- Added some `sleep(...)` to WeldWorkflow.
- Interrupted, got program exit.
- Added interrupt handlers and interrupted, got interrupt handling.
- With interrupt handlers, interrupted twice. Got program exit.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19703
2018-09-26 14:07:25 -07:00
epriestley
b6f93a46d7 [Wilds] Shove the logging stuff into a bit of an abstraction before it gets out of control
Summary:
Ref T13098. The logging stuff is starting to get a little sketchy, so wrap it in several layers of class-based indirection before it can escape its cage.

This at least gives us an actual API and structure to work with later instead of scattering a lot of `fprintf(STDERR, ....)` all over the place.

Test Plan: Ran a few commands, got slightly more sensible/consistent logging out of them.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19699
2018-09-25 15:59:44 -07:00
epriestley
e6c37bd4b3 [Wilds] Make "arc call-conduit ..." call Conduit methods
Summary:
Ref T13098. This repairs Conduit integration. Conduit was meaningfully updated in the `experimental` branch so a lot of this is just deleting code I don't plan to support going forward.

This removes "conduit.timeout", "http.basicauth.user" and "http.basicauth.pass". I believe these were all crazy niche calls with essentially no legitimate use. We could provide extension support if anyone actually uses this stuff, now.

Fixes an old "phutil" reference in HTTPSFuture.

Builds Conduit engine support into `ArcanistWorkflow`. There's perhaps some argument for trying to not make this core, but every upstream thing we'll ever write probably wants it (`arc`, `phage`) and there's not much of a cost to making it core. Even non-core stuff may include extensions which expect Conduit support (for example, for reporting workflow metrics to Phabricator).

There's no authentication support yet, I'm planning to update "hosts" config handling next.

Test Plan:
```
$ echo '{}' | arc call-conduit conduit.ping
 WARNING  Ignoring unrecognized configuration option ("hosts") from source: User Config File (/Users/epriestley/.arcrc).
 WARNING  Ignoring unrecognized configuration option ("load") from source: Project Config File (/Users/epriestley/dev/core/.arcconfig).
{
  "error": null,
  "errorMessage": null,
  "response": "secure001.phacility.net"
}
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19698
2018-09-25 15:58:29 -07:00
epriestley
5e70719306 [Wilds] Continue toward a generalized "arc alias" workflow
Summary:
Ref T13098. This leaves a lot of rough edges but nothing is overtly broken so here's where we're at so far:

Config sources get "scopes", like user configuration vs system configuration. The major reason for this is so that `arc set-config x y` can know where it's supposed to write. This is generalized enough that we can implement `arc set-config --system ...` and `arc alias --local ...` and so on relatively easily later, although scopes themselves are not modular (a third-party can't add a new type of config scope). Maybe we'll modularize this some day but it felt like that's probably YAGNI/overboard since we have no current use cases. For now, a source does not need to belong to any particular scope.

Config may be writable (like user config in `~/.arcrc`) or nonwritable (like `--config` flags). Writable config can now specify how to write to disk. Config files can actually write to disk now, although the only pathway for doing this that exists is via `arc alias`.

Aliases now parse properly and can write to disk. `arc alias` now lets you define aliases, and writes them to disk. **The first time you do this, your `~/.arcrc` file will be rewritten into a format which old `arc` can not read!** It's relatively easily to unmangle/repair these files so I'm planning to just let this happen.

When a toolset is invoked, it now reads and evaluates aliases. Aliases have a lot of new guard rails like suggesting the user try `arc draft` if they type `phage draft`, allowing alias chains, detecting cycles, and limiting chain length.

Workflows can provide help and argument lists in a more structured way. I've moved this to sub-objects: help is now on `WorkflowInformation` (instead of a bunch of different `getHelp()`, `getSynopsis()` methods) and arguments now have a `WorkflowArgument` object instead of a dictionary. I think this pattern is generally better for extending: it lets us add and change stuff with less impact (and greater explicitness) down the road.

`arc alias` now has reasonable help text and argument documentation. The `arc alias` (list) and `arc alias x` (details/remove) flows don't work yet but `arc alias x y` does.

`arc liberate` now uses the new help/argument stuff, although the help needs more beef eventually. I pruned a bunch of long-obsolete or questionable flags and renamed `--all` to `--clean` since `--all` sounds like "liberate all libraries", which is now the default behavior of `arc liberate`.

Test Plan:
You can now define chains of aliases. Finally!

```
$ arc draft4
 WARNING  Ignoring unrecognized configuration option ("hosts") from source: User Config File (/Users/epriestley/.arcrc).
 WARNING  Ignoring unrecognized configuration option ("load") from source: Project Config File (/Users/epriestley/dev/core/.arcconfig).
 ALIAS  arc draft4 -> arc draft3
 ALIAS  arc draft3 -> arc draft2
 ALIAS  arc draft2 -> arc diff
Usage Exception: Unrecognized argument '--draft'.
```

This also works now:

```
$ phage alias deploy-secure -- deploy --hosts secure001-4 --limit 1
```

General!

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19697
2018-09-25 15:57:22 -07:00
epriestley
c64f86c2f6 [Wilds] Flesh out most of the new Config objects
Summary:
Ref T13098. This is pretty rough, but sketches out the general shape of a more modern configuration flow. The new flow is very similar to the Phabricator flow.

Each configuration option is typed (string, bool, list of dictionaries, etc) so we can typecheck it, and each type is a class so the types are modular and can do fancy stuff. Some of this "fancy stuff" that I want to do includes:

  - transparently rewriting/reformatting various options for modernness/consistency;
  - having some options exposed as objects instead of raw JSON values (in particular, aliases);
  - merging "list" options (like "aliases") in a modular way instead of by having hard-coded stuff that says "this particular option is magic gets merged instead of getting replaced when defined in multiple places".

Generally, this makes everything modular and extensible and gets rid of the hard-coded `switch (...)` stuff.

Test Plan: Ran `arc get-config`, it sort of almost worked.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19695
2018-09-21 16:57:57 -07:00
epriestley
412484022b [Wilds] Prepare for more modular configuration management
Summary:
Ref T13098. This is kind of a catch-all diff with stuff that didn't fit in prior diffs, and which fixes some bugs with that stuff now that I made it at least sort of reachable.

Beyond bugs, the general idea is to replace `ConfigurationManager` (a big class which knew about config-end-to-end) with a more modern/modular `ConfigurationEngine` using the standard Engine + EngineExtension modularity pattern.

Configuration becomes a `ConfigurationSourceList` of `ConfigurationSource` objects, each of which represents one source (a config file, `--config x=y`, etc). The various sources will have the logic to parse values (e.g., decode `x=y` flags or JSON files on disk). A new `--config-file` allows you to replace the system (`/etc/arcconfig`) and user (`~/.arcrc`) files.

This also gets rid of `--library` support entirely for now since it's kind of messy to bridge until Config works. I expect to either restore it or replace it with `arc install` and similar.

Test Plan: Ran `arc liberate`; it actually works now. (The Config stuff does not actually work yet.)

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19694
2018-09-21 16:54:00 -07:00
epriestley
d936257018 [Wilds] Rewrite WorkingCopyIdentity in a more modern/modular way
Summary:
Ref T13098. Currently, `WorkingCopyIdentity` has rules for finding `.git/.svn/.hg` directories and `.arcconfig` files. It also has a ton of logic for reading and writing config files.

Generally:

  - Rename `WorkingCopyIdentity` to `WorkingCopy`.
  - Make it an abstract base with `Git`, `Mercurial` and `Subversion` subclasses, using the standard module/extension pattern.
  - Throw out almost all of the config file logic. This logic is going to move into Config classes. It is not unique to working copies.
    - Working copies retain the actual bit of this they need: knowing where stuff should go on disk.

This doesn't run yet, but sure looks a lot cleaner!

Test Plan: Doesn't run yet, since Config isn't working yet.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19689
2018-09-21 16:48:00 -07:00
epriestley
9a94aa216b [Wilds] Slightly simplify fatal handling during "arc" setup
Summary:
Ref T13098. We currently handle fatals by printing a message and returning an error code, since this was the most direct adaptation from the old `arcanist.php` script.

Instead, throw an exception and then handle it above, since we can reasonably do that in one place now.

Test Plan: Ran `arc`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19693
2018-09-21 16:46:05 -07:00
epriestley
fe0c293895 [Wilds] Remove include_path mangling and drop support for "externals/includes"
Summary:
Ref T13098. This logic comes from D3243 and was theoretically used to let you install `libphutil` with Homebrew. Since libphutil no longer exists and the other use cases I can come up with are questionable/obsolete, remove it.

(As we move into T5055, I expect to provide better tools for bundling/managing external dependencies.)

Test Plan: Ran `arc`, same as the old `arc`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19692
2018-09-21 16:44:11 -07:00
epriestley
8e0e07664a [Wilds] Remove libphutil
Summary:
Ref T13098. Historically, Phabricator was split into three parts:

  - Phabricator, the server.
  - Arcanist, the client.
  - libphutil, libraries shared between the client and server.

One imagined use case for this was that `libphutil` might become a general-purpose library that other projects would use.

However, this didn't really happen, and it seems unlikely to at this point: Phabricator has become a relatively more sophisticated application platform; we didn't end up seeing or encouraging much custom development; what custom development there is basically embraces all of Phabricator since there are huge advantages to doing so; and a general "open source is awful" sort of factor here in the sense that open source users often don't have goals well aligned to our goals.

Turning "arc" into a client platform and building package management solidify us in this direction of being a standalone platform, not a standalone utility library.

Phabricator also depends on `arcanist/`. If it didn't, there would be a small advantage to saying "shared code + client for client, shared code + server for server", but there's no such distinction and it seems unlikely that one will ever exist. Even if it did, I think this has little value.

Nowadays, I think this separation has no advantages for us and one significant cost: it makes installing `arcanist` more difficult for end-users.

This will need some more finesssing (Phabricator will need some changes for compatibility, and a lot of stuff that still says "libphutil" or "phutil" may eventually want to say "arcanist"), and some stuff (like xhpast) is probably straight-up broken right now and needs some tweaking, but I don't anticipate any major issues here. There was never anything particularly magical about libphutil as a separate standalone library.

Test Plan: Ran `arc`, it gets about as far as it did before.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19688
2018-09-21 16:38:53 -07:00
epriestley
d8f660ec6f [Wilds] Move ArcanistRuntime to support/ArcanistRuntime.php
Summary:
Ref T13098. Currently, it lives in `init-script.php`. Move it to a separate file for similarity with `support/PhabricatorStartup.php`.

Two small changes here:

  - `dirname(dirname(...))` adjustments for new path.
  - Remove `memory_limit(-1)`, this is adjusted by the libphutil init script already.

Test Plan: Ran `arc`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19687
2018-09-21 16:00:30 -07:00