Summary:
Ref T13209. This gives us a clean suite under Windows. The actual changes are a lot of miscellaneous stuff which I'll walk through inline in more detail.
The biggest change here is just rewriting some stuff like `cat`, `echo`, `sleep`, etc., in PHP. These commands either don't exist, don't work the same way, or are shell builtins (and we're now bypassing the shell) under Windows. So replace `cat ...` with `php -f cat.php -- ...` to make the tests portable.
Test Plan: No remaining test failures on Windows.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13209
Differential Revision: https://secure.phabricator.com/D19729
Summary:
Ref T13098. Makes some tests pass by updating `'phutil'` to `'arcanist'`. Skips some tests which won't pass for a while.
Also removes external test engines for now since they aren't realistically going to run for a while and they significantly complicate bootstrapping a set of passing tests out of `arc unit`.
Test Plan: Ran `arc unit`, saw fewer failures.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: aurelijus
Maniphest Tasks: T13098
Differential Revision: https://secure.phabricator.com/D19714
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
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
Summary: Fixes T9116. Ref T13098. Fix shell completion handling of filenames with spaces in them. This is a big mess -- see T9116 for discussion and I'll walk through things below.
Test Plan:
These all now seem to do the right thing:
```
arc we<tab> -> arc weld (+space)
arc weld fo<tab> -> arc weld foo\ bar (+space)
arc weld 'fo<tab> -> arc weld 'foo bar' (+space)
arc weld src<tab> -> arc weld src/ (no space)
arc weld src/w<tab> -> arc weld src/work (no space)
arc weld src/work<tab><tab> -> suggests "workflow/", "workingcopy/"
arc shell-complete --gen<tab> -> arc shell-complete --generate
```
These also work, which I think is nice-to-have:
```
arc WEL<tab> -> arc weld
arc shell-complete --GEN<tab> -> arc shell-complete --generate
```
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13098, T9116
Differential Revision: https://secure.phabricator.com/D19705
Summary:
Ref T13098. Major changes:
`arc shell-complete` now installs itself to your `~/.profile`. Running `arc shell-complete` again will update the hook and update the completion rules.
Completion rules work for all toolsets, so you can install once and then autocomplete in `arc`, `phage`, etc.
This code supports other shells in theory, and I developed most of it with ZSH support next to the Bash support. However, while actually testing ZSH support, I couldn't get it to even slightly work and found myself falling down a very, very deep rabbit hole of ZSH being entirely written in bash script and `${0🅰️h}` being a legitimate script construction. The existing ZSH support comes from one guy in 2012 and also does not work for me on `master`, so I ultimately removed it. Open to restoring it but I wasn't able to figure it out in 10 minutes of Googling and I'm not convinced it's worth 11 minutes of Googling.
I left a few rough edges here with notes on how to improve/fix them, but the basics all work.
Test Plan:
- Ran `arc shell-complete` under various stages of `~/.profile`, couldn't get it to do anything bad.
- Ran `arc lib<tab>`, `arc shell-complete --curr<tab>`, etc. Got sensible suggestions and completions.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13098
Differential Revision: https://secure.phabricator.com/D19700
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
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
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
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
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
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
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
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
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
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
