1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-30 02:32:41 +01:00
Commit graph

1338 commits

Author SHA1 Message Date
epriestley
ea1585d7fa Fix "empty diff" error in arcanist
Summary: Ref T2296. This error is unreachable right now -- when I fixed all the "\r\n" stuff, we always end up with a nonempty first line for an empty input. Do this test earlier and more explicitly. This results in a less useful error: "expected (some junk) on line 1" instead of "can't parse an empty diff".

Test Plan: Tried to parse an empty diff, got a "you can't parse an empty diff" error.

Reviewers: btrahan, vrana, codeblock

Reviewed By: codeblock

CC: aran

Maniphest Tasks: T2296

Differential Revision: https://secure.phabricator.com/D4370
2013-01-09 08:15:53 -08:00
Ricky Elrod
a5ddd7ebc0 Add a comma.
Summary: Add a comma because it was going to annoy the crap out of me.

Test Plan: See the comma. :)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4365
2013-01-08 15:51:36 -08:00
Roy Williams
7547ef7e96 Pass the constructed message to the TYPE_DIFF_DIDBUILDMESSAGE event.
Summary: We want to use TYPE_DIFF_DIDBUILDMESSAGE to abort arc diff when a message doesn't fit some

Test Plan: Created an EventListener subscribed to TYPE_DIFF_DIDBUILDMESSAGE, validated the 'message' field was filled in

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D4361
2013-01-08 15:06:46 -08:00
Jack Lindamood
48f5ecb05c Fix cppcheck path finding
Summary: This fix lets you run arc lint from any directory in the repository

Test Plan: Ran arc lint from any directory

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4356
2013-01-07 12:47:00 -08:00
epriestley
72b2b7b22c Limit arc branch to 16 concurrent processes
Summary:
Currently, this spawns 125 concurrent processes on my machine, which overflows some limit and gives me an error:

  PHP Warning:  proc_open(): unable to create pipe Too many open files in /INSECURE/devtools/libphutil/src/future/exec/ExecFuture.php on line 491

Instead, limit parallelism to 16. The runtime is approximately the same for me, and dominated by other concerns (conduit calls).

Test Plan: Ran `arc branch` successfully. Ran `arc branch --trace`, observed behavior.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4336
2013-01-07 08:44:55 -08:00
Jack Lindamood
cf8d445c9f CppCheck linter
Summary:
Lints cpp code using the cppcheck static linter.  This linter needs to
be downloaded and built from http://cppcheck.sourceforge.net/

Test Plan: Used it on a few files.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4353
2013-01-07 08:18:33 -08:00
Jack Lindamood
854c1b67b1 Add cpplint.py engine
Summary: Adds arc lint support for cpp files with Google's cpplint.py lint checking.

Test Plan: ran it on some cpp files. Added unit tests

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4344
2013-01-07 08:16:34 -08:00
Bob Trahan
0c8fa90040 make arc install-certificate <uri> work if uri is specified
Summary: see title

Test Plan: ran arc install-certificate <uri> in a directory without an .arcconfig and it worked! ran arc install-certificate in a directory with an .arcconfig and it worked! ran arc install-certificate <uri> in a directory with an .arcconfig and noted it correctly overrode the .arcconfig

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2251

Differential Revision: https://secure.phabricator.com/D4332
2013-01-03 09:23:20 -08:00
epriestley
59e13f666f Correct spelling for "arc" commands
Summary:
I type "arc brnach" about 300 times per day.

  - Allow arc commands to be specified by unique prefix ("exp" for "export", "lib" for "liberate").
  - Allow arc commands to be specified by unique levenshtein edit distance <= 2 ("brnach" for "branch", "halp" for "help", "ptach" for "patch").
  - Reorganize code out of "arcanist.php".

I think this will be uncontentious because arc commands are rarely destructive, but if people complain we can either require certain commands be typed exactly (maybe "land"?) or allow this feature to be disabled in configuration.

