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

280 commits

Author SHA1 Message Date
epriestley
ade9b51a1f Detect LFS by looking for tracks in ".gitattributes" instead of using "ls-tree"
Summary:
See PHI1718. See also <https://discourse.phabricator-community.org/t/arc-diff-fails-due-to-git-cmd-fails/3680/>.

Currently, `arc diff` detects Git LFS with `git ls-files -z -- ':(attr:filter=lfs)'` magic. This is an accurate test, but does not work on older Git.

Try a simpler, dumber test and see if that will work. If this also has issues, we can try this stuff:

  - do version detection;
  - pipe the whole tree to `git check-attr`;
  - try a command like `git lfs ls-files` instead, which is probably a wrapper on one of these other commands.

Test Plan:
  - In a non-LFS repository, ran "arc diff" and saw the repository detect as non-LFS.
  - In an LFS repository, ran "arc diff" and saw the repository detect as LFS.

Differential Revision: https://secure.phabricator.com/D21190
2020-04-29 16:23:44 -07:00
epriestley
196f8f54ce Remove "backout", "close", "flag", "start", "stop", "time", and "revert" workflows
Summary: Ref T13490. These workflows don't seem worth the maintenance cost, see T13488 for discussion.

Test Plan: Grepped for methods which looked like they might only be called by these flows, only dug up the backout stuff.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21100
2020-04-13 07:09:49 -07:00
epriestley
0f2e277cd9 Update "arc amend" for Toolsets
Summary: Ref T13490. Moves "arc amend" to Toolsets with modern ref/hardpoint code.

Test Plan: Ran "arc amend --show", "--revision", etc. Hit all the prompts and errors, probably?

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21095
2020-04-12 13:48:26 -07:00
epriestley
4cc05c377d Add a "CommitSymbolRef" for resolving symbolic commits into stable commit hashes
Summary:
Ref T13490. Frequently, we know the symbolic name of a commit (like "master") but need the immutable identifier for it (the commit hash).

Provide a Ref and Query for doing this lookup.

Test Plan: Ran `arc inspect symbol(...)` with various symbols, saw appropriate resolutions, nulls, or errors.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21090
2020-04-12 13:19:53 -07:00
epriestley
92be6df0eb Add a mode to "ExecFuture" that makes "resolvex()" semantics the default
Summary:
Ref T13490. "ExecFuture" can raise a "CommandException" when the subprocess fails. In most cases, this is desirable, since we usually do not expect subprocesses to fail.

Currently, "execx()" (which raises these exceptions) is the synchronous default (with the more verbose "exec_manual()" as an alternative if you expect the command may return an error code), but the Future variation has no way to hint that external resolution should raise an exception if the process exits with an error.

Since the "HardpointEngine" yield construct can resolve external futures, add a mode for this to simplify implementing "HardpointQuery" classes a bit. Without this, they either need to retain "$futures" and manually call "resolvex()", or check the "$err" result and throw some other exception, both of which are low-value boilerplate (queries that want to do this still can, of course).

This is basically like providing a hint that the futures should be resolved with "resolvex()" instead of "resolve()".

Also, make this the default behavior of a new "$api->newFuture()" wrapper.

Test Plan: Intentionally broke a command in a HardpointQuery, got a sensible exception automatically during external future resolution.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21089
2020-04-12 13:18:11 -07:00
epriestley
adea2550f5 Introduce "arc inspect" and some of the new ref/hardpoint classes
Summary:
Ref T11968. Inches toward the new ref/hardpoint code by introducing the modern refs as "RefPro" objects and supporting an "arc inspect <object>" to load objects and hardpoints.

This doesn't impact any existing runtime behavior.

Test Plan: Ran "arc inspect [--all] commit(...)", got hardpoint queries and yield-based data fetching.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21078
2020-04-10 05:01:25 -07:00
epriestley
0b3cd39230 Update the "WorkingCopy" API and create a fallback "Filesystem" working copy
Summary:
Ref T11968.

  - Allow "WorkingCopy" objects to maintain an API object and update callers ("get...()" instead of "new...()").
  - Always generate a WorkingCopy object and a RepositoryAPI object.

Currently, code has to look like this:

```
$working_copy = ...
if ($working_copy) {
  $repository_api = ...
  if ($repository_api [instanceof ... ]) {
```

This is clunky. There's also no reason some "arc" commands can't run outside a VCS working directory without special-casing how they interact with the filesystem.

