1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-10-24 17:48:50 +02:00
Commit graph

134 commits

Author SHA1 Message Date
Bob Trahan
6c613292f7 Make arc patch be less ROFL for mercurial
Summary:
pretty easy stuff as mercurial accepts git style patches...!

also fixed two issues where we were 1) storing the short hash and 2) storing it
with a trailing "\n".  This diff makes us store the full hash AND no trailing
return character

Test Plan:
in my mercurial repo
<note repo at revision $foo>
<did something dumb>
hg commit -m "something dumb"
arc diff
<go to web and fill out stuff for DX>
hg checkout $foo
arc patch DX
<verify patch DX successfully applied!>

use conduit console to verify a few diffs were returning the correct full
revision hash with a trailing \n

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T630

Differential Revision: https://secure.phabricator.com/D1431
2012-01-17 09:40:43 -08:00
Marke Hallowell
7e63e232ba Fix for arc install-certificate timeout.
Summary:
Extended the install-certificate workflow's timeout from 5
seconds to rely on the ConduitClient default (30 seconds), which matches the HTTP interface.

Test Plan: Run arc install-certificate

Reviewers: epriestley

CC: epriestley, aran

Differential Revision: https://secure.phabricator.com/D1404
2012-01-15 10:11:52 -08:00
epriestley
f271e1d8e8 Improve 'arc' behavior under git mutable history with ambiguous commit messages
Summary:
Currently, we throw a fairly perplexing error when there are multiple valid
commit messages. Installs can also remove the "test plan" field entirely, which
is the only really strong discriminator here.

When the message to use is ambiguous, show the user all the valid messages and
prompt them to choose one.

Also add a -C flag like "git commit -C", so they can choose a message
explicitly.

Test Plan: Ran "arc diff HEAD^^^^^", "arc diff -C <rev>".

Reviewers: cpiro, btrahan, jungejason

Reviewed By: cpiro

CC: aran, cpiro

Differential Revision: https://secure.phabricator.com/D1385
2012-01-12 20:09:53 -08:00
Bob Trahan
b61e4eacf1 Arc - add a sanity check to arc patch workflows to make sure the vcs base
revision is the correct base revision relative to the patch.

Summary: What the title says.   If not correct, warn the user.   This check
honors the --force flag to skip all these checks.   This change also includes
moving some Differential constants into Arc so they can be used for both
projects.   There is a corresponding phabricator diff (incoming) to address this
part of the change.

Test Plan:
For a project with actual diffs, a git repository tracked by phabricator, *AND*
development in master branch only, do some...
- git reset --hard HEAD^1
- arc patch DX, where X is what got us to HEAD in the first place
- verify successful patch
...then...
- git reset --hard HEAD^^
- arc patch DX, where X is what got us to HEAD in the first place
- verify warning
- verify Y versus N continues versus stops appropriately
Note if development were done outside the master branch this warning message
will fire early / often as git commit hashes are based on the commit *and* the
rest of the source code the commit is made against.  This is (unfortunately) the
"typical" case so this warning is pretty active at the moment.   T201 will
eventually land and when parsing a given commit update the corresponding diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1328
2012-01-10 11:48:05 -08:00
epriestley
f3eccfbe81 Unify arguments for 'arc lint', 'arc unit'
Summary: See T645. These commands take inconsistent and overly-magical arguments
right now. Instead, make them behave consistently and allow them both to operate
on "arc <workflow> path path2 path3 ...", which is a generally useful workflow.

Test Plan: Ran "arc lint <path>", "arc unit <path>", "arc lint --rev
HEAD^^^^^^", "arc unit --rev HEAD^^^^^^^^^^^^", etc. Ran "arc diff --trace" and
verified --rev argument to child workflows.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T645

Differential Revision: https://secure.phabricator.com/D1348
2012-01-10 10:42:22 -08:00
epriestley
d359532842 Add an "--update <revision>" flag to Arcanist
Summary:
See T614. This flag explicitly tells Arcanist to use the message for an existing
revision, and attach the change to it directly.

Next step is to have "arc diff" automatically choose "--create" or "--update" in
the absence of "--create", "--update", "--only", "--preview" or a template
commit message.

Test Plan:
Ran "arc diff --update <n>" for various revisions. Got updates or
errors as expected.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, jungejason

