1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 20:52:43 +01:00
Commit graph

7911 commits

Author SHA1 Message Date
Bob Trahan
c00733a292 Project - add editor-level support for detecting name conflicts
Summary: Fixes T4833. I wish there was an elegant way to catch this exception but I think the stack is written such that we really should just do this one-off query here...

Test Plan: from the "create project" link under "edit task" I received a more detailed exception than the report in T4833 post patch. I also tested editing an existing project - yay - and editing an existing project to some other existing project's name - got a nice error dialogue.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4833

Differential Revision: https://secure.phabricator.com/D8834
2014-04-21 15:44:35 -07:00
epriestley
5cbdda413c Paradigms, paradigms, paradigms
Summary: Fixes T4693.

Test Plan: {F146407}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4693

Differential Revision: https://secure.phabricator.com/D8829
2014-04-21 15:33:59 -07:00
epriestley
0cfc5aa0aa Support "public" policy in Tokens
Summary: Ref T4830. Allows logged-out users to browse tokens, policies permitting.

Test Plan: Browsed tokens as a logged-out user.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4830

Differential Revision: https://secure.phabricator.com/D8828
2014-04-21 15:33:07 -07:00
epriestley
27d426e3fe Allow Conduit console to be browsed by logged-out users
Summary:
Ref T4830. A few methods, like `conduit.ping`, are callable without authentication, so this even has some use cases. Also:

  - Make some Differential stuff a little more consistent.
  - Use slightly more modern rendering.
  - Deprecate the status-oriented `user` calls; these will be replaced by Calendar methods.

Test Plan: Browsed console as logged out / logged in users.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4830

Differential Revision: https://secure.phabricator.com/D8826
2014-04-21 15:32:48 -07:00
epriestley
4143ecf015 Minor updates to UIExamples
Summary:
Ref T4830.

  - If the application policy is public, allow logged-out users to browse examples.
  - Use standard elements instead of custom ones.

Test Plan: Browsed UIExamples.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4830

Differential Revision: https://secure.phabricator.com/D8825
2014-04-21 15:32:03 -07:00
epriestley
28696d08ac Remove indirect loads of Differential revisions from Releeph requests
Summary:
Ref T3551. Currently, there are many layers of indirection between pull requests and revisions. After D8822, revisions and other types of requested objects are recorded directly on the request. This allows us to simplify data access and querying.

A lot of stuff here is doing `instanceof` checks to keep APIs stable, but most of those can go away in the long run.

Test Plan:
  - Browsed requests.
  - Verified revision-dependent fields (like "Revision", "Size", "Churn") still render correctly.
  - Called `releeph.queryrequests`.
  - Called `releephwork.nextrequest`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8824
2014-04-20 11:55:47 -07:00
epriestley
65913162e7 Allow discovered but unparsed commits to be requested in Releeph
Summary:
Ref T3662. Releeph blocks users from requsting unparsed commits, but there's no real technical reason for this.

The `releephwork.getorigcommitmessage` method assumes data exists, but should be replaced with `diffusion.querycommits` anyway.

Test Plan: Ran `diffusion.querycommits`. Requested a commit.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3662

Differential Revision: https://secure.phabricator.com/D8823
2014-04-20 11:55:29 -07:00
epriestley
1a3ac09975 Add "requestedObjectPHID" to ReleephRequest
Summary:
Ref T3551. Currently, ReleephRequests don't have a direct concept of the //object// being requested. You can request `D123`, but that is just a convenient way to write `rXyyyy`.

When the UI wants to display information about a revision, it deduces it by examining the commit.

This is primarily an attack on T3551, so we don't need to load <commit -> edge -> revision> (in an ad-hoc way) to get revisions. Instead, when you request a revision we keep track of it and can load it directly later.

Later, this will let us do more things: for example, if you request a branch, we can automatically update the commits (as GitHub does), etc. (Repository branches will need PHIDs first, of course.)

This adds and populates the column but doesn't use it yet. The second part of the migration could safely be run while Phabricator is up, although even for Facebook this table is probably quite small.

Test Plan:
  - Ran migration.
  - Verified existing requests associated sensibly.
  - Created a new commit request.
  - Created a new revision request.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8822
