Improve `arc` handling of various binary file operations in Git

Summary:
See T866, D3521. Additional things this fixes:

  - `arc export` now exports binary data correctly.
  - `ArcanistBundle` unit tests now load and apply binary data correctly.
  - `arc patch` no longer relies on `base` configuration.
  - Adds tests to the tarball:

  commit df340e88d8aba12e8f2b8827f01f0cd9f35eb758
  Author: epriestley <git@epriestley.com>
  Date:   Wed Oct 17 15:46:11 2012 -0700

      Remove binary image.

  commit 3f5c6d735e64c25a04f83be48ef184b25b5282f0
  Author: epriestley <git@epriestley.com>
  Date:   Wed Oct 17 15:45:58 2012 -0700

      Copy binary image.

  commit b454edb3bb29890ee5b3af5ef66ce6a24d15d882
  Author: epriestley <git@epriestley.com>
  Date:   Wed Oct 17 15:45:35 2012 -0700

      Move binary image.

  commit 5de5f3dfda1b7db2eb054e57699f05aaf1f4483e
  Author: epriestley <git@epriestley.com>
  Date:   Wed Oct 17 15:45:09 2012 -0700

      Add a binary image.

Test Plan: Ran unit tests, `arc patch`, `arc export`, `arc diff`, `arc upgrade`.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T866

Differential Revision: https://secure.phabricator.com/D3732
Require defining workflows' synopses and help

Test Plan: Deleted `getCommandSynopses()` from workflow, ran `arc help`.

Reviewers: epriestley, edward

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3739
Temporarily unrequire defining getWorkflowName()

Summary:
We need to run new Arcanist over old code in Perflab.
We also need to run new Arcanist over new code (with already deleted custom `buildAllWorkflows()`).

This will work because old code overwrites `buildAllWorkflows()`.

Test Plan:
  $ arc help
  $ arc help # after deleting getWorkflowName() from one workflow

Reviewers: edward, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3741
Don't lint file content of directories

Summary: This also changes how we treat `@generated` and `@nolint` - after this diff, we will still lint their path.

Test Plan:
Created directory `a.php` and symlink `b.php` pointing to directory.
`arc lint` previously printed:

