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

100 commits

Author SHA1 Message Date
epriestley
71afde1988 Upgrade arcanist to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet because they aren't necessary for libraries to function.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2589
2012-05-30 14:22:59 -07:00
Nick Harper
c51590dc1e Provide api to get committed data in a hook
Summary:
We have a linter that needs to read data from the repo that isn't in the commit,
so that data isn't part of the svn transaction, thus not available in svnlook
cat --transaction. This change provides a method to get data from a committed
file.

Test Plan:
Successfully ran arc svn-hook-pre-commit with a linter that needs something
not in the transaction.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2582
2012-05-25 15:28:45 -07:00
Jason Ge
ed7034ab38 Fix arc ArcanistSvnHookPreCommitWorkflow
Summary:
there are two problems making the ArcanistSvnHookPreCommitWorkflow not working.

  - pathExists($path) will always return false because it hasn't loaded the path yet. Because of this, PhutilLintEngine will unset the path at https://secure.phabricator.com/diffusion/ARC/browse/master/src/lint/engine/phutil/PhutilLintEngine.php;032b9b30b0721c3f$46, so ArcanistSvnHookPreCommitWorkflow will have no path to lint against and never detects a problem.
  - In ArcanistSubversionHookAPI::getCurrentFileData(), the $path is already the full path in svnroot (path got from https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php;032b9b30b0721c3f$72). Prepending the project root to it will make the file path to be wrong so that the file content is empty. Again, ArcanistSvnHookPreCommitWorkflow never detects a problem.

Test Plan:
changed --transaction to --revision in related code and manually ran the following command. It detected the syntaxt error correctly. 558937 is a revision with syntax error.

/usr/local/bin/php -f /home/jungejason/nfs_devtools/arcanist/bin/arc svn-hook-pre-commit     --load-phutil-library=/tmp/testwww/www/lib/arcanist --load-phutil-library=/home/jungejason/nfs_devtools/facebook/src /svnroot 558937 2>&1

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, Girish, Koolvin

Differential Revision: https://secure.phabricator.com/D2485
2012-05-23 16:29:27 -07:00
epriestley
04606d2833 Improve default fill of Differential messages from commits
Summary:
  - When you run "arc diff" in Mercurial, we currently give you an empty template. Instead, prefill a likely template by parsing messages, as we do for Git.
  - Unify Git and Mercurial logic for acquiring messages, since `getLocalCommitInformation()` now provides this information. This should improve Git performance, and allows us to delete some code.
  - Merge messages more intelligently. Currently, we just overwrite fields. Instead, merge arrays (like ccs, reviewers, tasks) and concatenate strings (like summary and test plan). We need to special case "title", see inline.
  - @csilvers: this is a precursor to implementing "--reviewers", etc.

Test Plan: See next post, test plan includes giant output.

Reviewers: btrahan, csilvers, Makinde, jungejason, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2536
2012-05-22 11:08:57 -07:00
Jamie Wong
92819458b8 Make arc diff work with multiple heads in hg 2012-05-17 13:07:53 -04:00
Jamie Wong
b8fb074dcf Make arc diff not crash on hg bookmarks 2012-05-16 13:24:30 -04:00
epriestley
a31fc544c8 Fix "local commits" for Git and Mercurial to align with user expectations, and fix a parsing bug
Summary:
  - "git log" still includes "\n", so we're currently generating nonsense hashes like "\nabd9879bab86ad78ab...".
  - Correct the log range we use in Git. See comment. When users perform merges, their expectations about what the "included commits" and what the included changes are are different. Represent them with two different ranges.
  - Same deal for Mercurial

Test Plan: Ran "arc which" in various contexts.

Reviewers: btrahan, aurelijus, Makinde

Reviewed By: Makinde

CC: aran, nh, jungejason

Maniphest Tasks: T873

Differential Revision: https://secure.phabricator.com/D2460
2012-05-11 13:52:14 -07:00
epriestley
e50ea62271 Make "arc which" really really really verbose
Summary: Provide far more information about what "arc diff" intends to do.

