Fix parsing of PHPUnit JSON log format for PHPUnit 3.7.7

Summary: See https://github.com/facebook/arcanist/pull/55

Reviewed by: epriestley
Fix typo in comment
Don't pull message twice from Conduit in `arc diff`

Summary:
We currently pull message from Conduit twice in `arc diff` update.
Once in `buildCommitMessage()` and once in `buildRevisionFromCommitMessage()`.
Remeber that we already pulled it (and so it is authoritative) to save this call.

Even faster solution would be to not pull and update the message at all in common (non-`--edit`, non-`--verbatim` and such) update workflows but it's more involved.

Test Plan:
  $ arc diff --trace

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3716
Use git-cherry-pick for arc patch

Summary:
This changes what --nobranch does, though not what it means.  In git, the patch
is applied to the git commit it is based off where it will commit cleanly, and
if you specify --nobranch then arc-patch will then switch back to your original
branch and try to cherry-pick the commit there.

If this fails, you end up with merge-conflict markers in the file which can be
handy.

Test Plan: https://secure.phabricator.com/P572

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3712
Make Arcanist workflow names explicit

Summary:
Currently, adding a new workflow requires you to override ArcanistConfiguration, which is messy. Instead, just load everything that extends ArcanistBaseWorkflow.

Remove all the rules tying workflow names to class names through arcane incantations.

This has a very small performance cost in that we need to load every Workflow class every time now, but we don't hit __init__ and such anymore and it was pretty negligible on my machine (98ms vs 104ms or something).

Test Plan: Ran "arc help", "arc which", "arc diff", etc.

Reviewers: edward, vrana, btrahan

Reviewed By: edward

CC: aran, zeeg

Differential Revision: https://secure.phabricator.com/D3691
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