1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00
Commit graph

1384 commits

Author SHA1 Message Date
epriestley
6dade54efb Improve exception raised when an event listener fails to load
Summary: See IRC. The failing class might not be the listener itself.

Test Plan:
  $ arc list
  ERROR: Failed to load event listener 'depr': Failed to load class or interface 'depr': the class or interface 'depr' is not defined in the library map for any loaded phutil library. If this symbol was recently added or moved, your library map may be out of date. You can rebuild the map by running 'arc liberate'. For more information, see: http://www.phabricator.com/docs/phabricator/article/libphutil_Libraries_User_Guide.html

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7616
2013-11-20 12:46:38 -08:00
James Rhodes
fb8a0d32ae Set limit for execution in C# linter
Summary: This sets the limit for future execution in the C# linter to 8.  See D7606 for more information.

Test Plan:
Ran

```
arc lint --everything --trace --never-apply-patches --output json
```

and saw it only run 8 commands at once.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7607
2013-11-19 13:36:27 -08:00
James Rhodes
61753b6770 Upgrade C# linter to support linting multiple files at a time
Summary:
This upgrades `cslint` to support linting multiple files at a time.  As this required a backwards-incompatible to `cslint`, I've added a SUPPORTED_VERSION constant which can be used to detect these kinds of breaking changes in the future (and prompt users to upgrade the `cslint` they have in their repository).

The reason for this upgrade is mainly around running `arc lint --everything`, where there are significant performance benefits gained when bulk linting lots of files per command execution.

Test Plan: Upgraded `cslint` in the Tychaia repository and ran `arc lint --everything --trace`.  Saw a substantially less number of executions happening for linting and all of the results came through as expected.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, waynea

Differential Revision: https://secure.phabricator.com/D7599
2013-11-18 11:32:58 -08:00
Burak Yigit Kaya
19edbbb46e Make ScriptAndRegexLinter available to ConfigurationDrivenLintEngine
See: <https://github.com/facebook/arcanist/pull/118>

Reviewed by: epriestley
2013-11-18 10:08:19 -08:00
Anirudh Sanjeev
17e4e9e651 Fix markup in documentation for arcanist lint engine
Summary: Fixed wrong markup which was 1 space instead of 2

Test Plan: I couldn't test it as I don't know how to build documentation

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7587
2013-11-14 10:19:08 -08:00
epriestley
9310df9615 Handle empty output from hg --debug branches in the parser
Summary: Ref T1493. Also consolidate this a bit more.

Test Plan: Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1493

Differential Revision: https://secure.phabricator.com/D7481
2013-11-04 12:15:12 -08:00
Sven Axelsson
aabbdbd2ab Filter out messages from included files
Summary:
CppCheck shows lint messages from included files as well as the current
file. Filter out those, since they don't make much sense in the context
of `arc lint`.

Test Plan:
Before this patch, `arc lint` using `ArcanistCppcheckLinter`. Note that lint messages
from included files appear pointing to a line in the active file.
After this patch, only messages from the active file are included.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7444
2013-10-30 13:46:30 -07:00
Aviv Eyal
6dc04af6e8 Specify custom arcrc filename
Summary: allow specifing an replacement .arcrc file, for the odd cases.

Test Plan: `echo {} | arc call-conduit user.whoami` with and without `--arcrc-file=`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7208
2013-10-25 15:57:38 -07:00
Aviv Eyal
0d212ccf5a rename getConfig -> getProjectConfig, make all linters use getConfigFromAnySource
Summary:
Lingers on from D7271; Rename `ArcanistWorkingCopyIdentity.getConfig()`.

Changed all linters (Except one) to use `getConfigFromAnySource()`, because it seems to make sense.