Test Plan: Ran "arc which" in a variety of circumstances.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran

Maniphest Tasks: T1183

Differential Revision: https://secure.phabricator.com/D2454
2012-05-11 06:07:33 -07:00
epriestley
53161a1b84 Ship complete commit messages to Phabricator from Arcanist
Summary: Currently, we ship only the summary, but we need to ship the whole thing for T1189.

Test Plan: Added var_dump() + die, ran in git and hg working copies, verified 'message' included the whole message.

Reviewers: csilvers, btrahan, Makinde

Reviewed By: csilvers

CC: aran

Maniphest Tasks: T1189

Differential Revision: https://secure.phabricator.com/D2437
2012-05-09 15:56:16 -07:00
epriestley
5c684594d4 Allow 'arc' to run without '.arcconfig'
Summary:
This is mostly an onboarding thing, but also allows "arc upload", "arc download", and "arc paste" to work anywhere on the system.

  - Try to read the Phabricator install URI from arc global config if we can't find ".arcconfig".
  - Build a WorkingCopy anyway if we can't find ".arcconfig", as long as we can find ".svn", ".git", or ".hg".
  - Make all the workflows handle "no project ID" at least somewhat gracefully.

Test Plan:
  - Ran "arc diff" in .arcconfig-less Mercurial, Git, and Subversion working copies.
  - Ran "arc upload" and "arc download" from my desktop.
  - Ran "arc paste" from somewhere random.
  - Cleared my config and hit the error, got useful instructions.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2424
2012-05-07 15:24:58 -07:00
Nick Harper
6fcd2db646 Always use git.default-relative-commit, if present
Summary:
Always use the value in git.default-relative-commit when getting the relative
commit in git.

Test Plan:
Ran arc diff in a repo with git.default-relative-commit set to HEAD^ on a
branch tracking a remote (that is different from HEAD^), and checked that
the diff against HEAD^, not the remote, was published.

Reviewers: jungejason, epriestley, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2409
2012-05-07 09:54:47 -07:00
epriestley
b2dc11940f Clean up some "arc" edge cases in Mercurial
Summary:
  - We no longer need color options since we fake our way through parsing ANSI colorized diffs and use HGPLAIN (on Windows, too!). Drop 'em.
  - In the case where you have nothing outgoing, we don't cache the relative commit and thus run "hg outgoing" too many times, which is fairly slow (even if you have nothing outgoing). Cache it.

Test Plan: Ran "arc diff --trace" in a mercurial working copy with nothing outgoing; verified we run "hg outgoing" only once.

Reviewers: Makinde, csilvers, btrahan

Reviewed By: Makinde

CC: aran

Differential Revision: https://secure.phabricator.com/D2399
2012-05-04 15:46:10 -07:00
epriestley
52e08cc6c5 Fix "HGPLAIN" environmental variable in Windows
Summary: In Windows, you can't use `X=y cmd` syntax to set variables. Use "set X=y & cmd" instead.

Test Plan:
  - Ran "arc diff" in a Mercurial repo in Windows, created D2367.
  - Verified this does //not// cause 'HGPLAIN' to be set in the outer shell (where you type "arc diff").

Reviewers: Makinde, tido, indiefan, btrahan

Reviewed By: tido

CC: aran

Maniphest Tasks: T1179

Differential Revision: https://secure.phabricator.com/D2368
2012-05-02 12:40:02 -07:00
Marke Hallowell
9a718f210a Using a temp file for git commit --amend instead of passing multi-line message to the shell. Avoids escaping issue on windows.
Test Plan: Run arc diff locally, verify via git log that the commit is amended afterwards (using the mutable history paradigm)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2369
2012-05-02 12:39:46 -07:00
epriestley
c0c3c8a113 Remove "--no-decorate"
Summary: Apparently my advice here was terrible, and `--no-decorate` and `--decorate=no` are both very recent additions to Git which a bunch of users don't have. Get rid of them since D2344 allows us to parse all decorate levels anyway.

