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

189 commits

Author SHA1 Message Date
epriestley
ae66d4caa9 Use a temporary file to execute arc patch
Summary:
Piping data around on Windows doesn't work well when it contains zany characters like "null" and "newline". Fixes T3266.

Instead of piping data into `git apply`, write to a temporary file.

Test Plan:
Ran `arc patch`, got good results.

  >>> [17] <exec> $ git apply --index --reject -- '/private/var/folders/8k/c3vkmjy5335gcxdzxkhwq82w0000gn/T/7z9iea6srikoo0sc/4266-ZEyvz9'

Reviewers: btrahan, hach-que

Reviewed By: hach-que

CC: aran

Maniphest Tasks: T3266

Differential Revision: https://secure.phabricator.com/D6070
2013-05-30 21:03:21 -07:00
epriestley
592172c775 Remove "--allow-empty-message"
Summary: @alex has git 1.7.0.4 which doesn't have this flag. We don't actually need it: we always provide a commit message when calling this method. Remove the flag for compatibility, leaving a note in case we bump into this in the future.

Test Plan: N/A

Reviewers: btrahan, vrana, alex

Reviewed By: alex

CC: aran

Differential Revision: https://secure.phabricator.com/D5926
2013-05-14 13:35:42 -07:00
Howard Chan
3116d3656a Adding arc revert command[]
Summary:
Arc Revert does the following:
1. Git revert
2. Go to the differential of the rev you are reverting and either repoen it or set it to a reverted state
3. File a hipri task to orig author

[Preview] Creating Arc Revert workflow

Porting arc revert from FB4A to phabricator for general usage. This is my first stab,
so totally appreciate feedback and assistance. I'm currently focused
on making this work for git. However, I built out the functions through the GitAPI so this
could be easily extendable to Mercurial later on.

Stuck on the following (help):
1. Creating a task for FB internal. I tried building on top of existing arc listeners
but getting errors on failures to load the TaskCreator (and other) tasks.

2. I'm using a hacky way to grab the diff revision id from the newly created
revert diff. (see line 204) I'm looking for a way to just fetch the diff ID
from arc after the diff is created; is this possible?

Test Plan:
 -

1. Ran arc revert on a www and fbcode diff
2. Confirmed that revert was run on the diffs and a proper diff filed
 -

Reviewers: royw, sdwilsh, nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin, pti, keir

Maniphest Tasks: T1751

Differential Revision: https://secure.phabricator.com/D5553
2013-05-14 11:00:56 -07:00
Martin Kralik
3401b2a5d2 Added missing space between words in messages.
Summary:
During my attept to `arc land master` on `master` branch
I have discovered that error message is missing few spaces between words.

I have added them and used this ugly readonly command:

  pcregrep --include="\.php$" -M -r '(?<!\\n| )("|'"'"')\.\n\s*\1(?!\\n| )' ~/arc/arcanist/src

to detect other instances of this serious bug.

Two more were found.
This time they were probably introduced in order to abide
to the draconian lint rule about number of columns.

Since I want to be a good citizen,
I have added this missing space to the begining of the next line in both cases.
It is an ugly hack, but I think user should not suffer due to missing spaces.

Another solution could be preserving no leading spaces and splitting long lines.
Or just providing excuse to lint.

Test Plan: Ran `arc land master` on `master` branch.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3008

Differential Revision: https://secure.phabricator.com/D5827
2013-05-03 16:12:17 -07:00
durham
b3108661bb Enable lint amending commits in mercurial
Summary:
arc lint was hardcoded for git for amending commits with lint
patches. This enables the same functionality for mercurial.

Test Plan:
Made some changes that would result in a lint patch.
arc diff

Verify that the patches it produces were amended into the commit.

Verified it still works in git as well.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5716
2013-04-16 13:32:12 -07:00
durham
bfc1eeba07 Fix hg amend during arc diff
Summary:
When doing an arc diff with pending changes in your working copy
it was creating a new commit with the pending changes instead of amending
the existing one.  The problem was the author comparison was comparing
values like "John Smith <john@foo.com>" with "John Smith". The fix changes
$api->getAuthor() to return "John Smith" instead of the full string. This
matches the behavior (and implementation) found in the git api.