Test Plan:
  $ arc br
  Usage Exception: Unknown command 'br'. Try 'arc help'.

  Did you mean:
      branch
      browse

  $ arc brnachh
  Usage Exception: Unknown command 'brnachh'. Try 'arc help'.
  $ arc brnach
  (Assuming 'brnach' is the British spelling of 'branch'.)
     doc-security        No Revision      security
     sms                 Needs Revision   D319: Add SMS support to Phabricator
     phxtag              No Revision      derp
     arantag             No Revision      derp
  ...

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4305
2012-12-30 16:01:59 -08:00
David Cramer
0b833c2f02 Correct description and severity on Flake8 linter
Test Plan: do it live

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4274
2012-12-21 16:13:33 -08:00
David Cramer
4ca70d4e48 Add a flake8 linter
Summary: flake8 is the better maintained combination of pep8 and pyflakes

Test Plan: There's a test!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, jack

Differential Revision: https://secure.phabricator.com/D4082
2012-12-21 15:28:26 -08:00
epriestley
cffe1942ec Improve arc land recovery from issues with the remote.
Summary:
Fixes T2138.

  - When a pull fails, restore the original branch.
  - When a push fails, complain about it really loudly.

NOTE: No test plan for push yet since I'm not sure this is the right remedy, see T2138 for discsusion.

Test Plan:
  - Tested pull by changing "git pull" to "git xxpull" and running "arc land". Saw the pull fail and my original branch restored.

Reviewers: vrana, aran

Reviewed By: vrana

Maniphest Tasks: T2138

Differential Revision: https://secure.phabricator.com/D4265
2012-12-21 14:18:07 -08:00
vrana
f830b3bf3f Don't require clean working copy for arc diff path in SVN
Summary: If you are explicit then there is no need to ask you.

Test Plan:
  $ touch a
  $ arc diff
  $ arc diff a
  $ arc diff existing

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4186
2012-12-21 12:34:06 -08:00
vrana
c7c3f6a7f1 Update PEP8 to 1.3.4
Summary: Primarily to avoid false positives: http://pypi.python.org/pypi/pep8#id1

Test Plan:
  $ arc lint --engine ComprehensiveLintEngine externals/pep8/pep8.py # after uncommenting externals/ check

Saw 9 errors, haha.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4260
2012-12-21 11:34:40 -08:00
vrana
940d91d7b5 Speed up SVN discovery
Summary: `svn info` takes up to 10 seconds.

Test Plan: `arc diff` inside SVN repo and outside any repo.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4256
2012-12-20 18:10:37 -08:00
vrana
3f05751724 Don't amend commit with the same message in arc diff
Summary: It makes reflog ugly

Test Plan:
# `arc diff`, made error, saw "Message saved to".
# `arc diff`, didn't edit, didn't see "Message saved to".
# `arc diff`, edited, saw "Message saved to".

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4243
2012-12-20 12:18:05 -08:00
vrana
2d63d080df Make caching lint default
Summary: I use it couple of weeks without troubles.

Test Plan:
Added debug output and:

  $ arc lint
  $ arc lint --cache 0

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4242
2012-12-20 12:16:19 -08:00
epriestley
ffdf44e298 Fix arc diff in SVN with paths
Summary:
Recently, in D4097 or one of the precursors I refactored this. However, when $rev is null parseBaseCommitArgument() throws ("This VCS does not support commit ranges."). Shield the call so it only happens if if $rev is nonempty (we still want to make the call, so "arc lint --rev x" on SVN will throw and inform the user that "--rev" is incorrect usage).

(@vrana, this was reported by FB and might be worth pushing.)

Test Plan: Ran "arc diff --preview <path>". Grepped for other parseBaseCommitArgument() callsites and verified they don't have similar issues.

Reviewers: vrana, btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D4241
2012-12-20 11:12:30 -08:00
vrana
219dbe2374 Refuse writing to undeclared properties in workflows
Summary: I don't know how to not be strict here plus we (Arcanist developers) don't have access to user's error log.

Test Plan:
Undeclared `ArcanistDiffWorkflow::$console`, then:

  $ arc diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3607
2012-12-18 16:15:32 -08:00
vrana
2871d00f96 Allow checking out branches in arc branch
Summary:
The main added value is loading the branch name from revision.
Sometimes I know the revision ID but I don't know the branch name.

The missing piece is the starting point of the branch.
I was thinking about using `arc.land.onto.default` but we also need to get 'origin'.

