ArcanistBundle: fix attribution of $old_phid when building binary changes

Summary:
The variable $old_phid was not being set in a certain situation in
buildBinaryChange(), and that was causing the following error, during
`arc patch <revision>`:

"the patch applies to <file> (<hash>), which does not match the
current contents."

and hence it was failing to download/apply the patch.

Signed-off-by: Sergio Correia <sergio@correia.cc>

Test Plan:
I spotted the problem in a revision where I was renaming
some images, which are binary.

Reviewers: epriestley, vrana, btrahan

Reviewed By: btrahan

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3925
make arc patch workflow preserve author commit information

Summary: assumes D3917 (or something like it that populates 'author' value from conduit call) exists in production

Test Plan: stubbed out 'author' value and verified checkins as author worked

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T479

Differential Revision: https://secure.phabricator.com/D3918
Provide repository API method for getting all files

Test Plan:
  print_r(iterator_to_array($api->getAllFiles())); // Under Git.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3940
Provide repository API method for getting changed files

Test Plan:
  print_r($api->getChangedFiles('HEAD~36')); // Under Git.

Verified that deleted files are false.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3941
Introduce `arc lint --severity warning`

Summary: I plan to exclude advices from our saved results.

Test Plan:
  $ arc lint src/lint/engine/PhutilLintEngine.php
  $ arc lint --severity advice src/lint/engine/PhutilLintEngine.php
  $ arc lint --severity error src/lint/engine/PhutilLintEngine.php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3936
accommodate git's diff.suppress-blank-empty=true setting

Summary:
accommodate git's diff.suppress-blank-empty=true setting
Without this change, if you were to set diff.suppress-blank-empty=true
in your .gitconfig (as I do), then "arc diff" would always fail with the
cryptic diagnostic, "Diff Parse Exception: Found the wrong number of
hunk lines."

Test Plan:
Put this in ~/.gitconfig or .git/config
[diff]
         suppress-blank-empty = true
and run "arc lint".  It should pass.  Without this chnage,
it would fail as described above.

Reviewers: vrana, epriestley

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3963
Allow ArcanistDiffParser to parse `diff.suppress-blank-empty` Git diffs

Summary: See D3963. Instead, parse these diffs so they'll work with `--raw`, etc.

Test Plan:
Generated a failing diff, added it as a test case. Fixed issue. Ran test suite. Ran `arc` against it:

  $ git -c diff.suppress-blank-empty=true diff HEAD | arc diff --raw --only --conduit-uri=http://local.aphront.com:8080/
  Reading diff from stdin...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/103/

  Included changes:
    M       things

Reviewers: vrana, jiiix, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3969
Revert "accommodate git's diff.suppress-blank-empty=true setting (D3963)"

Summary: Reverts D3963, which is obsoleted by D3969.

Test Plan: Straight revert.

Reviewers: vrana, jiiix, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3970
Allow running `arc diff` without `git commit`

Summary:
There's quite some logic in here:

- It automatically decides whether to create a new commit or amend.
- It partially respects 'default-relative-commit'.
- However if it points to a closed revision then it creates a new commit.

Resolves T2025.

Test Plan:
`arc diff` on:

- Clean committed repository.
- Dirty repository without commit since 'default-relative-commit'.
- Dirty repository with non-revision commit since 'default-relative-commit'.
- Dirty repository with revision commit since 'default-relative-commit'.
- `arc diff HEAD^` on dirty repository on top of closed revision.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2025

Differential Revision: https://secure.phabricator.com/D3967
Reintroduce `arc diff --advice`

Summary:
Some users want be stopped even if there are lint advices.

NOTE: Deleted by D3364.

Test Plan:
On diff with lint advice:

  $ arc diff --preview # advice just printed
  $ arc diff --preview --advice # excuse required

Reviewers: epriestley

Reviewed By: epriestley

CC: akramer, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3982
make arcanist backwards compatible with various conduit versions

Summary: pragmatism wins the day, though I think eventually we might want something really fancy to deal with arcanist and conduit not being up to date with respect to one another

Test Plan: php -l

Reviewers: epriestley

Reviewed By: epriestley

CC: zeeg, aran, Korvin

Maniphest Tasks: T2088

Differential Revision: https://secure.phabricator.com/D3988
Don't use empty commit message in commit from `arc diff`

Summary:
There was a bug in Git: https://github.com/git/git/commit/d9a935

Also fix a bug with discovering existing commits.

Test Plan:
  $ arc diff # yes
  # abort
  $ arc diff # didn't see fatal

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2025

Differential Revision: https://secure.phabricator.com/D3983
Change lint_engine to lint.engine

Summary: Also unit_engine.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, aurelijus

Differential Revision: https://secure.phabricator.com/D4001
Don't amend commits with different author

Summary: Also delete extra newlines.

Test Plan:
  $ arc diff # on top of my commit

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2025

Differential Revision: https://secure.phabricator.com/D3996
Allow caching lint results

Summary:
There is one big decision: How to get linters version.
I've created `getCacheVersion()` which is supposed to be bumped every time engine or any linter is changed.
This is not very nice but the other alternative (detect this automatically) seems worse:

- The engine may be outside repo and may or may not be under version control so getting its version through something like `git log` may not be even possible.
- If it is in the same repo then every rebase will obsolete the whole cache.

Even though bumping the version manually is PITA I still think it's a better solution.

Test Plan:
  $ time arc lint --cache 1
  # verified file
  $ arc arc lint --cache 1
  # added some debug output to see the cached results
  # also observed better time (.57 s instead of 2.19 s)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4006
Emit advice for TODO in Phutil linter

Summary:
Let's try this.
It may be useful to see this together in Differential overview.

Test Plan: Linted a file with TODO.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T744

Differential Revision: https://secure.phabricator.com/D4014
Fix getting changed files in Git
Fix some arcanist warnings

Summary:
  - Rename some very old variables.
  - Wrap some contributed lines.

Test Plan: `arc lint` / `arc unit`. Viewed a diff in an uncacheable mode to verify intraline behavior.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4018
Set cache version per linter

Summary: Also delete cache with `--cache 0`.

Test Plan:
  $ arc lint --lintall --cache 1
  $ cat .git/arc/lint-cache.json
  $ arc lint --lintall --cache 1 # with debug output

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4013
Parse "--help" correctly in `arc`

Summary:
We delegate parsing to `PhutilArgumentParser` but then skip the parts of the code which do something with this flag.

In particular, `arc --help list` just runs `arc list`.

See https://github.com/facebook/arcanist/issues/58

Test Plan: Ran `arc --help`, `arc list --help`, `arc --help list`, `arc --help help --help`. Got expected help in all cases.

Reviewers: vrana, chad, btrahan

Reviewed By: chad

CC: aran

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