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

2276 commits

Author SHA1 Message Date
bootstraponline
04e340ab0f Fix rubocop lint tests
Summary: Fix tests to work with rubocop 0.92.0 released on September 25, 2020

Test Plan: Unit tests pass

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D21474
2020-09-30 15:19:04 +00:00
epriestley
524aa2aed2 Fix "PhutilOpaqueEnvelopeTestCase" under PHP 7.4 with "zend.exception_ignore_args"
Summary:
See PHI1894. PHP 7.4 introduced a new runtime configuration option, "zend.exception_ignore_args", which removes the "args" from exception backtraces.

The "PhutilOpaqueEnvelopeTestCase" relies on this behavior (since it explicitly inspects stack frames). Although the test isn't critical and could be restructured, it seems like there is little value to ever enabling this option in the context of Phabricator.

Disable it at startup so environments are more consistent across different PHP versions and configurations.

Test Plan:
  - Enabled "zend.exception_ignore_args" under PHP 7.4.
  - Ran "PhutilOpaqueEnvelopeTestCase".
  - Before: failure, expected signpost value not present in stack trace (because no "args" are present on the exception).
  - After: test passes.

Differential Revision: https://secure.phabricator.com/D21473
2020-09-30 07:49:25 -07:00
Paul Tarjan
7597f31b6a fail arc diff if second lfs push errors
Summary:
We are having issues where people run out of file descriptors and the first `git push` will succeed, but the
second one will not. We'd like the diff to not be created in this case as it leads to weird behavior like our tests
running against 0 changed files.

Test Plan: none

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D21471
2020-09-28 16:20:56 -07:00
epriestley
a716c4e55f In "phutil_passthru()", "resolve()" the future rather than calling "execute()" directly
Summary:
See PHI1862. This code calls "execute()" on the future directly, but that skips some steps -- notably, ServiceProfiler hooks.

Call "resolve()", which has the same effect but includes desirable/expected side effects.

Test Plan: Changed a workflow to run "phutil_passthru('ls')", ran it with "--trace". Before: no execution in trace; after: execution in trace.

Differential Revision: https://secure.phabricator.com/D21470
2020-09-18 11:22:52 -07:00
epriestley
563dc2a993 In ConduitCallFuture, only call Conduit exception messages on Conduit exceptions
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
2020-09-17 13:20:24 -07:00
epriestley
8e5e49984d Fix a slow memory leak in long-lived FutureIterator objects, as used by FuturePool
Summary:
See T13572. FutureIterator does not release futures, so long-lived iterators (like the one that FuturePool may build) can end up leaking memory.

This affects the FuturePool used by the daemon overseer.

See T13572 for more discussion.

Test Plan:
  - Ran the simple FutureIterator script from T13572. Before: memory held during iteration, script grows without bound. After: memory released, script uses stable memory.
  - Ran the overseer with memory logging and an immediate wakeup from hibernation. Before: saw memory usage grow without bound at a rate of ~300MB/day. After: saw memory usage stable.

Differential Revision: https://secure.phabricator.com/D21466
2020-09-17 12:56:48 -07:00
epriestley
de209ec064 When raising a Conduit client exception, show the called method in the error message
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
2020-09-15 17:34:22 -07:00
epriestley
7112ee3d59 Fix additional "xsprintf()"-family static parameter errors
Summary:
Ref T13577. After the lint rule fix in D21453, it can identify more errors. Fix the errors it identifies in "arcanist/".

These all seem fairly obscure/benign.

Test Plan: Ran `arc lint` on the files before and after these changes. Did not specifically re-test these particular messages, but they mostly very obscure.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21456
2020-09-08 11:45:54 -07:00
epriestley
83e63aeb07 Allow AAST to extract string literal values from HEREDOCs
Summary:
Ref T13577. I'd like to `arc lint --everything` to find other bad calls to `pht()` and similar functions, but `n_HEREDOC` nodes currently can not generate a response to "getStringLiteralValue()".

Support literal extraction from heredocs.

Test Plan: Added a test, made it pass. Will lint everything.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21455
2020-09-08 11:45:54 -07:00
epriestley
1c327208d7 Fix a missing "pht()" parameter in HTTPSFuture
Summary: Ref T13577. This call is missing a parameter. After D21453, this is detected properly by lint. Provide the parameter.