This is also the last step of a simple workflow where underlying VCS is not abstracted away.
In future, we can implement this for other APIs.

Test Plan:
  $ arc branch new_branch
  $ arc branch new_branch
  $ arc branch D4168

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4170
2012-12-18 16:13:22 -08:00
Evan Priestley
955c5b8820 Merge pull request #62 from bonneval/bugfixUndefinedVariable
Bugfixed undefined variable $commit.
2012-12-18 05:38:05 -08:00
bonneval
551d036a89 Bugfixed undefined variable $commit. 2012-12-18 14:31:48 +01:00
Brennan Taylor
b5b1fd62dc Support http basic auth and related configuration
This adds the http basic auth username and password if found in the
configuration to the ConduitClient.

Reviewed by: epriestley
2012-12-17 18:06:54 -08:00
epriestley
b9fa71f7e1 Allow arc to be run in arcanist/ or libphutil/
Summary:
Currently, if you run `arc` in arcanist/ or libphutil/ and your PATH and on-disk configuration are set up so a different version of arc or libphutil are the ones that actually load, we fail with an exception like "running arcanist in a different copy of arcanist is not supported".

This causes problems for Harbormaster, since we'd like to be able to run 'arc' in a copy of libphutil/ and have it execute unit tests for that copy rather than failing abruptly. So, if we detect that we're in arcanist/ or libphutil/, execute 'arc' again with the same arguments but force it to load the working copy in place of either the 'arcanist/' or the 'libphutil/' that it decided to load.

This is pretty much horrible black magic.

Test Plan: Ran 'arc list --trace' inside copies of libphutil/ and arcanist/ outside of the normal include chain. Saw it detect these, emit a message, and re-execute itself correctly.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D4225
2012-12-17 16:35:03 -08:00
epriestley
6823706c76 Stop passsing bogus 'user' parameter to differential.createrevision
Summary: After D4191 this is a fatal.

Test Plan: Created this revision.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4219
2012-12-17 14:41:27 -08:00
epriestley
716ce2482c Implement --everything, --json and --ugly flags for arc unit
Summary: Adds "arc unit --everything", which runs every available test, provided the test engine supports it. Also add JSON output.

Test Plan: Ran `arc unit --everything` in arcanist/, libphutil/ and phabricator/. Saw all tests run.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2065