2014-04-20 11:55:18 -07:00
epriestley
a588955bf7 Remove loadPhabricatorRepository from ReleephProject
Summary: Ref T3551. Repository is guaranteed if a product is loaded with modern mechanisms.

Test Plan:
  - Edited a request.
  - Called `releephwork.getbranchcommitmessage`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8821
2014-04-20 11:55:09 -07:00
epriestley
d75d0acba5 Remove loadReleephBranch and loadReleephProject from ReleephRequest
Summary: Ref T3551. Ref T3549. Mostly unnecessary with modern calls.

Test Plan:
- Called `releeph.queryrequests`.
- Called `releeph.request`.
- Called `releephwork.getbranchcommitmessage`.
- Called `releephwork.getcommitmessage`.
- Called `releephwork.nextrequest`.
- Viewed and edited branches and requests.
- Made a comment on a request.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549, T3551

Differential Revision: https://secure.phabricator.com/D8820
2014-04-20 11:54:58 -07:00
epriestley
a4f975ef3e Remove some ad-hoc handle loads from Releeph
Summary:
Ref T3551. Releeph does a bunch of old-school on-object data loading; start cleaning that up.

This doesn't change anything, just makes the code more modern/consistent.

Test Plan: Edited a request; called `releephwork.nextrequest`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8819
2014-04-20 11:54:50 -07:00
epriestley
230fce735e Move /project/ controllers to /product/
Summary: Ref T3549. Move only, no extra changes.

Test Plan: Loaded application.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549

Differential Revision: https://secure.phabricator.com/D8817
2014-04-20 11:54:37 -07:00
epriestley
6e6ad2cfcf Add modern releeph.queryproducts and releeph.querybranches
Summary:
Ref T3662. Ref T3549. These methods are pretty conservative for now, but get the structure in place.

Also do a bunch more project -> product stuff.

Test Plan: Made calls to both methods, browsed around the UI a fair amount.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549, T3662

Differential Revision: https://secure.phabricator.com/D8816
2014-04-20 11:54:22 -07:00
epriestley
f5cc5c122a Rename ReleephProjectQuery to ReleephProductQuery
Summary: Ref T3549.

Test Plan: grep/lint

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549

Differential Revision: https://secure.phabricator.com/D8815
2014-04-20 11:51:02 -07:00
austinkelleher
2e5065feb5 Update function name to follow naming convention.
See: <http://github.com/facebook/phabricator/pull/575>

Reviewed by: epriestley
2014-04-20 08:37:37 -07:00
Chad Little
ce1dbbec3c Add FontAwesome as a font/icon choice
Summary:
This adds FontAwesome and attempts to make use as icons as consistent as possible. May require additional tweaks once we start using, but in practice this is pretty finished.

 - Adds FontAwesome
 - Adds additional transforms (rotates, spins)
 - Adds additional colors
 - Better scopes halflings and fontawesome
 - Shares CSS between fonts for consistency

Test Plan:
Tested various browsers back to IE8, mobile.

{F146146}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8818
2014-04-19 10:00:37 -07:00
epriestley
5570ec5156 Rename Releeph "Project" PHID type to "Product"
Summary: Ref T3549.

Test Plan: `grep`, loaded some pages

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549

Differential Revision: https://secure.phabricator.com/D8811
2014-04-18 17:52:45 -07:00
epriestley
fde4ccf9b2 Use standard handle loading in Releeph
Summary: Ref T3718. Move from unbatched / ad-hoc loading to standard stuff for handles.

Test Plan: Looked at some requests and saw no changes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3718

Differential Revision: https://secure.phabricator.com/D8810
2014-04-18 17:52:32 -07:00
epriestley
c7f5dc0208 Remove some unused Releeph field methods
Summary: Ref T3718. Remove `bulkLoad()` and header-view rendering wrappers.

Test Plan: Viewed a request, looked the same. `grep`'d for stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3718

Differential Revision: https://secure.phabricator.com/D8809
2014-04-18 17:52:22 -07:00
epriestley
71ef48b3e5 When a diff has 12,345 lines, render "12,345 lines" instead of "12 lines"
Summary: This `%d` should be a `%s`, since the `PhutilNumber` value may get formatted according to locale settings.

