Summary: Currently, we don't expose these at top level, so you can't disable coverage if your coverage is explosively broken. Expose them as passthrough arguments.
Test Plan:
- Touched a file in `arc` which triggered unit tests.
- Without `xdebug` installed:
- Ran `arc diff --preview`, `arc diff --preview --no-coverage` (both fine).
- Ran `arc diff --preview --coverage`, got exception about coverage not being available.
- Installed `xdebug`.
- Ran `arc diff --preview`, got coverage.
- Ran `arc diff --preview --coverage`, got coverage.
- Ran `arc diff --preview --no-coverage`, no coverage.
Reviewers: indiefan, btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4745
Summary: `git pull` may fail in git-svn after rebasing (which is a side effect of dcommit).
Test Plan:
$ git svn rebase
$ git log trunk..master
$ git pull --ff-only; echo $?
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4716
Summary: Some people have 2GB+ untracked files in repo which significantly slows down this or even crashes it.
Test Plan: Added a debug output here and linted repo with untracked path.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, arudolph
Differential Revision: https://secure.phabricator.com/D4713
Summary: Fixes T2438. We currently escape everything with '@', but SVN rejects that for '.'
Test Plan:
Unit tests. Performed this commit:
$ svn st
M .
A x@123
$ arc commit --conduit-uri=http://local.aphront.com:8080/ --revision 53
Revision 'D53: asdf' has not been accepted. Commit this revision anyway?
[y/N] y
Committing 'D53: asdf'...
Sending .
Adding x@123
Transmitting file data .
Committed revision 37.
Done.
I grepped for more '@' adding but couldn't find any. It's a bit tricky to grep for though, so it's possible I missed some.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T2438
Differential Revision: https://secure.phabricator.com/D4703
Summary:
arc land on a hg-svn repository would fail because arc land
uses 'hg push -r' to specify which revs to push which is not supported
by hg-svn. Now we just use 'hg push' which, when used against svn, only
pushes the current branch (which happens to be the branch we're trying to land).
We can't use standard 'hg push' for non-svn repos though because when used
against a vanilla hg repo 'hg push' pushes all branches.
Also remove --new-branch from 'hg push' because it's extremely
unlikely that a person wants to create a new branch on the server via
arc land.
Test Plan:
Ran arc land on a normal hg repo, verified it used 'hg push -r'.
Ran arc land on a hg-svn repo, verified it used 'hg push' and it pushed the
correct changes.
Reviewers: epriestley, sid0, dschleimer
Reviewed By: epriestley
CC: bos, aran, Korvin
Maniphest Tasks: T2403
Differential Revision: https://secure.phabricator.com/D4653
Summary:
The cache key is repository base revision and hash of all modified files.
It isn't perfect in SVN where every file might have a different revision.
It's also suboptimal as just committing or amending changes the cache key even if the files contents weren't modified.
We can improve it later, perhaps by using previous revision and files modified since it.
Test Plan:
Changed granularity of XHPAST linter to repository.
Linted the same repo twice, verified that it was read from cache the second file.
Changed a single file, verified that all files were re-linted.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4608
Summary: The main reason for this is to not exit with 1 when no paths are lintable (which is more success than failure).
Test Plan:
Returned empty array from `buildLinters()`, then:
$ arc lint
$ echo $? # 0
Reviewers: epriestley
Reviewed By: epriestley
CC: wez, aran, Korvin
Differential Revision: https://secure.phabricator.com/D4625
Summary: Currently, we get an exception on empty %Ls for `arc diff --no-ansi` or similar (see P698).
Test Plan: Ran `arc diff --no-ansi`.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4624
Summary:
Previously, trying to arc land in a mercurial repo would
fail if the local branch was already at the tip of the onto branch
(since hg rebase exited with code 1). This change makes it check
if a rebase is needed before executing the rebase.
Test Plan:
hg init foo
cd foo
hg bookmark master
touch a && hg add a && hg commit -ma
// setup your .arcconfig
cd ..
hg clone foo foo2
cd foo2
hg bookmark mybook
touch b && hg add b && hg commit -mb
arc land --onto master --revision <your rev number>
Arc land should succeed. I also tried landing when a rebase was
necessary and it still worked.
Reviewers: epriestley, dschleimer, bos
Reviewed By: epriestley
CC: sid0, aran, Korvin
Differential Revision: https://secure.phabricator.com/D4588
Summary:
A git submodule looks a lot like a normal git repo, but the .git
directory is replaced with a file that git reads to find the real
location of the git directory. When arcanist tries to write a file into
a directory inside of there, it was failing silently, and then crashing
silently when it couldn't read results back out. Instead of assuming the
git directory is a directory named .git at the toplevel of the tree, we
use the appropriate git command to get the correct git directory.
Test Plan: submit a diff from a submodule
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4482
Summary:
If linter with file cache granularity stops linting then we don't run it on the same path next time.
But we still run linters with non-file cache granularity causing that they run even on the paths that would be stopped otherwise.
Test Plan:
Put global granularity linter after Generated linter.
Caused an error from this linter but in ignored path.
Verified that 'stopped' is saved in `lint-cache.json`.
Linted the same files second time, verified that the path is still skipped (wasn't before).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4446
Summary: Add a comma because it was going to annoy the crap out of me.
Test Plan: See the comma. :)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4365
Summary: We want to use TYPE_DIFF_DIDBUILDMESSAGE to abort arc diff when a message doesn't fit some
Test Plan: Created an EventListener subscribed to TYPE_DIFF_DIDBUILDMESSAGE, validated the 'message' field was filled in
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D4361
Summary:
Currently, this spawns 125 concurrent processes on my machine, which overflows some limit and gives me an error:
PHP Warning: proc_open(): unable to create pipe Too many open files in /INSECURE/devtools/libphutil/src/future/exec/ExecFuture.php on line 491
Instead, limit parallelism to 16. The runtime is approximately the same for me, and dominated by other concerns (conduit calls).
Test Plan: Ran `arc branch` successfully. Ran `arc branch --trace`, observed behavior.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4336
Summary: see title
Test Plan: ran arc install-certificate <uri> in a directory without an .arcconfig and it worked! ran arc install-certificate in a directory with an .arcconfig and it worked! ran arc install-certificate <uri> in a directory with an .arcconfig and noted it correctly overrode the .arcconfig
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2251
Differential Revision: https://secure.phabricator.com/D4332
Summary:
Fixes T2138.
- When a pull fails, restore the original branch.
- When a push fails, complain about it really loudly.
NOTE: No test plan for push yet since I'm not sure this is the right remedy, see T2138 for discsusion.
Test Plan:
- Tested pull by changing "git pull" to "git xxpull" and running "arc land". Saw the pull fail and my original branch restored.
Reviewers: vrana, aran
Reviewed By: vrana
Maniphest Tasks: T2138
Differential Revision: https://secure.phabricator.com/D4265
Summary: If you are explicit then there is no need to ask you.
Test Plan:
$ touch a
$ arc diff
$ arc diff a
$ arc diff existing
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4186
Summary:
Recently, in D4097 or one of the precursors I refactored this. However, when $rev is null parseBaseCommitArgument() throws ("This VCS does not support commit ranges."). Shield the call so it only happens if if $rev is nonempty (we still want to make the call, so "arc lint --rev x" on SVN will throw and inform the user that "--rev" is incorrect usage).
(@vrana, this was reported by FB and might be worth pushing.)
Test Plan: Ran "arc diff --preview <path>". Grepped for other parseBaseCommitArgument() callsites and verified they don't have similar issues.
Reviewers: vrana, btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4241
Summary: I don't know how to not be strict here plus we (Arcanist developers) don't have access to user's error log.
Test Plan:
Undeclared `ArcanistDiffWorkflow::$console`, then:
$ arc diff
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3607
Summary:
The main added value is loading the branch name from revision.
Sometimes I know the revision ID but I don't know the branch name.
The missing piece is the starting point of the branch.
I was thinking about using `arc.land.onto.default` but we also need to get 'origin'.
This is also the last step of a simple workflow where underlying VCS is not abstracted away.
In future, we can implement this for other APIs.
Test Plan:
$ arc branch new_branch
$ arc branch new_branch
$ arc branch D4168
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4170
Summary: After D4191 this is a fatal.
Test Plan: Created this revision.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4219
Summary: Adds "arc unit --everything", which runs every available test, provided the test engine supports it. Also add JSON output.
Test Plan: Ran `arc unit --everything` in arcanist/, libphutil/ and phabricator/. Saw all tests run.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2065
Differential Revision: https://secure.phabricator.com/D4214
Summary:
See D4049, D4096.
- Move commit range storage from Mercurial and Git APIs to the base API.
- Move caching up to the base level.
- Store symbolic name and resolved name separately, so we can re-resolve the correct commit from the symbolic name after dirtying caches.
- Rename `supportsRelativeLocalCommit()` to `supportsCommitRanges()` (old name wasn't very good, and not consistent with new terminology like the `--base` flag).
- Rename `getRelativeCommit()` and `setRelativeCommit()` to `getBaseCommit()` and `setBaseCommit()`.
- Introduce `reloadCommitRange()` and call it from `reloadWorkingCopy()`.
I think this fixes the problem in D4049, and provides a general solution for the class of problems we're running into here, with D4096. Specifically:
- We no longer get dirty caches, as long as you call reloadWorkingCopy() after changing the working copy (or call a method which calls it for you).
- We no longer get order-of-parsing-things problems, because setBaseCommit() reloads the appropriate caches.
- We no longer get nasty effects from calling `requireCleanWorkingCopy()` too early.
Test Plan: This is pretty far-reaching and hard to test. Unit tests; ran various arc commands. :/
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4097
Summary:
This method is used in three cases:
# For unit tests, to set the range to 'HEAD^' or '.^' in an agnostic way.
# For "amend", to set the range to the commit to be amended (also 'HEAD^' or '.^').
# For "patch" and "upgrade" so we don't fail just because there's an invalid "base" rule somewhere in the config when doing clean-working-copy tests.
For cases (1) and (2), introduce an "arc:this" rule to mean "the current commit". For case (3), remove the call; it is no longer necessary to check the commit range in order to do tests for the working copy state after D4095.
Test Plan: Ran unit tests, "arc upgrade", "arc patch", "arc amend".
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4096
Summary:
This allows using new methods without the need for bumping version number.
The usage is not neccessary because we already bumped the version number for this but I wanted to have a callsite.
Test Plan:
Made a typo in method name, then:
$ arc lint --only-new 1
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4193
Summary:
This changes arc land's hg support to allow you to land a branch or bookmark that has nested branches/bookmarks. Example:
// Initial state:
// -a--------b master
// \
// w--x mybranch
// \--y subbranch1
// \--z subbranch2
//
// arc land --branch mybranch --onto master :
// -a--b--wx master
// \--y subbranch1
// \--z subbranch2
Test Plan:
Created several repos like in the summary and ran 'arc land' and 'arc land --keep-branch'. Did this with both bookmarks and named branches. Scenarios tested:
- mybranch having no child commits
- mybranch having a child branch with several commits
- mybranch having two child branches
- mybranch having a child branch which has two more child branches
No code was added outside of a "if ($this->isHg)" so I didn't run git arc land.
Reviewers: epriestley, dschleimer, bos, sid0
Reviewed By: dschleimer
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4089
Summary:
See chatlog from https://secure.phabricator.com/chatlog/channel/%23phabricator/?at=37257
Basically, the install's developers sometimes use "git merge master" instead of "git rebase master" to pull changes from master into their branch. When we run "git rebase master" at the end, this creates a lot of conflicts.
Instead, optionally run "git merge master". This should be equivalent in our case (where we always rebase) and much better in their case (where they sometimes merge).
We're both going to use it for a bit and see if it creates problems. If it improves the "sometimes-merge" workflow without affecting the "always rebase" workflow, we can make it a default. If it improves theirs but damages ours, we can keep it a flag/option. If it's just terrible, we can figure out something else.
Test Plan:
Ran `arc land --mergeup --trace <feature> --keep-branch --hold`, see P624 for output. Observed merge update strategy and general success.
Also verified the conflict:
$ arc land --mergeup --merge
Usage Exception: Arguments '--mergeup' and '--merge' are mutually exclusive: The --merge strategy does not update the feature branch.
Reviewers: btrahan, vrana, DurhamGoode
Reviewed By: btrahan
CC: aran, keir
Differential Revision: https://secure.phabricator.com/D4080
Test Plan: Added a debug output there and ran.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4147
Summary:
arc browse will open a browser to the Diffusion view of a file. Convenient if you like
Diffusion for reading source. Naturally, it fixes relative filenames.
Combined with git-grep it can be an easy replacement for server-side search functions.
Test Plan:
use feature with and without 'browser' configured.
I've only tested this on Linux, because that's all I have right now, but the principle is sound.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4127
Test Plan: Will test it by closing this revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4122
Summary:
Fixes the exception:
Exception
Command 'git commit -a --author=Whatever Long Name <whatever@email.com> -F -' failed with error #1
Test Plan: Tested with full git name
Reviewers: epriestley, aurelijus
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3979
Summary:
Allow authors to publish a new or updated revision to Phabricator
without requesting code reviews. The revision will have status
"Needs Revision" instead of "Needs Review".
In order to avoid a change of Conduit API, this is done by adding
a comment with the "plan changes" action immediately after the
revision is published.
Test Plan:
Using my local repository, run "./bin/arc diff --plan-changes"
Check the resulting diff in Phabricator.
Reviewers: vrana
Reviewed By: vrana
CC: aran, epriestley
Maniphest Tasks: T2024
Differential Revision: https://secure.phabricator.com/D4084
Summary:
If you run `arc diff` in a repository which:
- has uncommitted or untracked changes; and
- has a .arcconfig with a never-before-seen project ID;
- we fatal: http://pastebin.com/raw.php?i=ykpfr4MT
This patch is a bit iffy, open to alternatives. The "right" patch is probably an `arcanistproject.query` which behaves more sensibly.
I return array() directly since we'll later create the project.
Test Plan: Ran `arc diff` in a repository with untracked files or uncommitted changes and a new project ID.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4057
Summary: Should have been part of D3934.
Test Plan:
$ arc diff
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4076
Summary:
Makes arc land support hg repositories. Both bookmarks and named branches can be landed. For the most part all the arc land options work, but there are a few caveats:
- bookmarks can only be landed on bookmarks
- branches can only be landed on branches
- landing a named branch with --merge creates a commit to close the branch before the merge.
- since mercurial doesn't start with a default master bookmark, landing a bookmark requires specifying --onto or setting arc.land.onto.default
Test Plan:
Tested arc land with all permutations of --merge, --keep-branch on both bookmark branches and named branches. Also tested --hold, --revision, --onto, --remote.
See https://secure.phabricator.com/P619
Also tested git arc land with --merge and --keep-branch.
Reviewers: dschleimer, sid0, epriestley, bos
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4068
Summary:
See discussion in D4056. In `findRevision()` we call `loadWorkingCopyDifferentialRevisions()`. However, this method depends upon the state of the working copy, because the end of the commit range it examines is HEAD. Prior to D4056 we checked out the target branch before calling `findRevision()`; after D4056 we call it earlier.
This isn't problematic in the `arc land` case, but in the `arc land <branch>` case it means we may fail to identify a revision, or identify the wrong revision, because HEAD isn't where we expect it to be.
Instead, unconditionally check out the target branch before finding the revision.
See <http://dl.dropbox.com/u/116385/Slingshot/Pictures/Screen%20Shot%202012-12-03%20at%203.43.45%20PM.png> for a transcript of the issue.
Test Plan: Reproduced issue as per link above. Ran `arc land --keep-branch --hold somebranch` successfully after this patch.
Reviewers: DurhamGoode, zeeg
Reviewed By: DurhamGoode
CC: aran
Differential Revision: https://secure.phabricator.com/D4072
Summary:
We can add `GRANULARITY_DIRECTORY` and `GRANULARITY_REPOSITORY` later.
Repository granularity may use current commit + changes.
Directory would need to use hashes of all files in dir which would be quite expensive.
Test Plan:
$ echo '<?php class A extends B {}' > A.php
$ arc lint --cache 1
$ arc lint --cache 1
$ echo '<?php class B {}' > B.php
$ arc lint --cache 1
$ arc lint --cache 1
$ rm B.php
$ arc lint --cache 1
$ arc lint --cache 1
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4021
Summary:
Refactor the arc land code into several
functions so it's easier to maintain. This is in
preparation for adding hg support to arc land
in my next commit.
Without this refactor, adding hg support makes the run()
function too big.
Test Plan:
Set up a git repo and clone with a branch scenario.
Example: https://secure.phabricator.com/P614
Ran and verified:
arc land
arc land --keep-branch
arc land --merge
arc land --merge --keep-branch
arc land --hold
arc land --revision <another phabricator rev>
Reviewers: epriestley
Reviewed By: epriestley
CC: dschleimer, sid0, aran, Korvin
Differential Revision: https://secure.phabricator.com/D4056