Test Plan: Tried to google "git changelog", got a bunch of pages about managing changelogs with git.

Reviewers: zeeg, ehren

Reviewed By: ehren

CC: ehren, aran, NorthIsUp

Differential Revision: https://secure.phabricator.com/D2354
2012-04-30 17:36:50 -07:00
epriestley
7070d0a065 Show text of moved files in Differential
Summary:
"git diff -M -C" generates useful metadata (moves/copies) but (for a pure move) no diff text. Synthetically build the diff text after the fact so this information is available in Differential.

This patch is kind of nasty but I couldn't see a cleaner way to do it. :/

This also needs some UI changes in Differential: we get a full-green new file right now, but it would be better to default-hide it with "This file was moved. Show More" or similar.

Test Plan: Moved a file, ran "arc diff", got textual diff.

Reviewers: aran, tuomaspelkonen, jungejason, btrahan, vrana

Reviewed By: vrana

CC: aran, epriestley, vrana

Maniphest Tasks: T230

Differential Revision: https://secure.phabricator.com/D479
2012-04-30 16:47:12 -07:00
Ehren Kret
b32b868a61 add support for using Arcanist with git's log.decorate setting enabled
Summary:
Arcanist fails to find git's 'commit <hash>' header when log.decorate is
set in one of git's config files. git adds the named refs that point to
the commit in parentheses following the hash. This changes the regex
used by arcanist to match git commit headers to optionally match the
branch names in parentheses following the hash.

Test Plan:
Run `git config --global log.decorate short` and check that `arc diff`
runs successfully.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2344
2012-04-30 14:14:23 -07:00
epriestley
9c4c1de512 Autocomplete branches to "arc land"
Summary:
  - Add branch name tab completion to "arc land".
  - Default to landing the current branch.
  - This is a little bit hacky but not too terrible. I'm planning to move the whole thing to PhutilArgumentParser at some point so that'll be an opportunity for a big refactor.

Test Plan: Hit tab, landed this branch.

Reviewers: zeeg, btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, kdeggelman

Differential Revision: https://secure.phabricator.com/D2293
2012-04-23 14:09:29 -07:00
epriestley
ea0f737e85 Drop <project + branch> herusitic from Git
Summary: See rage in T1117. Don't use the <project + branch> heuristic anymore..

Test Plan: Ran "arc diff --strict HEAD^" on a commit stacked on top of this one, got no matches.

Reviewers: btrahan, vrana, simpkins, beng

Reviewed By: btrahan

CC: aran, avive

Maniphest Tasks: T1117

Differential Revision: https://secure.phabricator.com/D2221
2012-04-13 10:55:28 -07:00
epriestley
56cdc31426 Fix some --only / --preview / SVN issues with Arcanist
Summary:
  - Historically, "--preview" was forbidden under SVN. No reason for that now.
  - The "--auto" patch moved the "--preview" / "--only" checks later than they should be.
  - Fix an issue with Conduit query construction in SVN.

Test Plan: Ran "arc diff --preview" in an SVN working copy. Ran "arc diff" in an SVN working copy.

Reviewers: svemir, btrahan, vrana, jungejason

Reviewed By: svemir

CC: aran

Differential Revision: https://secure.phabricator.com/D2218
2012-04-12 10:32:54 -07:00
Git user
83ad377bdb Read default relative commit from scratch file with newline
Summary:
When people create the .arc/default-relative-commit scratchfile with $EDITOR of
choice, their editor usually puts a newline at the end, which breaks arc diff.
We should trim the newline before using the contents of the scratchfile.

Test Plan:
ran arc diff in a working copy that contained a .arc/default-relative-commit
with a newline

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2209
2012-04-11 17:44:57 -07:00
epriestley
d2915d8c5f Don't let "arc which" identify the working copy as belonging to a different project
Summary: If you have two projects (say, libphutil and arcanist) and you prepare a patch for one of them on branch "master", run "arc diff", and then prepare a patch for the other one on the same branch, "arc diff" will try to update the first revision when you run it. Instead, make it smart enough to stay within arc projects.