Test Plan: Ran `arc lint` on HTTPSFuture before and after the change.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21454
2020-09-08 11:45:54 -07:00
epriestley
73847a4b19 Fix a false negative in lint for "xsprintf()"-family functions
Summary:
Ref T13577. This lint rule correctly detects the error in `pht('x %s y')` but the narrow test for `n_STRING_SCALAR` prevents it from detecting the error in `pht('x %s y'.'z')`.

Make the test broader.

Test Plan:
  - Ran `arc lint` on `HTTPSFuture.php`, got a detection of the issue in T13577.
  - Added a failing test and made it pass.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21453
2020-09-08 11:45:53 -07:00
epriestley
ceb082ef6b Give Futures clearer start/end and exception semantics
Summary:
Ref T13555. Currently:

  - If an exception is raised in "start()", the exception state is not set on the future.
  - Futures do not always call "startFuture()" before starting, and do not always call "endFuture()" once they become resolvable.
  - If you start an ExecFuture which immediately fails and then call "getPID()" on it, you get an unclear exception.

Simplify these behaviors:

  - In FutureIterator, only start futures which have not already started.
  - When starting a future on any pathway, run start code.
  - When a future becomes resolvable on any pathway, run end code.
  - Raise a more clear exception when calling "getPID()" on a future with no subprocess.

Test Plan: Faked a failing subprocess with "$proc = null", ran "bin/phd debug taskmaster" etc. Got clearer errors and more consistent future lifecycle workflows.

Maniphest Tasks: T13555

Differential Revision: https://secure.phabricator.com/D21423
2020-07-23 11:22:20 -07:00
epriestley
65cda1596f Preserve bookmarks across "hg rebase --keep --collapse", and destroy them before "hg strip/prune"
Summary:
See PHI1808. Currently, "arc land <some bookmark>" does not destroy the bookmark in Mercurial. There are three issues here:

  - "hg rebase --keep --collapse" moves bookmarks to the rewritten commits;
  - "hg strip" moves bookmarks backwards;
  - "hg prune" moves bookmarks backwards.

To get around "hg rebase", save and restore bookmark state.

To get around "hg strip" and "hg prune", explicitly destroy bookmarks pointing at commits before we strip/prune those commits.

Test Plan:
  - Ran "arc land <some bookmark> --trace". Saw arc reset the bookmark position after rebasing, and destroy the bookmark explicitly before stripping.
  - When the workflow exited, saw no more bookmark (previously: bookmark existed and pointed at a possibly-intermediate state).

Differential Revision: https://secure.phabricator.com/D21397
2020-07-08 17:43:15 -07:00
epriestley
354da1ddaa When saving and restoring local state in Mercurial, also save and restore bookmarks
Summary:
Ref PHI1808. In Mercurial, we must save and restore bookmark state explicitly.

  - Save and restore bookmarks.
  - Clean up concepts in "arc-ls-markers" slightly, so we don't need separate "isCurrent" and "isActive" flags, hopefully.

I believe the totality of Mercurial state is:

  - A (non-bare) working copy points at exactly one commit (which might be the empty/null commit, in an empty repository).
  - A working copy has exactly one active branch.
    - Each branch has zero or more heads.
    - Each head may be closed.
    - Each (non-null) commit belongs to exactly one branch.
    - Note that the active branch may have zero heads and zero commits which belong to it!
  - A working copy has zero or one active bookmark.

To capture this, we now emit:

  - A list of branch heads. If a branch head is a working copy commit, that head is flagged as active.
  - A list of bookmarks. If a bookmark is the current bookmark, that bookmark is flagged as active.
  - A single "branch-state" virtual marker. This covers the case where you have run "hg branch X" to create X, but no objects in the working copy actually correspond to X yet. It also covers the case where you are on a concrete branch, but not any head of that branch.
  - A single "commit-state" virtual marker. This always shows the current commit in the working copy.

Test Plan:
  - Useful states to test are:
    - Empty repository (not all commands currently work here).
    - Normal repository, on a bookmark.
    - Normal repository, no bookmark.
    - "hg up 123" to update to somewhere in history.
    - "hg branch X", to start a new branch with no commits.
  - Ran "arc branches" and "arc bookmarks" in various states. Saw generally sensible output.
  - Ran "arc land --hold ..." in various states against a failing remote. Saw generally sensible output, and saw working properly restored to the original state.

