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

54 commits

Author SHA1 Message Date
Christopher Speck
ac365c3ee5 Refactor how Mercurial runs commands that require extensions
Summary:
Currently there are a number of `--config extensions.blah=` sprinkled throughout the Mercurial APIs which aren't pleasant to look at. Additionally it turns out the `rebase` command requires the `rebase` extension which is not turned on by default. Uses of `rebase` should also be including this flag to enable the extension when it's in use.

This adds `ArcanistMercurialAPI::buildMercurialExtensionCommand()` which allows running a Mercurial command that requires an extension just by naming the extension instead of providing the full `--config..` argument.

It can be used e.g.
```lang=php
$api->execxLocalWithExtension(
  'strip',
  'strip --rev %s --',
  $rev);

$api->execxLocalWithExtension(
  'arc-hg',
  'arc-amend --logfile %s',
  $tmp_file);
```
Which produces
```lang=console
$ hg --encoding utf-8 --config 'extensions.strip=' strip --rev 82567759e2d703e1e0497f5f41363de3a1500188 --

$ hg --encoding utf-8 --config 'extensions.arg-hg=/Users/cspeckrun/Source/phacility/arcanist/support/hg/arc-hg.py' arc-amend --logfile /private/var/folders/cg/364w44254v5767ydf_x1tg_80000gn/T/6bwck66ccx0kwskw/89989-5F8JaL
```

Refs T13659

Test Plan: I ran through several scenarios of running `arc diff` and `arc land` with uncommitted changes on non-head commits, while not having the `evolve` extension enabled. Using the `--trace` argument I verified that `rebase`, `strip`, `arc-amend`, and `arc-ls-markers` were all invoked and the corresponding `arc diff` and `arc land` commands succeeded. I also ran `arc land --onto-remote default` while `arc-hg.py` raised an exception for the "remote" case and verified that the error was caught by Arcanist and displayed the error and prevented further execution. I removed the error from `arc-hg.py` and re-ran the command and verified it succeeded.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13659

Differential Revision: https://secure.phabricator.com/D21697
2021-07-21 00:40:14 -04:00
Christopher Speck
41f6c6ecb2 Update "arc diff" to amend non-head commits with Mercurial
Summary:
After `arc diff` creates a revision in Phabricator it amends the commit to include a link to the revision in the commit message. For Mercurial this is done with `hg commit --amend --logfile` however this will fail when trying to create a diff for a non-head commit.

This updates `ArcanistMercurialAPI::amendCommit()` to allow amending a non-head commit in two ways, depending on whether `evolve` is in use:

No evolve:
1. Rebasing the current commit onto the current commit's parent, using the new commit message
2. Rebasing all children + descendants of the current commit onto the new resulting commit
3. Stripping the original commit

With evolve:
1. Amend the commit with `hg amend --logfile`
2. Run `hg evolve` to tidy up all commits

Test Plan:
I created 6 commits in a row placing a bookmark at commits 2 `bookmark1`, 4 `bookmark2`, and 6 `bookmark3`, and ensured I had `arc:bookmark` in my base ruleset.

No evolve, non-head changeset:
1. I verified I did not have `evolve` enabled by running `hg debugextensions` and did not see `evolve` in the listed active extensions.
2. I updated to `bookmark1` and modified a file to leave a dirty working state.
3. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
4. I checked the status of my repository and verified it was still linear and the bookmarks pointed to the proper commits.
5. I ran `hg log -r bookmark1 --template {desc}` to view the full commit message and verified it contained both `Summary: ...` and `Differential Revision: https://...`.
6. I ran `hg diff -c bookmark1` and verified the changes for that commit included the changes I made in step 2.

No evolve, head changeset:
1. I updated to `bookmark3` which is the head commit and modified a file to leave a dirty working state.
2. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
3. I checked the status of my repository and verified it was still linear and all the bookmarks pointed to the proper commits.
4. I ran `hg log -r bookmark3 --template {desc}` to view the full commit message and verified it contained both `Summary: ...` and `Differential Revision: https://...`.
5. I ran `hg diff -c bookmar3` and verified the changes for that commit included the changes I made in step 1.