Test Plan: Ran "arc which" in circumstances where it previously generated a false positive, no false positive.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1100

Differential Revision: https://secure.phabricator.com/D2199
2012-04-10 16:06:57 -07:00
epriestley
7ea51b6bb7 If a Git upstream is configured for the current branch, always use that as the default relative commit
Summary: See discussion in D1861.

Test Plan: Ran "arc diff" on master, got an upstream-based relative commit. Ran "arc diff" on a feature branch, got a config-based relative commit. Ran "arc diff x", got an argument-based relative commit.

Reviewers: btrahan, vrana, davidreuss, elgenie

Reviewed By: davidreuss

CC: aran

Differential Revision: https://secure.phabricator.com/D2192
2012-04-10 15:33:31 -07:00
epriestley
bd7dc8abaa Allow configuration of a weak per-project default relative commit
Summary: Allows you to set a default in .arcconfig. This default is overriden by any .arc/ setting.

Test Plan: Ran "arc diff" with a .arcconfig setting but no .arc/ setting, got a diff against the specified relative commit.

Reviewers: nh, btrahan

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D2093
2012-04-03 16:06:43 -07:00
epriestley
39b3778287 Revenge of the git relative commit default
Summary:
The default of "arc diff" to "arc diff HEAD^" in git is universally confusing to everyone not at Facebook.

Drive the default with configuration instead. Even at Facebook, "origin/trunk" (or whatever) is probably a better default than "HEAD^".

See D863 for the last attempt at this.

NOTE: This is contentious!

Test Plan: Ran "arc diff", got prompted to set a default. Ran "arc diff" from a zero-commit repo, got sensible behavior

Reviewers: btrahan, vrana, nh, jungejason, tuomaspelkonen

Reviewed By: btrahan

CC: aran, epriestley, zeeg, davidreuss

Maniphest Tasks: T651

Differential Revision: https://secure.phabricator.com/D1861
2012-04-02 16:52:20 -07:00
epriestley
4ca58d1129 Improve compatibility of "arc patch" on Windows
Summary: Can't use (cd ...) on windows.

Test Plan: Ran "arc patch" successfully.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124, T1034

Differential Revision: https://secure.phabricator.com/D1987
2012-03-25 09:32:20 -07:00
epriestley
3bff9417e9 Set relative commit early in "arc diff"
Summary:
In Mercurial, we figure out if the working copy is dirty with "hg diff", and do a somewhat expensive merge-base / outgoing operation if the relative commit isn't set. We can set the relative commit earlier and avoid some extra work.

We also may incorrectly cache this state (diff from merge-base of outgoing to tip) and pass the wrong rev and file dirty list to the linters.

Test Plan: Made commits which changed (A, B) and then (A). Ran "arc diff tip^". Before this change, observed full outgoing + merge base resolve and both "A" and "B" passed to lint. Observed execution of fewer commands and lint executing against "A" only after this change.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1998
2012-03-22 17:22:52 -07:00
epriestley
e8accf4b9f Instead of looping forever, resolve the relative commit
Summary: Obvious error in refactoring code around getCanonicalRevisionName() in D1954.

Test Plan: Ran "arc diff" without looping. Can you verify this fixes your case?

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T1025

Differential Revision: https://secure.phabricator.com/D1970
2012-03-21 11:07:15 -07:00
epriestley
31a54d9b4a When a mercurial working copy has uncommitted changes, prompt to include them in "arc diff"
Summary:
See task. Allow mercurial users to diff with uncommitted changes.

  - By default, commit range is merge-base of `hg outgoing` to `.` (dirstate).
  - You can get JUST dirstate with `arc diff tip` or similar.
  - This ended up being a giant mess various other changes to deal with empty `hg outgoing` and empty dirstate.

Test Plan: Diffed with uncommitted changes, got sensible prompts and results.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T998