Test Plan: will make @zeeg

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Differential Revision: https://secure.phabricator.com/D8814
2014-04-18 17:52:11 -07:00
epriestley
9889892e5b Actually squelch Harbormaster "test passed" mail
Summary:
Ref T1049. When Harbormaster tests pass, don't bother sending an email about it.

(I tried to implement this earlier but didn't test it entirely properly, and we needed a little more code.)

Test Plan: Used `bin/harbormaster build` to build some junk, got no email about passes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8813
2014-04-18 17:51:59 -07:00
epriestley
f1245f4f34 Remove flavor text for action buttons
Summary: A small but appreciable number of users find flavor on buttons confusing. Remove this flavor. This retains flavor in headers, error messages, etc., which doesn't cause confusion.

Test Plan: Looked at a revision, task, paste, macro, etc.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8812
2014-04-18 17:51:46 -07:00
Wenyu Zhang
ba956711a5 Change password_hash() algorithm from CRYPT_BLOWFISH to PASSWORD_BCRYPT.
Summary:
PHP 5.5 specifies constant PASSWORD_BCRYPT should be used in password_hash()
instead of CRYPT_BLOWFISH. Using CRYPT_BLOWFISH is not supported in either PHP
or HHVM. This constant breaks Username / Password authentication.

Test Plan:
Login using Username/Password with bcrypt hash. Before applying the patch,
No matter what password entered, it will always fail authentication. After this
patch, user should be able to login with bcrypt hash.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8808
2014-04-18 13:38:36 -07:00
epriestley
35df988036 Use standard UI elements to render pull requests in Releeph
Summary:
Ref T3718. Ref T3644. Ref T3092. Switches from the Releeph UI elements to standard ones. I'll attach some screenshots.

Also fixes CSRF against the request action endpoint.

Test Plan:
  - Viewed request details.
  - Took actions on a request from detail page.
  - Viewed request list.
  - Took actions on a request from list page.
  - Used keyboard shortcuts to navigate list.
  - Used keyboard shortcuts to take actions.
  - Simulated errors.
  - Viewed on devices.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: grp, FacebookPOC, mattlqx, tala, beng, LegNeato, epriestley

Maniphest Tasks: T3718, T3092, T3644

Differential Revision: https://secure.phabricator.com/D8771
2014-04-18 06:44:45 -07:00
Chad Little
41ea90c686 Add Glyphicons Halflings Font and Examples
Summary: This adds in the Glyphicons Halflings Font/Iconset as an option for PHUIIconView along with a standard set of 10 colors. This will be a replacement for the standard action icon set in upcoming diffs, as well as obviously give us more flexibility, less KB, and less design resource time managing images.

Test Plan: UIExamples, Diviner

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8798
2014-04-17 17:31:23 -07:00
epriestley
95a405da10 Record build success or failure on buildable objects
Summary:
Fixes T4810. When a buildable completes, make an effort to update the corresponding object with a success or failure message. Commits don't support this yet, but revisions do.

{F144614}

Test Plan:
  - Used `bin/harbormaster build` and `bin/harbormaster update` to run a pile of builds.
  - Tried good/bad builds.
  - Sent some normal mail to make sure the mail reentrancy change didn't break stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4810

Differential Revision: https://secure.phabricator.com/D8803
2014-04-17 16:04:14 -07:00
epriestley
49bc32f12d Implement PhabricatorApplicationTransactionInterface in Differential
Summary:
Ref T4810. Ultimate goal is to let Harbormaster post a "build passed/failed" transaction. To prepare for that, implement `PhabricatorApplicationTransactionInterface` in Differential.

To allow Harbormaster to take action on //diffs// but have the transactions apply to //revisions//, I added a new method so that objects can redirect transactions to some other object.

Test Plan:
  - Subscribed/unsubscribed/attached/detached from Differential, saw transactions appear properly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4810

Differential Revision: https://secure.phabricator.com/D8802
2014-04-17 16:03:24 -07:00
epriestley
62973e0f53 Add activeDiffPHID to differential.query
Summary: Ref T4809. This saves us a few round trips to find a Buildable, and generally makes the notion of "active" more explicit (i.e., not just the diff with the largest ID). In the future, we may let you revert to previous diffs, which would make the "largest number" rule not always correct.

Test Plan: Ran `differential.query`, got sensible results.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8800
2014-04-17 16:01:27 -07:00
epriestley
0ef599e906 Give Buildables a status, populate it, and return it over Conduit
Summary:
Ref T4809. Currently, buildables have a status field but nothing populates it. Populate it:

  - When builds change state, update the Buildable state.
  - Use the new Buildable state on the web UI.
  - Return the new Buildable state from Conduit.

To make it easier to debug/test this:

  - Provide `bin/harbormaster update Bxxx ...` to force foreground update of a Buildable.

Test Plan:
  - Used `bin/harbormaster update Bxxx --force --trace` to update buildables.
  - Looked at buidlable list, saw statuses reported properly.
  - Used Conduit to read statuses.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8799
2014-04-17 16:01:16 -07:00
epriestley
4918773afe Drop nonsense buildStatus field from Buildable
Summary:
Ref T4809. Buildables currently have buildStatus and buildableStatus. Neither are used, and no one knows why we have two.

I'm going to use buildableStatus shortly, but buildStatus is meaningless; burn it.

Test Plan: `grep`, examined similar get/set calls, created a new buildable, ran storage upgrade.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8796
2014-04-17 16:01:06 -07:00
epriestley
7c1bcdea16 Add "harbormaster.querybuilds" Conduit API
Summary:
Ref T4809. This one is more straightforward. A couple of tweaks:

  - Remove the WAITING status, since nothing ever sets it and I suspect nothing ever will with the modern way artifacts work (maybe). At a minimum, it's confusing with the new Target status that's also called "WAITING" but means something different.
  - Consolidate 17 copies of these status names into one method.

Test Plan: Ran some queries via Conduit, got reasonable looking results.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8795
2014-04-17 16:00:58 -07:00
epriestley
3b0be0961c Add a rough harbormaster.querybuildables Conduit API method
Summary: Ref T4809. I need to sort out some of the "status" stuff we're doing before this is actually useful (there's no sensible "status" value to expose right now) but once that happens `arc` can query this to figure out whether it needs to warn the user about pending/failed builds.