With evolve:
1. I enabled `evolve` and verified it was enabled by running `hg debugextensions` and saw `evolve` in the listed active extensions.
2. I updated to `bookmark2` and modified a file to leave a dirty working state.
3. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
4. I checked the status of my repository and verified it was still linear and all the bookmarks pointed to the proper commits.
5. I ran `hg log -r bookmark2 --template {desc}` to view the full commit message and verfieid it contained both `Summary: ...` and `Differential Revision: https://...`.
6. I ran `hg diff -c bookmark2` and verified the changes for that commit included the changes I made in step 2.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D21686
2021-07-20 16:55:27 -04:00
Christopher Speck
a43a3a9aab An assortment of fixes and updates to using arc-land with mercurial
Summary:
Refs T13546

**Behavior Changes**
1. Currently, after landing the `--onto` bookmark will not be advanced to the newly pushed commit(s)
  - This updates so that after pushing commits upstream, onto-marker bookmarks are pulled back from the server to retrieve their updated state
2. Currently, after landing the working directory will typically be the old `--onto` commit
  - This updates the behavior in the following ways
    1. If the starting working state is a commit that is part of a revision being landed, the post-land state will be the newly published commit for that revision
    2. If the starting working state is a commit for a tip revision being landed and there is only a single `--onto` target, the post-land state will be the newly published commit and the `--onto` bookmark will be activated, if applicable. If there are multiple `--onto` targets defined (uncommon) then the resulting working state will be the same behavior as before, as the desired behavior for this case is ambiguous.
    3. If the starting working state is a commit that is not part of any revision being landed, then the post-land state will be that same commit, activating the same previous bookmark if applicable.

**Bugs Fixed**
1. When landing a diff that includes multiple commits, where the non-tip commit adds a file and later commit modifies that file, the land process would fail during rebasing.
2. When landing, the display of what commits are going to be landed only includes the commit hash but should include part of the commit message for easy identification.
3. If errors occur during a rebase while landing, the resulting state is an in-progress rebase. Users will typically not be able to resolve this in-progress rebase and possibly confuse them further as they have to first abort the rebase before trying to clean up. These rebases will now be aborted by arcanist before exiting.
4. If using evolve, landing a non-tip revision would leave behind orphaned commits.

Test Plan:
Bookmark test
- I created a diff with bookmark `test`, as a branch from before the `master` commit
- I updated working state to activate `test`
- I landed that diff
- I verified that the resulting working state was on the newly published `master` commit

Non-bookmark test
- I created a diff with bookmark `test`, as a branch from before the `master` commit
- I updated working state to be the new commit, but not activate `test` even though it points to that same commit
- I landed that diff
- I verified that the resulting working state was on the newly published `master` commit

Not-working state test
- I created a diff with bookmark `test`, as a branch from before the `master` commit
- I updated working state to an older commit not related to the one being landed
- I landed that diff
- I verified the ending resulting working state was on the older commit I was on prior to landing

Multiple commits test
- Using `test` bookmark I created a commit that added a new file, made a diff, made another new commit on top of it which modified that file, then updated the diff
- I updated my working state to the first commit for the diff (non-head)
- I ran `arc land test` to land the diff
- I verified that the resulting working state was on the newly published `master` commit

Landing while on `master`
- I created a diff with a bookmark `test`, as a branch from before the `master` commit
- I updated working state to the `master` bookmark
- I landed that diff
- I verified that the resulting working state was on the newly published `master` commit

Landing with no local `master` bookmark
- I created a diff consisting of two commits with bookmark `test`, as a branch from before the `master` commit
- I left working state on `test` and deleted my local `master` bookmark
- I landed that diff
- I verified that the resulting working state was on the newly published `master` commit and the `master` bookmark was pulled and active

Cascading diffs while on an earlier diff changeset
- I setup two diffs, each consisting of two commits, one depending on the other
- I put the working state onto a commit from the earlier diff
- I landed the later diff
- I verified that the resulting working state was on the newly published commit associated with the earlier diff, which is not the new `master` commit so there was no active bookmark

Cascading diffs while on the later diff changeset
- I setup two diffs, each consisting of two commits, one depending on the other
- I put the working state into a commit from the later diff
- I landed the later diff
- I verified that the resulting working state was on the newly published commit for the later diff which was `master` and `master` was active