> Requested path `/data/users/jakubv/devtools/arcanist/_.php' is not a file.

Now it prints:

> No lint warnings.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3737
Support --nobranch when not currently on a branch

Summary:
This is horrible and git specific, but fixes a case where people are using "arc
patch --nobranch ..." when they're not currently on a branch.

The old code assumed you were on a branch and used getBranchName() to record
this, in order to return to that branch later and cherry-pick the patch.

When not on a branch, and using arc patch --nobranch, this was trying to return
to the branch '(no branch)'.

Now, I detect that we're not on a branch and just record what HEAD is instead.

Test Plan:
Checkout the SHA of master (so I'm on master, but not on a branch) then try to
patch it with a feature diff:

  € git checkout e7a3ec68159d6847372cab5ad913f2f15aa7c249
  Warning: you are leaving 1 commit behind, not connected to
  any of your branches:

    ac1ad39 Updating a-file

  If you want to keep them by creating a new branch, this may be a good time
  to do so with:

   git branch new_branch_name ac1ad392350a51edd10343f12b9713f5e5b3707c

  HEAD is now at e7a3ec6... Fix arcconfig

  € arc patch --nobranch D7
  Created and checked out branch arcpatch-D7.
   OKAY  Successfully committed patch.

  € git branch
  * (no branch)
    feature
    haddock
    master

  € git log --oneline | head -2
  38c0235 Updating a-file
  e7a3ec6 Fix arcconfig

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3743
Don't unset linted paths from subdirectory and in commit hook mode

Summary: D3737#6

Test Plan:
  phabricator/src/ $ arc lint __phutil_library_init__.php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3746
Make parser more flexible in the face of Git diff prefix flags

Summary: See T1675 for discussion. We currently strip `[abicwo12]/` from Git patches, but the user can provide arbitrary prefixes with `--src-prefix` and `--dst-prefix`, or strip prefixes entirely with `--no-prefix`. In these cases, trust they know what they're doing rather than rejecting the diff.

Test Plan: Added a bunch of tests. We have existing tests for `diff.mnemonicprefix` and normal prefixes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1675

Differential Revision: https://secure.phabricator.com/D3744
Treat a file merged from another svn branch as an addition

Summary:
After a reintegration merge, "Copied From URL" will be different and current approach will result in a wrong path. If the path does not match, just mark it as a new file.

moved the comment before if so lines stay at 80 chars

Test Plan: in trunk, svn merge --reintegrate ^/branches/foo and arc diff - without this change it will say "Copied from es/foo/..."

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3748
Print lint results before throwing

Summary:
When some linter throws then we don't print any result.
This is bad in case when the linter threw e.g. because of syntax error in some file which some other linter will tell us about.

Test Plan: Threw from a linter, made lint error in a file, saw error then exception.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3756
Fix ArcanistSingleLintEngine for deleted paths

Summary:
Currently, ArcanisSingleLintEngine lints deleted paths and directories. These are sometimes appropriate, but SingleLintEngine is a less-sophisticated linter and should have more safe defaults.

Also fix an error where JSHint reported useless messages on failure.

Test Plan:
Reproduced the problem:

  $ git show
  commit d71efe2b13770c8861bcd3415c15503fc377339f
  Author: epriestley <git@epriestley.com>
  Date:   Fri Oct 19 12:22:50 2012 -0700

      WIP

  diff --git a/test.js b/test.js
  deleted file mode 100644
  index 8bd6648..0000000
  --- a/test.js
  +++ /dev/null
  @@ -1 +0,0 @@
  -asdf
  $ arc set-config --local lint.engine.single.linter ArcanistJSHintLinter
  Set key 'lint.engine.single.linter' = "ArcanistJSHintLinter" in local config (was null).
  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: JSHint returned output we can't parse. Check that your JSHint installation.
  Output:

Applied the error message fix:

  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: JSHint returned unparseable output.
  stdout:

  stderr:

  node.js:181
          throw e; // process.nextTick error, or 'error' event on first tick
          ^
  Error: ENOENT, No such file or directory '/INSECURE/repos/git-working-copy/test.js'
      at Object.statSync (fs.js:400:18)
      at _collect (/usr/lib/node_modules/jshint/lib/hint.js:77:12)
      at /usr/lib/node_modules/jshint/lib/hint.js:93:13
      at Array.forEach (native)
      at Object.hint (/usr/lib/node_modules/jshint/lib/hint.js:92:17)
      at Object.interpret (/usr/lib/node_modules/jshint/lib/cli.js:137:21)
      at Object.<anonymous> (/usr/lib/node_modules/jshint/bin/hint:2:25)
      at Module._compile (module.js:420:26)
      at Object..js (module.js:426:10)
      at Module.load (module.js:336:31)

Applied the remove paths fix:

  $ arc lint --engine ArcanistSingleLintEngine --rev HEAD^
  Usage Exception: No paths are lintable.

Reviewers: magazovski, btrahan, vrana

Reviewed By: btrahan

CC: aran, Korvin, vissi

Differential Revision: https://secure.phabricator.com/D3735
Fix two arc issues

Summary:
  - I caused $parser to be reused in D3732 which I belived was safe, but actually isn't. We end up writing to the same changes. We should make it safe but there's some mess in Phabricator that needs to be cleaned up first.
  - One minor error code thing, variable is undefined.

Test Plan: Ran `arc export --git` on a moved file, got a better result. Ran some command which made me hit the other case and didn't get a fatal anymore.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3749
Fix lint tests after D3756

Test Plan:
Threw normal exception, saw failed test.
Threw usage exception, saw skipped test.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3761
Set all lint paths at once

Summary:
This code is much more readable to me.
It should also be faster as building the array at once should be faster than one by one.

Test Plan: Made license and XHPAST errors in PHP file, made spelling error in JS file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3758
Pass key to aggregate exception

Summary: This information may be quite useful.

Test Plan: Threw exception from license linter.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3764
Make ArcanistDiffParser automatically load synthetic changes if the working copy is available

Summary: Make this harder to get wrong. Instead of requiring a separate call for synthetic data, automatically load it if we can.

Test Plan: Unit tests; `arc diff`.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T866

Differential Revision: https://secure.phabricator.com/D3750
Improve test coverage of diff/patch generation

Summary:
We have coverage for generating patches, applying them, and making sure they reproduce the original repository state. However:

  - The code uses simplified patch generation which omits some flags like `-M` and `-C`, and generally produces less rich patches than we really produce. Instead, produce patches the same way `arc diff` does.
  - We don't test the intermediate change representation. In theory it's not too important because if we get it wrong the output should be wrong, but in practice it makes it easier to nail down issues. We can also generate less-rich patches which still apply correctly, but would prefer not to.
  - Similarly, we don't test the intermediate patch representation. This is almost entirely redundant with simply applying the patch, but easier to visualize.

Add coverage for all that stuff and fix some bugs with `-M` / `-C` patch generation that weren't caught under the simpler patch generation.

Test Plan: Ran unit tests.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T866

Differential Revision: https://secure.phabricator.com/D3751
Add a large number of move/copy diff/patch tests

Summary: The way we represent some move/copy stuff is a bit messed up, but it mostly works, so add coverage before I mess with it.

Test Plan: Ran unit tests.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T866

Differential Revision: https://secure.phabricator.com/D3752
Fix some regexp issues for parsing SVN diffs with \r\n newlines

Summary: We need to tweak a few patterns to accommodate the possibility that lines end in "\r\n".

Test Plan: Added failing unit test and made it pass.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, mbishopim3

Maniphest Tasks: T1944

Differential Revision: https://secure.phabricator.com/D3772
Use PhutilAggregateException to aggregate unit test exceptions

Summary: Currently, a shutdown exception ("script exited with open transactions!") overwhelms the actual test failure exception, which is the one that needs to be fixed.

Test Plan: Ran a fixture test which opens a transaction and then throws. Got diagnostically useful output after this patch.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3780
Fix comprehensive lint engine