Maniphest Tasks: T614

Differential Revision: https://secure.phabricator.com/D1346
2012-01-09 11:42:14 -08:00
epriestley
c49e9863d4 Add a "--raw" flag to "arc diff"
Summary:
Some Mercurial users at Dropbox have very specific diff preparation
needs. Allow "arc" to read an arbitrary diff off stdin. This disables most
features.

Test Plan:
Ran "git diff HEAD | arc diff --raw", "git show | arc diff --raw",
"hg diff --rev 8 | arc diff --raw".

Reviewers: btrahan, jungejason, Makinde

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T617

Differential Revision: https://secure.phabricator.com/D1323
2012-01-05 14:13:30 -08:00
epriestley
560b339ad3 Refactor ArcanistDiffWorkflow to be a little more manageable
Summary:
I want to make some changes to this workflow but it a huge mess right now. Try
to refactor a bit to make it a little more manageable.

Broadly:

  - Moved lint/unit constant mapping into separate methods.
  - Moved lint/unit/local properties into separate methods.
  - Moved diff spec construction into a separate method.
  - Moved some message stuff into separate methods and reorganized related
methods near to one another.
  - Removed an unused findRevisionInformation() method.

I fixed a couple of small bugs, too:

  - --create now conflicts with --only and --preview.
  - --create now probably works in Mercurial.
  - --create messages now have basic reviewer validation.

This should have not have any significant behavioral changes.

Test Plan:
  - Created this revision.
  - Ran "arc diff --create".

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1320
2012-01-05 12:59:05 -08:00
vrana
085aa6093b Ignore non-plaintext changes in cover
Test Plan:
Run ##arc lint## after changing a binary file
"Warning:  Invalid argument supplied for foreach() in
src/workflow/cover/ArcanistCoverWorkflow.php on line 88" should not be displayed

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1314
2012-01-04 15:44:48 -08:00
Kunal Bhalla
62e527482b [arc svn-hook-pre-commit] Access working copy
Summary:
Creates a new hook API that can be used to interface with
SVN/Git/Mercurial in the context of a commit hook. Currently only adds a
function to read the modified file data in a Subversion commit hook.

An object of this API is created in the SvnHookPreCommitWorkflow and
passed on the Lint Engine which then uses it to access current file
data, of the way the APIs seem to be structured); linters use the
getData function which is essentially a wrapper around the engine's
call, with another layer of caching.

Task ID: #770556

Blame Rev:

Test Plan:
- Create a local svn repository and add a minimal hook to run the local
  version of arc to test commits