Differential Revision: https://secure.phabricator.com/D21396
2020-07-08 15:30:17 -07:00
epriestley
3633364bb9 Clean up push failure messaging in "arc land" slightly
Summary: Ref PHI1808. Currently, push failures are messaged awkwardly. Make this exception handling more selective and the user-facing behavior more readable.

Test Plan: Ran "arc land" against a failing remote, saw a human-readable message instead of a stack trace.

Differential Revision: https://secure.phabricator.com/D21395
2020-07-08 15:30:17 -07:00
epriestley
710bceab10 When "arc land" fails a Mercurial push, actually raise it as an exception
Summary: See PHI1808. Some refactoring of the "passthru" API resulted in error conditinos here being dropped. Instead, raise them as exceptions.

Test Plan: Forced "hg push" to fail, used "arc land" against a failed push, saw error behavior instead of "success" feedback.

Differential Revision: https://secure.phabricator.com/D21394
2020-07-08 15:30:17 -07:00
epriestley
41774ba9cc Fix additional Mercurial/Python compatibility issues in "arc land"
Summary:
Ref PHI1805. Under some combination of versions (Python 3.8?), "arc-ls-markers" is running into additional Python runtime issues.

Sprinkle more "b" around to resolve them? Also clean up a couple of plain "arc" issues.

Test Plan:
Landed a change in Mercurial.

Some of this works fine without changes in Python 3.7/2.7 against Mercurial 4.7/5.4, so this may not be exhaustive.

Differential Revision: https://secure.phabricator.com/D21393
2020-07-07 10:20:41 -07:00
epriestley
a28e76b7b3 Allow "hg arc-ls-markers" to run under Python 2 or Python 3
Summary:
Ref T13546. See PHI1805. Currently, the "arc-ls-markers" extension doesn't run under Python 3:

  - some stuff needs "b'...'" to mark it as a byte string;
  - "dict.iteritems()" is gone in Python 3, and "mercurial.pycompat" isn't always available;
  - in Python 3, "json" refuses to print byte strings; and
  - the compiler caching behavior in Python 3 has changed.

Try to get these things working in the same way under Python 2 and Python 3.

Test Plan:
Ran this command (with `python` as Python 2, locally):

```
$ python /usr/local/bin/hg --config 'extensions.arc-hg=/Users/epriestley/dev/core/lib/arcanist/support/hg/arc-hg.py' arc-ls-markers --
```

...and this command:

```
$ python3 /usr/local/bin/hg --config 'extensions.arc-hg=/Users/epriestley/dev/core/lib/arcanist/support/hg/arc-hg.py' arc-ls-markers --
```

..and saw the same output in both cases (previously, `python3 ...` fataled in various ways).

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21392
2020-07-06 15:29:35 -07:00
epriestley
79a6dfd7a9 Fix a MarkerRef call to get the active bookmark in Mercurial
Summary: See PHI1805. This call is constructed improperly and can lead to a fatal in `arc patch` under Mercurial.

Test Plan: In Mercurial, ran a valid `arc patch` operation.

Differential Revision: https://secure.phabricator.com/D21391
2020-07-06 14:08:11 -07:00
epriestley
a5480609f8 Render the state tree in "arc branches" slightly more cleanly
Summary:
Ref T13546. Try using unicode box drawing characters to render a more obvious tree struture in "arc branches".

Unclear if this has enough support to use, but seems okay so far.

Test Plan: Ran "arc branches", saw a nicer tree display.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21390
2020-07-03 12:43:34 -07:00
epriestley
01e91dc260 Clean up some service profiler behavior in Conduit futures
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
2020-07-01 06:37:31 -07:00
epriestley
b8a5191e3b Improve login/auth messages from Arcanist toolset workflows
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
2020-07-01 06:37:31 -07:00
epriestley
65e4927dca Drop intended support for "--anonymous" from Arcanist Toolsets
Summary:
Currently, modern "arc" workflows accept and parse "--anonymous" but don't do anything with it.

I intend to move away from anonymous workflows, which only really supported a tiny subset of unusual workflows on open source installs but added significant complexity to login/auth behavior.

Drop support for this flag.

Test Plan: Grepped for "anonymous", didn't turn up any relevant hits.

