1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-05 05:02:43 +01:00
Commit graph

1456 commits

Author SHA1 Message Date
tuomaspelkonen
fe9daa7ae3 'arc diff' passes the diff ID to Test Engine class.
Summary:
Test Engine classes might need the differential Diff ID to be
able to attach postponed test results to diffs.

Test Plan:
Added setDifferentialDiff function to PhutilUnitTestEngine class
and made sure it was called with the correct diff ID when running
'arc diff --preview'

Reviewed By: jungejason
Reviewers: jungejason, grglr
Commenters: sgrimm
CC: epriestley, sgrimm, slawekbiel, aran, tuomaspelkonen, jungejason
Differential Revision: 395
2011-06-02 12:19:05 -07:00
epriestley
03056c7206 Reorganize documentation. 2011-05-31 12:14:20 -07:00
epriestley
d3816f1c9e Allow 'arc diff' to detect non-UTF8 files and mark them binary
Summary:
There are a bunch of different ways we could approach this, but practically I
think this is probably the best one even though it's kind of yucky.

A big part of my motivation here is that if we just reject UTF-8 outright, users
are going to end up in a situation they don't understand how to resolve.

UPDATE: after discussion, going with a more conservative approach until such
time as we have a more compelling case for the less strict functionality.

Test Plan:
Created a diff with a text file that contained invalid UTF-8 subsequences.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 325
2011-05-31 12:13:24 -07:00
epriestley
5b6fbf70e0 Disable 'textconv' when diffing
Summary:
Adds "--no-textconv" to all 'git diff' commands so we don't invoke textconv. See
T178 for discussion.

Test Plan:
Added something like this to .gitattributes:

  *.txt diff=uppercase

And then this to .git/config:

  [diff "uppercase"]
    textconv = /path/to/uppercase

...where "uppercase" is a script which takes a file and emits an uppercase
version of it.

Then I added a "wisdom.txt" text file:

  The cow goes "moo".
  The duck goes "quack".

Without this patch, the file appears in uppercase in Differential, i.e. textconv
runs. With this patch, it appears as the original text.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: elgenie, aran, tuomaspelkonen
Differential Revision: 372
2011-05-31 12:13:07 -07:00
epriestley
857f08ae47 Make sure 'id' and 'idx' are not marked as builtin functions?
Summary:
This is sort of a guess. In D357, arc has gone crazy and removed a bunch of
'utils' dependencies for gc3. I think this is because his HPHP/i has them marked
as builtins.

However, I don't have an HPHP/i build (and probably don't have the fortitude to
build it outside of a Facebook environment, I spent about two hours on it at one
point and got maybe 25% of the way through the build process before running into
things I didn't know how to resolve) so I'm not sure this is the issue. No one
else's diffs have exhibited this problem eitehr, so I'm not confident this is
actually the problem or solution.

gc3, can you apply this locally to a copy of arcanist and then run 'arc lint'
(or 'arc lint --apply-patches') on your commit and see if it restores all the
libphutil/utils requirements?

Test Plan:
unable, see summary

Added 'array_keys' to this blacklist and verified the technical behavior of the
patch is correct by var_dump()'ing builtins.

Reviewed By: gc3
Reviewers: gc3, tuomaspelkonen, jungejason, aran
CC: aran, gc3, epriestley
Differential Revision: 361
2011-05-29 02:35:26 -07:00
epriestley
f87dd377df Add .divinercache to .gitignore. 2011-05-25 18:51:52 -07:00
Evan Priestley
010bd936e8 Merge pull request #1 from lovrop/master
Lint rule for duplicate keys in array literals
2011-05-25 17:16:07 -07:00
Lovro Puzar
4b041b8f84 Lint rule for duplicate keys in array literals
Summary:
Raise an error if an array is initialized with the same key present more than
once.

Test Plan:
bin/arc unit
Also added some duplicates to ArcanistXHPASTLinter.php and verified the output
of bin/arc lint.

