Instead of looping forever, resolve the relative commit

Summary: Obvious error in refactoring code around getCanonicalRevisionName() in D1954.

Test Plan: Ran "arc diff" without looping. Can you verify this fixes your case?

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T1025

Differential Revision: https://secure.phabricator.com/D1970
Resolve arc location through symlinks-of-symlinks, etc

Summary:
Stole this from stackoverflow; seems to work?

http://stackoverflow.com/questions/59895/can-a-bash-script-tell-what-directory-its-stored-in

Test Plan: Ran a symlink to bin/arc.

Reviewers: btrahan, zeeg

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1968
Fix a doc link in arcanist

Summary:
Missed this when I moved the document a while ago. See <https://github.com/facebook/arcanist/issues/21>.

NOTE: The actual document isn't very helpful in figuring out the error. I'll address this in a followup.

Test Plan: Hit error, clicked link, got docs.

Reviewers: btrahan

Reviewed By: btrahan

CC: jmhsieh, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1989
Set relative commit early in "arc diff"

Summary:
In Mercurial, we figure out if the working copy is dirty with "hg diff", and do a somewhat expensive merge-base / outgoing operation if the relative commit isn't set. We can set the relative commit earlier and avoid some extra work.

We also may incorrectly cache this state (diff from merge-base of outgoing to tip) and pass the wrong rev and file dirty list to the linters.

Test Plan: Made commits which changed (A, B) and then (A). Ran "arc diff tip^". Before this change, observed full outgoing + merge base resolve and both "A" and "B" passed to lint. Observed execution of fewer commands and lint executing against "A" only after this change.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1998
Update 'arc land': add --squash, add default config, fix windows compat

Summary:
  - We have "--merge", added similar "--squash" for completeness.
  - Switched away from (cd .. && ...) pattern which is not functional on Windows.
  - Allow a default --onto to be specified, if master isn't the best default for a project.

Test Plan:
  - Did --hold --keep dry runs, changes landed properly.
  - Set a default onto branch in .arcconfig.
  - Ran --merge and --squash together to verify conflict configuration.

Reviewers: btrahan

CC: aran, epriestley

Maniphest Tasks: T124, T1033

Differential Revision: https://secure.phabricator.com/D2001
Update 'arc land': add --squash, add default config, fix windows compat

Summary:
  - We have "--merge", added similar "--squash" for completeness.
  - Switched away from (cd .. && ...) pattern which is not functional on Windows.
  - Allow a default --onto to be specified, if master isn't the best default for a project.

Test Plan:
  - Did --hold --keep dry runs, changes landed properly.
  - Set a default onto branch in .arcconfig.
  - Ran --merge and --squash together to verify conflict configuration.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, gschmidt

Maniphest Tasks: T124, T1033

Differential Revision: https://secure.phabricator.com/D2001
Minor, fix inverted condition.
Minor, fix clowniness.
Improve compatibility of "arc patch" on Windows

Summary: Can't use (cd ...) on windows.

Test Plan: Ran "arc patch" successfully.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124, T1034

Differential Revision: https://secure.phabricator.com/D1987
Raise a better error message when "arc patch" fails because of filename case changes

Summary: Git can't apply filename case change patches on case-insensitive filesystems. Some day we could manually do this ourselves, but it's fairly rare and complicated -- just raise a useful warning.

Test Plan: Tried to apply a case-changing patch, got a good error.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T618

Differential Revision: https://secure.phabricator.com/D2015
Show a better expected vs actual for tests with large output

Summary: When tests have a lot of output, show a diff of the expected/actual.

Test Plan: Used this when developing D2016 to examine large output usefully.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2017
Minor, squelch a warning about reference variables.

Auditors: vrana
Avoid double indenting in console wrapping

Summary:
D2016 changed the behavior of `phutil_console_wrap()`.
The new behavior is better so I am fixing callsites instead of the function.

Test Plan:
  arc help help

Verify that the options descriptions is not indented with 28 spaces.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2047
Use assert_instances_of()

Summary: D2042

Test Plan:
- `arc lint` with dependent message
- `arc liberate`
- `arc diff`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2048
Minor Arcanist fixes

Summary:
Addresses concerns in rARCefb8219196abf047f14b505959e54d078e1df6d3:

  - As I recall, the intent of "generateFile" was that these warnings would replace all the other warnings for that file, under the theory that if one warning caused regeneration of
the file the other warnings were irrelevant.
  - However, this code never had any effect and I haven't seen any issues with the behavior.
  - So, just remove it.

Addresses concerns in rARC070e963d1c26879e47eab19a2377e388c2f166c5:

  - Ran "arc liberate --all".

Test Plan: Ran "arc lint" after removing an "__init__.php" with and without a "fixed" generateFile block, there was no behavioral change. So I just nuked it.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2049
Add a lint check for clobbering locals with iterators

Summary:
See D2049, D2050. Identify reuses of locals as iterator variables. Before raising an error, we require:

  - Variable is declared before the loop.
  - Variable is used after the loop, ignoring uses as an iterator variable.

I think this identifies all problems with a very low false positive rate (the false positives are suspicious/unconventional code, but not necessarily errors).

Also fix an issue identified by the linter.

Test Plan:
  - Verified this identified the bugs in D2049 and D2050.
  - Ran linter against libphutil/, arcanist/ and phabricator/ (see D2051, this, and next diff).
  - Ran unit tests.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley, jungejason

Differential Revision: https://secure.phabricator.com/D2052
Whitespace
[arcanist] properly handle arguments with spaces

Summary:
The new wrapper shell script that is bin/arc does not correctly
handle arguments with spaces in them.

Test Plan:
added a print_r($argv) to scripts/arcanist.php and ran bin/arc -m
"testing something" to see that "testing something" got passed in as
one argument instead of two.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2066
Minor, correct arc handling of relative symlinking (from @makinde).
Revenge of the git relative commit default

Summary:
The default of "arc diff" to "arc diff HEAD^" in git is universally confusing to everyone not at Facebook.

Drive the default with configuration instead. Even at Facebook, "origin/trunk" (or whatever) is probably a better default than "HEAD^".

See D863 for the last attempt at this.

NOTE: This is contentious!

Test Plan: Ran "arc diff", got prompted to set a default. Ran "arc diff" from a zero-commit repo, got sensible behavior

Reviewers: btrahan, vrana, nh, jungejason, tuomaspelkonen

Reviewed By: btrahan

CC: aran, epriestley, zeeg, davidreuss

Maniphest Tasks: T651

Differential Revision: https://secure.phabricator.com/D1861