If you arc which and have arc patch Dnnn'd someone else's revision into your working copy, we say "no matching revision". Other workflows will recognize the revision, e.g. arc diff.
Generally, on some workflows we exclude revisions you don't own. This is confusing, and we should avoid it; instead, we should show them but annotate them. e.g.:
D30 blah blah blah (owned by alincoln)
...or similar.
This has been the source of confusion a few times recently. My general attack on this would be:
A notable exception here is probably arc list, which should be filtering by author. Some of the others may not be totally clear-cut, either.
Hmmm, I can't repro the issue as described. See PS at the end of this comment where this works okay, albeit perhaps annotation could be better.
Moreover, I did a grep with some -A action and looking through *ALL* the callsites the only one that specifies authors is in the list workflow. ...so AFAIK this works okay already?
PS
12:37:56 ~/Dropbox/code/phalanx/src/applications (master)
~> arc patch D8685
Created and checked out branch arcpatch-D8685.
Checking patch src/__phutil_library_map.php...
Checking patch src/applications/people/controller/PhabricatorPeopleProfileController.php...
Checking patch src/view/layout/PhabricatorActionListView.php...
Checking patch src/view/layout/PhabricatorActionView.php...
Checking patch webroot/rsrc/css/layout/phabricator-action-list-view.css...
Applied patch src/phutil_library_map__.php cleanly.
Applied patch src/applications/people/controller/PhabricatorPeopleProfileController.php cleanly.
Applied patch src/view/layout/PhabricatorActionListView.php cleanly.
Applied patch src/view/layout/PhabricatorActionView.php cleanly.
Applied patch webroot/rsrc/css/layout/phabricator-action-list-view.css cleanly.
OKAY Successfully committed patch.
12:38:38 ~/Dropbox/code/phalanx/src/applications (arcpatch-D8685)
~> arc which
REPOSITORY
To identify the repository associated with this working copy, arc followed this process:
Configuration value "repository.callsign" is empty. Configuration value "project.name" is set to "phabricator"; this project is associated with the "P" repository. Found a unique matching repository.
This working copy is associated with the P repository.
RELATIVE COMMIT
If you run 'arc diff', changes between the commit:
af0b749369c2f044 Fix many lies in the "User Roles" document
...and the current working copy state will be sent to Differential, because
it is the merge-base of 'origin/master' and HEAD, as specified in
'.git/arc/default-relative-commit'.
You can see the exact changes that will be sent by running this command:
$ git diff af0b749369c2f044..HEADThese commits will be included in the diff:
8ab668bf1b76b0a5 Rough draft for action submenus
MATCHING REVISIONS
These Differential revisions match the changes in this working copy:
D8685 (epriestley) Rough draft for action submenus
Reason: Commit message for '8ab668bf1b76b0a5' has explicit 'Differential Revision'.Since exactly one revision in Differential matches this working copy, it will
be updated if you run 'arc diff'.
This is what happens for me:
~/git/puppet (master)$ arc patch D4933
Created and checked out branch arcpatch-D4933.
Checking patch modules/scribe/files/monitor/scribe.py...
Checking patch modules/scribe/manifests/init.pp...
Checking patch modules/scribe/manifests/monitor.pp...
Checking patch modules/scribe/templates/logr-client.conf.erb...
Checking patch modules/scribe/templates/logr-server.conf.erb...
Applied patch modules/scribe/files/monitor/scribe.py cleanly.
Applied patch modules/scribe/manifests/init.pp cleanly.
Applied patch modules/scribe/manifests/monitor.pp cleanly.
Applied patch modules/scribe/templates/logr-client.conf.erb cleanly.
Applied patch modules/scribe/templates/logr-server.conf.erb cleanly.
Submodule path 'files': checked out 'ef60b24e09dd8233b621df67dd072ffadadbcec8'
OKAY Successfully committed patch.
~/git/puppet (arcpatch-D4933)$ arc which
REPOSITORY
To identify the repository associated with this working copy, arc followed this process:
Configuration value "repository.callsign" is empty.
Configuration value "project.name" is set to "ops/puppet"; this project
is associated with the "PUP" repository.
Found a unique matching repository.
This working copy is associated with the PUP repository.
RELATIVE COMMIT
If you run 'arc diff', changes between the commit:
672fd102f7ef6a21 add a utility to fetch facts from puppetdb
...and the current working copy state will be sent to Differential, because
it is the merge-base of 'origin/master' and HEAD, as specified in
'.git/arc/default-relative-commit'.
You can see the exact changes that will be sent by running this command:
$ git diff 672fd102f7ef6a21..HEAD
These commits will be included in the diff:
5eef98c2f6e7153b set up scribe monitoring, refs T12654
MATCHING REVISIONS
These Differential revisions match the changes in this working copy:
(No revisions match.)
Since there are no revisions in Differential which match this working copy, a
new revision will be created if you run 'arc diff'.Sorry, a contributor fixed part of this yesterday and I didn't catch that it wasn't linked. Let me hook that up...
We can probably still improve behavior for other sites (for instance, I think arc land --hold will fail to identify the revision after arc patch, when it should instead say "this branch has Dxxxx, but you aren't the author -- land alincoln's revision?") but that solved the arc which case.
Yup, D8690 fixes it:
MATCHING REVISIONS
These Differential revisions match the changes in this working copy:
D4933 (drigh) set up scribe monitoring, refs T12654
Reason: Commit message for 'ef74edef440b1d41' has explicit 'Differential Revision'.Okay, so the better grep is on loadWorkingCopyDifferentialRevisions, which is a wrapper on differential.query. Here are the call sites
// Finding what to "amend"
ArcanistAmendWorkflow.php:94: $in_working_copy = $repository_api->loadWorkingCopyDifferentialRevisions(
ArcanistAmendWorkflow.php-95- $this->getConduit(),
ArcanistAmendWorkflow.php-96- array(
ArcanistAmendWorkflow.php-97- 'authors' => array($this->getUserPHID()),
ArcanistAmendWorkflow.php-98- 'status' => 'status-any',
ArcanistAmendWorkflow.php-99- ));
// shouldAmend() implementation
ArcanistBaseWorkflow.php:968: $in_working_copy = $api->loadWorkingCopyDifferentialRevisions(
ArcanistBaseWorkflow.php-969- $this->getConduit(),
ArcanistBaseWorkflow.php-970- array(
ArcanistBaseWorkflow.php-971- 'authors' => array($this->getUserPHID()),
ArcanistBaseWorkflow.php-972- 'status' => 'status-open',
ArcanistBaseWorkflow.php-973- ));
// Finding what revision to commit
ArcanistCommitWorkflow.php:78: $revisions = $repository_api->loadWorkingCopyDifferentialRevisions(
ArcanistCommitWorkflow.php-79- $this->getConduit(),
ArcanistCommitWorkflow.php-80- array(
ArcanistCommitWorkflow.php-81- 'authors' => array($this->getUserPHID()),
ArcanistCommitWorkflow.php-82- 'status' => 'status-accepted',
ArcanistCommitWorkflow.php-83- ));
// Finding what revision to update, if any.
ArcanistDiffWorkflow.php:1458: $revisions = $repository_api->loadWorkingCopyDifferentialRevisions(
ArcanistDiffWorkflow.php-1459- $this->getConduit(),
ArcanistDiffWorkflow.php-1460- array(
ArcanistDiffWorkflow.php-1461- 'authors' => array($this->getUserPHID()),
ArcanistDiffWorkflow.php-1462- 'status' => 'status-open',
ArcanistDiffWorkflow.php-1463- ));
...I guess basically all of them could be loosened? right now they tend to have some checks on 0 (create), 1 (success), > 1 (error!), and could have an additional "you not the author" bit. the shouldAmend() implementation one wouldn't have additional error messaging but the other ones would all add something like in D8709.
This is pretty arbitrary / gut-feel, but of those four I think the first one is the most worth loosening (I've actually hit it) and the other three less so (they're more involved, and I don't think users have hit them). Maybe do that first one and we'll call this done until we see more reports? I think these three cases (arc which, arc land, arc amend) are overwhelmingly more common (and more clearly better as find-by-any-author + prompt/show) than the other cases.