Summary: Fixes T2461. Similarly to `arc lint --everything`, `arc unit --everything` should work without a base commit.
Test Plan: Removed the `.git/arc/default-relative-commit` file and ran `arc unit --everything`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T2461
Differential Revision: https://secure.phabricator.com/D11459
Summary: Allow `--severity=warning` to mean the same as `--severity warning`. Longer term, we should convert this code to use `PhutilArgumentParser`, although it doesn't seem that `PhutilArgumentParser` support British spelling ;)
Test Plan: Ran `arc lint --severity=warning`. Also `var_dump`ed `$args` to make sure it looked reasonable.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11464
Summary: Ref T6822.
Test Plan: Visual inspection. These methods are only called from within the `ArcanistShellCompleteWorkflow` class.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11238
Summary: D7952 added namespaces to `ArcanistUnitTestResult` but these are not exported and thus don't get sent from client to server.
Test Plan: Uploaded a diff and inspected the contents of the `phabricator_differential.differential_diffproperty` table.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11208
Summary: Currently we output "Waiting for JSON parameters on stdin...", even if `stdin` is piped (for example, from `echo`).
Test Plan:
```lang=bash
> echo '{}' | arc call-conduit conduit.ping
{"error":null,"errorMessage":null,"response":"ip-10-161-81-110"}
> arc call-conduit conduit.ping
Waiting for JSON parameters on stdin...
{}
^D
{"error":null,"errorMessage":null,"response":"ip-10-161-81-110"}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11122
Summary: The `file.uploadhash` method was added a long time ago (in D4899). It should be safe to assume that this method exists on most installs.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11118
Summary: Ref T5112. The `arc inlines` workflow no longer works because the `differential.getrevisioncomments` API method has been deprecated (see T2222). The command is basically useless right now.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5112
Differential Revision: https://secure.phabricator.com/D9464
Summary:
Ref T5955. If the server supports token-based authentication, prefer it over certificate-based authentication.
Also fixes T3117.
Test Plan:
- Used `arc install-certificate` to install credentials from both token-based and certificate-based hosts.
- Used `arc list` with a token-based host.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3117, T2878, T5955
Differential Revision: https://secure.phabricator.com/D10988
Summary: the call to setHeadCommit() was accidentally placed in a `if ($background) {}` block, which was removed in 54bea94. This adds the removed call.
Test Plan: used --head, worked as expected
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10447
Summary:
Calling `arc patch` on a diff that's dependent on a different diff tries to patch the parent first.
To patch the parent a child workflow is created, and a conduit is passed down, but the credentials are not and it is not marked as authenticated.
Then when the child tries to get the commit message for the dependency, it checks isConduitAuthenticated() https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistPatchWorkflow.php;2c3268f03ed70d3221eb1642bIc99ebb39b12902e$800 and on failure pops up an interactive editor for the commit message.
Instead we just pass down the credentials to the childred and mark them as authenticated, so this is not a problem.
Test Plan: With two diffs where DA2 depends on DA1, run `arc patch --force --nobranch DA2` ... this no longer pops an interactive editor for the commit message for the dependency.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: hach-que, seshness, epriestley, Korvin
Maniphest Tasks: T5986
Differential Revision: https://secure.phabricator.com/D10381
Summary: Ref T5971. We lose validation of the line ranges, but I don't think that's a huge issue.
Test Plan: Ran `arc browse README`, `arc browse README:3`, and `arc browse README:3,6-7`
Reviewers: btrahan, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5971
Differential Revision: https://secure.phabricator.com/D10352
Summary: Ref T5781. This was broken in some earlier changes in T5781.
Test Plan: Ran `arc browse README`, `arc browse README:10`.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5781
Differential Revision: https://secure.phabricator.com/D10346
Summary: Fixes T3813. This error message isn't very helpful in SVN; be more helpful.
Test Plan: Ran `arc land` in a Subversion repository, got a reasonable error message.
Reviewers: btrahan, asherkin
Reviewed By: asherkin
Subscribers: aran
Maniphest Tasks: T3813
Differential Revision: https://secure.phabricator.com/D7275
Summary: Ref T5926. We only pass Phabricator an update hint from `arc land`, not from `arc commit`.
Test Plan:
- Ran `arc land` and `arc commit` with `--trace`, saw hints go over the wire.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5926
Differential Revision: https://secure.phabricator.com/D10324
Summary:
Ref T4281. A long time ago, we added a `--background` flag to let `arc lint` and `arc unit` run while you're typing a commit message, in some situations.
This code is only moderately beneficial and is way too complicated. Particularly, it has a long history of causing hangs (T4281, T2463), doesn't work on Windows, and is impossible to debug.
It's also running into a serious PHP bug with EAGAIN/EPIPE being indistinguishable that I haven't been able to find a reasonable workaround for in ~3-4 hours of trying.
All the pathways forward that I can see make this already-complex system more complex.
The major reason that this stuff is so complex is that the subprocess may need to prompt the user (notably, to apply patches from lint).
Instead, I'm going to simplify how `arc diff` interacts with `arc lint` and `arc unit`, so we can just fire-and-forget a background process, let it do as much work as it can without needing user input, and then pick up wherever it left off. This will be slightly less cool/magical, but it won't hang bizarrely and I will be able to debug it.
For now, simply remove the `--background` flag and behavior so `arc` works for everyone.
Test Plan: Ran `arc diff` to create this diff.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4281
Differential Revision: https://secure.phabricator.com/D10198
Summary: Ref T5781. `git show .` works like HEAD, but that isn't what `arc browse .` means.
Test Plan: Ran `arc browse .` with a repository at a published commit.
Reviewers: chad, btrahan, avive, avivey
Reviewed By: avivey
Subscribers: epriestley, avivey
Maniphest Tasks: T5781
Differential Revision: https://secure.phabricator.com/D10197
Summary: Ref T5781. This makes things like `arc browse master` work (but they open the commit, not a revision).
Test Plan: Ran `arc browse master`.
Reviewers: csilvers, btrahan
Reviewed By: btrahan
Subscribers: epriestley, spicyj
Maniphest Tasks: T5781
Differential Revision: https://secure.phabricator.com/D10143
Summary: Ref T5781. Add a flag to optionally open a web browser after creating a diff or revision.
Test Plan: Created //this revision// with `arc diff --browse` // !!! //
Reviewers: csilvers, btrahan
Reviewed By: btrahan
Subscribers: epriestley, spicyj
Maniphest Tasks: T5781
Differential Revision: https://secure.phabricator.com/D10141
Summary: Ref T5781. This implements `arc browse T234`, for any monogrammed object.
Test Plan:
- Ran `arc browse T234` outside of a working copy (does not require repository API).
- Ran `arc browse T234` in a working copy, got task.
- Ran `arc browse PATH` in a working copy.
- Ran `arc browse nonpath` with and without `--force`.
Reviewers: csilvers, btrahan
Reviewed By: btrahan
Subscribers: epriestley, spicyj
Maniphest Tasks: T5781
Differential Revision: https://secure.phabricator.com/D10140
Summary:
See T5690.
-arc land now respects tracked branch when choosing 'onto'; '--onto' option remains as an override.
-arc land now respects tracked branch when choosing a remote; the remote is taken from 'onto's upstream unless the '--remote' option is present; when 'onto' branch has no upstream the '--remote' option must be provided as before.
Since 'arc feature' branches are (already) created as tracking branches, 'arc.land.onto.default' if present, is only used as the default when a non-tracking branch created by some other means is landed with out explicit '--onto'. This may be surprising but is probably the correct go-forward behavior and is inline with the description in T5690.
Test Plan:
-checked having no arc.land.onto.default still assumes 'master'
-checked 'arc.land.onto.default' still overrides 'master'
-checked upstream branch (of feature branch) overrides 'master' and 'arc.land.onto.default'
-checked '--onto' overrides all
-checked origin is default for non-tracking branches
-checked the land onto branch's upstream remote is used instead of 'origin'
-checked '--remote' overrides 'origin' and the tracked upstream
-tested several crazy branch names including 'something/like/this' for both the upstream and tracking branches
-tested on linux and OS X
-rinse and repeat on Windows
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5690
Differential Revision: https://secure.phabricator.com/D10058
Summary:
Ref T5655. It is superfluous to include "base" in the name of an abstract base class. Furthermore, it is not done consistently within the code base.
In order to retain compatibility with external code, I have kept the `ArcanistBaseWorkflow` class (which trivially extends from `ArcanistWorkflow`), but it is now deprecated and should output a warning message. Similarly for `ArcanistBaseUnitTestEngine`.
Test Plan: Created a workflow which extends from `ArcanistBaseWorkflow`. Executed the workflow and saw a deprecation warning.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, aurelijus
Maniphest Tasks: T5655
Differential Revision: https://secure.phabricator.com/D9983
Summary: We can remove this "Stopped" code as per feedback on D7327. I think having the full text of "In Progress" and "Suspended" is much clearer than just a '*' though, so I've only removed the ongoing check.
Test Plan: Ran a combination of `arc start`, `arc stop` and `arc time` and it all worked.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9955
Summary:
Depends on D9906.
This adds `arc start`, `arc stop` and `arc tracking` for tracking tasks, diffs and other objects in Phrequent.
Test Plan: Tested this against a local install.
Reviewers: skyronic, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: maxhodak, hach-que, aran, epriestley, Korvin
Maniphest Tasks: T3569, T3969
Differential Revision: https://secure.phabricator.com/D7327
Summary: Fixes T4418. Allows Maniphests created through the `arc todo` workflow to have projects assigned.
Test Plan:
```
> ./bin/arc --trace --conduit-uri='http://phabricator.joshuaspence.com' todo "Test project" --project foo --project bar
libphutil loaded from '/home/joshua/workspace/github.com/phacility/libphutil/src'.
arcanist loaded from '/home/joshua/workspace/github.com/phacility/arcanist/src'.
Config: Reading user configuration file "/home/joshua/.arcrc"...
Config: Did not find system configuration at "/etc/arcconfig".
Working Copy: Reading .arcconfig from "/home/joshua/workspace/github.com/phacility/arcanist/.arcconfig".
Working Copy: Path "/home/joshua/workspace/github.com/phacility/arcanist" is part of `git` working copy "/home/joshua/workspace/github.com/phacility/arcanist".
Working Copy: Project root is at "/home/joshua/workspace/github.com/phacility/arcanist".
Config: Did not find local configuration at "/home/joshua/workspace/github.com/phacility/arcanist/.git/arc/config".
Loading phutil library from '/home/joshua/workspace/github.com/phacility/arcanist/src'...
>>> [0] <conduit> conduit.connect() <bytes = 618>
>>> [1] <http> http://phabricator.joshuaspence.com/api/conduit.connect
<<< [1] <http> 1,050,487 us
<<< [0] <conduit> 1,051,585 us
>>> [2] <conduit> project.query() <bytes = 199>
>>> [3] <http> http://phabricator.joshuaspence.com/api/project.query
<<< [3] <http> 294,584 us
<<< [2] <conduit> 294,986 us
>>> [4] <conduit> maniphest.createtask() <bytes = 313>
>>> [5] <http> http://phabricator.joshuaspence.com/api/maniphest.createtask
<<< [5] <http> 637,693 us
<<< [4] <conduit> 638,098 us
Created task T6: 'Test project' at http://phabricator.joshuaspence.com/T6
```
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T4418
Differential Revision: https://secure.phabricator.com/D9457
Summary: I'm pretty sure that `@group` annotations are useless now... I believe that they were originally used by Diviner?
Test Plan: Eye-balled it.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin, aurelijus
Differential Revision: https://secure.phabricator.com/D9855
Summary: Fixes T5442. `arc lint --everything` currently uses a `FileFinder` to discover paths for linting. A consequence of this is that files that are ignored or otherwise excluded from version control are linted.
Test Plan: Ran `arc lint --everything` in rPHU and noticed that I wasn't prompted to add a trailing newline to `src/.phutil_module_cache`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5442
Differential Revision: https://secure.phabricator.com/D9674
Summary:
In D9595, we stopped parsing short-form "Differential Revision:" fields in commit messages, and only accept URLs.
I have one of the older style commit messages in my local `arcanist/`, so now we go down this parse failure branch in `arc branch`. This has never worked quite correctly, and if the parse fails we end up with a bad branch dictionary that is missing fields.
Test Plan: Ran `arc branch` in a working copy with an old `Differential Revision:` field at the head of a branch. Before patch: explosions; after patch: works great.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9620
Summary: The output from `arc help --full` is missing a newline character.
Test Plan:
**Before**
```
> arc help --full
--skip-arcconfig
Skip the working copy configuration file
--arcrc-file filename
Use provided file instead of ~/.arcrc.>
```
**After**
```
> arc help --full
--skip-arcconfig
Skip the working copy configuration file
--arcrc-file filename
Use provided file instead of ~/.arcrc.
>
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9608
Summary: `PhutilConsoleTable` does a better job at aligning columns. We still probably need to do some work to `PhutilConsoleTable` to set a maximum width for a specified column, or elect which columns can be truncated etc but this can probably come later.
Test Plan: {F167687}
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9601
Summary:
This is useful for wrapper scripts that want to customize arcanist's behavior without affecting the global configuration.
This can be implemented with arcanist_configuration entry in .arcconfig, however it is currently limited to
per-project settings, and this feature makes writing wrapper scripts a little easier.
Test Plan: arc diff --set-config editor=vim (yeah yeah, crappy test case)
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9442
Summary: I can't find an analogous flag for hg.
Test Plan: I don't have any precommit hooks set up on my machine so I can't test this easily, but I think this seems pretty harmless?
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9523
Summary:
Introducing `--head` caused us to run `git diff base..head` explicitly.
However, we can now hit this workflow:
- We resolve `HEAD` as commit `aaaa1`.
- This is cached.
- We notice dirty working copy changes and prompt the user to amend them to HEAD.
- The user accepts the amend.
- We amend, creating commit `bbbb2`.
- We dirty the commit range and reload the working copy. This //does not// dirty the cache of HEAD.
- We run `git diff`, but it uses the old cached HEAD: `git diff base..aaaa1`.
- This works fine (`aaaa1` still exists, it's just not on any branch) but produces the wrong diff (without amended changes).
To resolve this, implement the "dirty the cache when the range reloads" hook.
Also never try to amend if the user provides `--head`.
Test Plan:
Ran `arc diff --only --trace` in a working copy with a new commit and some uncommitted changes.
- Prior to this change, saw a `git diff base..aaaa1` command and the wrong diff.
- After this change, saw a `git diff base..bbbb2` command and the correct diff.
Reviewers: chad, csilvers, talshiri
Reviewed By: talshiri
Subscribers: epriestley, spicyj
Differential Revision: https://secure.phabricator.com/D9506
Summary:
This adds support for passing range of commits for arc diff. This is useful when you want to submit code reviews for past commits without mucking around with the working copy.
This will probably require changes :)
Test Plan: Tested locally, but totally need to add tests for this
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9369
Summary: This function has been deprecated for a long time (see rARC8150fdf044818c503a588c1b3ef0ddbb93cfa1be). It should be safe to remove it now.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9465
Summary: This workflow has been deprecated for a long time now. At some stage, it should be removed.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9463
Summary: Applied various linter fixes. Also make the `.editorconfig` file a bit more specific. Unfortunately, `arc lint --apply-patches` currently modifies some test data that it shouldn't, but this should be fixed after T5105.
Test Plan: Ran `arc unit` to make sure things weren't broken.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin, aurelijus
Differential Revision: https://secure.phabricator.com/D9440
