Description:
I'm trying to "arc diff" against D16324 to update it, and there's an Owners package on it which is restricted from view. When I run "arc diff", I get:
git@ip-10-124-2-40:~/phabricator> EDITOR=vim ~/arcanist/bin/arc diff 9da15fd7ab7071e3b7361463229abd4e918d9d94 --update D16324 --trace ARGV '/srv/phabricator/arcanist/bin/../scripts/arcanist.php' 'diff' '9da15fd7ab7071e3b7361463229abd4e918d9d94' '--update' 'D16324' '--trace' LOAD Loaded "phutil" from "/srv/phabricator/libphutil/src". LOAD Loaded "arcanist" from "/srv/phabricator/arcanist/src". Config: Reading user configuration file "/srv/phabricator/.arcrc"... Config: Did not find system configuration at "/etc/arcconfig". Working Copy: Reading .arcconfig from "/srv/phabricator/phabricator/.arcconfig". Working Copy: Path "/srv/phabricator/phabricator" is part of `git` working copy "/srv/phabricator/phabricator". Working Copy: Project root is at "/srv/phabricator/phabricator". Config: Did not find local configuration at "/srv/phabricator/phabricator/.git/arc/config". Loading phutil library from '/srv/phabricator/phabricator/src'... >>> [0] <conduit> user.whoami() <bytes = 117> >>> [1] <http> https://secure.phabricator.com/api/user.whoami <<< [1] <http> 770,281 us <<< [0] <conduit> 771,102 us >>> [2] <exec> $ git diff --no-ext-diff --no-textconv --raw 'HEAD' -- >>> [3] <exec> $ git ls-files --others --exclude-standard <<< [3] <exec> 41,954 us <<< [2] <exec> 43,256 us >>> [4] <exec> $ git diff-files --name-only <<< [4] <exec> 19,677 us >>> [5] <event> diff.didCollectChanges <listeners = 0> <<< [5] <event> 114 us >>> [6] <conduit> differential.query() <bytes = 149> >>> [7] <http> https://secure.phabricator.com/api/differential.query <<< [7] <http> 243,793 us <<< [6] <conduit> 244,106 us >>> [8] <conduit> differential.getcommitmessage() <bytes = 172> >>> [9] <http> https://secure.phabricator.com/api/differential.getcommitmessage <<< [9] <http> 363,029 us <<< [8] <conduit> 363,368 us >>> [10] <conduit> differential.parsecommitmessage() <bytes = 1,526> >>> [11] <http> https://secure.phabricator.com/api/differential.parsecommitmessage <<< [11] <http> 251,126 us <<< [10] <conduit> 251,504 us [2016-07-28 04:17:12] EXCEPTION: (ArcanistDifferentialCommitMessageParserException) Error parsing field "Subscribers": The objects you have listed include objects which do not exist (PHID-OPKG-gm6ozazyms6q6i22gyam). at [<arcanist>/src/differential/ArcanistDifferentialCommitMessage.php:54] arcanist(head=f1c45a3323ae20eefe29c0a22c7923fe8b151bbf, ref.master=e5946ed1a5fa), phabricator(head=arcpatch-D16324, ref.master=7d87781f0bcf, ref.arcpatch-D16324=079d5b56c1c2), phutil(head=5fd2cf9d5ddd38424a54a8fba02398d527639970, ref.master=ecba5f543f25) #0 ArcanistDifferentialCommitMessage::pullDataFromConduit(ConduitClient) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1795] #1 ArcanistDiffWorkflow::getCommitMessageFromRevision(string) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1527] #2 ArcanistDiffWorkflow::buildCommitMessage() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:469] #3 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:394]
Replication Steps:
- Presumably create an account on this instance which doesn't have access to whatever that owners package is.
- Create a diff initially, and see the package get added by the Owners application after diff creation.
- Delete the branch locally.
- Some time later, use "arc patch Dxxxx" to get the branch back.
- Attempt to run "arc diff" again to update the diff and see Arcanist crash.
It should be noted that I ran "arc patch D16324" to get my branch back so I could work on it. That means that rather than the commit message I'm working off is actually:
commit e9cfb22ccf05718d2e3f3fb8e9acb528a6ae25f8
Author: June Rhodes <juner@pageuppeople.com>
Date: Thu Jul 28 04:12:03 2016 +0000
Handle crash when build plan has mix of valid and invalid build steps
Summary:
Resolves T11373. Currently Harbormaster gracefully handles a scenario where the build step implenementation is missing for a step. However, dependency calculations on valid build steps may cause them to attempt to access the implementations of other invalid build steps (in particular, when attempting to validate whether their input artifacts are available from other build steps).
When this happens, you get a blunt page crash like this:
{F1737168}
Because of this page crash, there's no way to fix the invalid configuration from the web UI, and the user has to go and remove things from the database manually.
Instead, after this change, the page doesn't crash, but shows "Unable to calculate dependencies for this build step" when this issue occurs.
Test Plan: Had a page with a mix of valid and invalid build steps, where the valid build step had an input artifact. Observed the crash. Applied this patch and saw the crash go away.
Reviewers: #blessed_reviewers, epriestley
Subscribers: yelirekim, Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11373
Differential Revision: https://secure.phabricator.com/D16324Expected Result:
Arcanist doesn't crash.
Actual Result:
Arcanist crashes.