Differential Revision: https://secure.phabricator.com/D1954
2012-03-19 19:17:10 -07:00
epriestley
89753d5d49 Don't fatal without Conduit for "--patch" and "--bundle"
Summary:
In some workflows, we don't have Conduit at all, so we'll fail with a raw exception. Test for conduit presence before trying to make the encoding call.

Also move some "instanceof" logic for updates into RepositoryAPI (factoring, windows compat).

Test Plan: Ran "arc patch --patch some.patch".

Reviewers: 20after4, davidreuss, nh, btrahan

Reviewed By: 20after4

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1935
2012-03-16 13:40:33 -07:00
Nick Harper
7494a95c40 [svn1.7] handle removed directories in svn in arc diff
Summary:
`svn rm $directory` removes the directory and its contents in svn 1.7, whereas
svn 1.6 only removes the directory's contents, so arc diff needs to not try
to read the directory.

Test Plan:
ran arc diff in a svn working copy that had a directory removed (with both
svn 1.6 and svn 1.7) and confirmed that the command did not throw an exception,
and that the removed directory was marked as directory (not a file).

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1889
2012-03-13 23:45:37 -07:00
Edward Speyer
454a7de87e ArcanistRepositoryAPI: add getCanonicalRevisionName
Summary:
Resolves crazy stuff like 'HEAD' or '{2001-01-01T01:01:01}' into an
actual revision, with a provided implementation for git.

Test Plan: Tested in a hacky script.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1849
2012-03-12 16:12:01 -07:00
Nick Harper
36709ece41 Improve arc support for svn 1.7
Summary:
svn 1.7 no longer has a .svn directory in every subdirectory of the working
copy, only one in the root of the working copy (like git, except you can still
check out subtrees). Thus, we can't check whether we're in an svn repo by
looking for a .svn directory alongside the .arcconfig file.

Test Plan: ran arc diff in an svn 1.7 working copy where it previously wasn't working

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1848
2012-03-09 13:51:52 -08:00
Bob Trahan
276b013d87 Arc patch - upgrade "same base rev" check to "does this commit exist in the working copy?" check
Summary: good title

Test Plan: ran "arc patch DX" a bunch

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T892

Differential Revision: https://secure.phabricator.com/D1789
2012-03-07 13:02:53 -08:00
epriestley
81019a790e Improve ArcanistMercurialParser's handling of unusual branch names
Summary:
See <https://github.com/facebook/phabricator/issues/97>. Mercurial branch names may contain any characters, but we currently reject branch names with spaces.

NOTE: Mercurial allows you to name branches things like `     ` (five spaces). We do not parse this correctly. Die in a well fire.

Test Plan:
  - Added some horrible-but-possible test cases for crazy branch names.
  - Unit tests now pass.

Reviewers: btrahan, Makinde, killermonk

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1795
2012-03-06 20:14:25 -08:00
epriestley
0e35dc64a9 Fix escaping of "git log ---format" command on Windows
Summary:
On Windows, the PHP function escapeshellarg() replaces '%' with ' ' (space). This is apparently because there is no safe way to escape % inside of strings.

cmd.exe does use "^" as an escape character, so I think replacing "xyz" with "^x^y^z" might work for arbitrary strings (maybe?), or at least some subset of strings, but I don't know cmd.exe well enough to make that call without being concerned I'm introducing a security issue.

Although this patch is dumb, it's certinaly safe, and can only do something wrong if the user has environmental variables like H, P, T, or x01, in which case they're sort of asking for it.

cmd.exe also truncates output on \0, so use \1 as a delimiter instead.

Seriously it's like this thing was written in 1982 and never ever changed.

Test Plan: Created D1783 successfully after applying this patch.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1785
2012-03-05 13:22:41 -08:00
epriestley
d7b10e4f44 Make git repo discovery work on Windows
Summary: This use of "(cd ..)" is outside of the RepositoryAPI proper (it's when we're trying to figure out which VCS a directory uses) and explodes on Windows.

Test Plan: @koolvin applied this patch manually and got farther than before.

