Summary: Ref T12331. These changes are intended to make it easier to debug T12331 since I'm having difficulty reproducing the issue locally.
Test Plan:
- Ran `bin/phd debug task --pool 4` and got an autoscaling pool.
- Ran `bin/worker flood --duration 3` and got some 3-second-long tasks to execute with `bin/worker execute ...`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12331
Differential Revision: https://secure.phabricator.com/D17431
Summary: Ref T12324. Adds back this query for search results in dashboards.
Test Plan: Use panel in Dashboard.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12324
Differential Revision: https://secure.phabricator.com/D17428
Summary: Ref T10319. This builds out a reasonably decent avatar generator. 256 colors x 74 images x 2 borders, 38k options. Not completely sure though how names disburse though, so likely half that number. I can add lowercase lettering to double the footprint if needed though.
Test Plan:
UIExamples. Color generator here: http://tools.medialab.sciences-po.fr/iwanthue/
{F3416622}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10319
Differential Revision: https://secure.phabricator.com/D17418
Summary:
Fixes T12322. Allows you to search for commits using the `tagged(...)` repository function, so you can find "any commmit in any repository tagged with android" or similar.
I moved the function from Differential (which was the application using it) to Diffusion (which is more accurately the application which provides it).
I fixed a bug where searching for `tagged(xyz)` would have no effect (constraint was ignored) if there were no repositories tagged with "xyz". The fix isn't perfectly clean, but should work properly for the moment.
Test Plan:
- Searched with `tagged(...)` in Diffusion and Differential.
- Searched by repository.
- Searched with `tagged(...)` for a project with no tagged repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12322
Differential Revision: https://secure.phabricator.com/D17426
Summary:
Ref T12319. With large datasets, the computation of which packages own paths in a revision is needlessly slow.
Improve performance through caching:
- Cache which paths belong to each repository.
- Cache the split fragments of each path.
- Cache the path fragment counts.
- Micro-optimize accessing `$this->path`.
Test Plan:
- Used `bin/lipsum` to generate 4,000 packages with 150,000 paths.
- Created a revision affecting 100 paths in `phabricator/` (these paths mostly overlap with `bin/lipsum` path rules, since Lipsum uses Phabricator-like rules to generate paths).
- Before optimizations, this revision spent about 5.5 seconds computing paths.
- After optimizations, it spends about 275ms.
{F3423414}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12319
Differential Revision: https://secure.phabricator.com/D17424
Summary: Ref T12319. Ref T12270. Allow badges to be generated with `bin/lipsum`. These aren't hugely sophisticated but I'm not sure about the fate of T9010 yet or what's happening with the quality levels, and didn't want to make those changes more difficult.
Test Plan:
- Used `bin/lipsum generate badges --force --quickly` to generate badges.
- Made some coffee and came back to 20K badges.
{F3422200}
Reviewers: chad
Reviewed By: chad
Subscribers: cspeckmim
Maniphest Tasks: T12319, T12270
Differential Revision: https://secure.phabricator.com/D17422
Summary:
Ref T12319.
- Lipsum can trash an install by creating a lot of junk that's hard to get rid of, so we're cautious about letting you run it. Add a `--force` flag if you're sure you know what you're doing. This makes the edit/test cycle a bit easier when actually writing Lipsum generators.
- Lipsum normally sleeps for a second before creating objects, to give users more control over how much stuff they create and limit the amount of damage caused by mistakes. Sometimes, you want to generate a LOT of stuff because you want to reproduce a performance/scale issue (like T12319). Add a `--quickly` flag to generate objects as fast as possible.
- When loading random users (used as authors, assignees, etc), also load user settings so we can `ConduitCall` with them.
- Allow generators to return a PHID instead of an actual object (more convenient for Conduit-based generators).
Test Plan:
- With next change, ran `lipsum generate badges --force --quickly`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12319
Differential Revision: https://secure.phabricator.com/D17421
Summary:
Ref T12319. Currently, `bin/lipsum` uses substring matches against human-readable text to chose which objects to generate.
Instead:
- Use separate selector keys which are guaranteed to be unique.
- When a match is exact, select only that generator.
- When a match is ambiguous, fail and warn the user.
Test Plan: Generated several types of objects, tried to generate ambiguous objects like "e".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12319
Differential Revision: https://secure.phabricator.com/D17420
Summary: Ref T12319. The product name is misspelled in some methods, and a few places in the documentation.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12319
Differential Revision: https://secure.phabricator.com/D17419
Summary: Looks nicer on profiles, cards. Added some additional colors.
Test Plan: change my avatar a few times
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: avivey, Korvin
Differential Revision: https://secure.phabricator.com/D17416
Summary: We moved to having "no data" strings render in italics, but sometimes it doesn't make sense. This renders out the panel a little more expected.
Test Plan: Clean install of Phabricator, read home page activity box.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17415
Summary: Ref T12270. Moves badges into their own page and menu item. Capable of displaying hundreds of useful tokens of appreciation and dedication.
Test Plan:
Test blank state, mobile, awards badges.
{F3284139}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12270
Differential Revision: https://secure.phabricator.com/D17410
Summary:
Ref T12296. This cache is used to cache Git ref heads (branches, tags, etc). Reasonable repositories may have more than 2048 of these.
When we miss the cache, we need to single-get refs to check them, which is relatively expensive.
Increasing the size of the cache to 65535 should only require about 7.5MB of RAM.
Additionally, fill only as much of the cache as actually fits. The FIFO nature of the cache can get us into trouble otherwise.
If we insert "A, B, C, D" and then lookup A, B, C, D, but the cache has maximum size 3, we get this:
- Insert A, B, C, D: cache is now "B, C, D".
- Lookup A: miss, single get, insert, purge, cache is now "C, D, A".
- Lookup B: miss, singel get, insert, purge, cache is now "D, A, B".
Test Plan:
- Reduced cache size to 5, observed reasonable behavior on the `array_slice()` locally with `bin/repository update` + `var_dump()`.
- Used this script to estimate the size of 65535 cache entries as 7.5MB:
```
epriestley@orbital ~ $ cat size.php
<?php
$cache = array();
$mem_start = memory_get_usage();
for ($ii = 0; $ii < 65535; $ii++) {
$cache[sha1($ii)] = true;
}
echo number_format(memory_get_usage() - $mem_start)." bytes\n";
epriestley@orbital ~ $ php -f size.php
7,602,176 bytes
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12296
Differential Revision: https://secure.phabricator.com/D17409
Summary:
Ref T12298. The trigger daemon already has routine long-term sleep, and few external events can impact when it should ideally wake up. The relevant events are:
- Someone creates a new Nuance source (ideally, we should wake up right away and start polling it).
- Someone creates a Calendar event about 16 minutes in the future (ideally, we should send them a reminder in about a minute).
- Someone changes GC config to be extremely aggressive (ideally, we should immediately respect the change).
None of these cases are very important. We don't hibernate for more than 3 minutes, so the worst case is that your Nuance source takes 3 minutes to start importing or your Calendar notification comes two minutes too late (13 minutes before the event instead of 15).
This change makes GC sightly more CPU-expensive on average: currently, we do a GC sweep every 4 hours. After this change, we'll end up doing one every 3 minutes, because we lose the fact that we did a sweep recently when the daemon restarts.
We could fix this by keeping track of when the last GC sweep was in the database, instead of in the Daemon process, but the cost of a sweep is normally very small so I don't plan to do this anytime soon.
Test Plan:
- Ran `bin/phd debug trigger`, saw daemon go through 3-minute hibernate + restart cycles.
- Ran `bin/phd debug task`, saw daemon run normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12298
Differential Revision: https://secure.phabricator.com/D17408
Summary: Ref T6049. This moves Phurl to modular transactions.
Test Plan: Everything works here, add phurl, edit phurl, use phurl. Test various error states. Left a TODO on the validate dupe keys, not sure how to implement that in modular-land.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T6049
Differential Revision: https://secure.phabricator.com/D17405
Summary:
Fixes T12304. If you have a Herald rule which tries to add a commit author as an auditor, it fails validation when trying to apply.
Stop trying to apply these transactions, and explicitly tell the user why. Differential already uses a similar ruleset around reviewers, but Audit was using older code.
Test Plan:
- Wrote a Herald rule to add A, B and C as auditors.
- Committed as A.
- After change, saw B and C added with transacript guidance that A was the author.
{F3235660}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12304
Differential Revision: https://secure.phabricator.com/D17404
Summary:
Fixes T12302. Currently, we aren't merging multiple "AddAuditors" transactions correctly.
This can occur when Herald triggers multiple auditor rules.
Instead, merge them.
Test Plan:
- Wrote two different Herald rules that add auditors.
- Pushed a commit which triggered them.
- After the change, saw all the auditors get added correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12302
Differential Revision: https://secure.phabricator.com/D17403
Summary: Ref T12270. This converts Badges to modular transactions for editing and awarding.
Test Plan: Add Badge, edit badge, award and revoke... Still going to test this some more but feel free to comment on anything obviously wrong?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12270
Differential Revision: https://secure.phabricator.com/D17402
Summary: Ref T12297. This could be fancier, but should make pulling profiles off `admin.phacility.com` significantly more realistic.
Test Plan: Dragged and dropped some profiles to upload them, then reviewed them via web UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12297
Differential Revision: https://secure.phabricator.com/D17401
Summary:
Ref T12297. This slightly modernizes the XHProf UI. Not included here:
- Some of the code acts like samples have PHIDs, but they currently do not. I plan to add them in the next change.
- I've intentionally left the actual list untouched for now -- it has some old/buggy code (like `flag-6` is no longer an icon) that I'll fix in a future change.
Test Plan: {F3224264}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12297
Differential Revision: https://secure.phabricator.com/D17400
Summary: Ref T12297. When a page is generated with the profiler active, keep it active by adding a `__profile__` input to any forms we generate.
Test Plan: Hit Conduit API page with `__profile__` active, saw it reflected in forms.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12297
Differential Revision: https://secure.phabricator.com/D17399
Summary:
Fixes T12172. Fixes T12060. This allows runtime code building CSS for mail to read CSS variables, then makes all the code do that.
It reverts the non-colorblind red/green to the colors in use before T12060, which seem better for non-colorblind users since no one really complained?
Test Plan:
- Viewed code diffs in Web UI.
- Viewed prose diffs in Web UI.
- Viewed code diffs in email.
- Viewed prose diffs in email.
All modes respected the accessibility color scheme.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12172, T12060
Differential Revision: https://secure.phabricator.com/D17269
Summary: Ref T10390. Basically hides policy controls when creating a panel on a dashboard. Shows when you edit them or through normal workflow. I think we should maybe also get rid of view policy? Not sure the benefit since results will be filtered anyways. Maybe Text panels? Not sure the use case.
Test Plan: Add a panel, edit a panel.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: hskiba, Korvin
Maniphest Tasks: T10390
Differential Revision: https://secure.phabricator.com/D17393
Summary:
Fixes T12301. In D17372, this changed to use generic EditEngines instead of the proper runtime engine. Normally this doesn't matter, but can in this case.
After loading the configurations normally, swap their attached engines for the specific configured runtime engine we're currently executing.
Test Plan: Clicked "Create Form" from the Maniphest form list, saw it go to "Create Maniphest Form", not "Create Generic Meta-Form".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12301
Differential Revision: https://secure.phabricator.com/D17398
Summary:
Fixes T12306. Currently, we warn about daemons not running even if they're in normal "alive" states, particularly "waiting to restart after a failure".
This check was made more strict in D12088, back when we tried to version check running daemons. Since we implemented auto-restart-after-config-change we don't do this anymore, so it should be fine to make this more lax again.
Test Plan:
- Faked an exception for all tasks.
- Before patch: reloading the daemon setup error sometimes raised a false positive ("waiting" daemon detected as dead).
- After patch: daemon setup error no longer triggers.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12306
Differential Revision: https://secure.phabricator.com/D17397
Summary:
Ref T12298. This updates `bin/phd` for minor changes to daemon configuration. In particular:
- Every daemon now has an autoscale pool (for trigger/pull, the maximum pool size is 1).
- Pools now have labels to make debugging a little easier.
- Some minor structural changes.
Test Plan: See D17389.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12298
Differential Revision: https://secure.phabricator.com/D17390
Summary: Ref T10390. Simplifies dropdown by rolling out canUseInPanel in useless panels
Test Plan: Add a query panel, see less options.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10390
Differential Revision: https://secure.phabricator.com/D17341
Summary:
See downstream <https://phabricator.kde.org/T5404>. This code was doing some `.firstChild` shenanigans which didn't survive some UI refactoring.
This whole UI is a little iffy but just unbreak it for now.
Test Plan: Allowed and rejected desktop notifications, got largely reasonable UI rendering.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17388
Summary: I broke this at the last second in D17374. `getStrList()` doesn't read arrays. It probably should (more modern analogs do) but don't rock the boat in the leadup to the release cut.
Test Plan: Hovered over a thing, saw a hovercard and no `getStrList()` error in my logs.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17383
Summary:
Ref T12287. See D17361. That fixed a mostly-theoretical bug with crumbs named things like "MMMMMMMMMMMMMMMMMMMM", but caused a less-theoretical buggy side effect in Safari.
For now, just keep the "MMMMMMMM" crumbs around since that's the easiest/least-bad/safest fix prior to the release cut. We can fix this more broadly when we have more time to look at it.
Test Plan: Looked at profiles, saw the entire name for "hector" and the too-long-crumb for "MMMMM".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12287
Differential Revision: https://secure.phabricator.com/D17382
Summary:
Ref T12268. Ref T12157. When you mention or interact with a user who is unlikely to be able to respond (for example, because their account is disabled), we try to show a colored dot to provide a hint about this.
Recently, we no longer send any normal mail to unverified addresses. However, the rules for showing a dot haven't been updated yet, so they only care about this if `auth.require-verification` is set. This can be misleading, because if you say `Hey @alice, what do you think about this?` and she hasn't verified her email, you may not get a response.
Update the rule so users with unverified email addresses get a grey dot in all cases. The hint is basically "you shouldn't expect a response from this user".
Make the meaning of this hint more clear on the hovercard and profile.
Also:
- Allow the non-ajax version of the hovercard page (which is basically only useful for testing hovercards) accept `?names=...` so you can just plug usernames, hashtags, etc., in there.
- Fix a bug where the user's join date was based on their profile creation date instead of account creation date on the hovercard. Users may not have a profile creation date (if they never changed any account details), and it may be different from their account creation date.
Test Plan: {F2998517}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12268, T12157
Differential Revision: https://secure.phabricator.com/D17374
Summary:
Fixes T12281. Some forms (like Settings) can't actually create new objects. Currently, though, you can select them and add them to profile menus; if you do, they fail when building an item.
Kick them out of the typeahead, and decline to render them in menus.
Test Plan:
Added "Create Settings" to a menu, no longer fatals after patch (item vanished from menu, still editable normally to get rid of it).
Tried to add another "Create Settings", no longer available in typehaead.
Added some normal stuff.
Viewed a choose-among-forms dropdown in Maniphest, which still worked normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12281
Differential Revision: https://secure.phabricator.com/D17372
Summary: See D14617. This could probably go either way but we don't currently need `$engine` in `newStandardEditField()`, so just get rid of it.
Test Plan: Edited a task with standard custom fields defined.
Reviewers: vrana, chad
Reviewed By: vrana
Differential Revision: https://secure.phabricator.com/D17370
Summary:
See D16676. When an export has an unsupported mode (bad database value, out-of-date object, etc) the intent of this code is to put it into the `<select />` so that you can save the form without silently changing the object.
However, it incorrectly calls `array_shift()` instead of `array_unshift()`.
Test Plan:
Edited a Calendar export with an invalid mode, saw the mode appear properly in the dropdown:
{F2957321}
Reviewers: vrana, chad
Reviewed By: vrana
Differential Revision: https://secure.phabricator.com/D17369
Summary: Ref T12270. Any project, badge, dashboard, etc, that uses names in crumbs can over generate a long title. Restrict to a sane but generous width.
Test Plan: Make a project with a really long name, test various crumb layouts, boards, tasks, desktop, mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12270
Differential Revision: https://secure.phabricator.com/D17361
Summary: Ref T10798. Cleans up the UI a little and adds a sidenav.
Test Plan: Review badge and recipients in sandbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10798
Differential Revision: https://secure.phabricator.com/D17358