Reviewed By: epriestley
Reviewers: jungejason, epriestley, aran
CC: aran, epriestley
Revert Plan:
Tags:

Differential Revision: 346
2011-05-25 16:26:33 -07:00
Andrew Gallagher
d762311a9d arc lint: add support for PyLint
Summary:
Provides a lint class as a wrapper around the external project PyLint.
This exposes some arc config variables to control the behavior:

lint.pylint.prefix - non-standard installation location of pylint
lint.pylint.logilab_astng.prefix - non-standard installation location
  of logilab-astng, a dependency of pylint
lint.pylint.logilab_common.prefix - non-standard installation location
  of logilab-common, a dependency of pylint
lint.pylint.codes.{error,warning,advice} - regexes matching against
  PyLint message codes which should trigger arc errors/warnings/advice
lint.pylint.options - options to pass PyLint

Test Plan:
used to lint python code

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley
Differential Revision: 343
2011-05-25 14:14:51 -07:00
tuomaspelkonen
1ef7aacff1 Image files should also be treated as binary files when patching.
Summary:
Arc patch for image files failed, because they were treated like text
files.

Test Plan:
Tested that 'arc patch' worked for a png file.

Reviewed By: jungejason
Reviewers: epriestley, jungejason
CC: aran, jungejason
Differential Revision: 341
2011-05-24 18:12:53 -07:00
epriestley
13ea6ea5b6 Add basic binary file support to 'arc patch'
Summary:
I'm probably missing some edge cases but it took me almost 3 hours to get this
far and I think it only makes things work that didn't work before. Some stuff
like SVN binary patches still won't work, although they should be far easier to
implement.

Most of the magic here just comes from reading the git source code. It appears
to work correctly; I sprinkled printf() around git liberally and recompiled it
during development. Took me about 45 minutes to figure out that "Index" vs
"index" causes git to silently fail in a confusing way. :/

Git has a diff mode for binary changes but I don't think we lose much by always
using the full binaries. We can enhance it later if we want.

Test Plan:
Exported and patched binary changes (a picture of a duck) into a working copy.

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: simpkins, aran, epriestley
Differential Revision: 327
2011-05-22 06:58:16 -07:00
Andrew Gallagher
f8c3a5d555 arc lint: add support for PyFlakes
Summary:
Provide a simple linter wrapper around pyflakes.  This relies on finding
pyflakes via:
- lint.pyflakes.path - arcconfig setting of absolute path to pyflakes
- lint.pyflakes.prefix - arcconfig setting of the prefix that pyflakes
  was installed under
- users path

Test Plan:
linted python code with PyFlakes warnings

Reviewed By: epriestley
Reviewers: jungejason, epriestley
Commenters: jungejason
CC: aran, epriestley, andrewjcg, jungejason
Differential Revision: 310
2011-05-19 16:48:05 -07:00
Andrew Gallagher
ce06ec00cc arc lint: fix typo in PEP8 linter
Test Plan:
linted!

Reviewed By: jungejason
Reviewers: epriestley, jungejason
CC: aran, jungejason
Differential Revision: 314
2011-05-19 13:41:18 -07:00
epriestley
7e4bde114e Detect library conflicts in Arcanist and raise a very specific error message
Summary:
When you run one copy of arcanist against another copy, you previously got a
meaningless error. After D311 you get a slightly more meaningful error.

Capture the new exception and raise an extremely specific error.

NOTE: Adding arcanist to .arcconfig forces the library conflict error to
trigger; otherwise there's just an implicit conflict because 'arc' reads the
running-copy library amp instead of the working-copy library map. This only
worked before because arcanist includes itself.

Test Plan:
Ran arcanist on a different copy of arcanist, got a good error message.

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 312
2011-05-19 11:32:53 -07:00
epriestley
292c995d57 Use phutil_passthru() in "arc commit" instead of passthru().
Summary:
passthru() doesn't show up on --trace, and one time a while ago someone had an
issue which was harder than necessary to debug because the command wasn't
avialable in the log. Use the logged version.

