Summary:
Diff of diffs display changes between new versions of file.
This is bad after rebase because there can be many unrelated changes so it is hard to spot the real change.
This diff unhighlights the lines that were added or removed in rebase.
The changes are still visible (they can be sometimes relevant) but very subtle.
Test Plan:
# Add, change and delete line. Display diff.
# Add and change some lines in parent. Rebase. Display diff. Display diff of diff.
# Change and add some lines. Display diff. Display diff to first diff. Display diff to second diff.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: jungejason, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2761
Summary:
The locks held by read-only pullLocal daemons were causing our discovery instance
to not get the lock and fail at discovery. We don't need to hold the lock while
pulling (only while discovering), so this moves the lock to the appropriate place.
Test Plan: tested in production
Reviewers: jungejason, epriestley, vrana
Reviewed By: jungejason
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2890
Summary: They just beg to be clicked.
Test Plan: clicked furiously with my mouse until i got tired - went expected places.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2889
Summary:
Even though `--encoding` is passed to the command, git still fails
in some cases to correctly convert the output. Attempt the conversion
ourselves if it's non UTF-8.
Test Plan: Reparsed message in a repository with ISO-8859-1 encoded commit messages.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T452
Differential Revision: https://secure.phabricator.com/D2888
Summary:
- Allow clients to query for specific closed statuses (invalid, resolved, wontfix, etc), not just "closed" tasks.
- Rename this method to maniphest.query and deprecate maniphest.find as an alias to maniphest.query, for API consistency.
Test Plan: Ran queries for all tasks, "wontfix" tasks, closed tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2887
Summary:
This adds:
1) A new "arc:lint-postponed" diff property which stores a list of
lint names that are postponed and a finishpostponedlinters conduit
method which removes linters from this list. Postponed linters are
shown in the lint details.
2) A updatelintmessages conduit message, which adds additional lint
messages to the "arc:lint" diff property.
In combination, this provides very basic support for running
asynchronous static analysis tools. When the diff is being created,
a list of asynchronous static analysis runs can be added to the
diff's postponed linters list. As these postponed linters finish
up, then can report new lint messages back to the diff then mark
themselves as complete.
The client is currently responsible for filtering the lint messages
by things like affected lines and files.
Test Plan:
Used conduit call API to add lint messages and remove postponed
linters from a test diff.
Reviewers: epriestley, vrana, nh, jungejason
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1332
Differential Revision: https://secure.phabricator.com/D2792
Summary: people want to get the info about how long the revision has been active since the last time the authoer ran 'arc diff'.
Test Plan: call it from conduit console
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: Girish, aran, epriestley
Differential Revision: https://secure.phabricator.com/D2885
Summary:
We have a race condition right now, where we may insert a commit without `seenOnBranches`. This means shouldAutocloseCommit will return false (since it's not on any autoclose branches) if the message parser runs fast enough, causing it to associate the commit but not close the revision. This happened for D2851.
Also prompt the user to repair broken repositories.
Test Plan: Ran discovery / repair. Ran discovery on new commits. Verified 'seenOnBranches' value. Deleted some data, verified "repair" error. Repaired repository.
Reviewers: jungejason, nh, vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2858
Summary:
- Provide a filter to show just unread notifications.
- Visually show which notifications are unread.
- Style tweaks to make notifications more readable.
Test Plan: {F13494}
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2883
Summary:
They were shifted by one because of calling `$day->setTime(24, 0, 0)`.
I've tried to clone the date and get the epoch from the clone but it was crashing for some reason.
Test Plan: /calendar/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2882
Summary: this lets people download diffs easily in case arc export, arc patch, etc isn't to their liking or whatever.
Test Plan: "Download Raw Diff" a few times with various things toggled in the "Revision Update History" widget
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1294
Differential Revision: https://secure.phabricator.com/D2855
Summary: Allow multiple daemons to run without contention.
Test Plan: Ran multiple daemons simultaneously in "debug" mode, observed them acquiring (and sometimes failing to acquire) locks.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1400
Differential Revision: https://secure.phabricator.com/D2877
Summary:
These are currently not available via Conduit.
Also fix a bug where bad JSON input triggers an error about undefined `$metadata`.
Test Plan: Ran 'repository.create' with and without a description and with and without autoclose. Verified the created repositories had the requested attributes.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2881
Summary: This error usually doesn't occur because we have only one hunk per changeset most of the time.
Test Plan:
$hunk = new ArcanistDiffHunk();
$hunk->setAddLines(1);
$hunk->setDelLines(1);
$change = new ArcanistDiffChange();
$change->addHunk($hunk);
$change->addHunk($hunk);
$diff = DifferentialDiff::newFromRawChanges(array($change));
var_dump($diff->getLineCount());
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2878
Summary: I will also need `getRemovedLines()` so refactor this first.
Test Plan:
New test case.
Viewed uncached diff.
Verified that the only callsite of `getAddedLines()` trims lines.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2875
Summary: When the commandeerer was a reviewer, after the commandeering, he stayed as a reviewer. He can no longer amend the diff. He has to go 'Edit Revision' to remove himself/herself. The fix is to remove it automatically.
Test Plan:
comamndeereded a revision and the behavior is correct now:
- I was removed from the revision list
- the comment transaction shows one more entry that I removed myself as a reviewer
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: nh, vrana, aran, Korvin
Maniphest Tasks: T1225
Differential Revision: https://secure.phabricator.com/D2872
Summary: Related to D2873.
Test Plan: Specified it and verified that highlighting still works.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2874
Summary:
When the absolute path is used for load file (loadFileContent(()), it fails in git. For example:
/var/repo/page_admin_app
> git cat-file blob '4d6c03923006d6c444660f2c734fe03e10fd20bd':'/ios/PageAdminApp/Resources/splash/De fault-Portrait@2x~ipad.png'
fatal: Not a valid object name 4d6c03923006d6c444660f2c734fe03e10fd20bd:/ios/PageAdminApp/Resources/s plash/Default-Portrait@2x~ipad.png
This is breaking the auto-closing for about 8 revisions like
https://phabricator.fb.com/rPPA4d6c03923006d6c444660f2c734fe03e10fd20bd ...
https://phabricatorcator.fb.com/rPPA51acb7e482aab0c491b530ed19dddc741d50f673 ...
Test Plan:
- reparsed https://phabricator.fb.com/rPPA4d6c03923006d6c444660f2c734fe03e10fd20bd successfully with corresponding differential revision being closed.
- verified that without leading '/', loadFileContent for svn still works. Both of the following commands worked (note the double '/' right before 'tfb':
svn cat svn+ssh://svn.vip.facebook.com/svnroot//tfb/trunk/www/flib/intern/cachearchiver/regenerators/wurfl/CacheArchiveWurflRegenerator.php@579700
svn cat svn+ssh://svn.vip.facebook.com/svnroot/tfb/trunk/www/flib/intern/cachearchiver/regenerators/wurfl/CacheArchiveWurflRegenerator.php@579700
Reviewers: vrana
Reviewed By: vrana
CC: nh, aran, epriestley
Differential Revision: https://secure.phabricator.com/D2847
Summary:
Sometimes people create their account in Phabricator using some OAuth provider, forget about it and then tries to login with another provider.
Provide this information to them.
Test Plan: Tried to link an account linked to already existing user.
Reviewers: epriestley
Reviewed By: epriestley
CC: robarnold, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2857
Summary: See T693. To do "arc branch" performantly in immutable history repositories, we need to be able to do a single query to identify revisions related by hash. Return hash information to enable this.
Test Plan: Ran `differential.query`.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T693
Differential Revision: https://secure.phabricator.com/D2859
Summary: show project profile image on pertinent edit page. also add a "Use Default Image" checkbox for both project and user profiles. Also added a function for projects to get the profile picture to prevent some copy + paste action.
Test Plan: set my user profile and project profile image. clicked "Use Default Image" and got the default image back.
Reviewers: epriestley, floatinglomas
Reviewed By: floatinglomas
CC: aran, Korvin
Maniphest Tasks: T1307
Differential Revision: https://secure.phabricator.com/D2852
Summary:
Improve performance of large discovery tasks in Git by using subprocess streaming, like we do for Mercurial.
Basically, we save the cost of running many `git log` commands by running one big `git log` command but only parsing as much of it as we need to. This is pretty complicated, but we more or less need it for mercurial (which has ~100ms of 'hg' overhead instead of ~5ms of 'git' overhead) so we're already committed to most of the complexity costs. The git implementation is much simpler than the hg implementation because we don't need to handle all the weird parent rules (git gives us to them easily).
Test Plan:
Before, `discover --repair` on Phabricator took 35s:
real 0m35.324s
user 0m13.364s
sys 0m21.088s
Now 7s:
real 0m7.236s
user 0m2.436s
sys 0m3.444s
Note that most of the time is spent inserting rows after discover, the actual speedup of the git discovery part is much larger (subjectively, it runs in less than a second now, from ~28 seconds before).
Also ran discover/pull on single new commits in normal cases to verify that nothing broke in the common case.
Reviewers: jungejason, nh, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1401
Differential Revision: https://secure.phabricator.com/D2851
Summary:
If a repository is missing commits because they mysteriously vanished, there's no reasonable way to get them back right now. Provide a way to ignore the state in the database and rediscover the entire repository unconditionally.
We don't queue any reparses or anything, but when I move reparse into this script we can hook things up or something. This generally shouldn't be too important anyway.
Test Plan: Ran `repository discover --repair` on Phabricator.
Reviewers: jungejason, nh, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D2850
Summary:
Nothing new or exciting here yet, just moving the random scripts/repositories/ things to bin/repository. Also add `repository list`.
(Console stuff comes from D2841.)
Test Plan: Ran `repository list`, `repository pull`, `repository discover`, `repository discover --verbose`, `repository help`.
Reviewers: jungejason, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D2849
Summary: Add verbose logging. This logging is activated by setting "phd.verbose" in the config, running "phd debug", or explicitly in scripts/repository/pull.php and scripst/repository/discover.php
Test Plan:
>>> orbital ~/devtools/phabricator $ ./scripts/repository/discover.php GTEST
Discovering 'GTEST'...
<VERB> PhabricatorRepositoryPullLocalDaemon Discovering commits in repository 'GTEST'...
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '()_+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch '_+abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'abcd$100', at a37bc285a12efa7224fe19f3df54cd90fa2b897a.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch', at 774c7737b2d560a291697126bf4513204ccf661a.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch-1', at dc97539bee07293f95990d71f4638335a2531d69.
<VERB> PhabricatorRepositoryPullLocalDaemon Skipping, HEAD is known.
<VERB> PhabricatorRepositoryPullLocalDaemon Examining branch 'arcpatch-2', at 1acfaec313c46dd3caa90448800181fb91b0270f.
Reviewers: jungejason
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2843
Summary:
See D2855 for usage.
This has a drawback that inlines without a comment (synthetic comments) are not attached anywhere.
I don't like adding more and more methods so I've chosen this solution.
Plus comments and inline comments are often useful together.
Test Plan: Called the method on a revision with inline comments.
Reviewers: epriestley
Reviewed By: epriestley
CC: vii, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2844
Test Plan:
Displayed repository.
Displayed repository history.
Wondered that we actually have bunch of commits without a revision.
Displayed blame.
Didn't display merge commit.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2840
Summary: in svn and hg (for now), no branch used.
Test Plan: will test live
Reviewers: epriestley
CC: nh, vrana, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2839
Summary: I need this much more than commit but it can be useful too.
Test Plan: Displayed blame, clicked on the link.
Reviewers: epriestley
Reviewed By: epriestley
CC: john, aran, Korvin, phunt
Differential Revision: https://secure.phabricator.com/D2820
Summary: People want to create filters checking if they are in CC which is almost impossible with multiplexing in Outlook.
Test Plan: Sent e-mail with multiple CCs, verified headers.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2829
Summary: it's calling pull daemon's failure. This is actually Nick's fix.
Test Plan: Nick already manually ran it on daemon machine.
Reviewers: vrana, nh
Reviewed By: vrana
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2828
Summary:
Currently, when rendering the panel we count the number of unread notifications in the last 15, but this means we can never render a number larger than 15. If the user has more unread notifications than that, or unread notifications older than the most recent 15, there will be a flash of the higher number and then it will update to the lower number afterward.
Instead, count all unread notifications. This uses the same method used to render both numbers.
Test Plan: Loaded a page, checked the menu, nothing exploded.
Reviewers: btrahan, jungejason, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2811
Summary:
"differential.creatediff" requires a mostly-parsed diff, but there's no reason we can't make DifferentialDiffs out of raw diffs.
This mainly serves to lower the adoption barrier if getting "arc" distributed is too much of a hassle.
Test Plan: Made a diff out of a raw block of diff text.
Reviewers: ffx, vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2751
Summary: D2230 did the heavy lifting spoken of in the comment this diff deletes. Also remove 'macro' => false.
Test Plan: Added an image macro to a wiki document and it showed up
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1023
Differential Revision: https://secure.phabricator.com/D2668
Summary: This prevents "8 active days" for last week.
Test Plan: `strtotime('1 week ago 24:00')`
Reviewers: john, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2794