(http://phabricator.com/docs/arcanist/article/Installing_Arcanist_SVN_Hooks.html)
- Create a temporary repository that can trigger any of the linters
  available, and test against a temporary linter by committing against
  the test repository: the linter should be able to access all required
  files by using loadData/getData in the LintEngine and Linter.

Revert Plan:

Tags: lint, svn-hook-pre-commit

Reviewers: jungejason, asukhachev, epriestley, aran

Reviewed By: epriestley

CC: aran, jungejason, epriestley, kunalb, asukhachev

Differential Revision: https://secure.phabricator.com/D1256
2011-12-29 14:28:50 -08:00
jungejason
6910fd77a4 Use getcwd() which is more reliable than $_SERVER['PWD']
Summary:
in arcanist we are using $_SERVER['PWD'], but in some cases it
is not returning the correct result. For example, when I'm using
PhpStorm (an php IDE) to launch the script, $_SERVER['PWD'] returns the
path of the binary of the IDE:
/home/jungejason/tools/PhpStorm-107.658/bin, not the path where arcanist
is at. getcwd() returns the correct value.

One difference between getcwd() and $_SERVER['PWD'] is that getcwd()
resolves symlink where $_SERVER['PWD'] does not. From what I can see,
using realpath should work.

Test Plan:
* ran arcanist as normal and it worked;
* run arcanist in PhpStorm and it worked.
* created a symlink pointing to the repository inside which
I ran the arc command, and it worked.
* created a symlink pointing to arcanist project, run `.
* resources/shell/bash-completion`  and it worked

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1285
2011-12-24 15:05:13 -08:00
vrana
736b0deaac arc cover doesn't support against_commit
Summary: Blame Rev:

Test Plan: arc help cover

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1272
2011-12-22 14:50:24 -08:00
epriestley
25ebebe0df Bump Arcanist client version to 3
Summary:
Thinking about this, I think it's worthwhile to bump versions for T1249/T1250.
The problem is that if you have an old Arcanist, trying to update diffs against
a new Phabricator will create new diffs instead, and it probably won't be
obvious what's wrong.

Bump the versions so users will get a message like "oh, hey, you should
upgrade".

Test Plan:
  - Tried to diff against a mismatched version.
  - Diffed against a matching version.

Reviewers: btrahan, jungejason, aran

Reviewed By: aran

CC: aran

Differential Revision: https://secure.phabricator.com/D1257
2011-12-22 06:47:15 -08:00
epriestley
ca879150c8 Improve detection of invalid unit and lint engines
Summary: We fatal confusingly if you specify a valid class that isn't of the
right subclass (like a linter rather than a lint engine). Improve the error
message.

Test Plan:
  - Ran "arc lint --engine PhutilSymbolLoader", "arc unit --engine
PhutilSymbolLoader", got expected failures.
  - Ran "arc diff" normally without errors.

Reviewers: btrahan, jungejason, aran

Reviewed By: aran

CC: aran

Differential Revision: https://secure.phabricator.com/D1259
2011-12-21 09:02:04 -08:00
epriestley
89cb92a22c Parse full URIs for "Differential Revision" in Arcanist
Summary:
  - Allow Arcanist to parse either "123", "D123" (existing behaviors) or
"http://phabricator.example.com/D123" (new behavior) values.
  - Drop support for labeling this field "DiffCamp". This should only impact
people trying to update revisions that are more than ~a year old, which should
be very very few.

Test Plan:   - Ran "arc diff" with values "74", "D74", "x74",
"http://local.aphront.com/D74", "http://local.aphront.com/x74". Got the expected
behaviors.

Reviewers: jungejason, btrahan

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T54, T692

Differential Revision: 1249
2011-12-20 19:57:55 -08:00
adonohue
72ee0ced4f Don't output "OKAY" for arc lint --output json.
Summary: --output json will be used for scripts, so support script writers a
little better.

Test Plan:
arc lint [--output {json, summary}]
arc lint --output json {--apply-patches, --never-apply-patches}

Reviewers: epriestley

Reviewed By: epriestley

CC: jack, aran, epriestley

Maniphest Tasks: T675

Differential Revision: 1220
2011-12-15 11:03:17 -08:00
Marek Sapota
f794b50b0e Merge branch 'amend_revision' 2011-12-13 11:41:31 -08:00
Marek Sapota
a89d2537a1 Allow revision numbers prefixed with 'D' in arc commit --revision
Test Plan: Tried `arc commit --revision D123` and it worked.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1194
2011-12-13 11:41:16 -08:00
Marek Sapota
3abebbebfd Make --revision switch of amend workflow easier to use
Summary:
--revision doesn't allow the revision to be prefixed with 'D'.  Also the error
message showed when specified revision doesn't exist is hard to understand.

Test Plan:
Used `arc amend --revision D123`, tried it without 'D' and with a non existing
revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1193
2011-12-13 11:41:11 -08:00
Marek Sapota
0788220af4 Fix arc commit workflow when new directories were added.
Summary: This adds parent directories of modified files to commit paths if
needed.

Test Plan:
Used https://reviews.facebook.net/D603 patch on Apache Hive repository, called
`arc commit` and SVN didn't complain about missing paths.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T667

Differential Revision: 1189
2011-12-09 16:06:47 -08:00
epriestley
331afdce87 Add an experimental "--create" flag to "arc diff"
Summary:
See T614. This adds a "--create" flag which I think works properly, but doesn't
make it the default.

Once I add "--update" and am confident the flags actually work, I'll work on
some heuristics to make "arc diff" automatically choose "--udpate" or "--create"
as per T614.

Test Plan:
This revision was created with "--create" and a bogus commit message ("derp").
I intentionally goofed the message at first to test the fail + file workflow.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Differential Revision: 1162
2011-12-03 09:57:09 -08:00
epriestley
79da3a6ff9 Remove client-side commit message validation from Arcanist
Summary:
Commit templates are fully configurable on the server now, so we should be doing
validation there, since an install can add or remove fields and change
validation rules. Remove these outdated client validations, as per comment.

Also update the API call to use the 'errors' field, which allows us to show the
user all the parse errors at once. See:

https://secure.phabricator.com/diffusion/P/browse/origin:master/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php;cfaab709df37739b$75

Test Plan:
Ran "arc diff" on a change with multiple errors:

```$ arc diff --conduit-uri=http://local.aphront.com/
Usage Exception: Commit message is not properly formatted:

Error parsing field 'Reviewers': Commit message references nonexistent users:
jjjderp.
Error parsing field 'CC': Commit message references nonexistent users and
mailing lists: jjjderp.

You should use the standard git commit template to provide a commit message. If
you only want to create a diff (not a revision), use --preview to ignore commit
messages.```

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1154
2011-12-02 07:29:31 -08:00
Bob Trahan
d81d97f9c6 make arc patch issue a warning if applying a patch against a different project
Summary: adds a little bit of sanity checking to the arc patch workflow.  in
short, if the working copy project is not the same as the patch project, don't
apply the patch

Test Plan:
ran

arc patch DX
arc patch DX --force

in the top line directory for project A and proejct B.   DX is for project A.

verified for project A that the patch was applied and for project B i was issued
warnings as expected.  also verified in project B case that saying Y or N to the
warning had the desired effect.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1140
2011-12-01 09:44:55 -08:00
David Reuss
1f69ab5fdb Added local encoding parameter for arc diff workflow
Summary:
If any hunks is detected as non-utf8, and you've never submitted diffs
for a certain project before, you would get a ERR-BAD-ARCANIST-PROJECT
exception. This makes it possible to submit the patch properly, so you
can set the encoding in the interface afterwards. Further this fixes
cases where you don't supply a diff but will result in hunks getting
treated as binary, but that still beats the exception behaviour.

Test Plan:
Ran `arc diff` with and without the new --encoding param and
got the expected results. Also ensured the diff (with non utf-8 hunks)
would be properly created even when no encoding is specified.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1135
2011-12-01 08:56:03 -08:00
Jakub Vrana
647c047b3c Document paths parameter in arc unit
Summary:
arc unit supports paths parameter
It is not documented in arc help

Test Plan:
arc help
Search for unit

Reviewers: slawekbiel, aran

Reviewed By: aran

CC: aran, vrana

Differential Revision: 1143
2011-11-30 15:23:09 -08:00
epriestley
c3c4f6ed4c Improve git behavior in the zero- and one- commit case
Summary:
Git works completely differently for commits zero and one than for 2..N so add
more special casing to handle them. See:

  - {T206}
  - {T596}

The getCommitRange() block is also fatal land, although I wasn't able to reach
it. I'll follow up with @s on T596.

Test Plan:
  - Created a new, empty repository ("mkdir x; cd x; git init").
  - Ran "arc lint", "arc unit", "arc diff" against it with no commits (the first
two work, the third fails helpfully).
  - Made an initial commit.
  - Ran "arc lint", "arc unit", "arc diff" against it (all work correctly).

Reviewers: btrahan, jungejason, aran

Reviewed By: aran

CC: s, aran

Differential Revision: 1142
2011-11-30 11:17:37 -08:00
Marek Sapota
4ea0541aeb Allow modification of the svn commit message via an event listener
Test Plan:
Wrote an event listener modifying the commit message and the message was
successfully changed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, jungejason, epriestley

Differential Revision: 1103
2011-11-16 16:40:19 -08:00
Bob Trahan
16caf63d98 make arc issue a warning if file upload fails during diff creation
Summary:
just a little try / catch action in ArcanistDiffWorkflow.

"Ideally, it would be good to flag these changes somehow so that "arc patch" can
issue the inverse warning (e.g., "This patch could not be completely applied
because some binary data was not uploaded.") but that hasn't come up in a real
use case yet."

I think a change w/ filetype of FILE_IMAGE || FILE_BINARY *and* no phid means
that the upload failed so there's no additional flag needed.  (True?)  however,
it would be easy enough to store metadata that explicilty stated whether or not
the file upload succeeded.

(also / related - i looked through the arc patch workflow a bit and i don't
understand how the svn codepath loads up the actual binary files...  for git,
toGitPatch => buildBinaryChange => getBlob is the right path )

Test Plan:
 - set 'storage.mysql-engine.max-size' to 0 in my conf, uploaded diffs with
files
 - noted in differential that it correctly detected images versus binary despite
file upload failing
 - noted in differential that images had some empty UI
 - reverted conf change, uploaded diffs with files
 - noted in differential file showed up
 - ran arc patch with DX, where DX had broken files
 - noted "Downloading binary data...

Exception:
ERR-BAD-PHID: No such file exists."

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1100
2011-11-12 09:57:22 -08:00
Nick Harper
28eae60821 Check that rev is accepted in arc mark-committed --finalize
Summary:
If arc amend is run on a rev that isn't accepted, it runs arc mark-committed
--finalize. The rev shouldn't be marked as committed if it hasn't been accepted,
so this diff adds in that check.

Test Plan:
Ran arc amend on a rev that hasn't been accepted, checked that it didn't get
marked as completed.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, epriestley, nh

Differential Revision: 1104
2011-11-10 16:18:17 -08:00
epriestley
5150252f91 Add very hacky encoding transformation support for arc
Summary:
Adds a secret, undoucmented "encoding" key to ".arcconfig" which makes a very
half-hearted effort to convert encodings. This is probably good enough that
Differential can be used for code review, but there will be issues with 'arc
patch', 'arc export', paste, maybe conduit stuff, Diffusion, and whatever else I
haven't thought of.

This also doesn't store the original encoding so anything converted like this
won't reasonably be able to be made to work with all that stuff in the future.

See T452 for a broader discussion of the issues involved.

Test Plan:
Short circuited the UTF-8 detection to always fail, had my files "converted"
from ISO-8859-1 to UTF-8.

@davidreuss: you can test this by applying this patch to arcanist/, adding
'"encoding" : "ISO-8859-1"' to your .arcconfig, touching some non-ASCII file,
and then running "arc diff".

Reviewers: davidreuss, jungejason, tuomaspelkonen, aran

Reviewed By: davidreuss

CC: aran, davidreuss, epriestley, nshamg123

Differential Revision: 812
2011-11-04 14:07:12 -07:00
Marek Sapota
08ce2a2e2c Save committed revision id in workflow.
Summary:
Save commited revision id in workflow, so it can be used in Arcanist
didRunWorkflow hook.

Test Plan:
Running getRevisionID on the workflow should return id of the committed
revision.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, mareksapota

Differential Revision: 1078
2011-11-04 12:13:03 -07:00
epriestley
d57ea379c6 Fix "arc lint" exploding on new directories
Summary:
D1061 introduced a 'text file' check, but it fails under SVN for new
directories.

  - Revert D1061 (This reverts commit b2cd18252701be2093b52652fb3d1d94c5df571e.)
  - Make getChangedLines() return null to indicate that the operation doesn't
make sense. I think this was the intent of the code in the lint engine.
  - Fix a bug where running "arc lint" on a change in an SVN working copy from a
subdirectory would fail.
  - Fix a bug where warnings with no line information were incorrectly
discarded.

Test Plan:
  - Ran "arc lint" in an SVN working copy with a new directory (no failure).
  - Forced FilenameLinter to always raise a warning. Added a binary file and ran
"arc lint". The warning was reported for the new binary file, a new text file,
and a new directory.

Reviewers: jungejason, andrewjcg, nh, tuomaspelkonen, aran

Reviewed By: andrewjcg

CC: aran, andrewjcg, epriestley

Differential Revision: 1076
2011-11-03 14:28:04 -07:00
Marek Sapota
9070a123d3 Allow running arc patch without authentication.
Summary:
Allow `arc patch` without authentication if Phabricator instance has
'differential.anonymous-access' set to true.

Test Plan:
Set 'differential.anonymous-access' in Phabricator to true and run `arc patch`
without installing a certificate.  `arc patch` should work as expected.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1069
2011-11-01 15:31:04 -07:00
Andrew Gallagher
b2cd182527 Show lint warnings for non-"line-able" files
Summary:
Currently, lint messages are only included if they are errors or
if they affect lines which the diff changed.  The implementation
of this caused issues for non-text files (e.g. binaries), as line
change information is not available, and the corresponding lint
messages were dropped (for non-errors).

In this diff, only lint messages concerning text files are dropped
based on this line filtering.

Test Plan: arc lint with binary file

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, aravindn, andrewjcg, liat, epriestley

Differential Revision: 1061
2011-10-28 13:49:58 -07:00
Marek Sapota
bb05ddfb5a Allow anyone to run arc commit on an accepted revision
Test Plan:
Using SVN make some changes to the repo, run `arc diff`.  As other user accept
the revision, add the changes to your repo using `arc patch` and then run `arc
commit --revision revisionID` to commit them.  Arcanist should ask if you are
sure that you want to commit this revision and if you answer `Y` it should
commit to SNV repo.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, mareksapota, epriestley

Differential Revision: 1055
2011-10-28 09:18:32 -07:00
Marek Sapota
6db055222a Allow anyone to mark Differential revisions as commited.
Test Plan:
Run `arc mark-committed` on a revision that you don't own, you should see a
prompt asking you if you really want to do that and if you answer `Y` it should
mark the revision as committed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota

Differential Revision: 1052
2011-10-25 17:24:57 -07:00
epriestley
df8c0e5e25 Don't do working copy checks for "arc amend --show"
Summary: See T587. Reduce the strictness of working copy checks when using
"--show", since there's a reasonable workflow where you 'arc patch' and then
'arc amend --revision X --show | git commit -a -F -' that currently won't work.
There are other ways to accomplish the same thing but this increases flexibility
overall.

Test Plan: Ran 'arc amend --show' with a dirty working copy, didn't get yelled
at.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh

Differential Revision: 1033
2011-10-24 23:37:47 -07:00
Jason Ge
ab50afe583 Fix breaking because of D935
Summary:
D935 missed one place of parseGitRelativeCommit() in
ArcanistExportWorkflow.

Test Plan: ran arc export and verify that it worked.

Reviewers: epriestley, tuomaspelkonen, aran

Reviewed By: aran

CC: aran

Differential Revision: 1015
2011-10-17 19:32:38 -07:00
epriestley
2fd37a1728 Automatically detect when to mark revisions committed
Summary:
See D945. We have this kludgy "remote_hooks_installed" mess right now, but we
have enough information on the server nowadays to figure this out without it.

Also reduce code duplication by sharing the "mark-committed" workflow.

This causes "arc merge" to effect commit marks.

Test Plan:
In Git, Mercurial and SVN working copies ran like a million
amend/merge/commit/mark-committed commands with and without --finalize in
various states of revision completion.

This change is really hard to exhaustively test because of the number of
combinations of VCS, revision state, command, command flags, repository state
and tracking state. All the reasonable tests I could come up with worked
correctly, though.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 967
2011-09-27 11:06:02 -07:00
epriestley
c84d6255b4 Support "immutable_history" doctrine in arc
Summary: When a project uses a conservative history mutability doctrine, never
try to amend history.

Test Plan: Ran "arc amend" in an "immutable_history" working copy.

Reviewers: fratrik, Makinde, aran, jungejason, tuomaspelkonen

Reviewed By: Makinde

CC: aran, Makinde, epriestley

Differential Revision: 862
2011-09-15 07:44:29 -07:00
epriestley
44959afd4b Add an "arc merge" workflow
Summary:
This should support conservative rewrite policies in git fairly well, under an
assumed workflow of:

  - Develop in local branches, never rewrite history.
  - Commit with "-m" or by typing a brief, non-template commit message
describing the checkpoint.
  - Provide rich information in the web console (reviewers, etc.)
  - Finalize with "git checkout master && arc merge branch && git push" or some
flavor thereof.

This supports Mercurial somewhat. The major problem is that "hg merge" fails if
the local is a fastforward of the remote, at which point there's nowhere we can
throw the commit message. Oh well. Just push it and we'll do our best to link
them up based on local commit info.

I am increasingly forming an opinion that Mercurial is "saftey-scissors git".
But also maybe I have no clue what I'm doing. I just don't understand why anyone
would think it's a good idea to have a trunk consisting of ~50% known-broken
revisions, random checkpoint parts, whitespace changes, typo fixes, etc. If you
use git with branching you can avoid this by making a trunk out of merges or
with rebase/amend, but there seems to be no way to have "one commit = one idea"
in any real sense in Mercurial.

Test Plan: Execute "arc merge" in git and mercurial.

Reviewers: fratrik, Makinde, aran, jungejason, tuomaspelkonen

Reviewed By: Makinde

CC: aran, epriestley, Makinde

Differential Revision: 860
2011-09-15 07:42:45 -07:00
epriestley
31ec011922 Move some VCS-specific logic into VCS APIs
Summary:
We have some git-specific logic on main pathways which should be in the API
class, move it around so "arc lint" with an engine works under Mercurial. This
resovles the error @makinde reported:

> PHP Catchable fatal error:  Argument 1 passed to
ArcanistBaseWorkflow::parseGitRelativeCommit() must be an instance of
ArcanistGitAPI, instance of ArcanistMercurialAPI given, called in
/home/makinde/.arc_install/arcanist/src/workflow/lint/ArcanistLintWorkflow.php
on line 131 and defined in
/home/makinde/.arc_install/arcanist/src/workflow/base/ArcanistBaseWorkflow.php
on line 830

Test Plan: Ran "arc diff" in git and hg working copies, plus "arc lint" with a
configured "lint_engine".

Reviewers: Makinde, aran, jungejason, nh, tuomaspelkonen

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 935
2011-09-15 07:39:34 -07:00
mgummelt
e9b7f8e3ca Merge branch 'master' of github.com:facebook/arcanist into get_engine 2011-09-08 18:26:07 -07:00
mgummelt
3250cbb4d3 two small changes for "arc unit" post hook
Summary:
1) unit engine getter method in the unit workflow

   we store some information (unit test results) in the engine that we
   need to access in the "arc unit" post hook

2) make requireCleanWorkingCopy() public

   "arc unit" doesn't need to be clean in general, but we'd like the
   option to upgrade the workflow to require it if necessary.  We use
   this for ensuring a repo is clean before updating unit test
   results.