Also fix a locale issue I ran into on my local machine, with "en_US.utf8" not
being a valid locale.

Test Plan:
Created an SVN repo and used "arc commit" to make commits against it.

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 306
2011-05-19 10:19:01 -07:00
Andrew Gallagher
399c505f1a arc lint: fix/enable PEP8 linter
Summary:
This diff:
- Adds the PEP8 linter to the externals directory
- Changes the path for finding pep8.py
- Removes use of execx since pep8.py return an errors code
  when it finds PEP8 violations

Test Plan:
tested linting python code

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley
Differential Revision: 309
2011-05-18 16:47:23 -07:00
Adam Simpkins
10bdc77f96 arc diff: --only doesn't conflict with --nounit or --nolint
Summary:
The --only flag implies --nounit and --nolint, so there's really no
conflict if these flags are specified together.

(My wrapper around arc diff automatically specifies --nounit in some
cases.  It is useful to be able to manually add --only in some cases,
and not have it fail because it conflicts with the automatically
specified --nounit flag.)

Test Plan:
Ran "arc diff --only --nounit".

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley, simpkins
Differential Revision: 298
2011-05-17 13:08:21 -07:00
Adam Simpkins
952fa69f72 lint: fail early if no lint engine is defined
Summary:
Update the lint workflow to exit immediately it there is no lint engine
configured.  Previously it scanned the repository to find the paths to
check before failing in this case.  Scanning the repository can be
relatively slow on large repositories.

Test Plan:
Ran "arc lint" in a large repository with no lint engine configured.
Verified that it failed quickly, without scanning the repository first.

Reviewed By: jungejason
Reviewers: jungejason, epriestley
CC: aran, jungejason
Differential Revision: 297
2011-05-17 12:59:49 -07:00
epriestley
3a559ddd13 'arc liberate', convenience wrapper for various libphutil operations
Summary:
The story for creating and maintaining libphutil libraries and modules
is pretty terrible right now: you need to know a bunch of secret scripts and
dark magic. Provide 'arc liberate' which endeavors to always do the right thing
and put a library in the correct state.

Test Plan:
Ran liberate on libphutil, arcanist, phabricator; created new
libphutil libraries, added classes to them, liberated everything, introduced
errors etc and liberated that stuff, nothing was obviously broken in a terrible
way..?

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 269
2011-05-17 09:53:19 -07:00
epriestley
ade681e0db Update arcanist to use PhutilServiceProfiler.
Summary:
Switch to the new PhutilServiceProfiler API.

Test Plan:
Ran "arc" with "--trace", got mostly reasonable output. I'll clean up conduit
output in a followup.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran
Differential Revision: 292
2011-05-16 17:09:54 -07:00
epriestley
3edd525f83 Stop undeclared variable warning for $obj->{$prop} syntax
Summary:
This isn't necessarily a root-cause solution but this syntax is really really
bad. If we want to fix the root cause, I'd recommend making its use a lint
error?

Test Plan:
Unit tests failed before patch, passed afterward.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 270
2011-05-11 22:06:03 -07:00
epriestley
23afdb99f0 Allow ArcanistLinter to load files not directly affected by lint
Summary:
This is necessary for the Javelin linters. The libphutil and flib linters do it
implicitly, so there's no real tradeoff here.

Test Plan:
Ran javelin linters.

Reviewed By: tuomaspelkonen
Reviewers: tomo, aran, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 238
2011-05-08 01:43:49 -07:00
epriestley
71d233410b mark remote hooks 2011-05-06 15:35:07 -07:00
epriestley
caed8a2830 Restore "arc patch D12345" syntax
Summary:
I only dropped this because it's slightly inconvient to accommodate, but
empirically it's pretty confusing to users (who often use --diff 12345 when they
mean --revision 12345).