Differential Revision: https://secure.phabricator.com/D21386
2020-07-01 06:37:31 -07:00
epriestley
17f2668d1f When tab-completing "arc" commands, suggest paths if the argument is empty and a path wildcard argument exists
Summary:
Currently, if you type "arc upload <tab>", we do not autocomplete the working directory. We should, but the "current argument" is the empty string and that's technically a prefix of every flag, so we suggest that you might want a flag instead.

You probably don't. Suggest paths in this case.

Test Plan:
  - Ran "arc upload <tab>", saw path completions.
  - Ran "arc land <tab>" (this workflow does NOT take paths as arguments), saw flag completions.

Differential Revision: https://secure.phabricator.com/D21385
2020-07-01 06:37:31 -07:00
epriestley
7e9f80971b Implement Conduit login prompt behavior as a pure FutureProxy, not a Future-like object
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
2020-07-01 06:37:31 -07:00
epriestley
2daf9b16ae Improve resolution behaviors of FutureProxy
Summary:
See PHI1764. See PHI1802. Address two resolution behaviors for FutureProxy:

  - FutureProxy may throw an exception directly from iteration via "FutureIterator" (see PHI1764). This is wrong: futures should throw only when resolved.
  - FutureProxy can not change an exception into a result, or a result into an exception, or an exception into a different exception. Being able to proxy the full range of result and exception behavior is useful, particularly for Conduit (see PHI1802).

Make "FutureProxy" more robust in how it handles exceptions from proxied futures.

Test Plan:
Used this script to raise an exception during result processing:

```
<?php

require_once 'support/init/init-script.php';

final class ThrowingFutureProxy
  extends FutureProxy {

  protected function didReceiveResult($result) {
    throw new Exception('!');
  }

}

$future = new ImmediateFuture('quack');
$proxy = new ThrowingFutureProxy($future);
$iterator = new FutureIterator(array($proxy));

foreach ($iterator as $resolved) {
  try {
    $resolved->resolve();
  } catch (Exception $ex) {
    echo "Caught exception properly on resolution.\n";
  }
}
```

Before this change, the exception is raised in the `foreach()` loop. After this change, the exception is raised at resolution time.

Differential Revision: https://secure.phabricator.com/D21383
2020-07-01 06:37:30 -07:00
epriestley
98ca5cfa81 Remove an unused method in "ArcanistUploadWorkflow"
Summary: This method is private and has no callers. The code has moved to "FileUploader" in a prior change.

Test Plan: Grepped for callers, found none.

Differential Revision: https://secure.phabricator.com/D21382
2020-07-01 06:37:30 -07:00
epriestley
4b8a32ee02 Give Mercurial more plausible marker behavior
Summary: Ref T13546. Fixes some issues where marker selection in Mercurial didn't work, and selects "draft()" as the set of commits to show, which is at least somewhat reasonable.

Test Plan: Ran "arc branches" and "arc bookmarks" in Mercurial, got more reasonable output.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21380
2020-06-30 15:50:07 -07:00
epriestley
8c95dc0d29 Support date-range commit graph queries, and multiple disjoint commits in Git
Summary: Ref T13546. Allow the commit graph to be queried by date range, and Git to be queried for multiple disjoint commits.

Test Plan: Ran "arc branches" and future code which searches for alternate commit ranges for revisions.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21379
2020-06-30 15:50:06 -07:00
epriestley
c7093a2e57 In "arc branches", group linear sequences of published revisions together
Summary: Ref T13546. If your history includes a long linear sequence of published revisions, summarize them.

Test Plan: Ran "arc branches", saw better summarization of linear published revision sequences.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21367
2020-06-30 15:50:06 -07:00
epriestley
5d305909eb When a commit graph set has many commits, summarize them
Summary: Ref T13546. In cases where a given set has a large number of commits, summarize them in the output.

Test Plan: Ran "arc branches", saw long lists of commits (like the history of "stable" summarized).

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21366
2020-06-30 15:50:06 -07:00
epriestley
0ad3222d59 Improve grid layout in "arc branches" at various terminal widths
Summary: Ref T13546. Make "arc branches" use a flexible grid width and try to match the content to the display width in a reasonable way.