Test Plan: Ran query with various different parameters.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8794
2014-04-17 16:00:25 -07:00
epriestley
b5df5af04d Link to Herald transcripts from Herald transactions
Summary: See IRC. Some users are having difficulty figuring out why Herald is taking some actions. Make it easier to get to the transcript.

Test Plan: {F144622}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: dctrwatson, epriestley

Differential Revision: https://secure.phabricator.com/D8804
2014-04-17 15:57:34 -07:00
epriestley
e49251ec98 Improve robustnesss of feed text rendering
Summary:
Couple of minor cleanup things here:

  - Pass handles to ApplicationTransactions when rendering their stories; this happened implicitly before but doesn't now.
  - Add `?text=1` to do ad-hoc rendering of a story in text mode.
  - Make Conduit skip unrenderable stories.
  - Fix/modernize some text in the Commit story.

Test Plan: Rendered text versions of stories via Conduit and `?text=1`.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: zeeg, spicyj, epriestley

Differential Revision: https://secure.phabricator.com/D8793
2014-04-17 15:57:18 -07:00
epriestley
4a6d2e9c97 Allow tasks to yield to other tasks
Summary:
For Harbormaster tasks which want to poll or wait, this lets them say "try again a little later" without having to sleep and hold a queue slot.

This is basically the same as failing, except that we don't increment the failure counter. Instead, we just set the current lease to the correct length and then exit. The task will be retried after the lease expires.

Test Plan: Using both `bin/harbormaster` and `phd debug taskmaster`, ran a lot of waiting tasks through the queue, faking them to either yield or not yield in a controlled manner. The queue responded as expected, yielding tasks appropraitely and retrying them later.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8792
2014-04-16 13:02:12 -07:00
epriestley
afd04731ab Add a "Create build step" transaction to Harbormaster
Summary:
Without this, build steps that have no options (like "wait for previous commits") don't actually save, since the transaction array is empty.