Test Plan:
hg book foo
touch a && hg commit -Ama
touch b && hg add b
arc diff
When prompted, amend the pending changes to the existing commit.

Verified that the changes were amended instead of making a new commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5706
2013-04-16 10:22:00 -07:00
durham
f5c8430188 Change hg arc diff to detect up to the nearest parent with a diff
Summary:
Changes arc diff to choose the base commit as the first ancestor
that has a diff.  So if your tree looks like master->A->B->C->D, if you
have a diff on B (which will include A), when you run arc diff on D it will
only include C and D.

This makes the scenario for stacked diffs nicer.  A user can commit A, commit B,
arc diff, commit C, commit D, arc diff, arc land B, arc land D.

Test Plan:
Commit A on top of master
Commit B on top of A
arc diff
Commit C on top of B
Commit D on top of C
arc diff

Verify the second diff contains the changes in C and D, but not A and B.

hg up B
arc land --preview
Verify that arc land shows A and B

hg up D
arc land --preview
Verify that arc land shows A, B, C, and D
(arc land should be unaffected by this change.
It always tries to land the entire branch)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5639
2013-04-12 14:11:51 -07:00
Nick Harper
b1fe436cfc [svn] Fix removing a binary file without svn:mime-type set
Summary:
If we're removing a binary file that didn't have svn:mime-type set properly,
we can't propset it (because the file doesn't exist locally). Instead, just
return a synthetic diff for the removed file.

Test Plan:
run arc diff in an svn working copy where I ran svn rm on a binary file that
doesn't have svn:mime-type set, and the diff correctly gets uploaded to
phabricator instead of erroring when trying to set properties.

Reviewers: epriestley, wez

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5655
2013-04-10 15:22:58 -07:00
durham
7c22017562 Allow 'arc diff .^' for hg repos
Summary:
Previously arc diff for hg only allowed bookmark names, rev numbers,
and commit hashes as the input base commit. This was because it escaped all
inputs and treated them as raw identifiers.

This change makes it treat the input as a revset if the escaped version fails.
This allows users to do things like "arc diff .^" when they only want to diff
the top commit.

Test Plan:
Created a stack of commits, master->A->B.
hg up B
arc diff .^
Verified the diff message only showed B as part of the diff.

arc diff .^~
Verified an error occurred ("Commit '.^~' is not a valid Mercurial commit id.")

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2888

Differential Revision: https://secure.phabricator.com/D5638
2013-04-09 17:00:34 -07:00
Jakub Vrana
dbe0f7dc09 Invalidate SVN status cache after adding files
Summary: Cache invalidation is really one of the two hardest computer science problems.

Test Plan:
  lang=sh
  # in Subversion repository
  $ arc diff .arcconfig # untracked

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan

Differential Revision: https://secure.phabricator.com/D5600
2013-04-06 09:25:35 -07:00
epriestley
ac0aa330f5 Don't use emailuser in Mercurial
Summary:
The `emailuser` template is a relatively recent addition to Mercurial, and a few users have complained about it. It also doesn't actually do what I thought it did, e.g. in an address like this:

  "Abraham Lincoln" <alincoln@whitehouse.gov>
   ^^^^^^^^^^^^^^^   ^^^^^^^^
         (1)            (2)

                     ^^^^^^^^^^^^^^^^^^^^^^^
                              (3)

...I want (1), but `emailuser` means (2). Instead, extract (1) with `getDisplayName()` and (3) with `getAddress()` using PhutilEmailAddress.

The implementation in Mercurial is not particularly sophisticated or magical (it just looks for "@" and "<") so we aren't really missing anything by doing this ourselves, at least today.

Also fix some issues in `arc export`, which literally no one uses, but which is occasionally useful for testing (as here).