Test Plan: arc unit --everything; arc lint in github.com:epriestley/arclint-examples.git (Except for phpcs, flake8, cpplint and csslint which I don't have installed).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7382
2013-10-22 15:34:06 -07:00
Aviv Eyal
4103bc423c get Config Manager to Unit engine and use getConfigFromAnySource
Summary:
somewhat related to D7271 and D7377.
Not actually broken right now, but might be worth it for completeness.

Test Plan: arc unit invokes PHPUnit; Can't test Csharp/XUnit on my end.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: aurelijus, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7381
2013-10-22 13:58:32 -07:00
epriestley
ddbc14ade1 Provide ConfigurationManager to LintEngine in Arcanist
Summary: Unbreaks ArcanistSingleLintEngine / ArcanistScriptAndRegexLinter from recent config churn.

Test Plan: `arc lint --engine ArcanistSingleLintEngine --rev HEAD^`

Reviewers: btrahan

Reviewed By: btrahan

CC: csilvers, aran

Differential Revision: https://secure.phabricator.com/D7377
2013-10-21 16:57:22 -07:00
epriestley
5c2f7b2abd Autocomplete branch names in arc feature
Summary: SPOOKY

Test Plan: `arc feature <tab>`

Reviewers: cpojer, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7376
2013-10-21 16:29:08 -07:00
Pascal Borreli
831fc9a92b Fixed typos
See: https://github.com/facebook/arcanist/pull/110

Reviewed by: epriestley
2013-10-20 07:53:23 -07:00
epriestley
2c64f1b072 Minor, fix a missing config source change. 2013-10-19 13:32:59 -07:00
Aviv Eyal
359e1c2803 Couple of fixes from refactor
Test Plan: arc set-<tab> <enter> from a not-workspace.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7354
2013-10-18 16:41:50 -07:00
epriestley
d85f7bee66 Minor, fix a typo. 2013-10-18 16:19:22 -07:00
Aviv Eyal
a2285b2b5a Extract configuration read/write methods out of BaseWorlkflow
Summary:
Create a new class for them, pass instance around as need.

This looks like it's mostly working, but I'd like to replace the various `new ArcanistConfigurationManager()`
calls with something more suitable.

And maybe get a better name for ArcanistConfigurationManager ("Configuration" is already taken).

Test Plan: arc unit --everything, and then some.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, chad

Differential Revision: https://secure.phabricator.com/D7271
2013-10-18 16:10:45 -07:00
Bob Trahan
b2021586d4 Arcanist - make Patch workflow automagically apply dependencies
Summary: Nice title. Ref T479.

Test Plan:
Actually, help on that? I want to make sure I properly build up the "depends" on data. Is it as simple as

  -- observe at some commit hash RAZZMATAZZ
  -- git checkout -B "foo"
  -- <work>
  -- git commit -m "stash"
  -- arc diff -> yields DX
  -- git checkout -B "foo_prime"
  -- <work>
  -- git commit -m "stash"
  -- arc diff -> yield DY
  -- git checkout RAZZMATAZZ
  -- arc patch DY
  -- get prompted in workflow, agree
  -- git log and observe DX and DY applied

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran, davidressman

Maniphest Tasks: T479

Differential Revision: https://secure.phabricator.com/D6790
2013-10-17 14:59:04 -07:00
epriestley
79be59863e Write ARCANIST into the environment while running arc
Summary:
Addresses a request to let Git commit hooks react differently when run via `arc`. While executing an `arc` command, write ARCANIST into the environment.

(I wrote the actual command as the value since it seemed like it could plausibly be more useful than `true` or `1`.)

Test Plan: Added `pre-commit` to `.git/hooks` which did `echo $ARCANIST`, verified the envvar was accessible when running via arc.

Reviewers: btrahan, frgtn

Reviewed By: frgtn

CC: aran

Differential Revision: https://secure.phabricator.com/D7281
2013-10-10 06:18:37 -07:00
epriestley
13e422e123 Remove spurious +x from arcanist
Summary: A few files here have +x, but should not. Also correct a
spelling mistake.
2013-10-05 05:20:05 -07:00
root
10a9bb1e99 Keep Arcanist from mistaking paths that look like svn:externals as svn:externals
Summary: Fixes T3920. Added a slash to the path and external name so that "public" and "publicnotexternal" won't appear to be the same root.

Test Plan:
We've had this issue in one of our projects for some time, just ran into it again today. Ran the patched arc against the same directory structure and the troublesome file was added to the diff. Confirmed that files
modified in the "public" (svn external) folder are still caught as external modifications.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3920

Differential Revision: https://secure.phabricator.com/D7226
2013-10-04 17:31:18 -07:00
Tal Shiri
4e892e7269 get jshint error codes
Summary:
makes jshint slightly more useful by printing out error numbers. I love memorizing numbers.
Sorry about the crappy getLintMessageName(). There was no list of error names, just the long descriptions that are already rendered as 'reason'.

Test Plan: Verify that the numbers are mezmorizing. Been tested on OSX with jshint v2.1.11

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7213
2013-10-04 06:29:47 -07:00
epriestley
587addfd94 Strip comments from all diffs before parsing them
Summary:
See <https://github.com/facebook/phabricator/issues/400>. Since all of `patch`, `git apply` and `hg export` either accept or emit header comments, parse them unconditionally.

This is a tiny bit messy because we already had a less-general parser for `hg export` diffs, which have a large header section.

This is less permissive than GNU `patch`, which allows comments anywhere. We could do that, but `git apply` won't read them and they seem pretty crazy.

Test Plan: Added and ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7207
2013-10-03 15:06:35 -07:00
Tal Shiri
0ece525d6c added getLinterConfigurationName() to ArcanistJSHintLinter.
Summary: Needed for it to be usable from ArcanistConfigurationDrivenLintEngine, which is pretty ok.

Test Plan: put jshint in your .arclint and feel the electricity in the air.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7200
2013-10-02 19:02:40 -07:00
James Rhodes
02e4a690dd Add C# linter for Arcanist.
Summary:
Completes T3859.  This implements a C# linter for Arcanist, which in turn uses `cslint` from `cstools` to actually perform the linting.  `cslint` internally uses StyleCop in addition to it's own lint rules.

Unlike other linters, C# is a compiled language, which means that the StyleCop integration must be aware of the full project.  To this end, there is the `discovery` setting in `.arclint`.  This allows users to define mappings between C# files and the projects they belong to.  Here is an configuration for `.arclint` (and is the one we use):

```
{
  "linters": {
    "csharp": {
      "type": "csharp",
      "include": "(\\.cs$)",
      "binary": "cstools/cslint/bin/Debug/cslint.exe",
      "discovery": {
        "([^/]+)/(.*?)\\.cs": [
          "$1/$1.Linux.csproj"
        ],
        "([^\\\\]+)\\\\(.*?)\\.cs": [
          "$1\\$1.Windows.csproj"
        ]
      }
    }
  }
}
```

Test Plan: Tested under both Linux and Windows.  Changed some files, ran `arc lint` and it all worked correctly.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran, jamesr

Maniphest Tasks: T3859

Differential Revision: https://secure.phabricator.com/D7170
2013-10-01 11:37:26 -07:00
Aviv Eyal
0b8ea973ae Fix coverage for NoseTestEngine
Summary:
In case that the $source_path is `.` or empty, it produces filenames that
look like `./foo.py`, which differential doesn't like.

Test Plan: arc diff, see coverage data.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7181
2013-09-30 10:44:38 -07:00
Aviv Eyal
aebcd7a985 Extract xUnit test results parser
Summary:
Many test frameworks can format their output in xUnit-like format.

Test Plan: Tested the Nose engine with a Nose one, and the pytest with a demo project.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

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

Conflicts:
	src/__phutil_library_map__.php
2013-09-30 10:44:13 -07:00
epriestley
9bae517a38 Fix a stiring issue in the External linter
Summary: See error message in D7170 --  this should be a `.`, not a `,`.

Test Plan:
Faked interprerter and got reasonable error message:

> Unable to locate interpreter "TESTpython2.6" to run linter ArcanistPEP8Linter. You may need to install the intepreter, or adjust your linter configuration.
> TO INSTALL: Install PEP8 using `easy_install pep8`.

This doesn't fix the //real// error, which is that the test should skip if you don't have the interpreter/binary, but that's a little more involved.

Reviewers: hach-que, chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7172
2013-09-29 07:44:33 -07:00
Gareth Evans
1ead3cc307 Set the CWD before applying a git patch
Summary:
If in a subdirectory, any changes made in a different location
is missed from the patch with no errors from git. The other
option was to run some logic to compare the files being changed.

Test Plan:
Run arc patch from a subdirectory that would miss some files
previously.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3733

Differential Revision: https://secure.phabricator.com/D7167
2013-09-28 06:53:20 -07:00
Chris Dentel
08536d8917 Adding arc lint flag that will show lint only on changed lines - even when paths are specified
Summary:
The existing lint configurations do not allow for linting only the lines that have
changed when paths are added.  --lintall has a default behavior of true when paths are specified,
and false when paths are not specified.  Because of this (and because it does not take a boolean
param) it is not possible to lint a path for only the errors on changed lines - only-new is not
working presently.

Test Plan: play around with the linter

Reviewers: lifeihuang

Reviewed By: lifeihuang

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D7055
2013-09-27 10:13:31 -07:00
epriestley
6270dd0de5 Add https.blindly-trust-domains to Arcanist
Summary:
This allows users to specify that they want to implicitly trust their weird self-signed certificate without verification.

This can either be specified per user (which will make it apply every time the user runs `arc`, in any project):

  arc set-config https.blindly-trust-domains '["example.mycompany.com"]'

...or added to a `.arcconfig` file (which will make it apply to every user who runs `arc` in that project):

  "https.blindly-trust-domains" : ["example.mycompany.com"]

Depends on D7130.

Test Plan: Tweaked config and verified this setting sends HTTPSFuture down the right branch.

Reviewers: btrahan

Reviewed By: btrahan

CC: hlau, aran

Differential Revision: https://secure.phabricator.com/D7131
2013-09-25 15:26:38 -07:00
epriestley
0d0333d50a Fix an issue where method prototypes have different signatures
Summary: PHP whines about this:

> [2013-09-23 08:09:22] ERROR 2048: Declaration of CSharpToolsTestEngine::loadEnvironment() should be compatible with XUnitTestEngine::loadEnvironment($config_item = 'unit.xunit...') at [/INSECURE/devtools/arcanist/src/unit/engine/CSharpToolsTestEngine.php:13]

Auditors: jamesr, btrahan
2013-09-23 08:34:28 -07:00
James Rhodes
4ba895c30d Add C# tools and xUnit unit test engines.
Summary:
This implements Arcanist support for the xUnit testing framework.  It also supports code coverage by way of `cstools` (which is something I've written).

The unit test support works under both Linux and Windows, while the code coverage support has only been tested under Linux (one would assume it would also work under Windows given that Windows has a super-set of functionality in the C# world).

The Arcanist support assumes that the directory layout will be something like:

  * MyProject
  * MyProject.Tests

When files are changed in either MyProject or MyProject.Tests, it causes MyProject.Tests to be built and the xUnit runner to be executed on the resulting binary.

Test Plan: I guess if really wanted to, you could create a C# project in MonoDevelop, set up `.arcconfig` to point to this unit test engine, and download and build xUnit and cstools.  Run `arc unit --coverage` to see the results of your unit test coverage.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3859

Differential Revision: https://secure.phabricator.com/D7058
2013-09-23 05:32:34 -07:00
James Rhodes
75737c6d89 Prevent detailed code coverage from accessing missing report.
Summary:
Fixes T3856.

Detailed code coverage iterates over all of the files with changes in the current repository, however the code coverage tool might not generate a report for all changed files in the repository, and this would result in an undefined index error.

As an additional bonus, since changes to binary files won't be reported by code coverage tools, this also prevents binary data from being outputted to the console.

Test Plan:
Change a binary file in a repository and run a code coverage tool.  The binary file should be reported as 0% code coverage, but the file contents should not be rendered to the console.

Change a code file that is not covered by a code coverage tool and run code coverage.  The file should be reported as 0% code coverage, and the file contents should not be displayed.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3856

Differential Revision: https://secure.phabricator.com/D7051
2013-09-20 09:18:02 -07:00
Eric Stern
bfcb3cfcd0 Perform case-insensitive check for PHPUnit test finder to not return original file
Summary: PhpunitTestEngine incorrectly included some source files as tests when run on a case-insensive fiesystem. This fixes that behavior. It could introduce problems for case-sensive users, but it's extremely unlikely to be an issue in practice (who would put a file in both Tests/ and tests/ and expect different results for the two?)

Test Plan: arc unit before and after change on OS X. Stubs and autoloaders in .../tests/... directories are no longer picked up.

Reviewers: epriestley

Reviewed By: epriestley

CC: aurelijus, Korvin, aran

Differential Revision: https://secure.phabricator.com/D7048
2013-09-19 15:14:58 -07:00
Jakub Vrana
bfbb16f322 Pluralize add files questions
Test Plan: Saw 'Do you want to add these files to the commit?'

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6991
2013-09-14 08:06:50 -07:00
epriestley
c00d8c551c Correct capitalization of "new" in lint
Summary: Automatically correct `New` to `new` in lint.

Test Plan: Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6948
2013-09-12 13:01:25 -07:00
epriestley
ba3c8e3356 Ignore errors from "svn upgrade" in unit tests
Summary: See discussion in D6893. Different versions of `svn` do different stuff, just ignore any possible error here.

Test Plan: Ran unit tests.

Reviewers: andrewjcg, btrahan

Reviewed By: andrewjcg

CC: aran

Differential Revision: https://secure.phabricator.com/D6946
2013-09-11 21:23:30 -07:00
Aviv Eyal
1fa8e861b2 parse ls-tree with git-submodule correctly
Summary:
git ls-tree gives type 'commit' for submodules.

This code-path is only used when a binary file is present in the diff.

Test Plan: arc diff with a binary file

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6943
2013-09-11 10:36:23 -07:00
epriestley
699f3b3c05 Fix an issue with PHPCS output formatting
Summary:
See <https://github.com/facebook/arcanist/issues/102>. PHPCS changed its output format sometime between 1.4.6 (stable) and 1.5.0RC3.

Add a "no errors" test and make the linter work on both versions.

Test Plan: Ran `arc unit` on PHPCS 1.5.0RC3.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6926
2013-09-10 15:10:34 -07:00
katherine
5fd07856c0 [lint][spelling] delimeter -> delimiter
Summary:
I got bit by this one earlier (I swear I thought it was delimeter),
so I figured I would add it to the spelling data lint rule.

Test Plan:
changed an instance of delimiter to delimeter, ran `arc lint` fixed the
typo.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6936
2013-09-10 11:36:17 -07:00
Evan Priestley
70567e48ae Merge pull request #90 from dcramer/pytest
Add PytestTestEngine
2013-09-06 11:49:56 -07:00
epriestley
9f6035b2c6 List all DOM extension classes as extension classes for static analysis purposes
Summary: See D6895. We currenty whitelist one of the DOM classes, but should whitelist them all. Classes not in this whitelist raise warnings when users who don't have the corresponding extensions installed lint code. We don't want to require you to install every extension just to send diffs, so we whitelist stuff that we know would exist if you did install everything.

Test Plan: bwahaha

Reviewers: btrahan, Firehed

Reviewed By: Firehed

CC: aran

Differential Revision: https://secure.phabricator.com/D6896
2013-09-05 17:28:19 -07:00
epriestley
5b869c2349 Fix arc + svn + delted binary file with properties + image heuristic
Summary:
See IRC. This fixes an issue when deleting an SVN file that ends with one of the extensions in the regexp, which may only affect newer versions of SVN.

Possibly we shouldn't have this heuristic, or should move it elsewhere or make it more explicit, but at least stop it from being broken for now.

Test Plan: Ran `arc diff --only` in a working copy with a deleted binary file ending in ".jpg".

Reviewers: btrahan, nh

Reviewed By: nh

CC: aran, mbishopim3

Differential Revision: https://secure.phabricator.com/D6893
2013-09-05 15:05:26 -07:00
Bob Trahan
67061480f9 Tighten up "arc land"
Summary:
Make sure on failure (restoreBranch()) we call `git submodule update --init --recursive` to handle all those purdy submodules. For the pushing step, wrap the push commands in the try / catch block so everything gets cleaned up nice if there's failure. BONUS - add --recursive to arc patch workflow to so nested submodules work correctly. (Crazy git users)

Fixes T3407, T2945.

Test Plan: I wasn't sure how to simulate a good "push" failure but I think this should work.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T2945, T3407

Differential Revision: https://secure.phabricator.com/D6885
2013-09-05 12:45:59 -07:00
Nick Harper
db3581b8fa Don't error on first run of arc diff
Summary:
When running arc diff in a repository that supports commit ranges, it is
possible that the setting for the default relative commit hasn't been set.
If this is the case, the user will be prompted. This change makes sure that
the prompt happens (and thus the setting is set) before we run the
background lint and unit runs.

Test Plan:
```
rm .git/arc/default-relative-commit
arc diff
```

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T2351

Differential Revision: https://secure.phabricator.com/D6854
2013-08-31 14:57:10 -07:00
Wez Furlong
4a3d829223 Fixup lint testing for changes in D6798
Summary:
We have some linters that trigger based on the path name
in the tree (some rules apply in some dirs and not others).
The changes in D6798 caused all the paths to appear to be outside
the tree, so allow for passing a fake through from those test cases
that are sensitive to this.

We also have a test for the copyright linter, and that needs to read
settings from the .arcconfig file.  The change to faking a working
copy meant that this config option was effectively unset, so add a way
to pass the entire arcconfig through from the tests that need it.

Lastly, the logic to skip deleted files needs to be special cased
when we're faking paths like this: if we've added data for a file
in the testable engine, we should also consider that file as existing.

Test Plan:
`arc unit --everything` here, and passing our tests in
our repo over there.

Reviewers: epriestley, mareksapota

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6841
2013-08-29 09:55:30 -07:00
epriestley
2c5c9815c0 Support PHPCS as a .arclint linter
Summary:
Ref T3186. Ref T2039. Ref T3771. A few effects here:

  # Expose PHPCS as a `.arclint` linter.
  # Turn PHPCS into an ArcanistExternalLinter linter.
  # Add test coverage for PHPCS.
  # Add a `severity.rules` option to `.arclint`. Some linters have very explicit builtin severities ("error", "warning") but their meanings are different from how arc interprets these terms. For example, PHPCS raises "wrong indentation level" as an "error". You can already use the "severity" map to adjust individual rules, but if you want to adjust an entire linter it's currently difficult. This rule map makes it easy. There's substantial precedent for this in other linters, notably all the Python linters.

For `severity.rules`, for example, this will turn all PHPCS "errors" into warnings, and all of its warnings into advice:

      "severity.rules" : {
        "(^PHPCS\\.E\\.)" : "warning",
        "(^PHPCS\\.W\\.)" : "advice"
      }

The user can use `severity` (or more rules) to get additional granularity adjustments if they desire.

Test Plan: 5bb919bc3a

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, ajtrichards

Maniphest Tasks: T2039, T3186, T3771

Differential Revision: https://secure.phabricator.com/D6830
2013-08-29 06:47:27 -07:00
Bob Trahan
96a47759ae Make arc patch slightly better about submodules
Summary:
Ref T3776, Ref T479. Say you have some DN, with a submodule X@Y. Later, X@Z in your working copy / repo. If you run arc patch DN, you'd end up with a dirty working copy claiming that X@Z was wrong and it should be X@Y.

To fix, basically run 'submodule init' and 'submodule update'. This makes it so after "arc patch" if you run "git status" it looks clean.

Gross part though now is if you then "git checkout master" you'll have a dirty checkout the other way. I think this is better though.

Test Plan: made a new repository where I added libphutil @ X, did some work (DX), then made libphutil @ y. When I arc patch'd DX, things looked good!

Reviewers: epriestley

Reviewed By: epriestley

CC: csilvers, Korvin, aran

Maniphest Tasks: T479, T3776

Differential Revision: https://secure.phabricator.com/D6837
2013-08-28 16:47:30 -07:00
epriestley
3ad72195bf When converting a file to a binary, populate the binary's data
Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.

A simple reproduction case is to build a test file (I used one with bytes from 1..255):

  $ # Don't include \0, since Git treats that specially.
  $ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt

Then add it:

  $ git add example.txt
  $ git commit -a -m derp
  $ arc diff --only HEAD^

You'll be prompted to convert the file to binary:

  Do you want to mark this file as binary and continue? [Y/n] y

Before this patch, that would be followed by:

  Uploading 0 files...

...which is incorrect; we need to upload the new data. After this patch, this shows:

  Uploading 1 files...

...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.

Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.

Reviewers: zeeg, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6815
2013-08-27 09:34:30 -07:00