Reviewers: btrahan, Koolvin, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1779
2012-03-05 13:22:04 -08:00
epriestley
075c4f84d4 Use execxLocal() for ArcanistGitAPI
Summary:
  - This is simpler and reuses more code than doing "(cd %s && ...)" every time.
  - If we have an issue like HGPLAIN, we have one place to fix it now.
  - On Windows, the construct "(cd %s && ...)" does not mean what we'd like it to.

Test Plan: Ran "arc diff" in a git repo with this change.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1761
2012-03-02 16:47:34 -08:00
epriestley
f9a7cb225e Fix "arc lint" when HEAD has been reverted in the working copy
Summary:
If a user reverts HEAD in the index or working copy, "git diff" shows no changes
and we explode.

Instead, we should simply return no changes; the rest of the lint pipeline
accommodates this correctly.

Test Plan: Reverted HEAD in a working copy, ran "arc lint", got an error.
Applied patch, ran "arc lint", got accurate lint.

Reviewers: Koolvin, btrahan

Reviewed By: Koolvin

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1729
2012-02-29 12:18:46 -08:00
epriestley
1f13e022cd Prefill template update message in Mercurial during --update
Summary:
When a user invokes the update flow from Mercurial, prefill the update message
like we do for Git.

Also add a little caching to improve performance since "hg" execs cost ~100ms.

Test Plan: Updated a Mercurial diff repeatedly, got sensible default message
fills.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T28

Differential Revision: https://secure.phabricator.com/D1718
2012-02-28 16:59:40 -08:00
epriestley
bb4616cd7c Use HGPLAIN when executing mercurial commands
Summary: See comments. Skip [defaults] in .hgrc when executing commands.

Test Plan: Ran "arc diff" in a mercurial working copy.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T922

Differential Revision: https://secure.phabricator.com/D1707
2012-02-28 16:56:57 -08:00
epriestley
8fe38f8b6d Finalize Arcanist Classes
Summary:
Mark all applicable Arcanist classes as "final", except PhutilLintEngine, which
needs a little finesse.

@jungejason / @nh, does this break any Facebook stuff?

Test Plan: Linter no longer raises warnings. Ran "testEverythingImplemented" in
Phabricator.

Reviewers: btrahan, jungejason, nh

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1519
2012-01-31 12:07:05 -08:00
epriestley
3ee01bacac Add 'arc which', and
ArcanistRepositoryAPI->loadWorkingCopyDifferentialRevisions()

Summary:
  - See T787.
  - @cpiro has an immediate use case for this, which is ##arc amend --revision
`arc which --id` --show master## for "git merge --autosquash" or similar.
  - For T614, we need this to choose "--create" vs "--update".
  - Other workflows should also use this to improve how often we automatically
get things right, particularly in Mercurial and SVN.

Test Plan:
Ran "arc which" in SVN, Git and HG working copies with various flags;
results seemed reasonable.

Reviewers: btrahan, cpiro, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T787

Differential Revision: https://secure.phabricator.com/D1478
2012-01-24 09:47:53 -08:00
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
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
9249ede952 Approximate "git merge-base" in Mercurial
Summary: Find the relative commit by finding the first non-outgoing commit, so
we don't show changes caused by merges we've performed since the last time we
pushed.

Test Plan: Checked out two hg working copies, A and B. Made a change in A. Made
a change in B. Pushed B. Merged in A. Made another change in A. Ran "arc diff"
in A. Got only changes I made in A in the diff, not the change from B.

Reviewers: Makinde, btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1339
2012-01-06 15:10:16 -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
Chris Piro
52d14ef8e6 Use --allow-empty when git amending to HEAD
Summary: ... in case the head commit is empty. Empty commits are useful for
injecting an Arcanist commit message in a branch then sending the whole thing
off for review. As far as I know there is no situation in which an empty commit
would exist unintentionally and using ##--allow-empty## would suppress an error.

Test Plan: works fine for a branch I just sent for review

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1241
2011-12-20 13:21:58 -08:00