Landing a non-tip revision, with evolve
- I setup three diffs, each consisting of two commits, creating a dependency chain
- I verified I had the evolve extension enabled via `hg debugextensions`
- I put the working state on a commit in the first revision
- I landed the second revision
- I verified that the resulting working state was clean, that revisions 1 and 2 were properly landed and published, and the resulting working state was on the published commit for the first revision.

Landing a non-tip revision, without evolve
- I setup three diffs, each consisting of two commits, creating a dependency chain
- I verified I did not have the evolve extension enabled via `hg debugextensions`
- I put the working state on a commit in the first revision
- I landed the second revision
- I verified that the resulting working state was clean, that revisions 1 and 2 were properly landed and published, and the resulting working state was on the published commit for the first revision.

Landing a non-tip revision, while the non-landing revision is active
- I setup three diffs, each consisting of two commits, creating a dependency chain
- I put the working state on the latest tip revision commit and its bookmark
- I landed the second revision
- I verified that revisions 1 and 2 were properly landed and published, and the resulting working state was still on the latest tip revision commit and its bookmark was active.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21680
2021-07-11 23:40:53 -04:00
epriestley
953d742a1a In "arc land", if rebasing a range fails, attempt to "reduce" it
Summary:
Ref T13576. See that task for discussion.

When a user runs `arc land --pick A`, they may be selecting a range of commits ("X..Y") which have ancestors ("V..W") that should NOT land.

We must slice "X..Y" out of history before we can merge it, to avoid landing changes from "V..W".

When "X..Y" is simple and linear, we can rebase the range to pick our desired slice out of history.

When "X..Y" includes merge commits, we frequently can not, and I could not identify any simple alternative. The best alternative I came up with is this "reduce" operation:

  - squash "into" onto Y, producing S, to guarantee there are no natural conflicts;
  - squash S onto X^, producing T, to get rid of the merge commits;
  - rebase T onto "into", producing R, to slice "X..Y" out of history;
  - squash R onto "into", producing Q. (R and Q will be the same, but this simplies the code.)

This feels flimsy and fragile, but I can't immediately find a way to break it. See T13576 for more discussion.

Test Plan:
  - Applied conflicting changes to `example.txt` in `master` and `feature1`.
  - Ran `arc land`, got a merge conflict.
  - Resolved the conflict with `git merge master`.
  - Ran `arc land`.
    - Before: merge conflict.
    - After: `arc land` resolves the merge correctly.
  - Stacked `feature2` on `feature1`, and made various mutations to `feature1` and `feature2`, then ran `arc land --pick feature2`. Changes made in `feature1` should not land, and they mostly do not. See T13576.

Maniphest Tasks: T13576

Differential Revision: https://secure.phabricator.com/D21590
2021-03-03 13:40:56 -08:00
epriestley
d72fad6461 Add a character marker to the "IMPLICIT COMMITS" warning in "arc land"
Summary:
Ref T13576. The "implicit commits" prompt in "arc land" shows a list of implicit and non-implicit commits.

The implicit commits are marked with a background color, but this doesn't survive if you copy/paste the output into a support ticket.

Make my life easier by also marking commits so the marker survives copy/paste.

Test Plan: Ran "arc land" with implicit commits, saw a copy-pastable indicator.

Maniphest Tasks: T13576

Differential Revision: https://secure.phabricator.com/D21589
2021-03-03 13:40:56 -08:00
epriestley
b2e715fc5a Provide "gitsprintf(...)" and disambiguate Git ref selectors
Summary: Ref T13589. See that task for discussion.

Test Plan:
  - Created this diff, ran most commands in isolation.
  - This change is difficult to test extensively.

Maniphest Tasks: T13589

Differential Revision: https://secure.phabricator.com/D21509
2021-01-13 12:31:15 -08: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
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
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
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
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
epriestley
50c534b591 Correct some minor "arc land" workflow issues in Mercurial
Summary: Ref T9948. Ref T13546. Clean up some minor behaviors to allow "arc land" to function in the simplest cases again. Also, do a capability test for "prune" rather than just falling back.