Test Plan: Ran "arc branches" at various terminal widths, got generally sensible output.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21365
2020-06-30 15:50:06 -07:00
epriestley
10c4a551ae Remove implicit sorting from "MarkerRefQuery"
Summary: Ref T13546. This is no longer necessary after the introduction of "msortv_natural()", which can handle natural string sorting.

Test Plan: Ran "arc branches", saw the same sorting applied.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21364
2020-06-30 15:50:05 -07:00
epriestley
cd19216ea2 Render "arc markers" workflows as a tree, not a list
Summary:
Ref T13546. Currently, each "land" workflow executes custom graph queries to find commits: move toward abstracting this logic.

The "land" workflow also has a potentially dangerous behavior: if you have "master > A > B > C" and "arc land C", it will land A, B, and C. However, an updated version of A or B may exist elsewhere in the working copy. If it does, "arc land" will incorrectly land an out-of-date set of changes.

To find newer versions of "A" and "B", we need to search backwards from all local markers to the nearest outgoing marker, then compare the sets of changes we find to the sets of changes selected by "arc land".

This is also roughly the workflow that "arc branches", etc., need to show local markers as a tree, and starting in "arc branches" allows the process to be visualized.

As implemented here ,this rendering is still somewhat rough, and the selection of "outgoing markers" isn't good. In Mercurial, we may plausibly be able to use phase markers, but in Git we likely can't guess the right behavior automatically and probably need additional configuration.

Test Plan: Ran "arc branches" and "arc bookmarks" in Git and Mercurial.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21363
2020-06-30 15:50:05 -07:00
epriestley
80f5166b70 Identify published commits in working copies by using remote configuration
Summary:
Ref T13546. When running "arc branches", we want to show all unpublished commits. This is often a different set of commits than "commits not present in any remote".

Attempt to identify published commits by using the permanent ref rules in Phabricator.

Test Plan: Ran "arc look published", saw sensible published commits in Git.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21378
2020-06-30 14:56:34 -07:00
epriestley
50f7a853b5 Load and map repository objects for remote URIs
Summary:
Ref T13546. Query and associate known Phabricator repositories to working copy remotes by normalizing and comparing URIs.

This primarily gives us access to "permanentRefRules" so we can tell which branches have published changes.

Test Plan: Ran "arc look remotes" in Git and Mercurial working copies, saw repositories map properly.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21377
2020-06-30 13:43:14 -07:00
epriestley
6bf7a40358 Provide "arc look", a user-facing inspection command
Summary:
Ref T13546. Currently, "arc which" provides some amount of inspection but it generally isn't very helpful to users and is too limited and inflexible. "arc inspect" is an internal/debugging workflow.

The new "arc look" is much more aggressively unhelpful.

Test Plan: I'm not sure if this command should allow you to continue at night, because it's too dark.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21376
2020-06-30 13:08:31 -07:00
epriestley
ffb027e85c Support generating remote refs in Git
Summary: Ref T13546. Allow construction of remote refs in Git; previously they were only supported in Mercurial.

Test Plan: Ran "arc inspect remote(origin)" in Git, got a ref.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21375
2020-06-30 13:08:19 -07:00
epriestley
89f9eb66a7 Support inspection of remote refs with "arc inspect remote(...)"
Summary: Ref T13546. Expose remote refs for inspection via "arc inspect". For now, this only works in Mercurial.

Test Plan: Ran "arc inspect remote(default)" in Mercurial, got a ref out.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21374
2020-06-30 13:07:25 -07:00
epriestley
b19985a4bd Copy repository URI normalization code from Phabricator to Arcanist
Summary: Ref T13546. Move toward smarter remote repository lookup by providing URI normalization code in Arcanist. This diff duplicates code from Phabricator; the next change will collapse it.

Test Plan: Ran unit tests.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21372
2020-06-30 13:07:12 -07:00
epriestley
c53c05e5b2 Introduce "phutil_partition()" and natural case sorting for "msortv(...)"
Summary:
Ref T13546. Pull some small utility changes out of the deeper stack of "land/markers" changes.

"phutil_partition()" makes it easier to write code that loops over a list grouping elements, then acts on each group. This kind of code is not terribly common, but often feels awkward when implemented with raw primitives.

"msortv()" can support "natural" sorting, which sorts "feature1", "feature2", ..., "feature10" in a more human-readable order.

