Add dependencies to releeph
ClosedPublic
Actions

Authored by • sowedance on Jan 13 2014, 4:25 AM.

Details

Reviewers

epriestley
lifeihuang
JoelB

Group Reviewers

Blessed Reviewers

Commits

Restricted Diffusion Commit
rP60a6ba2b2501: Add dependencies to releeph

Summary

Add the 'Depends On' field to releeph requests. This will help the release engineers to be aware of the dependencies and make sure pick them altogether.

Test Plan

Check sandbox. This field shows up when a revision has some dependencies.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

Your arc is out of date, at a minimum, because that error wording no longer exists at HEAD. My guess is that there are several copies of arcanist/ and libphutil/ at play here. An up to date arc will give you a more specific error message, pointing at the copy of libphutil/ in question.

Specifically (using 3-year-old-paths), I'm guessing that when you run arc it's loading /home/engshare/devtools/libphutil, which is wildly out of date, while your sandbox copy of Phabricator is using /home/pengli/sandbox/libphutil or whatever.

Two minor nitpick inlines. The big one here is the guarantee that dependent handles load -- I don't think anything is actually loading them?

src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php
11	This should be wrapped in `pht()` (which is like `fbt()`) so it can be translated.
21	Is there any guarantee this actually loads the dependent revisions' handles? I don't see where in the code we can be sure about this.
24	Consider using `clone` on the object before making hacky mutations like this. That will prevent this from propagating to other sites which might use the same handle.

Yeah, I can fatal this locally with some slight tweaks: the handles are not guaranteed to load.

[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP Stack trace:"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   1. {main}() /INSECURE/devtools/phabricator/webroot/index.php:0"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   2. ReleephBranchViewController->processRequest() /INSECURE/devtools/phabricator/webroot/index.php:84"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   3. AphrontController->delegateToController() /INSECURE/devtools/phabricator/src/applications/releeph/controller/branch/ReleephBranchViewController.php:27"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   4. PhabricatorApplicationSearchController->processRequest() /INSECURE/devtools/phabricator/src/aphront/AphrontController.php:52"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   5. PhabricatorApplicationSearchController->processSearchRequest() /INSECURE/devtools/phabricator/src/applications/search/controller/PhabricatorApplicationSearchController.php:85"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   6. PhabricatorController->buildApplicationPage() /INSECURE/devtools/phabricator/src/applications/search/controller/PhabricatorApplicationSearchController.php:272"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   7. AphrontPageView->render() /INSECURE/devtools/phabricator/src/applications/base/controller/PhabricatorController.php:222"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   8. PhabricatorStandardPageView->willRenderPage() /INSECURE/devtools/phabricator/src/view/page/AphrontPageView.php:46"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP   9. PhabricatorBarePageView->willRenderPage() /INSECURE/devtools/phabricator/src/view/page/PhabricatorStandardPageView.php:109"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  10. phutil_implode_html() /INSECURE/devtools/phabricator/src/view/page/PhabricatorBarePageView.php:58"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  11. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:146"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  12. AphrontView->producePhutilSafeHTML() /INSECURE/devtools/libphutil/src/markup/render.php:85"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  13. AphrontSideNavFilterView->render() /INSECURE/devtools/phabricator/src/view/AphrontView.php:159"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  14. AphrontSideNavFilterView->renderFlexNav() /INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:184"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  15. phutil_tag() /INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:297"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  16. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:65"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  17. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:107"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  18. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:107"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  19. AphrontView->producePhutilSafeHTML() /INSECURE/devtools/libphutil/src/markup/render.php:85"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  20. ReleephRequestHeaderListView->render() /INSECURE/devtools/phabricator/src/view/AphrontView.php:159"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  21. phutil_tag() /INSECURE/devtools/phabricator/src/applications/releeph/view/request/header/ReleephRequestHeaderListView.php:67"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  22. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:65"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  23. phutil_escape_html() /INSECURE/devtools/libphutil/src/markup/render.php:107"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  24. AphrontView->producePhutilSafeHTML() /INSECURE/devtools/libphutil/src/markup/render.php:85"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  25. ReleephRequestHeaderView->render() /INSECURE/devtools/phabricator/src/view/AphrontView.php:159"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  26. ReleephRequestHeaderView->renderFields() /INSECURE/devtools/phabricator/src/applications/releeph/view/request/header/ReleephRequestHeaderView.php:51"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  27. ReleephRequestHeaderView->renderOneField() /INSECURE/devtools/phabricator/src/applications/releeph/view/request/header/ReleephRequestHeaderView.php:118"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: PHP  28. ReleephDependsOnFieldSpecification->renderValueForHeaderView() /INSECURE/devtools/phabricator/src/applications/releeph/view/request/header/ReleephRequestHeaderView.php:156"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "NOTICE: PHP message: >>> UNRECOVERABLE FATAL ERROR <<<"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: ""
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "Call to a member function setStatus() on a non-object"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: ""
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "/INSECURE/devtools/phabricator/src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php:25"
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: ""
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: ""
[12-Jan-2014 21:35:00] WARNING: [pool www] child 22684 said into stderr: "┻━┻ ︵ ¯_(ツ)_/¯ ︵ ┻━┻"

The easiest fix is probably to use PhabricatorHandleQuery to just load them explicitly inline, although this will create an N+1 query issue and impact performance. The ideal upstream approach here is T3718, which would align these custom fields with general purpose custom fields which have mechanisms for bulk loading handles.

Updated based on comments

Looks good on my end, modulo one inline.

{F101545}

src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php
26	In Zend PHP 5.5, this is a syntax error. It can be resolved by using `id()`. I don't know offhand if this is specific to HHVM or not, but we support old versions of PHP in the upstream. I'll just fix this in the pull.