Differential Revision: https://secure.phabricator.com/D4214
2012-12-17 12:56:41 -08:00
epriestley
0bf5c3603c Refactor commit ranges and base commit handling
Summary:
See D4049, D4096.

  - Move commit range storage from Mercurial and Git APIs to the base API.
  - Move caching up to the base level.
    - Store symbolic name and resolved name separately, so we can re-resolve the correct commit from the symbolic name after dirtying caches.
  - Rename `supportsRelativeLocalCommit()` to `supportsCommitRanges()` (old name wasn't very good, and not consistent with new terminology like the `--base` flag).
  - Rename `getRelativeCommit()` and `setRelativeCommit()` to `getBaseCommit()` and `setBaseCommit()`.
  - Introduce `reloadCommitRange()` and call it from `reloadWorkingCopy()`.

I think this fixes the problem in D4049, and provides a general solution for the class of problems we're running into here, with D4096. Specifically:

  - We no longer get dirty caches, as long as you call reloadWorkingCopy() after changing the working copy (or call a method which calls it for you).
  - We no longer get order-of-parsing-things problems, because setBaseCommit() reloads the appropriate caches.
  - We no longer get nasty effects from calling `requireCleanWorkingCopy()` too early.

Test Plan: This is pretty far-reaching and hard to test. Unit tests; ran various arc commands. :/

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4097
2012-12-17 12:54:08 -08:00
epriestley
2ae0cb797d Remove ArcanistRepositoryAPI::setDefaultBaseCommit()
Summary:
This method is used in three cases:

  # For unit tests, to set the range to 'HEAD^' or '.^' in an agnostic way.
  # For "amend", to set the range to the commit to be amended (also 'HEAD^' or '.^').
  # For "patch" and "upgrade" so we don't fail just because there's an invalid "base" rule somewhere in the config when doing clean-working-copy tests.

For cases (1) and (2), introduce an "arc:this" rule to mean "the current commit". For case (3), remove the call; it is no longer necessary to check the commit range in order to do tests for the working copy state after D4095.

Test Plan: Ran unit tests, "arc upgrade", "arc patch", "arc amend".

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4096
2012-12-17 12:53:38 -08:00
epriestley
0e27dbfd17 Refactor getWorkingCopyStatus() into getUncommittedStatus() and getCommitRangeStatus()
Summary:
See discussion in D4049.

The getWorkingCopyStatus() method gets called from requireCleanWorkingCopy() in a lot of places, which triggers resolution of the base of the commit range. This is unnecessary; we do not need to examine the base commit in order to determine whether the working copy is dirty or not. This causes problems, in D4049 and elsewhere (we currently have a lot of fluff calls to setDefaultBaseCommit() in workflows which need to call requireCleanWorkingCopy() but do not ever use commit ranges, such as `arc patch`). This is mostly an artifact of SVN, where the "commit range" and "uncommitted stuff in the working copy" are always the same.

  - Split the method into two status methods: getUncommittedStatus() (uncommitted stuff in the working copy, required by requireCleanWorkingCopy()) and getCommitRangeStatus() (committed stuff in the commit range).
  - Lift caching out of the implementations into the base class.
  - Dirty the cache after we commit.

This doesn't do anything useful on its own and creates one caching problem (`commitRangeStatusCache` is not invalidated when the commit range changes because of `setBaseCommit()` or similar) but I wanted to break things apart here. I won't land it until there's a more complete picture.

This creates a minor performance regression in git and hg (we run less stuff in parallel than previously) but all the commands should be disk-bound anyway and the regression should be minor. It prevents a larger regression in `hg` in D4049, and lets us do less work to arrive at common error states (dirty working copy). We can examine perf at the end of this change sequence.

Test Plan: Ran unit tests, various `arc` commands.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4095
2012-12-17 12:53:28 -08:00
vrana
5f5392df8a Suggest upgrading Phabricator for unknown Conduit methods or parameters
Summary:
This allows using new methods without the need for bumping version number.

The usage is not neccessary because we already bumped the version number for this but I wanted to have a callsite.

Test Plan:
Made a typo in method name, then:

  $ arc lint --only-new 1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4193
2012-12-14 18:16:01 -08:00
vrana
7133c76d37 Update PEP8
Test Plan:
Linted this Python file:

  def get_mapper_sets(
          compression=4.0,  # Currently works for most tables
          **kwargs):
      pass

Didn't see:

> E225 missing whitespace around operator

Reviewers: andrewjcg, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4172
2012-12-14 17:50:18 -08:00
durham
f476d1a0c5 add arc land hg support for landing with nested branches
Summary:
This changes arc land's hg support to allow you to land a branch or bookmark that has nested branches/bookmarks.  Example:

      // Initial state:
      // -a--------b  master
      //   \
      //    w--x  mybranch
      //        \--y  subbranch1
      //         \--z  subbranch2
      //
      // arc land --branch mybranch --onto master :
      // -a--b--wx  master
      //          \--y  subbranch1
      //           \--z  subbranch2

Test Plan:
Created several repos like in the summary and ran 'arc land' and 'arc land --keep-branch'. Did this with both bookmarks and named branches. Scenarios tested:

- mybranch having no child commits
- mybranch having a child branch with several commits
- mybranch having two child branches
- mybranch having a child branch which has two more child branches

No code was added outside of a "if ($this->isHg)" so I didn't run git arc land.

Reviewers: epriestley, dschleimer, bos, sid0

Reviewed By: dschleimer

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4089
2012-12-14 11:23:44 -08:00
epriestley
2d7224f0e3 Add an experimental "--mergeup" flag to use a less volatile merge strategy in "arc land"
Summary:
See chatlog from https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=37257

Basically, the install's developers sometimes use "git merge master" instead of "git rebase master" to pull changes from master into their branch. When we run "git rebase master" at the end, this creates a lot of conflicts.

Instead, optionally run "git merge master". This should be equivalent in our case (where we always rebase) and much better in their case (where they sometimes merge).

We're both going to use it for a bit and see if it creates problems. If it improves the "sometimes-merge" workflow without affecting the "always rebase" workflow, we can make it a default. If it improves theirs but damages ours, we can keep it a flag/option. If it's just terrible, we can figure out something else.

Test Plan:
Ran `arc land --mergeup --trace <feature> --keep-branch --hold`, see P624 for output. Observed merge update strategy and general success.

Also verified the conflict:

  $ arc land --mergeup --merge
  Usage Exception: Arguments '--mergeup' and '--merge' are mutually exclusive: The --merge strategy does not update the feature branch.

Reviewers: btrahan, vrana, DurhamGoode

Reviewed By: btrahan

CC: aran, keir

Differential Revision: https://secure.phabricator.com/D4080
2012-12-13 14:11:52 -08:00
vrana
02694d73ab Simplify code by not passing local data to methods 2012-12-12 14:21:06 -08:00
vrana
48077c80f2 Use repository info from arcanist.projectinfo if available
Test Plan: Added a debug output there and ran.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4147
2012-12-10 15:18:52 -08:00
vrana
c68a048213 Support git svn dcommit in arc land
Test Plan:
  $ arc land # in Git SVN repository

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4123
2012-12-10 12:28:44 -08:00
Aviv Eyal
7b2ce3a987 Add arc browse workflow
Summary:
arc browse will open a browser to the Diffusion view of a file. Convenient if you like
Diffusion for reading source. Naturally, it fixes relative filenames.

Combined with git-grep it can be an easy replacement for server-side search functions.

Test Plan:
use feature with and without 'browser' configured.

I've only tested this on Linux, because that's all I have right now, but the principle is sound.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4127
2012-12-09 14:10:21 -08:00
vrana
65e3583235 Reuse code in close-revision workflow
Test Plan: Will test it by closing this revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4122
2012-12-07 18:22:23 -08:00
epriestley
c43d627cf2 Remove ArcanistSubversionAPI::hasMergeConflicts()
Summary: No callsites, redundant with getMergeConflicts().

Test Plan: Grepped for callsites.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4093
2012-12-06 12:20:02 -08:00
Edward Speyer
ecdb885236 Quote author parameter
Summary:
Fixes the exception:

  Exception
  Command 'git commit -a --author=Whatever Long Name <whatever@email.com> -F -' failed with error #1

Test Plan: Tested with full git name

Reviewers: epriestley, aurelijus

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3979
2012-12-05 15:45:54 -08:00
Xingyao Ye
6c3418a0e8 Introduce "arc diff --plan-changes"
Summary:
Allow authors to publish a new or updated revision to Phabricator
without requesting code reviews. The revision will have status
"Needs Revision" instead of "Needs Review".

In order to avoid a change of Conduit API, this is done by adding
a comment with the "plan changes" action immediately after the
revision is published.

Test Plan:
Using my local repository, run "./bin/arc diff --plan-changes"
Check the resulting diff in Phabricator.

Reviewers: vrana

Reviewed By: vrana

CC: aran, epriestley

Maniphest Tasks: T2024

Differential Revision: https://secure.phabricator.com/D4084
2012-12-05 14:04:11 -08:00
epriestley
ba1a17ac31 Fix a fatal for git uncommitted changes in new arc projects
Summary:
If you run `arc diff` in a repository which:

  - has uncommitted or untracked changes; and
  - has a .arcconfig with a never-before-seen project ID;
  - we fatal: http://pastebin.com/raw.php?i=ykpfr4MT

This patch is a bit iffy, open to alternatives. The "right" patch is probably an `arcanistproject.query` which behaves more sensibly.

I return array() directly since we'll later create the project.

Test Plan: Ran `arc diff` in a repository with untracked files or uncommitted changes and a new project ID.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4057
2012-12-05 09:20:15 -08:00
vrana
209ffba8c0 Bump Conduit client version number
Summary: Should have been part of D3934.

Test Plan:
  $ arc diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4076
2012-12-03 18:09:45 -08:00
durham
aada1440ef Add hg support for arc land
Summary:
Makes arc land support hg repositories. Both bookmarks and named branches can be landed.  For the most part all the arc land options work, but there are a few caveats:

- bookmarks can only be landed on bookmarks
- branches can only be landed on branches
- landing a named branch with --merge creates a commit to close the branch before the merge.
- since mercurial doesn't start with a default master bookmark, landing a bookmark requires specifying --onto or setting arc.land.onto.default

Test Plan:
Tested arc land with all permutations of --merge, --keep-branch on both bookmark branches and named branches.  Also tested --hold, --revision, --onto, --remote.

See https://secure.phabricator.com/P619

Also tested git arc land with --merge and --keep-branch.

Reviewers: dschleimer, sid0, epriestley, bos

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4068
2012-12-03 17:59:12 -08:00
vrana
5025c06f3d Introduce arc lint --only-new 1
Test Plan:
  $ arc lint src/workflow/ArcanistLintWorkflow.php
  $ arc lint --only-new 1 src/workflow/ArcanistLintWorkflow.php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3934
2012-12-03 17:21:17 -08:00
epriestley
886c1721dd Restore order of operations after D4056
Summary:
See discussion in D4056. In `findRevision()` we call `loadWorkingCopyDifferentialRevisions()`. However, this method depends upon the state of the working copy, because the end of the commit range it examines is HEAD. Prior to D4056 we checked out the target branch before calling `findRevision()`; after D4056 we call it earlier.

This isn't problematic in the `arc land` case, but in the `arc land <branch>` case it means we may fail to identify a revision, or identify the wrong revision, because HEAD isn't where we expect it to be.

Instead, unconditionally check out the target branch before finding the revision.

See <http://dl.dropbox.com/u/116385/Slingshot/Pictures/Screen%20Shot%202012-12-03%20at%203.43.45%20PM.png> for a transcript of the issue.

Test Plan: Reproduced issue as per link above. Ran `arc land --keep-branch --hold somebranch` successfully after this patch.

Reviewers: DurhamGoode, zeeg

Reviewed By: DurhamGoode

CC: aran

Differential Revision: https://secure.phabricator.com/D4072
2012-12-03 16:31:45 -08:00
vrana
e8eacb6ae3 Allow setting lint cache granularity
Summary:
We can add `GRANULARITY_DIRECTORY` and `GRANULARITY_REPOSITORY` later.
Repository granularity may use current commit + changes.
Directory would need to use hashes of all files in dir which would be quite expensive.

Test Plan:
  $ echo '<?php class A extends B {}' > A.php
  $ arc lint --cache 1
  $ arc lint --cache 1
  $ echo '<?php class B {}' > B.php
  $ arc lint --cache 1
  $ arc lint --cache 1
  $ rm B.php
  $ arc lint --cache 1
  $ arc lint --cache 1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4021
2012-12-03 15:30:06 -08:00
David Cramer
0d1d04e434 canRun should return false on scala linter instead of raising an exception
Summary:
This was causing the following error in environments that didnt have scala configured:

Some linters failed:
   - ArcanistScalaSBTLinter: ArcanistUsageException: This directory does not appear to be maintained by SBT, as we can't seem to find a working build file (project/Build.scala or build.sbt).

Test Plan: .

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4070
2012-12-03 15:25:28 -08:00
durham
3602d2aa15 Break arc land run() into smaller functions
Summary:
Refactor the arc land code into several
functions so it's easier to maintain. This is in
preparation for adding hg support to arc land
in my next commit.

Without this refactor, adding hg support makes the run()
function too big.

Test Plan:
Set up a git repo and clone with a branch scenario.
Example: https://secure.phabricator.com/P614
Ran and verified:
arc land
arc land --keep-branch
arc land --merge
arc land --merge --keep-branch
arc land --hold
arc land --revision <another phabricator rev>

Reviewers: epriestley

Reviewed By: epriestley

CC: dschleimer, sid0, aran, Korvin

Differential Revision: https://secure.phabricator.com/D4056
2012-12-03 13:03:13 -08:00
Ricky Elrod
b549f565c9 Add a very basic Scala (SBT) linter.
Summary:
SBT is the most common Scala buildsystem. This adds an extremely basic and
slightly horrible linter to check SBT's output for warnings and errors.

Test Plan:
Tested this with a Scala project I've been working on for some time.
It seemed relatively sane.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4064
2012-12-03 10:04:18 -08:00