Page MenuHomePhabricator

Differential commit message parser immediately rejects objects the user can't see, but Herald can put them there
Closed, ResolvedPublic

Assigned To
None
Authored By
michaeljs1990
Jul 18 2016, 10:29 PM
Referenced Files
F1728606: Screen Shot 2016-07-18 at 6.28.27 PM.png
Jul 18 2016, 10:29 PM
Tokens
"Haypence" token, awarded by chad."Hungry Hippo" token, awarded by epriestley.

Description

Phabricator currently support a great workflow for stopping interns from landing diffs.

  1. Create a project only I can see.
  2. Observe intern creating a diff that I have been added to.
  3. Having seen the code and not agreeing with it although I can find no technical fault I apply my secret project. Which looks like this for the other user.

Screen Shot 2016-07-18 at 6.28.27 PM.png (778×1 px, 111 KB)

  1. Ask intern to fix a lint error and re diff.

When they run arc diff they will get this error.

 ARGV  '/usr/share/arcanist/bin/../scripts/arcanist.php' 'diff' '--trace'
 LOAD  Loaded "phutil" from "/usr/share/libphutil/src".
 LOAD  Loaded "arcanist" from "/usr/share/arcanist/src".
Config: Reading user configuration file "/home/schuettm/.arcrc"...
Config: Reading system configuration file "/etc/arcconfig"...
Working Copy: Reading .arcconfig from "/home/schuettm/webroot/hudson/.arcconfig".
Working Copy: Path "/home/schuettm/webroot/hudson" is part of `git` working copy "/home/schuettm/webroot/hudson".
Working Copy: Project root is at "/home/schuettm/webroot/hudson".
Config: Did not find local configuration at "/home/schuettm/webroot/hudson/.git/arc/config".
Loading phutil library from '/home/schuettm/webroot/integrator/client/source'...
Loading phutil library from '/home/schuettm/webroot/hudson/source/phab'...
>>> [0] <conduit> user.whoami() <bytes = 117>
>>> [1] <http> http://phabricator.dev/api/user.whoami
<<< [1] <http> 235,474 us
<<< [0] <conduit> 236,233 us
>>> [2] <exec> $ git diff --no-ext-diff --no-textconv --raw 'HEAD' --
>>> [3] <exec> $ git ls-files --others --exclude-standard
<<< [2] <exec> 4,827 us
<<< [3] <exec> 4,743 us
>>> [4] <exec> $ git diff-files --name-only
<<< [4] <exec> 2,259 us
>>> [5] <event> diff.didCollectChanges <listeners = 0>
<<< [5] <event> 87 us
>>> [6] <exec> $ git rev-parse --verify HEAD^
<<< [6] <exec> 15,505 us
>>> [7] <exec> $ git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}'
<<< [7] <exec> 2,059 us
>>> [8] <exec> $ git cat-file -t 'origin/master'
<<< [8] <exec> 13,357 us
>>> [9] <exec> $ git merge-base 'origin/master' HEAD
<<< [9] <exec> 2,197 us
>>> [10] <exec> $ git rev-parse 'HEAD'
<<< [10] <exec> 1,963 us
>>> [11] <exec> $ git log --first-parent --format=medium 'c90b10d91e8dd3301be5e7af6a669fc5c53fc549'..'af6640a87e49fe488c1cc18d6332733841b1e3e0'
<<< [11] <exec> 2,293 us
>>> [12] <conduit> differential.query() <bytes = 236>
>>> [13] <http> http://phabricator.dev/api/differential.query
<<< [13] <http> 273,756 us
<<< [12] <conduit> 273,983 us
>>> [14] <conduit> differential.query() <bytes = 146>
>>> [15] <http> http://phabricator.dev/api/differential.query
<<< [15] <http> 288,834 us
<<< [14] <conduit> 289,060 us
>>> [16] <conduit> differential.getcommitmessage() <bytes = 169>
>>> [17] <http> http://phabricator.dev/api/differential.getcommitmessage
<<< [17] <http> 294,728 us
<<< [16] <conduit> 294,963 us
>>> [18] <conduit> differential.parsecommitmessage() <bytes = 341>
>>> [19] <http> http://phabricator.dev/api/differential.parsecommitmessage
<<< [19] <http> 247,195 us
<<< [18] <conduit> 247,403 us

[2016-07-18 22:26:56] EXCEPTION: (ArcanistDifferentialCommitMessageParserException) Error parsing field "Tags": The objects you have listed include objects which do not exist (PHID-PROJ-2h3zn6kuntthz6v6weoj). at [<arcanist>/src/differential/ArcanistDifferentialCommitMessage.php:54]
arcanist(), integrator-client(), phab(), phutil()
  #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:392]

I ran across this about a month ago with a user who had a project they didn't have access to added to a diff however it did not occur to me that this will stop secret projects from monitoring users.

Event Timeline

michaeljs1990 renamed this task from Block user from landing diffs to differential.parsecommitmessage error parsing restricted projects.Jul 18 2016, 10:32 PM
michaeljs1990 added a project: Restricted Project.

Are you looking for alternative ways to block commits, or is this a bug report about "arc diff fails when revision has secret project tags?"

No this is just a general bug report since it's a fairly confusing error message for end users.

We can have this render "Tags: PHID-PROJ-abcdef" instead, which is an "improvement".

Where did the phid get to the user anyway? If it's arc diff --update, it shouldn't try to parse the comment, and if it's arc diff --create, it shouldn't have any tags?

I think arc actually calls differential.getcommitmessage and then passes the result right back to differential.parsecommitmessage to split it into fields. That could/should work differently than it does.

epriestley renamed this task from differential.parsecommitmessage error parsing restricted projects to Differential commit message parser immediately rejects objects the user can't see, but Herald can put them there.Jan 1 2017, 5:07 PM
epriestley triaged this task as Normal priority.