Test Plan: Ran "arc land <mark>" in Mercurial, got changes pushed.

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21350
2020-06-10 17:31:50 -07:00
epriestley
488a24c40a In "arc land" in Mercurial, inch closer to making complex branch/bookmark workflows function
Summary:
Ref T9948. Ref T13546. This change moves toward a functional "arc land" in Mercurial.

Because of how "bundlerepo.getremotechanges()" works, "hg arc-ls-markers" does not actually list markers in the remote that aren't different from local markers so it's hard to get anywhere with this.

Test Plan: Got somewhat-encouraging output from "arc land" and "hg arc-ls-markers", but too many things are still broken for this to really work yet.

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21348
2020-06-10 17:31:50 -07:00
epriestley
727d73fec9 In "arc land", fix some coarse issues with build warnings
Summary: Ref T13546. In the new "arc land": actually reach build warnings; and show buildable URIs.

Test Plan: Ran "arc land ..." with intentionally broken builds, got more useful build warnings.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21347
2020-06-10 10:27:18 -07:00
epriestley
705c48effc Realign "arc land" closed/published warning around more modern language
Summary: Ref T13546. The modern constant from the modern API method for this state is "published", and this more narrowly covers the desired behavior (notably, excluding "Abandoned" revisions).

Test Plan: Ran "arc land ... --revision X" where "X" is a published revision, got an appropriate prompt.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21345
2020-06-10 10:27:18 -07:00
epriestley
3cad824e38 In "arc land" in Mercurial, show a tidier "ls-remote" command
Summary:
Ref T9948. Ref T13546. We must passthru "hg ls-remote" because it might prompt the user for credentials.

Since "ls-remote" is implemented as an exension and we can't rely on using stdout beacuse of passthru, the actual command we execute is:

```
$ hg --config extensions.arc-hg=<huge long path to extension> arc-ls-remote --output <huge long path to temporary file> -- remote
```

This is meaningless and distracting; show the intent of the command we're executing instead. Users can see the raw command in "--trace" if they're actually debugging behavior.

Test Plan: Ran "arc land" in a Mercurial repository, got a tidier command output.

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21344
2020-06-10 10:27:17 -07:00
epriestley
b1f807f7ca Disambiguate various types of Mercurial remote markers with "hg arc-ls-remote"
Summary: Ref T13546. Ref T9948. It seems challenging to examine a remote in vanilla Mercurial. Provide an "hg arc-ls-remote" command which functions like "git ls-remote" so we can figure out if "--into X" is a bookmark, branch, both, neither, or a branch with multiple heads without mutating the working copy as a side effect.

Test Plan: Ran various "arc land --into ..." commands in a Mercurial working copy, saw apparently-sensible resolution of remote marker names.

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21343
2020-06-10 10:27:17 -07:00
epriestley
1bb054ef47 Verify remotes ("paths") in Mercurial during "arc land"
Summary: Ref T13546. Parse "hg paths" and validate that the remotes "arc land" plans to interact with actually exist.

Test Plan: Ran "arc land" with good and bad "--into-remote" and "--onto-remote" arguments, got sensible validation behavior.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21342
2020-06-10 10:27:17 -07:00
epriestley
091aebe014 Refine "arc land" behavior when pushing "onto" a new branch
Summary:
Ref T13546. If the "onto" branch doesn't exist yet and has a "/" in it, we need to preface it with "refs/heads" explicitly.

Fix a "false/null" issue with argument validation.

Possibly, "arc land" should prompt you before creating branches in the remote.

Test Plan: Ran "arc land --onto does-not-exist/1.1" and created a branch.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21341
2020-06-08 16:56:46 -07:00
epriestley
ab70626c12 Support "arc land --pick" to pick specific changes out of a sequence
Summary:
Ref T13546. If you have "feature1", "feature2", etc., "arc land feature4" will now land the entire sequence.

Provide "arc land --pick feature4" to work more like the old "arc land" did. This cherry-picks the commits associated with "feature4", then cascades onto the ancestor of the range.

Test Plan: Ran "arc land --pick land14" to pick a change out of a stack.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21340
2020-06-08 16:30:53 -07:00
epriestley
7ddaed9aba Improve "arc land" behavior in the presence of merge conflicts and change sequences
Summary:
Ref T13546. When we encounter a merge conflict, suggest "--incremental" if it's likely to help.

