Arc land and wrong state

Description: arc land states that the revision is not accepted when it is Accepted.

This behaviour was not seen in the previous versions of arcanist.

Reproduction Instructions

  1. create a test branch
  2. push test branch to remote origin
  3. create revision based of test branch
  4. accept revision
  5. run arc land

Expected: one question is asked “Land these changes?” and revision can be landed onto local/remote test branch

Actual: two questions are asked: “Land these changes?” and “Land 1 revision(s) in the wrong state?” when the revision is in the right state.

Phabricator/Arcanist Version

Cient:

arcanist 091aebe0149a6a905369b9fa33dac5dbc2eaddbb (8 Jun 2020)

Server:

Library Version Date Branchpoint
phabricator 18a099b42168 Tue, Jun 9 36075f6ce525
arcanist 38d7a07d3722 Mon, May 25
Binary Version Path
php 7.2.20-1+0~20190710.23+debian9~1.gbp2428c5 fpm-fcgi
diff 3.5 /usr/bin/diff
git 2.11.0 /usr/bin/git
hg Not Available
pygmentize 2.2.0 /usr/bin/pygmentize
svn Not Available

Screen copies:

Phabricator

Repository: rPIM Kotryna/Product Information Manager
Branch: f-separete_deployment_and_test_ci_scripts (branched from stage)
Lint: Lint OK
Unit: No Unit Test Coverage

Build Status

Arcanist:

 ARGV  /usr/local/bin/arc land --trace
>>> [1] (+0) <http> https://phabricator.kotryna.lt/api/user.whoami
>>> [2] (+0) <conduit> user.whoami() <bytes = 117>
<<< [2] (+260) <conduit> 259,980 us
<<< [1] (+261) <http> 261,723 us
 STRATEGY  Merging with "squash" strategy, the default strategy.
>>> [3] (+265) <exec> $ git symbolic-ref --quiet HEAD
<<< [3] (+269) <exec> 3,845 us
 SOURCE  Landing the current branch, "f-separete_deployment_and_test_ci_scripts".
>>> [4] (+270) <exec> $ git rev-parse --verify f-separete_deployment_and_test_ci_scripts
<<< [4] (+279) <exec> 8,239 us
>>> [5] (+281) <exec> $ git rev-parse --symbolic-full-name f-separete_deployment_and_test_ci_scripts@{upstream}
<<< [5] (+285) <exec> 3,942 us
>>> [6] (+285) <exec> $ git rev-parse --symbolic-full-name stage@{upstream}
<<< [6] (+289) <exec> 3,381 us
 ONTO REMOTE  Remote "origin" was selected by following tracking branches upstream to the closest remote.
>>> [7] (+290) <exec> $ git remote get-url --push -- origin
<<< [7] (+299) <exec> 8,852 us
>>> [8] (+300) <exec> $ git rev-parse --symbolic-full-name f-separete_deployment_and_test_ci_scripts@{upstream}
<<< [8] (+303) <exec> 3,673 us
>>> [9] (+304) <exec> $ git rev-parse --symbolic-full-name stage@{upstream}
<<< [9] (+307) <exec> 3,397 us
 ONTO TARGET  Landing onto target "stage", selected by following tracking branches upstream to the closest remote branch.
 INTO REMOTE  Will merge into remote "origin" by default, because this is the remote the change is landing onto.
 INTO TARGET  Will merge into target "stage" by default, because this is the "onto" target.
>>> [10] (+310) <exec> $ git rev-parse --verify refs/remotes/origin/stage
<<< [10] (+317) <exec> 7,014 us
 FETCH  Fetching "stage" from remote "origin"...

  $   git fetch --no-tags --quiet -- origin stage

>>> [11] (+318) <exec> $ git fetch --no-tags --quiet -- origin stage
<<< [11] (+1,657) <exec> 1,338,322 us

>>> [12] (+1,657) <exec> $ git rev-parse --verify refs/remotes/origin/stage
<<< [12] (+1,662) <exec> 4,483 us
 INTO COMMIT  Preparing merge into "stage" from remote "origin", at commit "00ef2bd9b078".
>>> [13] (+1,662) <exec> $ git log 8cbe839783da766f75d6487f81b5616c2d5430ba --not 00ef2bd9b0789efa9b97c8c39fd671941dfca82e --format='%H%x00%P%x00%s%x00'
<<< [13] (+1,666) <exec> 3,965 us
>>> [14] (+1,668) <http> https://phabricator.kotryna.lt/api/differential.query
>>> [15] (+1,668) <conduit> differential.query() <bytes = 243>
>>> [16] (+1,669) <exec> $ git log -n1 --format='%s%n%n%b' ec1bf666bf8eaace01250e7f7f06ee2599481906 --
>>> [17] (+1,670) <exec> $ git log -n1 --format='%s%n%n%b' 385e91cf6d6ca5bdbf7c63765ef1625a8c5a28fa --
>>> [18] (+1,672) <exec> $ git log -n1 --format='%s%n%n%b' 8cbe839783da766f75d6487f81b5616c2d5430ba --
<<< [16] (+1,674) <exec> 4,844 us
<<< [17] (+1,675) <exec> 4,260 us
<<< [18] (+1,680) <exec> 8,233 us
>>> [19] (+1,682) <mystery> 
>>> [20] (+1,682) <conduit> differential.revision.search() <bytes = 140>
<<< [15] (+1,825) <conduit> 157,211 us
<<< [14] (+1,826) <http> 157,762 us
<<< [20] (+1,905) <conduit> 223,068 us
<<< [19] (+1,905) <mystery> 223,679 us
 LANDING  These changes will land:
>>> [21] (+1,907) <exec> $ tput cols > /tmp/apgzx27w9igo4kcw/19246-fGlGvN
<<< [21] (+1,911) <exec> 3,369 us

  *   D126 separate ant builds scripts to build test ci and deploy
        ec1bf666bf8e  separate ant builds scripts to build test ci and deploy
        385e91cf6d6c  add test suffix so build and test environments can have…
        8cbe839783da  add production deployment

 >>>  Land these changes? [y/N/?] y

 <!> 1 REVISION(S) ARE NOT ACCEPTED 
You are landing 1 revision(s) which are not in state "Accepted", indicating
that they have not been accepted by reviewers. Normally, you should land
changes only once they have been accepted. These revisions are in the wrong
state:

  *   D126 separate ant builds scripts to build test ci and deploy
        Status: Accepted

 >>>  Land 1 revision(s) in the wrong state? [y/N/?] 

This looks like a bug, but I can’t immediately reproduce it locally.

Can you show me the output of this command?

$ echo '{"constraints":{"ids":[126]}}' | arc call-conduit differential.revision.search --

(If you’ve already dealt with this revision so it’s in a different state, change the 126 in that command to the new revision number the next time you hit this issue.)

You can censor any sensitive information, I’m mostly interested in this part:

I also had this issue several times. The output you requested:
"status": { "value": "accepted", "name": "Accepted", "closed": false, "color.ansi": "green" }

(I suspect this is fixed by https://secure.phabricator.com/D21355, but that change hasn’t landed yet.)

{

“error”: null,
“errorMessage”: null,
“response”: {
“data”: [
{
“id”: 124,
“type”: “DREV”,
“phid”: “PHID-DREV-4iohljaapc32hiyfs45y”,
“fields”: {
“title”: “[sylius] build/ci/deploy”,
“uri”: “https://xxxxxx.yyy/D124”,
“authorPHID”: “PHID-USER-dkcf6m6xny6eptpzxixn”,
“status”: {
“value”: “accepted”,
“name”: “Accepted”,
“closed”: false,
“color.ansi”: “green”
},
“repositoryPHID”: “PHID-REPO-fsvdalo2ekhbig3tgbnq”,
“diffPHID”: “PHID-DIFF-tz4lgi7yanrimmlbbxer”,
“summary”: “test ci parameters, build script and stage deployment”,
“testPlan”: “n/a”,
“isDraft”: false,
“holdAsDraft”: false,
“dateCreated”: 1591363018,
“dateModified”: 1592313030,
“policy”: {
“view”: “users”,
“edit”: “users”
}
},
“attachments”: []
}
],
“maps”: [],
“query”: {
“queryKey”: null
},
“cursor”: {
“limit”: 100,
“after”: null,
“before”: null,
“order”: null
}
}
}

I look forward to the fix then.

Hello,

arcanist 33dfa859d8e68a66559c15c78cea4da19204653b (12 Jun 2020)

Another diff had the same:

echo '{"constraints":{"ids":[149]}}' | arc call-conduit differential.revision.search --
{
  "error": null,
  "errorMessage": null,
  "response": {
    "data": [
      {
        "id": 149,
        "type": "DREV",
        "phid": "PHID-DREV-k7aabqoqft6rolaaoywb",
        "fields": {
          "title": "add database dumps loading for testing and deployment",
          "uri": "https://aaa.xxx.yyy/D149",
          "authorPHID": "PHID-USER-dkcf6m6xny6eptpzxixn",
          "status": {
            "value": "accepted",
            "name": "Accepted",
            "closed": false,
            "color.ansi": "green"
          },
          "repositoryPHID": "PHID-REPO-frrjhzwggu6htawlpizt",
          "diffPHID": "PHID-DIFF-jgbqtibvb2cxep2mixly",
          "summary": "",
          "testPlan": "jenkins",
          "isDraft": false,
          "holdAsDraft": false,
          "dateCreated": 1593421860,
          "dateModified": 1593430932,
          "policy": {
            "view": "users",
            "edit": "users"
          }
        },
        "attachments": []
      }
    ],
    "maps": [],
    "query": {
      "queryKey": null
    },
    "cursor": {
      "limit": 100,
      "after": null,
      "before": null,
      "order": null
    }
  }
}