Test Plan: Used both features in a custom post hook in arc unit

Reviewers: epriestley

Reviewed By: epriestley

CC: dpepper, aran, epriestley

Differential Revision: 917
2011-09-08 18:25:54 -07:00
epriestley
aa138a80d2 Attach local commit information to DVCS revisions
Summary: When a revision is created, attach relevant information about the local
commits which it came from if applicable. This supports T473, for DCVSes and
DCVS workflows with immutable history where we can't just amend commit messages.
It will also allow us to enrich the web interface.

Test Plan: Will verify this info shows up for this very diff.

Reviewers: fratrik, aran, jungejason, tuomaspelkonen

Reviewed By: fratrik

CC: aran, epriestley, fratrik

Differential Revision: 857
2011-08-25 18:13:53 -07:00
epriestley
58c09ab36d Improve Arcanist Mercurial support
Summary:
  - Build the manifest of file changes so unit and lint workflows work.
  - Default to creating a diff between the parent of the first outgoing change
and the tip.

Test Plan:
  - Ran "arc diff" in a dirty mercurial repo, got warned about
untracked/uncommitted changes.
  - Ran "arc diff" in a clean mercurial repo, got a diff of everything I'd done
locally.

Reviewed By: aran
Reviewers: Makinde, aran, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 796
2011-08-09 19:16:36 -07:00
epriestley
268de6428c Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.

This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.

Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:22:58 -07:00
epriestley
40b445b387 Fix a variable usage
Summary: This stopped being available in scope when I refactored this
recentlyish.
Test Plan: Got error, saw useful message.
Reviewed By: jungejason
Reviewers: mgummelt, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 787
2011-08-07 16:04:20 -07:00
Edward Speyer
344fbb8d35 Fix documentation: stop __init__ rendering in bold
Summary:
Requires
https://secure.phabricator.com/rPHU0acf708b9b0fdbf59e4399f14dd8295b6a96972c
from libphutil, which allows a backslash prefix to control escape
sequences such as bold, underline, and invert.

Test Plan: "arc help liberate"
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: 767
2011-08-04 14:09:13 +01:00
epriestley
57ff80e6c4 Provide "arc upload --json"
Summary:
  - Provide a "--json" flag for "arc upload"
  - Unify some of the stderr stuff across upload/download/paste.

Test Plan:
  - Ran "arc upload" and "arc upload --json", piped stderr away with 2>/dev/null
to verify only JSON got emitted to stdout
  - Ran "arc paste"

Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 749
2011-08-01 17:44:43 -07:00