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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Summary:
In D1079, I added "--color never", but this flag is provided by the "color"
extension, which is why I missed it originally, because it doesn't show up until
you enable that extension. Providing it causes installs which don't have it
enabled (disabled is the default) to fail.
Use "--config" to disable color instead. This sets a configuration setting and
works regardless of whether the color extension is present.
Test Plan: Ran "arc diff" in a mercurial working copy with the color extension
enabled and disabled.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, nh
Differential Revision: 1092
Summary: Mercurial has a --color flag which disables the use of ANSI colors. Use
it to prevent "hg diff" from giving us colorized diff output.
Test Plan: Ran "hg diff --color never", verified it disabled ANSI color in diff
output.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: aran
CC: aran
Differential Revision: 1079
Summary: Apparently I just never tested this or something. Make it work
correctly.
Test Plan: Ran "arc diff" in a Mercurial working copy with a binary file in
outgoing.
Reviewers: Makinde
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1074
Summary: We incorrectly add the "Uncommitted" flag to untracked files, which
causes them to raise various prompts and not respect "--allow-untracked".
Test Plan: Ran "arc diff --allow-untracked" in a clean working copy with
untracked files, was not fatally error'd.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde, epriestley
Differential Revision: 1073
Summary:
In D962, I switched us from using "hg summary" to "hg id" to find the working
copy revision. However, "hg id" reports the revision with a trailing "+" if the
working copy is dirty, so we fail before hitting the explicit check for this
later.
Trim off the trailing '+' if it is present.
Test Plan:
Ran "arc diff" in a dirty Mercurial working copy and got a good error message
about uncommitted changes.
~/repos/hg-working-copy $ arc diff
WARNING: Mercurial support is largely imaginary right now.
Usage Exception: You have uncommitted changes in this branch. Commit (or
revert) them before proceeding.
Working copy: /INSECURE/repos/hg-working-copy/
Uncommitted changes in working copy
hello.c
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1070
Summary:
Expand 12-character hashes to 40-character hashes so other things will work
properly.
Also use "hg id" instead of "hg summary" to figure out where the working copy
is, since it's substantially simpler.
Test Plan: Ran "arc diff" and got properly 40-character hashes.
Reviewers: Makinde, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 962
Summary: Move code to actually parse "hg" output into a separate class with some
tests, so I can reuse it in the import scripts. We should probably do this for
Git/SVN at some point, too.
Test Plan: Ran unit tests, used this class in Phabricator importers, grepped for
calls to removed private methods.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 942
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
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
Summary:
- Use "hg parents" to figure out where outgoing changes originate from, not
"~1", since that's a new feature in Mercurial.
- Add "--style default" all over the place since hg config can override this
(similar to the date config issues we saw in git).
- Cache working copy status so we don't run full hg diffs like 30 times
(similar to git/svn APIs).
- Use full "--git" flag instead of rather cryptic "-g".
Test Plan: Ran "arc diff" in my hg working copy, got the diff I expected.
Reviewers: Makinde, aran, jungejason, nh, tuomaspelkonen
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 934
Summary:
- Noncontentious Mercurial stuff from D863.
- Use "hg outgoing --branch" to make sure we aren't including outgoing
revisions on other branches.
- Use "hg summary" to figure out where the working copy is, so "hg up <older
rev> && arc diff" works like expected.
Test Plan: Ran "arc diff" from multiple branches with the working copy in
various states.
Reviewers: Makinde, aran, jungejason, nh, tuomaspelkonen
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 936
Summary: Parse some other fields which I hadn't seen yet from "hg log". Select
the right "hg log" range for local commit info.
Test Plan: Ran "arc diff" in my test mercurial repository from a branch with
tags. Looked at "local commits" in Differential.
Reviewers: Makinde, aran, jungejason, tuomaspelkonen
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 880
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
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
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