Conceptually, model the filesystem as a trivial VCS (which stores exactly one commit, always amends onto it, and discards history). Provide a trivial WorkingCopy and API for it.

(This change isn't terribly interesting on its own, but chips away at landing the new Hardpoint infrastructure.)

Test Plan: Ran `arc version`, `arc upgrade`.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21070
2020-04-08 09:22:43 -07:00
epriestley
368aec16a1 Update some ancient "set X=Y" environment code for new Windows execution without a shell
Summary:
Depends on D21051. Ref T13504. Ref T13209. This very old Mercurial code uses "X=Y hg ..." on Linux and "set X=Y & hg ..." on Windows.

The latter construct no longer works because we bypass the shell. The former construct is obsolete.

Additionaly, delete some ancient "branch merge" code which has no callers.

Test Plan: Created a diff in a Mercurial repository on Linux. I minimally vetted this on Windows since I don't have a "hg + Windows" environment at the moment.

Maniphest Tasks: T13504, T13209

Differential Revision: https://secure.phabricator.com/D21052
2020-04-02 13:50:57 -07:00
epriestley
8c4f6ce161 Merge the remainder of the "experimental" branch
Summary:
Depends on D20986. Ref T13395. This //mostly// collapses the entire "experimental" branch into "master".

I plan to change the "Ref/Hardpoint" pattern to become future oriented, but this is more steps forward than sideways.

Test Plan: Ran various `arc` workflows.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20987
2020-02-13 06:05:08 -08:00
epriestley
21a1828ea0 Omit "--" in older fallback commands for Git remote URIs
Summary: Ref T13481. Some older versions of Git appear to not support "--" in these commands. Just drop it. This can lead to ambiguous results with certain obviously-silly remote names, but doesn't appear to lead to anything dangerous.

Test Plan: Will followup with user on ancient Git.

Maniphest Tasks: T13481

Differential Revision: https://secure.phabricator.com/D20952
2020-01-23 16:50:54 -08:00
epriestley
70c0fd3f22 In Git, fall back across versions more cleanly when trying to get the URI for a remote
Summary: Fixes T13481. When identifying the URI for a remote, fall back from "git remote get-url" to "git ls-remote --get-url" to "git config remote.<name>.url" based on command output and version tests.

Test Plan: Ran `arc land --hold`, rigged the subcommands to fail to try all three fallbacks, ran `arc land --hold --remote asdfasdf` to get an explicit failure.

Maniphest Tasks: T13481

Differential Revision: https://secure.phabricator.com/D20950
2020-01-23 15:19:28 -08:00
epriestley
cc1ff38843 When generating diffs in "arc diff", disable Git config option "diff.suppressBlankEmpty"
Summary:
Ref T13432. Git has a "diff.suppressBlankEmpty" config option which makes it emit nonstandard diffs with trimmed trailing whitespace on unchanged blank lines.

Currently, we don't parse these diffs correctly. Even if we do in the future, emitting a more standard diff is desirable.

Explicitly disable this option when executing "git diff" so we build more standard diffs.

Test Plan:
  - Configured this option.
  - Modified a file with a blank line in it without changing the blank line, got this goofy display diff:

{F6985234}

  - Applied patch, rediffed the same change, saw "-c diff.suppressBlankEmpty" in "--trace" output and got this sensible diff:

{F6985235}

Maniphest Tasks: T13432

Differential Revision: https://secure.phabricator.com/D20877
2019-10-29 10:14:40 -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
83661809e5 Work around a Windows escaping issue and security conecern in "hg cat --output ..."
Summary:
See PHI904. Ref T13210. Ref T13209. Currently, we have an `hg cat` construction which attempts to pass a literal `%p` to Mercurial. This fails because you can't pass `%` through `%s` outside of `wilds`.

It also uses `%C` to pass a list of file paths. This is broadly unsafe and can cause command execution if you modify a file named, e.g., `; rm -rf xyz` or similar. I think it would be difficult to turn this into an attack but it's fairly bad. This dates from D5144 in 2013.

Test Plan: With this patch, created D19757 which has valid binary data (see F5962134).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13210, T13209

Differential Revision: https://secure.phabricator.com/D19758
2018-10-26 07:28:50 -07:00
Aviv Eyal
875d018360 Fix arc diff when adding large new file with new git
Summary:
See https://discourse.phabricator-community.org/t/arc-not-supporting-git-2-17-1/.
When treating it a large file  binary, we try to get the "old" and "new" content using `git ls-tree` and `cat-file`.
If the file is new or deleted, there is no old file, so we try to work with filename `null`.

Under git < 2.17.1, that gets treated as `git ls-tree -- .`, which falls in the next condition under "no such path".
In git 2.18, etc, this is an error.

Explicitly bail out if there is no filename.

Test Plan: Add a new, large (>4Mb) file, arc diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D19513
2018-07-09 17:59:15 +00:00
epriestley
222800a86e Parse Mercurial changeset evolution "instability" log field
Summary: See PHI718. Modern Mercurial with the "evolve" extension enabled may emit this field.

Test Plan: As D19262.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19498
2018-06-19 16:15:30 -07:00
Alex Vandiver
ad3087e5e1 Correctly parse git status --porcelain=2 output with filenames with spaces
Summary:
Filenames are last in `git status --porcelain=2` lines; they
are not escaped in any way, despite the fields being
whitespace-delimited.  `explode` thus happily chops apart filenames
with spaces in them, causing later git operations to operate only on
the filename up to the first space.

Split the lines into the right number of elements -- in all cases,
this is one more than the index we're using, since filenames come last.

Test Plan:
Altering a file with a space in its path, and running `arc diff -a`.

Added tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D19389
2018-04-19 19:17:16 -07:00
epriestley
b8c9c385a7 Survive extra "obsolete:" log output from the Mercurial evolve extension
Summary: See PHI502; see <https://bugzilla.mozilla.org/show_bug.cgi?id=1448137>.

Test Plan: I spent all of three minutes trying to install the `evolve` extension without success and gave up, but this probably does the right thing based on the example output in the Bugzilla issue.

Differential Revision: https://secure.phabricator.com/D19262
2018-03-26 14:12:15 -07:00
Alex Vandiver
9144658e69 Accelerate working tree operations in git
Summary:
Currently, `arc` on `git` uses the following commands to examine the
state of the working tree and history; example times for a no-op diff in a
165k-file working tree are also shown:

```
1)  git diff --no-ext-diff --no-textconv --submodule=short --raw 'fb062d4ecce5d9c1786b7bfc8a0dedf6b11fdd96' --
  = 1,722,514 us

2a) git diff --no-ext-diff --no-textconv --submodule=short --raw 'HEAD' --
  = 1,715,507 us

2b) git ls-files --others --exclude-standard
  = 2,359,202 us

3)  git diff-files --name-only
  = 1,333,274 us
```

Steps (2a) and (2b) are run concurrently; this results in a total elapsed
wallclock time of approximately 5.4 seconds.  This is inefficient -- all four of
the above steps must both load the index and examine the working copy, which may
be slow operations when large repositories are used.  Additionally, none of the
effort of those stat calls on the working tree, or load time of the index, is
shared across the processes.

Step (1) is called from `getCommitRangeStatus`, which was split out in D4095; it
is currently never called on its own, only ever from `getWorkingCopyStatus`,
where it it combined with `getUncommittedStatus`.  The current behavior of the
method is to return the set of changes //either// in local commits //or//
uncommitted in the working tree, which duplicates work that
`getUncommittedStatus` is intended to do.  Changing the behavior of this method
(in Git, and other VCSes) to only examine _committed_ status seems both inline
with the name of the method and the original description of it in D4095 -- and
also serves to make it much faster, as it is an operation that need not inspect
the working tree at all.

Steps (2a), (2b), and (3) attempt to gather the state of the working copy, and
as such are all I/O bound but must examine nearly identical data.  For git
2.11.0 and higher, we can instead rely on the machine-parseable `git status
--porcelain=2` format, which provides the information from all of these commands
at once.  It also allows additional performance improvements, as `git status`
has been the focus of several optimizations in the latest versions of git (the
untracked cache and fsmonitor services, for instance), which are not available
in the lower-level `diff`, `ls-files`, and `diff-files` commands.

This has the added benefit of fixing a bug noticed in T9455, in that uncommitted
or unstaged changes in modules can now be detected, regardless of if they also
have changed their base commit.  It further resolves a bug where `.gitmodules`
appeared to have unstaged changes, when in reality the unstaged changes were in
submodules elsewhere in the tree.

For backwards compatibility with versions of git < 2.11.0, the old code is left
in place.  It is possible that the simpler output from v1 of `git status
--porcelain` would also suffice for some of the above benefits, but the payoff
of parsing yet another format is deemed insufficient; users wishing improved
performance should simply upgrade `git`.

Alltogether, these result in the following, for a no-op diff in a
165k-working-file tree:

```
1) git diff --no-ext-diff --no-textconv --submodule=short --raw 'fb062d4ecce5d9c1786b7bfc8a0dedf6b11fdd96' HEAD --
 = 9,227 us
2) git status --porcelain=2 -z
 = 739,964 us
```

...for a total of 749ms, an improvement of 4.7s.

Depends on D18841.

Test Plan: Existing tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D18842
2017-12-23 20:12:50 -08:00
Alex Vandiver
f34467914c Add tests for git submodules, based on current behavior
Summary:
This adds tests that detail the current behavior of `arc` in
the presence of `git` submodules.

Test Plan: No behavior change; wrote the tests such that they pass.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D18841
2017-12-23 20:01:20 -08:00
epriestley
76b54ce0a9 Fix parsing of Git branches with common and useful name "0"
Summary: See <https://discourse.phabricator-community.org/t/arc-has-a-spurious-failure-on-arc-diff/521>. Oh, PHP!

Test Plan: Created a branch named "0", ran `arc diff`. Before: fatal. After: this beautiful revision.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18678
2017-10-04 10:20:48 -07:00
Jakub Vrana
3b6b523c2b Fix errors found by PHPStan
Test Plan: None.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D17376
2017-02-18 09:24:19 +00:00
Alex Vandiver
f3037bf216 [git] Override diff.submodule so git diff output is always parseable
Test Plan:
Removed a submodule with `diff.submodule` set to `log`, saw
`arc diff` error; with this change, it no longer does.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T10881

Differential Revision: https://secure.phabricator.com/D17327
2017-02-13 17:40:26 -08:00
Chris Burroughs
c243cbbd9f tighten remote URI error handling with idiosyncratic remote names
Summary:
`git ls-remote` has an unusual way to indicate a URL was not
found: echoing back user input
```
$ git ls-remote --get-url does_not_exist
does_not_exist
$ echo $?
0

```

`getRemoteURI` handles checking for remotes other than 'origin', but
the error handling always matched against the string 'origin'
regardless of remote name.

Test Plan:
With a git config along the lines of:
```
[remote "my_special_name"]
        url = ssh://secure@secure.phabricator.com/diffusion/ARC/arcanist.git
        fetch = +refs/heads/*:refs/remotes/my_special_name/*
[branch "master"]
        remote = github
        merge = refs/heads/master
[remote "github"]
         # url = git@github.com:phacility/arcanist.git
         fetch = +refs/heads/*:refs/remotes/github/*
```

and running in a branch tracking `master` (github).  `arc which` would
(without this diff) show:
```
The remote URI for this working copy is "github".
```
With this diff, `arc which` correctly shows:
```
Unable to determine the remote URI for this repository.
```

When diffing against a tracking branch with a propertly configured
remote (the happy path), `arc which` still correctly identifies the
remote URI:
```
The remote URI for this working copy is
"ssh://secure@secure.phabricator.com/diffusion/ARC/arcanist.git".
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, chad, epriestley

Differential Revision: https://secure.phabricator.com/D17110
2016-12-29 10:35:59 -05:00
Alex Vandiver
ee6357386d Correctly parse file renames and copies from git diff --raw
Summary:
`parseGitRawDiff` dealt with the `A`, `M`, and `D` status flags from
`git diff --raw`, for file additions, modifications, and deletions
respectively.  However, it failed to cope with `C` and `R` flags, for
copies and renames.  Git version 2.9 and above default to resolving
renames, even in `git diff --raw` output, making this lack of support
only salient now (though users with Git's `diff.rename` set
encountered it previously).

Those two flags differ from the other three in that they offer both the
source and destination filename, separated by a tab.  As
`parseGitRawDiff` was not aware of this property, it returned a
"filename" of `"oldfile\tnewfile"`.  This is surfaced in several
places, including as passed to linters as a filename to check.
Needless to say, this file is nearly guaranteed to never exist on
disk.

Detect both the `C` and `R` flag types, and generate either a file
addition, or a pair of addition/deletion entries.

Test Plan:
Renamed a file, with a linter that printed each file it was
called with.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: jboning, Korvin

Differential Revision: https://secure.phabricator.com/D16387
2016-08-16 17:27:20 -07:00
Allan Jude
19608cffe6 SVN buildSyntheticAdditionDiff: exit sooner if path is a directory
Summary:
When doing svn copy, or svn mv, a SynthenticAdditionDiff is generated.

If the path is a directory, an error will occur when checking the
mime-type of the directory. Immediately after the properties check,
the function returns null if the path is a directory. Move this
check to before the properties check to avoid exiting with an error.

```
Command failed with error #1!
COMMAND
svn propget 'svn:mime-type' '/home/trasz/svn/ports/cad/py-pycam'@

STDOUT
(empty)

STDERR
svn: warning: W200017: Property 'svn:mime-type' not found on '/home/trasz/svn/ports/cad/py-pycam@'
svn: E200000: A problem occurred; see other errors for details

(Run with `--trace` for a full exception trace.)
```

Test Plan: Created differentials of changes with `svn copy` and `svn mv`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Tags: #subversion

Differential Revision: https://secure.phabricator.com/D15985
2016-06-05 15:12:46 -07:00
epriestley
2234c8cacc Fix arc which for svn repos
Summary:
`arc which` is currently broken for svn repos.

Fixes T6635

(I think I independently wrote an identical change to yours)

Test Plan: `∴../../p/arcanist/bin/arc which` on a svn repo.

Reviewers: aik099, epriestley, #blessed_reviewers

Reviewed By: aik099, epriestley, #blessed_reviewers

Subscribers: aik099, Korvin

Maniphest Tasks: T6635

Differential Revision: https://secure.phabricator.com/D15922
2016-05-16 12:54:37 +00:00
epriestley
a2ab38df78 Improve performance of arc branch in Git with many branches
Summary:
This is mostly just a personal quality-of-life fix. I run this command fairly often and having it return a little faster is nice.

This replaces a `git show` for each individual branch with a big `git for-each-ref` which we were already running anyway. This is quite a bit faster.

This command also occasionally hangs or segfaults for me while executing the huge pile of subprocesses. This is unreliable to reproduce, probably some bug in some PHP extension I have, and likely hard to narrow down, and this approach is better in every way anyway.

Test Plan:
  - Ran `arc branch` in Git, observed faster output (in my `phabricator/`, about 2000ms -> 1200ms).
  - Ran `arc feature` in Mercurial.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15735
2016-04-16 16:39:32 -07:00
epriestley
d0e73bb656 Don't use "-c" flag in "git:branch-unique()" revision range rule
Summary:
Fixes T9953.

  - "-c" was introduced in 1.7.2.
  - "--no-color" has existed forever as far as I can tell.
  - "--no-column" was introducd in 1.7.11, but there was nothing that needed to be disabled before that (hopefully).

Test Plan:
  - Ran `arc which --trace` and observed a reasonable `git branch` command with correct output.
  - Ran `arc which --trace` with a faked older Git version, observed command omit `--no-column`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9953

Differential Revision: https://secure.phabricator.com/D14735
2015-12-10 14:19:39 -08:00
Joshua Spence
b52e9dc702 Linter fixes
Summary: Minor linter fixes

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14633
2015-12-07 21:35:34 +11:00
epriestley
c71fe67ccb Address feedback from D14530
Summary: See D14530.

Test Plan: Careful scrutiny.

Reviewers: chad, alexmv

Reviewed By: alexmv

Differential Revision: https://secure.phabricator.com/D14554
2015-11-23 10:32:13 -08:00
Alex Vandiver
e730ececbc Examine upstream path instead of assuming "origin"
Summary:
Instead of blindly assuming that "origin" is the repository that
arcanist should communicate with, use the remote that is configured
for the branch in git.

Test Plan:
Used `arc which` with a branch with no upstream, an
origin/master upstream, and an upstream/master upstream -- the last of
which is being used to create and land this diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: joshuaspence, Korvin

Differential Revision: https://secure.phabricator.com/D14530
2015-11-23 10:29:20 -08:00
epriestley
c844669326 After pushing at the end of "arc land", cascade the origin through all local tracking branches
Summary:
Fixes T9661. Users can construct arbitrarily long chains from the remote, like:

  (remote) origin/master -> (local) cascade-a -> (local) cascade-b -> (local) cascade-c -> (local) cascade-d

When a user lands "cascade-d" onto "origin/master", we should pull A, B and C if they aren't ahead of the remote.

If a user lands "cascade-d" onto itself, we should pull A, B, and C if they aren't ahead of the remote, then reset D to the remote.

We also find this chain if the last component of it is connected by the local branch having the same name as the remote branch (typical for "master") instead of an actual connection through tracking brnaches.

Test Plan: See comment below.

Reviewers: chad

Reviewed By: chad

Subscribers: edibiase

Maniphest Tasks: T9661

Differential Revision: https://secure.phabricator.com/D14361
2015-10-28 14:01:35 -07:00
epriestley
2a2fd6e338 Pull git upstream-path logic into a separate class
Summary:
Ref T9661. I need to reuse this to fix the complex workflow described in T9661 where we need to follow multiple paths to the upstream and cascade updates across them.

Pull the logic into a separate class to make this easier and less copy/pastey.

This shouldn't change any behavior.

Test Plan: Ran `arc land --preview` from detached head, remote-tracking branch, non-tracking branch, local-tracking branch. Selection of target/remote seemed correct in all cases.

Reviewers: chad

Reviewed By: chad

Subscribers: edibiase

Maniphest Tasks: T9661

Differential Revision: https://secure.phabricator.com/D14360
2015-10-28 13:19:30 -07:00
epriestley
aa5c023fe8 Make rules for guessing onto/remote more powerful and more explicit in arc land
Summary:
Fixes T9543. Fixes T9658. Ref T3855.

Major functional change is that you can have a sequence of branches like:

  origin/master -> notmaster -> feature1

...where they track each other, but you named your local master something else. Currently, we resolve only one level of upstreams, so we try to land onto "notmaster" in this case, which is wrong.

Instead, keep resolving upstreams until we either hit a cycle, don't have another upstream to look at, or find someting in a remote. In this case we'll eventually find "origin/master" and select "origin" as the remote and "master" as the target.

Other minor changes:

  - Make this selection process explicit.
  - Make the help 3000x longer.

Also fix a bug where we could incorrectly try to tell Differential to update awith `--preview`.

Test Plan:
  - Landed from a tag.
  - Landed from a tracking branch.
  - Landed from an nth-degree tracking branch.
  - Tried to land from a local branch with a cycle in upstreams.
  - Landed with --remote and --onto.
  - Read `arc help land`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9658, T3855, T9543

Differential Revision: https://secure.phabricator.com/D14357
2015-10-28 09:37:17 -07:00
epriestley
9c056c5cc8 Improve arc's handling of dirty submodules in Git
Summary:
Fixes T9455. Depends on D14136. When you have a dirty submodule:

  $ nano submodule/file.c # save changes

...we currently ask you to make a commit when you run `arc diff`, which is meaningless and misleading.

Instead, prompt the user separately.

This behavior isn't perfect but I think it's about the best we can do within reason.

Test Plan:
  - Ran `arc diff` in a working copy with uncommitted submodule changes only, got new prompt.
  - Ran `arc diff` in a working copy with submodule base commit changes only, got old (correct) prompt.
  - Ran `arc diff` in a working copy with both, got only old prompt (which is incomplete, but reasonable/meaningful).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9455

Differential Revision: https://secure.phabricator.com/D14137
2015-09-21 12:40:06 -07:00
Javier Arteaga
c22dfe61ed Use 'remote.origin.url' fallback for git < 1.7.5
Summary:
'git ls-remote --get-url' is more correct, but younger and less
supported. This commit tempers previous optimism about its availability,
improving support for users of older git packages.

Test Plan:
* Set `git config url.xttps.insteadOf https` rewrite rule.
* Ran `arc which` with git 1.7.5 in `$PATH`, saw rewritten configured remote.
* Ran `arc which` with git 1.7.4 in `$PATH`, saw configured remote.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13998
2015-08-26 15:59:03 -07:00
Javier Arteaga
4c3d75401f Use 'git blame --porcelain' for git blame info
Summary:
This guards against stability issues with the output format of 'git
blame' (such as git config, localization (ref T5554) or future changes).

For example, `git config blame.blankboundary true` breaks `arc cover`
before this patch.

Test Plan:
* Set `git config blame.blankboundary true` on a test repo.
* Ran `arc cover`. It failed with an exception ("Bad blame?").
* Applied this patch.
* `arc cover` works.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T5554

Differential Revision: https://secure.phabricator.com/D13993
2015-08-25 09:36:16 -07:00
Javier Arteaga
46009145f7 Minimize reliance on 'git branch' output format
Summary:
Ref T5554. Both the current branch name (if on a branch), as well as the
list of all local branches, can be retrieved without having to parse the
output from "git branch".

Unfortunately, there seems to be no git plumbing for "get list of
branches containing this commit" yet.
(see http://marc.info/?l=git&m=141408477614635&w=2)

For that case, this commit whitelists the output from "git branch" using
the known valid branch names from "git for-each-ref".

Test Plan:
Set up a test repo with this structure:
```
|   *  Commit B1, on branch "subfeature"
|  /
| *    Commit A1, on branch "feature"
|/
*      Commit M1, on branch "master"
|
```

In `subfeature`, I tried:
* `arc which --base 'git:branch-unique(master)'`
* `arc feature`
After that, I detached my HEAD (don't worry, I got better) and tried again.

Nothing looked broken.

(Tested with git 1.7.2.5 and 2.5.0.)

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T5554

Differential Revision: https://secure.phabricator.com/D13989
2015-08-24 17:57:41 -07:00
Javier Arteaga
6ecb3fb87d Avoid parsing git "remote show" using "ls-remote"
Summary:
Ref T5554. This makes git remote URL detection locale-agnostic.

The previously suggested `git config remote.origin.url` command does
almost the same, but does not support the URL rewriting features in
git-config (`url.<base>.insteadOf`).

This one does, although it has the unintuitive behavior of just printing
the passed remote name when the remote does not exist, or even when
called outside a git repo.

Test Plan:
* Switched to non-english locale in which git has a translation.
* Ran `arc which` on the Arcanist repo. It could not determine the remote URI.
* Applied patch, `arc which` found the URI.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: johnny-bit, Korvin

Maniphest Tasks: T5554

Differential Revision: https://secure.phabricator.com/D13983
2015-08-24 04:51:03 -07:00
Joshua Spence
e00e2939c1 Various linter fixes
Summary: Self explanatory.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13861
2015-08-11 22:35:40 +10:00
Joshua Spence
956bfa701c Extend from Phobject
Summary: All base classes should extend from `Phobject` or some other classes. See D13275 for some explanation.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13281
2015-06-15 15:47:33 +10:00
Joshua Spence
64d03ff68b Remove arcanist projects from working copy code
Summary: Ref T7604. Remove "arcanist projects" from `ArcanistWorkingCopy` and a few other callsites. Depends on D12999.

Test Plan: Can't really think of how to test this.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12945
2015-05-26 07:08:56 +10:00
Joshua Spence
6f86866104 phtize a bunch more strings
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
2015-05-22 17:09:56 +10:00
Joshua Spence
e3311f0832 Remove "arcanist projects" from arcanist
Summary: Ref T7604. Remove "arcanist projects". Depends on D12894.

Test Plan: This diff (so meta).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12901
2015-05-20 11:29:41 +10:00
Joshua Spence
753705b2c5 Rename ArcanistPhutilTestCase to PhutilTestCase and Remove ArcanistTestCase
Summary: Ref T7977. The `ArcanistTestCase` class is pointless and can be replaced by `ArcanistPhutilTestCase`. Furthermore, it sorta makes sense to just rename `ArcanistPhutilTestCase` to `PhutilTestCase`. Depends on D12664 and D12666.

Test Plan: `arc unit`

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12665
2015-05-20 09:40:24 +10:00
Joshua Spence
d2b38cdf94 pht all the things
Summary: `pht`ize almost all strings in rARC.

Test Plan: ¯\_(ツ)_/¯

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12607
2015-05-13 21:00:53 +10:00
Joshua Spence
8919a9c5b5 Remove hook functionality
Summary: Fixes T7674. Remove remaining commit hook functionality.

Test Plan: Unit tests still pass?

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12698
2015-05-05 21:10:47 +10:00
Joshua Spence
7bba30f66c Various linter fixes
Summary: Apply various linter fixes.

Test Plan: Unit tests + eyeballing.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12388
2015-04-14 06:29:07 +10:00
Joshua Spence
bb8e9d7357 Fix visibility of ArcanistMercurialAPI::didReloadCommitRange
Summary: Ref T6822.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11369
2015-01-14 06:48:25 +11:00