Page MenuHomePhabricator

Add dependencies to releeph
ClosedPublic

Authored by sowedance on Jan 13 2014, 4:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 11 2024, 3:40 PM
Unknown Object (File)
Feb 1 2024, 11:38 PM
Unknown Object (File)
Jan 29 2024, 12:00 AM
Unknown Object (File)
Jan 23 2024, 5:12 AM
Unknown Object (File)
Jan 2 2024, 12:12 PM
Unknown Object (File)
Dec 4 2023, 12:00 AM
Unknown Object (File)
Nov 25 2023, 5:12 PM
Unknown Object (File)
Oct 27 2023, 8:40 PM

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.

sowedance updated this revision to Unknown Object (????).Jan 14 2014, 2:15 AM

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.