Test Plan:
  - Ran `arc diff --only` in an `hg` repo, checked DB to see that name/email were correctly extracted.
  - Ran `arc export --git` in an `hg` repo, didn't get a long series of fatals.

Reviewers: btrahan, DurhamGoode

Reviewed By: DurhamGoode

CC: aran

Maniphest Tasks: T2866, T2858

Differential Revision: https://secure.phabricator.com/D5539
2013-04-02 14:06:46 -07:00
durham
ba78000b38 Make arc patch create a bookmark by default in hg
Summary:
Previously, arc patch would create a new commit under the existing
current bookmark in mercurial. There have been two discussions about what the
right behavior should be (D3334 and D3658). One side wants no commit at all,
and one side wants a commit under a new bookmark. The current implementation is
the worst of both worlds :(

This change makes it create a new bookmark at the revision's base before commiting,
same as the --bookmark flag used to do (which is now obsolete). That way the
existing bookmark doesn't move (in mercurial >=1.8). This is the same behavior
git has, which is convienent for groups migrating between the two.

Also makes hg's getCanonicalRevision handle svn revisions just like git. This way
arc patch will try to apply the patch to the appropriate revision in the history.

Test Plan:
Ran:
arc patch - Verified it created a new bookmark and commited on top of the
revision's base commit.

arc patch --nobranch - Verified it put the new commit on top of the current
bookmark without a new bookmark.

arc patch --nocommit - Verified it left all the changes in the working copy.

Also verified arc patch still works with git.

Reviewers: epriestley

Reviewed By: epriestley

CC: sid0, bos, dschleimer, aran, Korvin

Differential Revision: https://secure.phabricator.com/D5408
2013-03-21 17:52:07 -07:00
James Rhodes
2900ab6abd Added automatic stash / unstash support for Git in arc diff.
Summary:
- Added arc.autostash option to have this behaviour off by
default (but configurable on a per-project basis).
- Automatic stashing of changes now informs the user of how
to restore their working directory if Arcanist unexpectedly
terminates.
- Fixed an issue with finalizeWorkingCopy when the workflow
didn't require a clean working copy.

Test Plan:
Test `arc diff` when there are changes in the working
directory; by default it should tell you to stash or commit.
Turn on the arc.autostash option and try again; it should
automatically stash with a message on how to recover, and
it should restore the working directory automatically under
almost any circumstances (other than an unrecoverable error).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5385
2013-03-21 16:00:05 -07:00
vrana
91154c9cdf Ignore remote errors in parsing Mercurial log
Summary:
This probably indicates some none fatal error, e.g.:

> remote: Certificate invalid: name is not a listed principal

The best thing here would be to avoid the error but we shouldn't explode even if it is there.
I tried to mute the error from the output but didn't find a switch or config option to do it.

Test Plan:
  $ hg outgoing --branch default --style default

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5267
2013-03-07 11:14:49 -08:00
durham
4af7c865aa Use hg phases to detect outgoing when phases are supported
Summary:
There was an accidental ! in the phase vs outgoing condition
which caused it to use 'hg outgoing' when it should have used the draft()
phase.  Fixing this shaves 4.5 seconds off 'arc diff' on large repos.

Test Plan:
Ran arc diff --trace.  Noted that the draft() was used and that the diff
contained the correct files and commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: sid0, bos, dschleimer, aran, Korvin

Differential Revision: https://secure.phabricator.com/D5182
2013-03-01 14:10:25 -08:00
durham
8412f7cfd7 Improve hg arc diff perf when image files are uploaded
Summary:
arc diff called 'hg cat' twice for every image binary in the diff.
This turns out to take 1 second per call on a large repo because mercurial
has to parse the manifest every time.

Now arc diff batches up all the files and does only two 'hg cat'
commands.  This makes the cost constant relative to the number of
images being uploaded.

Test Plan:
Ran arc diff on a diff with 30 images on both git and hg.
Verified that it was fast and that the images showed up in the web ui.

Reviewers: epriestley

Reviewed By: epriestley

CC: sid0, dschleimer, bos, aran, Korvin

Differential Revision: https://secure.phabricator.com/D5144
2013-02-28 09:46:42 -08:00
durham
0cb1b7efad Fix arc lint on working copy changes in mercurial repos
Summary:
Previously, running arc lint on a set of changes that only
existed in your working copy threw an exception in mercurial repos.
It was trying to use the revset "...." (i.e. the range from . to .),
which didn't parse. Even if I fix that it still doesn't work because
getRawDiffText did not include the working copy changes (which it does
in git).  I removed the check so the function now acts the same as in
git and arc lint works on working copy changes. I've seen this error before
in other places so hopefully this change will also fix any other areas,
that depended on getRawDiffText working the same as git.

The logic I removed was added in D1954 to support diffing against
uncommited changes.  That workflow should be unchanged.  arc diff will
still prompt the user if there are uncommited changes, and the user can
still choose to abort or continue.

Let me know if I missed something important which makes this a bad idea.

Test Plan:
Edited a file in the working directory of a hg repo.
arc lint
Verified lint ran successfully.

Also ran arc diff and land with and without working copy changes to make sure
they still work.  I'd kill for some tests in this area...

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: dschleimer, bos, sid0, aran, Korvin

Maniphest Tasks: T1631

Differential Revision: https://secure.phabricator.com/D5130
2013-02-26 16:15:29 -08:00
epriestley
3e3ea378ed Remove hgsprintf() from arcanist
Summary: Moving to libphutil.

Test Plan: unit

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5058
2013-02-21 16:44:19 -08:00
epriestley
7a67173b97 Apply new whitespace lint rules to arcanist/
Summary: Automated changes from `arc lint`.

Test Plan: Looked them over.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5017
2013-02-19 14:09:20 -08:00
durham
eda3fc2ab4 Improve mercurial 'arc land' perf by avoiding updates
Summary:
Mercurial 'arc land --hold' was taking 90+ seconds on our large
repository.  Since most of arc land doesn't require any particular working
directory, I've changed the mercurial logic to avoid all updates except for
two: the one prior to finding the revision (only applies if the user specified
--branch), and the one at the end to leave the user in a good state.

Also got rid of a 'hg outgoing' call when phases are supported. Also changed
the hg-subversion detection to just look for .hg/svn instead of running 'hg
svn info', which was taking 4 seconds.

Now arc land takes about 50 seconds. Still much worse than git's 25 seconds.
One big hot spot is in the two 'hg rebase' calls, which account for 25 seconds
(versus 11 seconds of git).

Test Plan:
Tested arc land with mercurial and git. Tested with and without the --branch
options.

Reviewers: epriestley, bos, sid0, dschleimer

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5014
2013-02-19 13:36:02 -08:00
epriestley
1e612eebc3 Fix another SVN issue with "@" files
Summary: This chunk of code is kind of iffy and not really correct, but make it not fail, at least.

Test Plan:
  - Added a file named `swamp@2x.jpg` to a working copy.
  - Used `svn propedit svn:mime-type swamp@2x.jpg@` to incorrectly set its mime type to `text/plain`.
  - Ran `arc diff`.
  - Saw `arc diff` "correctly" change its mime-type to a binary mime type. This isn't really correct, but doing it successfully is better than throwing an exception.

Reviewers: mbishopim3, chad

Reviewed By: mbishopim3

CC: aran

Differential Revision: https://secure.phabricator.com/D4998
2013-02-18 08:24:47 -08:00
epriestley
aba9d49449 Fix "none" in SVN XML summary
Summary: The `svn diff --xml --summarize` command reports a bunch of item statuses, which may include "none" if you make property changes to a directory (this is fairly rare).

Test Plan: Created property changes, saw "none" status.

Reviewers: chad, mbishopim3

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D4978
2013-02-15 14:53:31 -08:00
epriestley
0f57b8d2de Fix various SVN escaping issues in arc patch
Summary: Followup to D4703. When we give paths to `svn`, we need to escape them if they contain an `@`.

Test Plan:
Created a patch full of modifications to files with `@` in their names, and applied it:

  $ arc patch --diff 192
  A         A@2xcopy2
  A         A@2xcopy
  D         A@2x
   OKAY  Successfully applied patch to the working copy.

Reviewers: chad, mbishopim3

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D4977
2013-02-15 14:53:25 -08:00
epriestley
39704f1e49 Fix "arc:this" and "arc:amended" base rules after D4838
Summary:
After D4383, we escape the base commit when constructing a command like this:

  hg log --rev (base::. - base)

However, if the base commit is a revset like ".^", we now escape it and Mercurial looks for a commit named ".^" (a valid mercurial branch name) instead.

Fix this by returning nodes for these rules instead of revsets. The "arc:this" rule is automatically used in some operations, like "arc amend", so users can hit this during normal workflows, not just with weird `--base` rules.

Test Plan: Ran "arc amend" in a Mercurial repository, didn't fatal out.

Reviewers: DurhamGoode, sid0

Reviewed By: DurhamGoode

CC: tido, aran

Differential Revision: https://secure.phabricator.com/D4949
2013-02-14 13:57:34 -08:00
vrana
299b9c4c6b Support arc bookmark in Mercurial
Summary:
Branch in Mercurial means something else.
Hopefully users wouldn't be too confused.

Test Plan:
  $ arc help
  $ arc help branch
  $ arc help feature
  $ arc feature
  $ arc bookmark
  $ arc branch
  # In hg repo:
  $ arc feature
  $ arc feature new
  $ arc feature new

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2332

Differential Revision: https://secure.phabricator.com/D4753
2013-02-13 13:58:07 -08:00
epriestley
446e5c4599 Apply rtrim() to arc:upstream
Summary: Git spits these out with \n at the end.

Test Plan:
```
>>> orbital ~/devtools/arcanist $ git branch --set-upstream trim master
Branch trim set up to track local branch master.
>>> orbital ~/devtools/arcanist $ arc which --base arc:upstream
RELATIVE COMMIT
If you run 'arc diff', changes between the commit:

    acf7600e6e  Temporarily restore apache/license linters

...and the current working copy state will be sent to Differential, because
it is the merge-base of the upstream of the current branch and HEAD, and
matched the rule 'arc:upstream' in your args 'base' configuration.

You can see the exact changes that will be sent by running this command:

    $ git diff acf7600e6e728395..HEAD

These commits will be included in the diff:

    3580555e4b30598f  WIP

MATCHING REVISIONS
These Differential revisions match the changes in this working copy:

    (No revisions match.)

Since there are no revisions in Differential which match this working copy, a
new revision will be created if you run 'arc diff'.
```

Reviewers: brennantaylor

Reviewed By: brennantaylor

CC: aran

Differential Revision: https://secure.phabricator.com/D4913
2013-02-11 15:58:47 -08:00
epriestley
84254f111a Minor, fix an exception variable.
Auditors: DurhamGoode
2013-02-11 04:54:20 -08:00
durham
4c35af9283 Make 'arc diff X' in mercurial match git by using the GCA of X as the base
Summary:
Previously 'arc diff X' with mercurial meant to use X as the base
to diff against.  Now it means use gca(X,working directory) as the base to
diff against.  This matches the git behavior.

Test Plan:
Ran 'arc diff master' on a repo where master was ahead of the feature branch.
Verified that the diff result included only the diffs in the feature branch.

Reviewers: epriestley, sid0, bos, dschleimer

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4865
2013-02-08 07:09:30 -08:00
durham
db053e1eb7 Improve performance of mercurial arc diff
Summary:
arc diff on large mercurial repos was taking 14 seconds just to get
to the commit message prompt.  With these optimizations it takes 4.

- "ancestor(.) - ancestor(XYZ)" is expensive because it has to build the
entire 400000+ revision history for both.  "XYZ::. - XYZ" is much cheaper
because it only looks at the revisions between XYZ and the working directory.

- "hg outgoing" has to talk to the server, which is slow.  "hg log -r draft()"
gives us the same information and is much cheaper.  We fall back to 'outgoing'
on older versions of mercurial.

Of the remaining 4 seconds, 2.5 are spent in 'hg status', which is a bit harder
improve.

Test Plan: Ran arc diff on our hg repo. Verified it ran faster and the diff was created.

Reviewers: epriestley, sid0, bos, dschleimer

Reviewed By: epriestley

CC: aran, Korvin, tido

Differential Revision: https://secure.phabricator.com/D4838
2013-02-08 05:54:32 -08:00
epriestley
99feb5c28e Record commit email information from arc
Summary:
Record author email information in `arc diff`, so we can recreate it in `arc patch` and elsewhere without creating any kind of email exposure issues.

In Mercurial, we currently store the whole string ("username <email@domain.com>"). Make this consistent with Git.

Test Plan: Created git and hg diffs, saw authorEmail populated.

Reviewers: btrahan, edward, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4825
2013-02-05 20:11:20 -08:00
epriestley
be73bd8716 Allow arc diff and similar to run on systems with no git
Summary:
We assume `git` is available now, but should not. Specifically, if a user runs a working copy operation like `arc list` in an SVN working copy without `git` available, they get this error: P707

We interpret git errors very narrowly; be more liberal in how we interpret them. This assumes users working with `git` will have a functional `git`, but this seems like a reasonable assumption and lets us remove some error text matching code.

Test Plan: Changed `git` to `girt`, ran `arc list`, saw a reasonable exception. Changed back to `git`, saw git detected.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: svemir, aran

Differential Revision: https://secure.phabricator.com/D4804
2013-02-04 11:34:53 -08:00
vrana
6cd3d8e995 Fix getting changed files in SVN
Summary:
This is pretty awkward but I don't have anything better.

Also don't compute cache key if caching is disabled.

Test Plan:
  $ svn diff --xml --summarize '' -r 701319:HEAD
  $ svn diff --xml --summarize svn+ssh://tubbs/svnroot/projects/lolbunny -r 701319:HEAD

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4771
2013-01-31 13:47:59 -08:00
epriestley
2613ea196f Provide hgsprintf() for building hg revsets
Summary: I guess this is correct? See T2387 for discussion.

Test Plan: Unit tests.

Reviewers: bos, DurhamGoode

Reviewed By: DurhamGoode

CC: aran

Maniphest Tasks: T2387

Differential Revision: https://secure.phabricator.com/D4711
2013-01-28 16:27:47 -08:00
vrana
edd585a3d7 lint_untracked 2013-01-28 15:15:18 -08:00
epriestley
53691cdcf9 Fix an escaping issue with "svn commit"
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
2013-01-28 14:11:31 -08:00
vrana
d3a6e456a6 Mark deleted paths as removed in committing
Test Plan: Deleted file in Git, ran `arc diff`, confirmed the question, saw the file as deleted.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4603
2013-01-23 14:42:49 -08:00
Nick Harper
40102252bf Fix arc diff when run from a git submodule
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
2013-01-17 11:28:13 -08:00
epriestley
d399354822 Normalize Windows directory separators in SVN
Summary: Fixes T1946.

Test Plan: Ran `arc diff --only` with added paths inside directories. Saw only "/" entries in the diff.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran, mbishopim3

Maniphest Tasks: T1946

Differential Revision: https://secure.phabricator.com/D4374
2013-01-09 12:34:37 -08:00
epriestley
57ec5a026d Fix arc diff x y in SVN
Summary:
D4186 added an "svn status --xml x y" form to getSVNStatus(), but the parser doesn't work for multiple files, since we get multiple <target /> elements in the XML output. So, curently, `arc diff` works (one target, all files) and `arc diff x` works (one target, x) but `arc diff x y` does not (more than one target, hits the exception).

  $ arc diff QUACK2 QUACK3
  Exception
  Expected exactly one XML status target.

Test Plan: Ran `arc diff QUACK2 QUACK3` in a working copy with modified QUACK2, QUACK3. Ran `arc diff`; `arc diff QUACK2`.

Reviewers: vrana, btrahan, codeblock, JThramer

Reviewed By: codeblock

CC: aran

Differential Revision: https://secure.phabricator.com/D4372
2013-01-09 09:11:26 -08:00
vrana
f830b3bf3f Don't require clean working copy for arc diff path in SVN
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
2012-12-21 12:34:06 -08:00
vrana
940d91d7b5 Speed up SVN discovery
Summary: `svn info` takes up to 10 seconds.

Test Plan: `arc diff` inside SVN repo and outside any repo.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4256
2012-12-20 18:10:37 -08:00
bonneval
551d036a89 Bugfixed undefined variable $commit. 2012-12-18 14:31:48 +01:00
epriestley
0bf5c3603c Refactor commit ranges and base commit handling
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
2012-12-17 12:54:08 -08:00
epriestley
2ae0cb797d Remove ArcanistRepositoryAPI::setDefaultBaseCommit()
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
2012-12-17 12:53:38 -08:00
epriestley
0e27dbfd17 Refactor getWorkingCopyStatus() into getUncommittedStatus() and getCommitRangeStatus()
Summary:
See discussion in D4049.

The getWorkingCopyStatus() method gets called from requireCleanWorkingCopy() in a lot of places, which triggers resolution of the base of the commit range. This is unnecessary; we do not need to examine the base commit in order to determine whether the working copy is dirty or not. This causes problems, in D4049 and elsewhere (we currently have a lot of fluff calls to setDefaultBaseCommit() in workflows which need to call requireCleanWorkingCopy() but do not ever use commit ranges, such as `arc patch`). This is mostly an artifact of SVN, where the "commit range" and "uncommitted stuff in the working copy" are always the same.

  - Split the method into two status methods: getUncommittedStatus() (uncommitted stuff in the working copy, required by requireCleanWorkingCopy()) and getCommitRangeStatus() (committed stuff in the commit range).
  - Lift caching out of the implementations into the base class.
  - Dirty the cache after we commit.

This doesn't do anything useful on its own and creates one caching problem (`commitRangeStatusCache` is not invalidated when the commit range changes because of `setBaseCommit()` or similar) but I wanted to break things apart here. I won't land it until there's a more complete picture.

This creates a minor performance regression in git and hg (we run less stuff in parallel than previously) but all the commands should be disk-bound anyway and the regression should be minor. It prevents a larger regression in `hg` in D4049, and lets us do less work to arrive at common error states (dirty working copy). We can examine perf at the end of this change sequence.

Test Plan: Ran unit tests, various `arc` commands.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4095
2012-12-17 12:53:28 -08:00
epriestley
c43d627cf2 Remove ArcanistSubversionAPI::hasMergeConflicts()
Summary: No callsites, redundant with getMergeConflicts().

Test Plan: Grepped for callsites.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4093
2012-12-06 12:20:02 -08:00
durham
aada1440ef Add hg support for arc land
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
2012-12-03 17:59:12 -08:00
vrana
6cb8d483b2 Improve getting Git author
Summary:
It has one more than source, see http://www.kernel.org/pub/software/scm/git/docs/git-commit-tree.html#_commit_information.
Also 'user.name' may not be set at all.
Also `git config` returns non-zero for non-existing value.

Test Plan:
  var_dump($api->getAuthor());

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4053
2012-11-30 11:09:31 -08:00
vrana
9917c1f06a Provide a method for getting Git SVN revision
Summary: I plan to use it in `save_lint.php`.

Test Plan:
  $api->getUnderlyingWorkingCopyRevision(); // In Git SVN repo

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4054
2012-11-30 11:06:23 -08:00
vrana
c12dee997d Prefer Mercurial and Git over Subversion in discovering repository
Summary: `svn info` is the slowest command for discovering repository (up to 300 ms) and Subversion is probably the least used repository type with Arcanist. Let's discover it last.

Test Plan:
  $ arc lint --trace

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4039
2012-11-27 13:48:31 -08:00