Test Plan:
Ran "arc patch D45", "arc patch --revision 45", "arc help patch"

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 241
2011-05-06 12:40:16 -07:00
epriestley
1fdead8c20 Provide 'arc call-conduit' to facilitate script integration with Phabricator
Summary:
Provide a formal mechanism for making conduit calls from other scripts.

Test Plan:
Called 'conduit.ping'.

Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 239
2011-05-06 11:56:21 -07:00
jungejason
8f59aff31a Make the rule for applying git diff less restrict
Summary:
the current command to apply a git diff is 'git apply --index',
which will fail the whole patch and does not touch the working tree when
some of the hunks do not apply. We want to allow the user to patch the
ones which apply.

Test Plan:
run 'arc apply' against a diff whcih partially applies and
verify that the hunks which apply are applied, and .rej files are
generated for the rest.

Reviewed By: simpkins
Reviewers: simpkins, epriestley
Commenters: ju
CC: aran, slawekbiel, simpkins, ju
Differential Revision: 235
2011-05-05 20:33:23 -07:00
jungejason
64ecf63680 init invest 2011-05-03 23:33:21 -07:00
tuomaspelkonen
e04ea5c7d8 Added 'edit' field to 'getcommitmessage' conduit calls.
Summary:
Server log was showing errors for undefined index 'edit'.

Test Plan:
arc lint

Reviewed By: jungejason
Reviewers: jungejason, grglr
Commenters: grglr
CC: aran, epriestley, grglr, jungejason, tuomaspelkonen
Differential Revision: 217
2011-05-03 11:26:01 -07:00
epriestley
17ac4d9ab1 Provide setup/teardown hooks for ArcanistPhutilTestCase
Summary:
I'm making a move on Lisk stubbability and need these so I can set up
Lisk isolation unconditionally in Phabricator tests. Also document and organize
this class.

Test Plan:
Changed the expected counts in the meta-tests and got failures. Ran
all unit tests.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran
Differential Revision: 192
2011-04-30 12:04:11 -07:00
slawekbiel
d70e5db39e Aggregate display of postponed tests
Summary:
Displaying each postponed test separately was spammy.
Now it will only display a single line with the total count

Test Plan:
~/local/arcanist/bin/arc unit --sandcastle-tests
35 tests to run.
Completed 'ParentInfoTestCase.php'
Interrupted 'MentionsUntagTestCase.php'
    PASS  ParentInfoTestCase
  POSTPONED  34 tests

Reviewed By: sgrimm
Reviewers: jungejason, sgrimm
CC: sgrimm
Revert Plan:
sure

Other Notes:

Differential Revision: 173
2011-04-28 12:05:00 -07:00
slawekbiel
8332d86ad2 add short option for fetching git revision hash
Summary:
Sometimes we need to show abbreviated hashes, passing --short is more
reliable than substring of fixed number of characters.

Test Plan:
- called with and without the parameter, got correct results
           -

Reviewed By: jungejason
Reviewers: jungejason
CC: jungejason
Revert Plan:
sure

Other Notes:

Differential Revision: 170
2011-04-27 15:26:20 -07:00
epriestley
8ffba323ea Stop this from failing on directories, etc.; u-u-u-u-ultra clown. 2011-04-11 16:13:22 -07:00
epriestley
c00da5bf4f Load more context for lint only if the files actually exist. 2011-04-11 14:54:17 -07:00
tuomaspelkonen
d67a081243 Arc cover handles a file that is added and modified correctly.
Summary:
Modifying a new file and running 'arc cover' before committing the
modifications confused arc. The problem was that 'unstaged' status
erased 'added' status and this caused problems.

Test Plan:
Tested that arc cover ignores added files when they are modified, but
old files that are modified are still handled correctly by arc cover.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley
Differential Revision: 122
2011-04-11 14:08:54 -07:00
epriestley
2cf1393219 Load files which we don't directly examine, since there are some common cases
where we either generate an entire file or an external script examines it.
2011-04-10 16:05:44 -07:00
epriestley
d87fda8561 Improve workflow for missing certificates. 2011-04-10 13:09:47 -07:00
adonohue
0472a5ff95 Improve lint rendering
Summary:
Declare victory like it's the United States.