When merging multiple changes, rebase ranges before merging them. This reduces conflicts when landing sequences of changes.

Test Plan: Ran "arc land" to land multiple changes. Hit better merge conflict messaging, then survived merge conflicts entirely.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21339
2020-06-08 16:30:41 -07:00
epriestley
3d64140ff3 Implement "arc work", to replace "arc feature"
Summary: Ref T13546. Fixes T2928. Adds a new "arc work" workflow which functions like the older "arc feature" workflow, but with modern infrastructure.

Test Plan: Used "arc work" to begin work on branches, bookmarks, and revisions in Git and Mercurial.

Maniphest Tasks: T13546, T2928

Differential Revision: https://secure.phabricator.com/D21336
2020-06-08 16:27:27 -07:00
epriestley
5abf0b96c8 Use MarkerRefs to resolve landing symbols in Mercurial
Summary: Ref T13546. Update the Mercurial code which finds default targets and maps symbols to targets under "arc land" to use the new MarkerRef workflow.

Test Plan: Ran "arc land" with (and without) various arguments in Mercurial, saw them resolve in a seemingly sensible way.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21335
2020-06-08 16:27:24 -07:00
epriestley
78e9cc9c01 Add a check for ambiguous merge strategies after the "history.immutable" behavioral change
Summary: Ref T13546. When users hit a situation where we would select "squash" but would previously select "merge", prevent them from continuing under ambiguous conditions.

Test Plan: Ran "arc land" in Git with "history.immutable" true, false, and not configured.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21332
2020-06-08 16:26:18 -07:00
epriestley
f3f31155b7 Format "arc land" passthru commands more nicely, and execute them from CWD
Summary:
Fixes T13380. Ref T13546. Use slightly nicer command formatting for passthru commands, to make it more obvious what's going on and which pieces of output are from "arc" vs various subcommands.

Also, execute repository API passthru commands from the working copy root. All other commands already did this, the older API just didn't support it.

Test Plan: Ran "arc land" in Git and Mercurial repositories, saw nicer output formatting.

Maniphest Tasks: T13546, T13380

Differential Revision: https://secure.phabricator.com/D21330
2020-06-08 16:26:18 -07:00
epriestley
0bf4da60f6 Make Mercurial use "hg shelve" and "hg unshelve" in dirty working copies in "arc land"
Summary: Ref T13546. Implement the equivalents of "git stash" in Mercurial.

Test Plan: Dirtied a working copy in Mercurial, ran "arc land", saw dirty changes survive the process.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21329
2020-06-08 16:22:44 -07:00
epriestley
4d61c00531 Improve final messages under "arc land --hold"
Summary: Ref T13546. Update some of the "arc land --hold" behavior to be more functional/consistent with the updated workflow.

Test Plan: Ran "arc land --hold" under various conditions, got sensible forward/restore instructions.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21328
2020-06-08 16:22:44 -07:00
epriestley
709c9cb6fb Improve the logic for identifying ambiguous commits and applying "--revision" to them
Summary: Ref T13546. This is mostly minor cleanup that improves behavior under "--revision".

Test Plan: Ran `arc land --into-empty` and `arc land --into-empty --revision 123` with ambiguous revisions in history to hit both the force and non-force outcomes.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21325
2020-06-08 16:22:43 -07:00
epriestley
8a53b5a451 When landing changes in an empty repository, merge cleanly in Git
Summary:
Fixes T12876. Ref T13546. When you make the first change in a new Git repository, "arc land" currently can not merge it because there's nothing to merge into.

Support merging into the empty state formally, reachable by using "--into-empty" (which should be uncommon) or "arc land" in an empty repository.

Test Plan:
  - Used "arc land --into-empty --hold ..." to generate merges against the empty state under "squash" and "merge" strategies in Git.
    - Got sensible result commits with appropriate parents and content.

Maniphest Tasks: T13546, T12876

Differential Revision: https://secure.phabricator.com/D21324
2020-06-08 16:19:55 -07:00
epriestley
57d0d690cc Modernize output when pruning branches in Git during "arc land"
Summary: Ref T13546. Make this output look more similar to other modern output.