Test Plan: Ran unit tests, used new behaviors elsewhere in "arc markers" workflows.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21371
2020-06-30 06:45:03 -07:00
epriestley
33484b43c9 Introduce "GridView", an updated version of "ConsoleTableView"
Summary:
Ref T13546. In a future change, I'm providing a fancier version of "arc branches" that requires more sophisticated table rendering.

Implement a new view which can do some fancier things, like handle alignment of multi-line table cells.

Test Plan: See future changes.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21360
2020-06-30 06:31:24 -07:00
epriestley
98bf58db4a Correct a leftover reference to "--keep-branch"
Summary: See <https://discourse.phabricator-community.org/t/arc-land-keep-branch-no-longer-works/4004/>. This flag is now "--keep-branches".

Test Plan: Grepped for "keep-branch".

Differential Revision: https://secure.phabricator.com/D21356
2020-06-30 06:30:51 -07:00
epriestley
f52222ad19 Add more "RepositoryRef" legacy status mappings
Summary: Ref T13546. The old "differential.query" call is still used to fill refs when all we have locally is hashes. Add some mappings to improve the resulting refs.

Test Plan: Viewed "arc branches", saw statuses colored more consistently.

Reviewers: ptarjan

Reviewed By: ptarjan

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21355
2020-06-30 06:30:21 -07:00
epriestley
b0a9ef8351 In "arc land" under Git, confirm branch creation
Summary: Ref T13546. If "arc land" would create a branch, warn the user before it does.

Test Plan: Ran "arc land --onto mtarse", a typo of "master".

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21354
2020-06-30 06:29:51 -07:00
epriestley
33bb0acf97 Collect scattered implementations of "getDisplayHash()" into RepositoryAPI
Summary: Ref T13546. All of LandEngine, LocalState, and RepositoryAPI implement "getDisplayHash()". Always use the RepositoryAPI implementation.

Test Plan: Grepped for symbols.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21353
2020-06-30 06:28:38 -07:00
epriestley
63f2e667b9 Update "arc land" display of build failures, and rename "DisplayRef" to "RefView"
Summary:
Ref T13546. Show ongoing and failed builds more clearly in "arc land" output.

Also rename "DisplayRef" (which is not a "Ref") to "RefView" with the goal of improving clarity, and let callers "build...()" it so they can add more status, etc., information.

Get rid of "[DisplayRef|RefView]Interface". In theory, future refs (say, in Phabricator) might not do anything here, but every Ref just ends up implementing it. This could perhaps be subclassed more narrowly in the future if necessary.

Test Plan: Ran "arc land", grepped for various symbols.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21352
2020-06-30 06:27:56 -07:00
epriestley
33dfa859d8 On Windows, don't try to set "stdin" nonblocking, as it does not work
Summary:
See <https://discourse.phabricator-community.org/t/arc-land-fail-unable-to-set-stdin-nonblocking/4006/>.

See also <https://bugs.php.net/bug.php?id=34972>.

Note that you can't ^C during a prompt (or at any other time) in Windows currently, see T13549.

Test Plan: On Windows, hit a prompt in "arc land", then answered it successfully.

Differential Revision: https://secure.phabricator.com/D21358
2020-06-12 14:29:52 -07:00
epriestley
1e09a0ee7e When a linter raises a message at a nonexistent line, don't fatal during rendering
Summary:
See PHI1782. If a linter raises a message at a line which does not exist in the file, render a confused warning rather than fataling.

This is a long-existing issue which was exacerbated by D21044.

Test Plan: Modified a linter to raise issues at line 99999. Before change: fatal in console rendering. After change: reasonable rendering.

Differential Revision: https://secure.phabricator.com/D21357
2020-06-12 12:31:02 -07:00
epriestley
92f860ae9b Improve "--hold", save/restore state, bookmark creation, and some warnings for "arc land" in Mercurial
Summary:
Ref T13546. Ref T9948.

  - Make "--hold" show the same set of commands to manually push that the normal workflow would use.
  - Make save/restore state work.
  - Make bookmark creation prompt for confirmation.
  - Improve / provide some additional warnings and help text.

Test Plan: Ran various increasingly complex "arc land" workflows, e.g. "arc land --hold --onto fauxmark1 --onto fauxmark2 --into default . --revision 118 --trace"

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21351
2020-06-10 17:31:51 -07:00