This also generally nice and consistent.

Test Plan: Created a new "wait" step, viewed transaction log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8791
2014-04-16 13:01:56 -07:00
epriestley
78bf266bde Allow Harbormaster build targets to wait for messages
Summary:
This hooks up all the pieces of the build pipeline so `harbormaster.sendmessage` actually works. Particularly:

  - Candidate build steps (i.e., those which interact with external systems) can now "Wait for Message". This pauses them indefinitely when they complete, until something calls `harbormaster.sendmessage`.
  - After processing a target, we check if we should move it to PASSED or WAITING.
  - Before updating a build, we move WAITING targets with pending messages to either PASSED or FAILED.
  - I added an explicit "Building" state, which doesn't affect workflows but communicates more information to human users.

A big part of this is avoiding races. I believe we get the correct behavior no matter which order events occur in:

  - We update builds after targets complete and after we receive messages, so we're guaranteed to update once both these conditions are true. This means messages can't be lost (even if they arrive before a build completes).
  - The minor changes to the build engine logic mean that firing additional build updates is always safe, no matter what the current state of the build is.
  - The build itself is protected by a lock in the build engine.
  - The target is not covered by an explicit lock, but for all states only the engine (waiting) //or// the worker (all other states) can interact with it. All of the interactions also move the target state forward to the same destination and have no other side effects.
  - Messages are only consumed inside the engine lock, so they don't need an explicit lock.

Test Plan:
  - Made an HTTP request wait after completion, then ran a pile of builds through it using `bin/harbormaster build` and the web UI.
  - Passed and failed message-awaiting builds with `harbormaster.sendmessage`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Differential Revision: https://secure.phabricator.com/D8788
2014-04-16 13:01:46 -07:00
epriestley
803c50c1e7 Allow Harbormaster HTTP steps to pass credentials
Summary: Fixes T4590. Use the credentials custom field to allow Harbormaster HTTP requests to include usernames/passwords.

Test Plan: Ran a build plan with credentials, verified they were sent to the remote server.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4590

Differential Revision: https://secure.phabricator.com/D8786
2014-04-16 13:01:38 -07:00
epriestley
c402d7d307 Implement smart waits for rarely updated repositories
Summary:
Ref T4605. When figuring out how long to wait to update a repository, factor in when it was last pushed. For rarely updated repositories, wait longer between updates.

(A slightly funky thing about this is that empty repos update every 15 seconds, but that seems OK for the moment.)

Test Plan:
Ran `bin/phd debug pulllocal` and saw sensible calculations and output:

```
...
<VERB> PhabricatorRepositoryPullLocalDaemon Last commit to repository "rPOEMS" was 1,239,608 seconds ago; considering a wait of 6,198 seconds before update.
>>> [79] <query> SELECT * FROM `repository` r   ORDER BY r.id DESC
<<< [79] <query> 514 us
>>> [80] <query> SELECT * FROM `repository_statusmessage` WHERE statusType = 'needs-update'
<<< [80] <query> 406 us
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIH" is not due for an update for 8,754 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rDUCK" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTESTX" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rQWER" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBT" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVNX" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIG" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rHGTEST" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rBTX" is not due for an update for 14 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGX" is not due for an update for 13 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rMTX" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPOEMS" is not due for an update for 6,198 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHU" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rSVN" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rPHY" is currently updating.
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rGTEST" is not due for an update for 21,598 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIS" is not due for an update for 6,894 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARCLINT" is not due for an update for 21,599 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rLPHX" is not due for an update for 1,979 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rARC" is not due for an update for 1,824 second(s).
<VERB> PhabricatorRepositoryPullLocalDaemon Repository "rINIHG" is not due for an update for 21,599 second(s).
...
```

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8782
2014-04-16 13:01:04 -07:00
epriestley
5671c4b276 Make PullLocal daemon more flexible and transparent about scheduling
Summary:
Ref T4605. Fixes T3466. The major change here is that we now run up to four simultaneous updates. This should ease cases where, e.g., one very slow repository was blocking other repositories. It also tends to increase load; the next diff will introduce smart backoff for cold repositories to ease this.