Test Plan: Ran "arc land", saw consistent-looking output.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21319
2020-06-08 16:19:55 -07:00
epriestley
94f78cf87c Provide more information about merge progress in "arc land" under Git
Summary: Ref T13546. Communicate more progress information and provide additional details when merge conflicts occur.

Test Plan: Hit a merge conflict, saw more helpful output.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21318
2020-06-08 16:19:55 -07:00
epriestley
1552397c86 Sometimes discard already-closed revisions in "arc land"
Summary:
Ref T13546. When we find commits in history which are associated with already-closed revisions, and they weren't named explicitly on the command line, and we're using a squash strategy, discard them.

This generally happens when "feature2" is on top of "feature1", but "feature1" gets amended or branched elsewhere and lands independently.

Test Plan: Ran "arc land feature3" where prior revisions had already landed, got discards on the duplicated changes.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21322
2020-06-08 16:17:20 -07:00
epriestley
25afb93f7a In "arc land", rebase branches in natural order
Summary: Ref T13546. When "arc land" performs cascading rebases, do them in "feature1", "feature2", etc., order so they're easier to follow. The outcome is not dependent on execution order.

Test Plan: Landed a change which cascaded many other branches, saw more comprehensible update order.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21320
2020-06-08 16:17:19 -07:00
epriestley
68f28a1718 Substantially modernize the "arc land" workflow
Summary: Ref T13546. This has a lot of dangerously rough edges, but has managed to land at least one commit in each Git and Mercurial.

Test Plan:
  - Landed one commit under ideal conditions in Git and Mercurial.
  - See followups.

Maniphest Tasks: T13546

Differential Revision: https://secure.phabricator.com/D21315
2020-06-08 16:17:19 -07:00
epriestley
73943d1bc9 Make "arc land --merge" an explicit error when targeting a Perforce remote
Summary: Ref T13434. Since "git p4 submit" gets more complicated when submitting merges, and empty merges (as with "--no-ff") seem to vanish, and it's not clear this is desirable or useful anyway, just make the "merge" strategy an explicit error with Perforce remotes.

Test Plan: Ran "arc land --merge ..." in a Git/Perforce repository, got an explicit error. Ran "arc land --squash ...", got existing working behavior.

Maniphest Tasks: T13434

Differential Revision: https://secure.phabricator.com/D20871
2019-10-28 11:59:42 -07:00
epriestley
7383c2f4e6 In "arc land", when "remote/onto" does not exist locally, try to fetch it before giving up
Summary:
Fixes T10650. It's valid to `arc land --remote origin --onto master` without first fetching that ref. If we can't find a local ref for a specified remote branch, try to fetch it before giving up.

(In the long run, this should be valid even if the remote branch does not exist at all and the user intends to create it -- see T12876 -- but this is a step toward that.)

Test Plan:
  - Ran `rm .git/refs/remotes/origin/master`, then landed into "master".
  - Before: "arc land" bailed out immediately.
  - After: "arc land" fetches the missing ref.

```
$ arc land
 TARGET  Landing onto "master", the default target under git.
 REMOTE  Using remote "origin", the default remote under Git.
 TARGET  No local ref exists for branch "master" in remote "origin", attempting fetch...
 FETCHED  Fetched branch "master" from remote "origin".
...
```

Maniphest Tasks: T10650

Differential Revision: https://secure.phabricator.com/D20870
2019-10-28 11:31:18 -07:00
epriestley
ca66743905 Support Perforce/Git repositories in "arc land"
Summary: Ref T13434. Detect perforce remotes and use "git p4" commands in place of "git" commands when operating in Perforce mode.

Test Plan:
  - Landed "master" onto itself, saw master update.
  - Landed "feature1" onto clean "master", saw master update.
  - Landed "feature2" onto dirty "master", saw master stay dirty.
  - Landed with "--hold", got sensible submit instructions.

Maniphest Tasks: T13434