Test Plan:
www copyright, apache copyright, control keyword whitespace and trailing
whitespace lint warnings

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 117
2011-04-08 19:18:34 -07:00
epriestley
b1acee6ee5 Fix repository UUID parsing. 2011-04-05 22:27:32 -07:00
epriestley
ceb8a42003 Ship up repository UUIDs with diffs. 2011-04-05 21:09:59 -07:00
epriestley
1139eb1a73 Correctly detect uncommitted files in Git
Summary:
I simplified this code at some point and broke it horribly
in the process. Mask in the right flag here and combine the maps
correctly. This solves the great mystery of increased developer
clowniness.

Test Plan:
Ran 'arc diff' in a working copy with staged but
uncommitted files, got an error message (previously, I didn't).

Reviewed By: aran
Reviewers: mroch, aran
CC: aran
Differential Revision: 104
2011-04-05 21:08:55 -07:00
jungejason
1dd69bdbee Improve 'arc' error message when failing to load a library
Summary:
display better message about the .arcconfig. The message will
be something like this now:
  Usage Exception: Failed to load library "phabricator" at location
  "srcaaaaa/". Please check the "phutil_libraries" setting in your
  .arcconfig file. Refer to page
  http://phabricator.com/docs/arcanist/article/Setting_Up_.arcconfig.html
  for more info.

Test Plan:
modify the .arcconfig file to test it. Also make sure it
works for correct settings.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley
Differential Revision: 99
2011-04-04 16:44:09 -07:00
Aran Donohue
76c45a74f3 JSON output for Lint
Summary:
Refactor ArcanistLintRenderer into three independent classes, and let
the Workflow select between them based on its 'output' parameter.

Test Plan:
Introduce a Lint warning and lint with no --output, with --output summary and
with --output JSON.

Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 96
2011-04-04 10:07:06 -07:00
epriestley
4bce8732e5 Expand ArcanistDiffParser to accept awkward Diffusion + SVN workflows. 2011-03-30 19:33:50 -07:00
epriestley
11e25f5840 Add an empty file lint test. 2011-03-22 20:51:45 -07:00
epriestley
b8e7e9017a Special case ArcanistTextLinter for completely empty files. 2011-03-22 11:09:28 -07:00
slawekbiel
aa44db2f96 [Adding postpone status for unittests]
Summary:
That's for the tests that will be run asynchronously.

Test Plan:
run arc unit with some tests that returned postponed status - they showed up.
Background was yellow.

Reviewed By: epriestley
CC: epriestley
Revert Plan:
sure

Other Notes:

Differential Revision: 77
2011-03-21 19:11:58 -07:00
jungejason
51b371481b Improve the error message for ERR-NO-CERTIFICATE
Summary:
list the hosts for which the .arcrc file have certificate and
which conduit-uri the user is trying to access.

Test Plan:
test the case where a cert is missing.

Reviewed By: epriestley
CC: epriestley
Differential Revision: 76
2011-03-21 16:40:57 -07:00
epriestley
bdde006484 Detect SVN 1.6 "tree-conflicts". 2011-03-20 15:06:55 -07:00
epriestley
efcd70c2d7 Use phutil_console_confirm() correctly, not vestigal $this->confirm().
Summary:

Test Plan:
$ arc patch --diff 553961

    Patch deletes file 'html/miniprofile.php', but the file does not exist in
    the working copy. Continue anyway? [y/N] y

    Patch deletes file 'html/index.php', but the file does not exist in the
    working copy. Continue anyway? [y/N] y

 OKAY  Successfully applied patch to the working copy.

Reviewers:

CC:
2011-03-19 12:16:47 -07:00
epriestley
b7be6dbf32 Fix symbol reference in arcconfig.diviner (thanks, jason!) 2011-03-17 19:19:31 -07:00