Summary:
Ref T7604. Remove a call to `arcanist.projectinfo` from `arc export`. This Conduit call was only used to detect the repository encoding, which we should be able to do locally anyway.
I was considering removing the `$projectName` from `ArcanistBundle` as well, but I'm not convinved that this is a good idea. Specifically, doing so would make it difficult to issue the "This patch is for the 'X' project, but the working copy belongs to the 'Y' project" error. We could potentially use the repository callsign for this purposes, but this isn't portable across installs.
Test Plan: I'm not quite sure how to test this. I suspect that this functionality isn't widely used anyway.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D12962
Summary: I found a few strings that I had missed, using a mostly-broken-but-somewhat-okay custom linter ruler (https://secure.phabricator.com/differential/diff/30988/).
Test Plan: Intense eyeballing.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: aurelijus, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12888
Summary: Ref T5655. After D9983, `ArcanistBaseWorkflow` and `ArcanistBaseUnitTestEngine` are deprecated.
Test Plan: Wait a sufficient amount of time before landing this.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5655
Differential Revision: https://secure.phabricator.com/D10004
Summary: This lost formatting in a pht() conversion; give the bold back and make it properly translatable.
Test Plan: Will land.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D12843
Summary: Refs D12607. Fixes T8195. Replace period in the sprintf() arguments with a comma.
Test Plan: Ran `arc diff` in the patch applied, did not get the sprintf() error
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T8195
Differential Revision: https://secure.phabricator.com/D12837
Summary:
Ref T5955. This logic is a little cleaner than the previous version.
Don't require `~/.arcrc` to exist if the caller provides `--conduit-token`.
Test Plan:
- Made calls with `--conduit-token` and no `~/.arcrc`.
- Made calls with `--conduit-token` and a normal `~/.arcrc`.
- Made calls with normal `arc`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: mbishopim3, epriestley
Maniphest Tasks: T5955
Differential Revision: https://secure.phabricator.com/D12750
Summary: Ref T5955. This makes it easier to write scripts which call Conduit via `arc call-conduit`.
Test Plan: Used `arc --conduit-token ... call-conduit user.whoami` to make calls as various users.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: mbishopim3, epriestley
Maniphest Tasks: T5955
Differential Revision: https://secure.phabricator.com/D12717
Summary: Remove the `ArcanistUncommittedChangesException` class which is unused after D11843.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12515
Summary: Although these don't do any harm, they show up in my editor which is configured to highlight trailing whitespace.
Test Plan: Submitted this diff... saw no trailing whitespace.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12369
Summary:
Fixes T7521. This separates things into two prompts and doesn't try to automatically add untracked files.
This also fixes T7512, or at least I can't reproduce it after this change.
Test Plan:
- Ran `arc diff` in a `git` directory with untracked files.
- Ran `arc diff` in a `svn` directory with untracked files.
- Ran `arc diff` in a `hg` directory with untracked files.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7512, T7521
Differential Revision: https://secure.phabricator.com/D12258
Summary:
Ref T7594. Currently, if a chunk upload fails, we incorrectly swallow the failure and fall back to single-file upload, which will often fail by hitting size limits. This also silences the original error.
Instead, do chunk uploads outside the block so that any exceptions escape, and we don't try to fall back to single-file upload.
Mostly just trying to get more info about what's going wrong on @joshuaspence's install.
Test Plan: Faked an exception in chunk upload, ran `arc upload` on a big file, saw the exception displayed on the console.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, joshuaspence
Maniphest Tasks: T7594
Differential Revision: https://secure.phabricator.com/D12111
Summary: Ref T7149. Make sure we sit at "Resuming: 60%" or whatever while uploading the first chunk.
Test Plan: Ran `arc upload` on a large file, cancelled it, resumed it, got sensible progress bar.
Reviewers: chad, btrahan
Reviewed By: chad, btrahan
Subscribers: epriestley
Maniphest Tasks: T7149
Differential Revision: https://secure.phabricator.com/D12082
Summary: Ref T7149. This was just for testing and is no longer required.
Test Plan: `grep`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7149
Differential Revision: https://secure.phabricator.com/D12077
Summary: Ref T7149. This makes the client try to use the new `file.allocate` API before falling back to the old stuff.
Test Plan: Used `arc upload` to upload files. With chunking forced, uploaded chunked files.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T7149
Differential Revision: https://secure.phabricator.com/D12061
Summary:
Refs D11990. When using `arc diff` with untracked files in the working
copy, add the untracked file(s) to the commit (as they weren't stashed or
ignored). Add the untracked paths to the list of changes in the editor
template, indicating that the files were added to the commit.
This doesn't add a separate prompt to add untracked files as per the
behaviour prior to D11843.
Test Plan:
Ran `arc diff` with only untracked files, answered yes to the 'create
new commit' prompt. Saw the commit-message with the updated changes
including untracked files. Completed the arc template, and got commit
containing uncommitted, unstaged and untracked files.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11995
Summary: Fixes T7465. I think I just missed this when untangling the old logic.
Test Plan: Ran `arc diff` with //only// untrakced files, saw warning.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7465
Differential Revision: https://secure.phabricator.com/D11990
Summary:
We end up with one too few newline here in some workflows, like `arc land` with unstaged changes.
Root issue here is that `phutil_console_prompt|confirm` lead with too much whitespace but that's a harder fix.
Test Plan: Saw reasonable whitespace.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11927
Summary: This is supposed to just print out the base revision, but actually prints out the repository section first.
Test Plan: Ran `arc which`, `arc which --show-base`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11888
Summary:
Fixes T7344.
Currently, we use `phutil_console_prompt()`, which isn't a very good editor. Use the real $EDITOR instead.
100% of the logic here was also a gigantic mess. Clean it up.
Test Plan: Will update in a second with console output from this run.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7344
Differential Revision: https://secure.phabricator.com/D11843
Summary: Fixes T7113. This one was a bit trickier than others as the API output changed a bit. In particular, there is no "errors" emitted so much as the result set just doesn't include the answer if things are garbage. Ergo, check the "identifier map" to either check for diff existence or to lookup the phid to grab the actual diff data from the "data" part of the result.
Test Plan: called `arc backout D11665` and got some working output...!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7113
Differential Revision: https://secure.phabricator.com/D11667
Summary: Fixes T7112. Nothing too difficult here.
Test Plan:
meta - submitting this with the new arcanist code
used conduit API to verify the difference between getdiff (just the latest diff) and querydiffs (all diffs that match, with the latest diff first in the set)
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7112
Differential Revision: https://secure.phabricator.com/D11665
Summary:
Some commands (like `get-config`) do not require a working copy at all. Recent changes prevented these commands from running outside a VCS working copy.
Let `null` mean "no working copy".
See also <https://github.com/phacility/phabricator/issues/800>.
Test Plan:
- Ran `arc get-config` from outside a working copy.
- Ran `arc list` from outside a working copy, got an error.
- Ran `arc commit` in a Git working copy, got an error.
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11643
Summary: Explicitly declare that the `arc branch` command is only supported under `git`.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11552
Summary: This workflow only works on git + mercurial.
Test Plan: I don't even subversion.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11626
Summary: This workflow does not work on subversion.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11625
Summary: It doesn't make any sense to run this command on another VCS.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11624
Summary: It doesn't make any sense to run this command on any other VCS.
Test Plan: Ran `arc svn-hook-pre-commit` and hit the usage exception.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11623
Summary: Instead of having an `ArcanistWorkflow` subclass explicitly throw an exception when run in an unsupported VCS, consolidate this code and move it to `arcanist.php`. In doing so, we lose some specificity in some of the error messages, but this otherwise feels cleaner. We could consider adding a `getUnsupportedRevisionControlSystemMessage()` method to provide a more tailored error message. Depends on D11604.
Test Plan:
Ran `arc bookmark` in a `git` working copy:
```
Usage Exception: `arc bookmark` is only supported under hg.
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11550
Summary: Using `array('any')` to represent `array('git', 'hg', 'svn')` is a bit magical and leads to a lot of special-casing.
Test Plan: Verified that tab completion (ala `ArcanistShellCompleteWorkflow`) still worked.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11604
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