Differential Revision: https://secure.phabricator.com/D20868
2019-10-28 11:25:00 -07:00
epriestley
9c7bbb760a Move Git-specific "arc land" parsing of "--onto" and "--remote" into GitLandEngine
Summary: Ref T13434. Move some git-engine-specific handling of "arc land" arguments into the Git engine. This prepares to handle Perforce workflows.

Test Plan: Will "arc land" this change.

Maniphest Tasks: T13434

Differential Revision: https://secure.phabricator.com/D20867
2019-10-28 11:24:34 -07:00
epriestley
3cdfe1fff8 When running "arc land" from a detached HEAD, don't try to delete the source ref
Summary:
Fixes T10321. Some reasonable but less-common workflows involve running `arc land` from a detached HEAD state.

When users do this, we currently try to delete the raw hash as though it were a branch during cleanup. Instead, detect if the thing we're thinking about deleting is a branch or not, and just leave it alone if it isn't.

Test Plan:
  - Ran `git checkout <some hash>`, then `arc land --revision <some revision>`.
  - Before, everything worked but cleanup tried to `git branch -D <some hash>`.
  - After, everything worked and cleanup skipped branch deletion.

Maniphest Tasks: T10321

Differential Revision: https://secure.phabricator.com/D20786
2019-09-05 05:31:00 -07:00
Jakub Vrana
d0353d2381 Fix errors found by PHPStan
Test Plan:
Ran `phpstan analyze -a autoload.php arcanist/src` with `autoload.php` containing:

  <?php
  require_once 'libphutil/src/__phutil_library_init__.php';
  require_once 'arcanist/src/__phutil_library_init__.php';

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17367
2017-02-16 13:54:43 +00:00
epriestley
e5be662d15 Use --no-ext-diff in arc land call to git diff
Summary:
Fixes T11435. This isn't a perfect solution since there's a little code duplication, but a perfect solution is probably a bit more involved.

See T11435 for some discussion. In particular, most `git diff` commands already get this flag via `ArcanistGitAPI->getDiffBaseOptions()`.

Test Plan: Will land this change.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11435

Differential Revision: https://secure.phabricator.com/D16375
2016-08-06 09:00:01 -07:00
epriestley
fcc11b3a27 Add --quiet to git fetch on arc land
Summary: This makes it a bit quieter.

Test Plan: shh

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15236
2016-02-10 14:00:24 -08:00
epriestley
d6b1531b94 Use passthru to run git fetch in arc land so password prompts work
Summary:
Fixes T10314. In `arc land`, we currently run `git fetch` as a subprocess.

However, this can prevent password prompts (when fetching over HTTP with basic authentication) from working properly.

Instead, use passthru to redirect stdin/stdout to the subprocess so the user can type their password.

This adds a little extra clutter to the `arc land` output but I think that's OK.

Test Plan: See T10314, @maxie confirmed this fixes the issue.

Reviewers: chad

Reviewed By: chad

Subscribers: maxie

Maniphest Tasks: T10314

Differential Revision: https://secure.phabricator.com/D15233
2016-02-10 12:31:18 -08:00
epriestley
9a373c88d7 Explain how to move forward or backward from arc land --hold
Summary: Fixes T9973. When users run `arc land --hold`, give them the commands to move forward (push) or backward (checkout the branch/tag/commit they were on) and be explicit that branches have not changed.

Test Plan: Ran `arc land --hold`, got lots of explanatory text.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9973

Differential Revision: https://secure.phabricator.com/D14762
2015-12-13 08:19:04 -08:00
Alex Vandiver
a77a77817a Try switching to the branch of the same name, instead of detached head
Summary:
Landing from a branch that directly tracks origin/master places one in
a detached HEAD state.  Instead, examine if there is a local branch of
the name that we landed onto, that also tracks the upstream; if so,
switch to that.

Test Plan:
Made a branch via `git checkout -b testing origin/master`
and tried to `arc land`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9723

Differential Revision: https://secure.phabricator.com/D14420
2015-11-20 10:58:37 -08:00
epriestley
baf5eb8a87 Explicitly provide "--ff" when performing squash merges in "arc land" under Git
Summary: Ref T3855. See discussion leading up to T3855#142304.

Test Plan: See T3855#142304.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3855

Differential Revision: https://secure.phabricator.com/D14365
2015-10-28 17:13:15 -07:00