The rest of this is just a ton of logging so I can IRC debug these things by having users run them in `phd debug pulllocal` mode.

For T3466:

  - You now have to hit four simultaneous hangs to completely block the update process.
  - Importing repository updates are killed after 4 hours.
  - Imported repository updates are killed after 15 minutes.

Test Plan:
  - Ran `phd debug pulllocal` and observed sensible logs and behavior.
  - Interrupted daemon from sleeps and processing with `diffusion.looksoon`.
  - Ran with various `--not`, `--no-discovery` flags.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3466, T4605

Differential Revision: https://secure.phabricator.com/D8785
2014-04-16 13:00:49 -07:00
epriestley
417056932e Make discovery slightly cheaper in the common case
Summary:
Ref T4605. Before discovering branches, try to prefill the cache in bulk. For repositories with large numbers of branches, this allows us to issue dramatically fewer queries.

(Before D8780, this cache was usually held across discovery events, so being able to fill it cheaply was not as relevant.)

Test Plan: Ran discovery on Git, Mercurial and SVN repositories. Observed fewer queries for Git/Mercurial.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8781
2014-04-16 13:00:38 -07:00
epriestley
118c696f72 Separate repository updates from the pull daemon
Summary:
Ref T4605. Currently, the PullLocal daemon is responsible for two relatively distinct things:

  - scheduling repository updates; and
  - actually updating repositories.

Move the "actually updating" part into a new `bin/repository update` command, which basically runs the pull, discover, refs and mirror commands. This will let the parent process focus on scheduling in a more understandable way and update multiple repositories at once. It also makes it easier to debug and understand update behavior since the non-scheduling pipeline can be run separately.

Test Plan:
  - Ran `update --trace` on SVN, Mercurial and Git repos.
  - Ran PullLocal daemon for a while without issues.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4605

Differential Revision: https://secure.phabricator.com/D8780
2014-04-16 13:00:29 -07:00
Chad Little
2cf2117eee Remove extra workboard margin on mobile
Summary: We have too much space on workboards when displayed on mobile devices.

Test Plan: Shrink browser display, note that all workboards align to common gutters.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8790
2014-04-16 11:09:53 -07:00
Chad Little
f5eb7b6404 Add ability to edit Projects on mobile
Summary: sets action list to crumbs

Test Plan: shrink browser, see mobile action list, click on it, edit

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8789
2014-04-16 11:06:04 -07:00
epriestley
041c3fd19d Let project prefilling accept PHIDs
Summary: I recently made this better about accepting project names, but we use it in some cases with PHIDs. Make that work properly again.

Test Plan: Clicked "New Task" from a project page.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8778
2014-04-15 11:17:37 -07:00
epriestley
20abf37b88 Fix a lookup issue in Owners
Summary:
Fixes T4477. Sort of winging this but it's probably the right fix?

One error in T4477.

One error via email:

```
[2014-04-15 17:44:34] ERROR 8: Undefined index: /some_index/ at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:213]
  #0 PhabricatorOwnersPackage::findLongestPathsPerPackage(Array of size 3 starting with: { 0 => Array of size 3 starting with: { id => 5 } }, Array of size 8 starting with: { / => Array of size 2 starting with: { /some_index/some_file.py => true } }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:170]
  #1 PhabricatorOwnersPackage::loadPackagesForPaths(Object PhabricatorRepository, Array of size 2 starting with: { 0 => /some_index/some_file.py }) called at [/phab_path/phabricator/src/applications/owners/storage/PhabricatorOwnersPackage.php:119]
...
```

Test Plan: Will make @zeeg do it.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Maniphest Tasks: T4477

Differential Revision: https://secure.phabricator.com/D8779
2014-04-15 11:12:42 -07:00
Bob Trahan
c845b757ee Maniphest - remove "attach file" action
Summary: Fixes T4655. Basically leaves the display code intact for legacy installs but removes the option from the UI and removes "create" code.

Test Plan:
tried to attach file and the action was not in the dropdown!
made a new task and it worked!
commented on an old task and it worked!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4655

Differential Revision: https://secure.phabricator.com/D8777
2014-04-15 10:49:05 -07:00
epriestley
04c07a7a7b Remove the developer-specific CSRF help in phabricator_form()
Summary:
Fixes T4802. For context, see T1921.

Originally (in T1921), a developer ran into an issue where rendering `phabricator_form()` with an absolute URI confusingly dropped CSRF tokens, and it wasn't obvious why. This is a security measure, but at the time it wasn't very clear how all the pieces fit together. To make it more clear, we:

  # expanded the exception text in developer mode to include a description of this issue; and
  # added an exception in developer mode when rendering a form like this.

However, (2) causes some undesirable interactions for file downloads. In particular, if:

  - developer mode is on; and
  - there's no alternate file domain configured; and
  - you try to download a file...

...we produce CDN URIs that are fully-qualified, and you get the exception from (2) above.

This is kind of a mess, and producing fully-qualified CDN URIs in all cases is simple and clear and desirable. To resolve this, just revert (2). We still have the clarification from (1) above and this hasn't caused further issues, so I think that's sufficient. This is a rare issue anyway and not particularly serious or error prone (at worst, a bit confusing and annoying, but hopefully easy to understand and resolve after the changes in (1)).

Test Plan: With develper mode and no alternate file domain, downloaded files from Files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4802

Differential Revision: https://secure.phabricator.com/D8776
2014-04-15 10:18:41 -07:00
epriestley
cb545856a9 Make task queue more robust against long-running tasks
Summary:
See discussion in D8773. Three small adjustments which should help prevent this kind of issue:

  - When queueing followup tasks, hold them on the worker until we finish the task, then queue them only if the work was successful.
  - Increase the default lease time from 60 seconds to 2 hours. Although most tasks finish in far fewer than 60 seconds, the daemons are generally stable nowadays and these short leases don't serve much of a purpose. I think they also date from an era where lease expiry and failure were less clearly distinguished.
  - Increase the default wait-after-failure from 60 seconds to 5 minutes. This largely dates from the MetaMTA era, where Facebook ran services with high failure rates and it was appropriate to repeatedly hammer them until things went through. In modern infrastructure, such failures are rare.

Test Plan:
  - Verified that tasks queued properly after the main task was updated.
  - Verified that leases default to 7200 seconds.
  - Intentionally failed a task and verified default 300 second wait before retry.
  - Removed all default leases shorter than 7200 seconds (there was only one).
  - Checked all the wait before retry implementations for anything much shorter than 5 minutes (they all seem reasonable).

Reviewers: btrahan, sowedance

Reviewed By: sowedance

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8774
2014-04-15 08:42:02 -07:00
Peng Li
6a4f126000 Give the commitownersparser a little more time
Summary:
Recently we see issues with huge commits (branch cuts for www) where people received hundreds of emails for the same commit. By checking all the active and archived tasks related to such commits, I saw the following pattern:
 - The commit itself is marked as importStatus = 15 which means all the processing was actually done;
 - In archived tasks, I see one PhabricatorRepositorySvnCommitMessageParserWorker, one PhabricatorRepositorySvnCommitChangeParserWorker, followed by many PhabricatorRepositoryCommitHeraldWorker, which means that the PhabricatorRepositoryCommitOwnersWorker (who schedule those herald tasks) was never done;
 - PhabricatorRepositoryCommitOwnersWorker is always active (for days) with failureCount = 0;
 - In daemon log I see a lot of lease expire exception for PhabricatorRepositoryCommitOwnersWorker.
So to me it looks like the following happened:
 - Everything is fine until we schedule the PhabricatorRepositoryCommitOwnersWorker
 - PhabricatorRepositoryCommitOwnersWorker actually successfully finished but its running time exceed 60s. Before it finishes, it scheduled the PhabricatorRepositoryCommitHeraldWorker task
 - When we try to archive it, the lease expiration exception happened. As a result, it stayed active and will be picked up immediately since it is in the head of the queue
 - The two steps above repeat forever until we kill it
I am not sure why we want to check lease expiration when we are archiving the task. For now I am giving the worker a little more time since parsing half million affected path needs some time..

Test Plan: Patched in our production and it worked.

Reviewers: lifeihuang, JoelB, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8773
2014-04-14 15:52:02 -07:00