diff --git a/.github/workflows/merge.yml b/.github/workflows/tvmbot.yml similarity index 62% rename from .github/workflows/merge.yml rename to .github/workflows/tvmbot.yml index efbada4b00a4..c9d2cf71e6a7 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/tvmbot.yml @@ -1,5 +1,5 @@ -name: Merge +name: tvm-bot on: status: pull_request_review: @@ -12,16 +12,19 @@ concurrency: cancel-in-progress: true jobs: - maybe-merge: + run-tvm-bot: if: github.repository == 'apache/tvm' runs-on: ubuntu-20.04 + if: ${{ github.event.issue.pull_request }} steps: - uses: actions/checkout@v2 - - name: Merge if requested and possible + - name: Run tvm-bot env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TVM_BOT_JENKINS_TOKEN: ${{ secrets.TVM_BOT_JENKINS_TOKEN }} PR_NUMBER: ${{ github.event.issue.number }} + ISSUE_COMMENT: ${{ toJson(github.event.comment) }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | set -eux - python tests/scripts/github_mergebot.py --pr "$PR_NUMBER" --run-url "$RUN_URL" + python tests/scripts/github_tvmbot.py --pr "$PR_NUMBER" --run-url "$RUN_URL" --trigger-comment-json "$ISSUE_COMMENT" diff --git a/tests/python/ci/sample_prs/pr10786-badci.json b/tests/python/ci/sample_prs/pr10786-badci.json index b49899b86bca..7e9d10d0b648 100644 --- a/tests/python/ci/sample_prs/pr10786-badci.json +++ b/tests/python/ci/sample_prs/pr10786-badci.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { @@ -119,6 +119,7 @@ "commit": { "oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd" }, + "id": 123, "author": { "login": "kparzysz-quic" }, diff --git a/tests/python/ci/sample_prs/pr10786-changes-requested.json b/tests/python/ci/sample_prs/pr10786-changes-requested.json index 46b13a7f6c6c..24e261099a4f 100644 --- a/tests/python/ci/sample_prs/pr10786-changes-requested.json +++ b/tests/python/ci/sample_prs/pr10786-changes-requested.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { @@ -120,6 +120,7 @@ "commit": { "oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd" }, + "id": 123, "author": { "login": "kparzysz-quic" }, diff --git a/tests/python/ci/sample_prs/pr10786-co-authors.json b/tests/python/ci/sample_prs/pr10786-co-authors.json index a660c9d9b214..75f272825059 100644 --- a/tests/python/ci/sample_prs/pr10786-co-authors.json +++ b/tests/python/ci/sample_prs/pr10786-co-authors.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr10786-invalid-author.json b/tests/python/ci/sample_prs/pr10786-invalid-author.json index d19d6dad8a44..81b028e3196a 100644 --- a/tests/python/ci/sample_prs/pr10786-invalid-author.json +++ b/tests/python/ci/sample_prs/pr10786-invalid-author.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { @@ -114,6 +114,7 @@ "nodes": [ { "body": "@tvm-bot merge", + "id": 123, "updatedAt": "2022-03-25T22:13:50Z", "authorCanPushToRepository": false, "commit": { diff --git a/tests/python/ci/sample_prs/pr10786-merges.json b/tests/python/ci/sample_prs/pr10786-merges.json index c7b6940f0d5b..0226c8ab5245 100644 --- a/tests/python/ci/sample_prs/pr10786-merges.json +++ b/tests/python/ci/sample_prs/pr10786-merges.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free.\n\n\nThanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.\n\n\nPreviously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\n\n\ncc @someone\n\r\n\r\nCo-authored-by: Adam Straw \n\n\nThanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.\n\n", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr10786-missing-job.json b/tests/python/ci/sample_prs/pr10786-missing-job.json index 81be0ebe4795..13739b793fb5 100644 --- a/tests/python/ci/sample_prs/pr10786-missing-job.json +++ b/tests/python/ci/sample_prs/pr10786-missing-job.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr10786-nottriggered.json b/tests/python/ci/sample_prs/pr10786-nottriggered.json index 11c5976bd6e4..0da541c4342d 100644 --- a/tests/python/ci/sample_prs/pr10786-nottriggered.json +++ b/tests/python/ci/sample_prs/pr10786-nottriggered.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr10786-oldreview.json b/tests/python/ci/sample_prs/pr10786-oldreview.json index 27ba0e872918..1a2556cb6f5f 100644 --- a/tests/python/ci/sample_prs/pr10786-oldreview.json +++ b/tests/python/ci/sample_prs/pr10786-oldreview.json @@ -3,7 +3,7 @@ "body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw ", "state": "OPEN", "author": { - "login": "Lunderberg" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr11244-unauthorized-comment.json b/tests/python/ci/sample_prs/pr11244-unauthorized-comment.json index 206adc9a9eac..beafc05958b6 100644 --- a/tests/python/ci/sample_prs/pr11244-unauthorized-comment.json +++ b/tests/python/ci/sample_prs/pr11244-unauthorized-comment.json @@ -3,7 +3,7 @@ "body": "See [this thread ](https://discuss.tvm.apache.org/t/crt-add-platform-specific-pre-and-post-function-calls-in-crt-runtime/12723)for an explanation.", "state": "OPEN", "author": { - "login": "fPecc" + "login": "abc" }, "comments": { "pageInfo": { diff --git a/tests/python/ci/sample_prs/pr11267-no-review.json b/tests/python/ci/sample_prs/pr11267-no-review.json index 31577671f0b6..d2ad164673e5 100644 --- a/tests/python/ci/sample_prs/pr11267-no-review.json +++ b/tests/python/ci/sample_prs/pr11267-no-review.json @@ -3,7 +3,7 @@ "body": "This adds `/opt/sccache` to the PATH of each of the CI docker images so when cmake looks for a C compiler it will pick up the sccache wrapper by default. This fixes some issues where compiler invocations weren't being run though sccache. With this approach the invoker doesn't need to do anything specific to set up sccache.\n\nThis will require a follow up PR to update the Docker images and remove some of the sccache logic in `task_build.py`\n\n\n\ncc @Mousius @areusch", "state": "OPEN", "author": { - "login": "driazati" + "login": "abc" }, "comments": { "pageInfo": { @@ -15,6 +15,7 @@ "author": { "login": "areusch" }, + "id": 124, "updatedAt": "2022-05-11T16:54:32Z", "body": "just confirming--we can disable this when doing a local build, correct? what's the mechanism by which we do that?" }, @@ -23,6 +24,7 @@ "author": { "login": "driazati" }, + "id": 123, "updatedAt": "2022-05-11T18:46:54Z", "body": "@tvm-bot merge" } diff --git a/tests/python/ci/sample_prs/pr11276-no-review.json b/tests/python/ci/sample_prs/pr11276-no-review.json deleted file mode 100644 index 3f8459eb00f7..000000000000 --- a/tests/python/ci/sample_prs/pr11276-no-review.json +++ /dev/null @@ -1,157 +0,0 @@ -{ - "title": "[COMMUNITY] mikepapadim -> Reviewer", - "body": "Please join us to welcome Michalis Papadimitriou (@mikepapadim) as a new reviewer to TVM. Michalis has contributed a lot to BYOC and TensorRT backend.\r\n\r\n- [Commits History](https://github.com/apache/tvm/commits?author=mikepapadim)\r\n- [Code Review](https://github.com/apache/tvm/pulls?utf8=%E2%9C%93&q=reviewed-by:mikepapadim)\r\n- [Community Forum Summary](https://github.com/apache/tvm/commits?author=mikepapadim)", - "state": "OPEN", - "author": { - "login": "ZihengJiang" - }, - "comments": { - "pageInfo": { - "hasPreviousPage": false - }, - "nodes": [] - }, - "authorCommits": { - "nodes": [ - { - "commit": { - "authors": { - "nodes": [ - { - "name": "ZihengJiang", - "email": "ziheng@apache.org" - } - ] - } - } - } - ] - }, - "commits": { - "nodes": [ - { - "commit": { - "oid": "96075744cc687caafc131361d006c5967edddbc6", - "statusCheckRollup": { - "contexts": { - "pageInfo": { - "hasNextPage": false - }, - "nodes": [ - { - "name": "MacOS", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "CI" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391733373" - }, - { - "name": "cc-reviewers", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "PR" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391732791" - }, - { - "name": "cc-reviewers", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "PR" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391754960" - }, - { - "name": "tag-teams", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "Teams" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391732788" - }, - { - "name": "tag-teams", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "Teams" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391754947" - }, - { - "name": "Windows", - "checkSuite": { - "workflowRun": { - "workflow": { - "name": "CI" - } - } - }, - "status": "COMPLETED", - "conclusion": "SUCCESS", - "url": "https://github.com/apache/tvm/runs/6391733127" - }, - { - "state": "SUCCESS", - "context": "tvm-ci/branch", - "targetUrl": "https://ci.tlcpack.ai/job/tvm/job/ziheng%252Fcommunity/1/display/redirect" - }, - { - "state": "SUCCESS", - "context": "tvm-ci/pr-head", - "targetUrl": "https://ci.tlcpack.ai/job/tvm/job/PR-11276/1/display/redirect" - } - ] - } - } - } - } - ] - }, - "reviewDecision": "APPROVED", - "reviews": { - "pageInfo": { - "hasPreviousPage": false - }, - "nodes": [ - { - "body": "", - "updatedAt": "2022-05-11T16:50:16Z", - "url": "https://github.com/apache/tvm/pull/11276#pullrequestreview-969701502", - "authorCanPushToRepository": true, - "commit": { - "oid": "96075744cc687caafc131361d006c5967edddbc6" - }, - "author": { - "login": "tqchen" - }, - "state": "APPROVED" - } - ] - } -} \ No newline at end of file diff --git a/tests/python/ci/sample_prs/pr11442-no-recomment.json b/tests/python/ci/sample_prs/pr11442-rerun-ci.json similarity index 95% rename from tests/python/ci/sample_prs/pr11442-no-recomment.json rename to tests/python/ci/sample_prs/pr11442-rerun-ci.json index 77af805f2180..0199b2921f64 100644 --- a/tests/python/ci/sample_prs/pr11442-no-recomment.json +++ b/tests/python/ci/sample_prs/pr11442-rerun-ci.json @@ -3,7 +3,7 @@ "body": "(See https://discuss.tvm.apache.org/t/byoc-supporting-cutlass-byoc-with-collage/12796/6 for\r\ncontext, which in turn is part of Collage (https://github.com/apache/tvm-rfcs/blob/main/rfcs/0062-collage.md).\r\n\r\nThis adds a new 'DSO exportable' runtime module representing the contents of a .o file. It\r\nallows external codegen toolchains to yield a result which:\r\n - Like CSource modules, can be conveyed directly to the final export_library compilation\r\n step for linking into the final .so and saved to a know location without risk the\r\n underlying code artifact will be lost.\r\n - Like DSOLibrary modules, are self contained so that no additional compile-time arguments\r\n need be conveyed from the CSource module to the final export_library command line\r\n\r\nSince this is the third flavor of 'DSO exportable' module, add a Module::IsDSOExportable.\r\n\r\nSince adding the above, can't resist also adding a Module::ImplementsFunction virtual and\r\ncalling it from TEComplier to check if an external codegen function actually provided the\r\nimplementation it promised.\r\n\r\nNote:\r\n - I've left the existing implementation of runtime.load_module alone which\r\n relinks .o files to .so files.\r\n - Though also contained in the .o metadata, I require static libraries to always\r\n carry their list of exported function names.\r\n\r\nThis is all pretty stop gap pending a good rework of TVM to supoprt the notion of artifacts\r\nand, perhaps, build rules.\r\n", "state": "OPEN", "author": { - "login": "mbs-octoml" + "login": "abc" }, "comments": { "pageInfo": { @@ -64,15 +64,7 @@ "login": "mbs-octoml" }, "updatedAt": "2022-05-25T22:12:37Z", - "body": "Hmff." - }, - { - "authorAssociation": "NONE", - "author": { - "login": "github-actions" - }, - "updatedAt": "2022-05-25T22:12:55Z", - "body": "Cannot merge, did not find any approving reviews from users with write access on 96d4e62da5a7b78da18d0ee28cc6261d8fbf31c4" + "body": "@tvm-bot rerun" } ] }, diff --git a/tests/python/ci/test_mergebot.py b/tests/python/ci/test_mergebot.py index b9f944e897d3..a565cc76a5c1 100644 --- a/tests/python/ci/test_mergebot.py +++ b/tests/python/ci/test_mergebot.py @@ -29,8 +29,8 @@ class TempGit: def __init__(self, cwd): self.cwd = cwd - def run(self, *args): - proc = subprocess.run(["git"] + list(args), cwd=self.cwd) + def run(self, *args, **kwargs): + proc = subprocess.run(["git"] + list(args), cwd=self.cwd, **kwargs) if proc.returncode != 0: raise RuntimeError(f"git command failed: '{args}'") @@ -50,87 +50,118 @@ def run(self, *args): "number": 10786, "filename": "pr10786-merges.json", "expected": SUCCESS_EXPECTED_OUTPUT, + "comment": "@tvm-bot merge", + "user": "abc", "detail": "Everything is fine so this PR will merge", }, "no-request": { "number": 10786, "filename": "pr10786-nottriggered.json", - "expected": "No merge requested, exiting", + "expected": "Command 'do something else' did not match anything", + "comment": "@tvm-bot do something else", + "user": "abc", "detail": "A PR for which the mergebot runs but no merge is requested", }, "bad-ci": { "number": 10786, "filename": "pr10786-badci.json", "expected": "Cannot merge, these CI jobs are not successful on", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "A PR which failed CI and cannot merge", }, "old-review": { "number": 10786, "filename": "pr10786-oldreview.json", "expected": "Cannot merge, did not find any approving reviews", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "A PR with passing CI and approving reviews on an old commit so it cannot merge", }, "missing-job": { "number": 10786, "filename": "pr10786-missing-job.json", "expected": "Cannot merge, missing expected jobs", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "PR missing an expected CI job and cannot merge", }, "invalid-author": { "number": 10786, "filename": "pr10786-invalid-author.json", - "expected": "No merge requested, exiting", + "expected": "Comment is not from from PR author or collaborator, quitting", + "comment": "@tvm-bot merge", + "user": "not-abc", "detail": "Merge requester is not a committer and cannot merge", }, "unauthorized-comment": { "number": 11244, "filename": "pr11244-unauthorized-comment.json", - "expected": "No merge requested, exiting", + "expected": "Comment is not from from PR author or collaborator, quitting", + "comment": "@tvm-bot merge", + "user": "not-abc2", "detail": "Check that a merge comment not from a CONTRIBUTOR is rejected", }, "no-review": { "number": 11267, "filename": "pr11267-no-review.json", "expected": "Cannot merge, did not find any approving reviews from users with write access", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "Check that a merge request without any reviews is rejected", }, "changes-requested": { "number": 10786, "filename": "pr10786-changes-requested.json", "expected": "Cannot merge, found [this review]", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "Check that a merge request with a 'Changes Requested' review on HEAD is rejected", }, "co-authors": { "number": 10786, "filename": "pr10786-co-authors.json", "expected": "Co-authored-by: Some One ", + "comment": "@tvm-bot merge", + "user": "abc", "detail": "Check that a merge request with co-authors generates the correct commit message", }, - "no-recomment": { + "rerun-ci": { "number": 11442, - "filename": "pr11442-no-recomment.json", - "expected": "No merge requested, exiting", - "detail": "Check that comments after a failed merge don't trigger another merge", + "filename": "pr11442-rerun-ci.json", + "expected": "Rerunning ci with", + "comment": "@tvm-bot rerun", + "user": "abc", + "detail": "Start a new CI job", }, } @pytest.mark.parametrize( - ["number", "filename", "expected", "detail"], + ["number", "filename", "expected", "comment", "user", "detail"], [tuple(d.values()) for d in test_data.values()], ids=test_data.keys(), ) -def test_mergebot(tmpdir_factory, number, filename, expected, detail): - mergebot_script = REPO_ROOT / "tests" / "scripts" / "github_mergebot.py" +def test_mergebot(tmpdir_factory, number, filename, expected, comment, user, detail): + mergebot_script = REPO_ROOT / "tests" / "scripts" / "github_tvmbot.py" test_json_dir = Path(__file__).resolve().parent / "sample_prs" git = TempGit(tmpdir_factory.mktemp("tmp_git_dir")) - git.run("init") - git.run("checkout", "-b", "main") + git.run("init", stderr=subprocess.PIPE, stdout=subprocess.PIPE) + git.run("checkout", "-b", "main", stderr=subprocess.PIPE, stdout=subprocess.PIPE) git.run("remote", "add", "origin", "https://github.com/apache/tvm.git") with open(test_json_dir / filename) as f: test_data = json.load(f) + comment = { + "body": comment, + "id": 123, + "user": { + "login": user, + }, + } + collaborators = [] + proc = subprocess.run( [ str(mergebot_script), @@ -141,10 +172,17 @@ def test_mergebot(tmpdir_factory, number, filename, expected, detail): "https://example.com", "--testing-pr-json", json.dumps(test_data), + "--testing-collaborators-json", + json.dumps(collaborators), + "--trigger-comment-json", + json.dumps(comment), ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", + env={ + "TVM_BOT_JENKINS_TOKEN": "123", + }, cwd=git.cwd, ) if proc.returncode != 0: diff --git a/tests/scripts/git_utils.py b/tests/scripts/git_utils.py index 9f2468638cad..7cd1b6b2fe59 100644 --- a/tests/scripts/git_utils.py +++ b/tests/scripts/git_utils.py @@ -19,6 +19,7 @@ import json import subprocess import re +import base64 from urllib import request from typing import Dict, Tuple, Any, Optional, List @@ -29,6 +30,27 @@ def compress_query(query: str) -> str: return query +def post(url: str, body: Optional[Any] = None, auth: Optional[Tuple[str, str]] = None): + print(f"Requesting POST to", url, "with", body) + headers = {} + if auth is not None: + auth_str = base64.b64encode(f"{auth[0]}:{auth[1]}") + request.add_header("Authorization", f"Basic {auth_str}") + + if body is None: + body = "" + + req.add_header("Content-Type", "application/json; charset=utf-8") + req = request.Request(url, headers=headers, method="POST") + data = json.dumps(body) + data = data.encode("utf-8") + req.add_header("Content-Length", len(data)) + + with request.urlopen(req, data) as response: + response = json.loads(response.read()) + return response + + class GitHubRepo: def __init__(self, user, repo, token): self.token = token diff --git a/tests/scripts/github_mergebot.py b/tests/scripts/github_tvmbot.py similarity index 80% rename from tests/scripts/github_mergebot.py rename to tests/scripts/github_tvmbot.py index 76e0803efc23..bfdbeb4039e5 100755 --- a/tests/scripts/github_mergebot.py +++ b/tests/scripts/github_tvmbot.py @@ -23,17 +23,21 @@ import logging import traceback import re -from typing import Dict, Any, List, Optional +from typing import Dict, Any, List, Optional, Callable from pathlib import Path -from git_utils import git, GitHubRepo, parse_remote +from git_utils import git, GitHubRepo, parse_remote, post from cmd_utils import init_log Review = Dict[str, Any] CIJob = Dict[str, Any] +Comment = Dict[str, Any] +CommentChecker = Callable[[Comment], bool] EXPECTED_JOBS = ["tvm-ci/pr-head"] +TVM_BOT_JENKINS_TOKEN = os.environ["TVM_BOT_JENKINS_TOKEN"] +JENKINS_URL = "https://ci.tlcpack.ai/" THANKS_MESSAGE = r"(\s*)Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from \[Reviewers\]\(https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers\) by them in the pull request thread.(\s*)" @@ -41,6 +45,19 @@ def to_json_str(obj: Any) -> str: return json.dumps(obj, indent=2) +COLLABORATORS_QUERY = """ +query ($owner: String!, $name: String!, $user: String!) { + repository(owner: $owner, name: $name) { + collaborators(query: $user, first: 1) { + nodes { + login + } + } + } +} +""" + + PR_QUERY = """ query ($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { @@ -60,6 +77,7 @@ def to_json_str(obj: Any) -> str: author { login } + id updatedAt body } @@ -119,6 +137,7 @@ def to_json_str(obj: Any) -> str: body updatedAt url + id authorCanPushToRepository commit { oid @@ -202,6 +221,17 @@ def checker(obj, parent_key): def __repr__(self): return json.dumps(self.raw, indent=2) + def plus_one(self, comment: Dict[str, Any]): + """ + React with a thumbs up to a comment + """ + url = f"issues/comments/{comment['id']}/reactions" + data = {"content": "+1"} + if self.dry_run: + logging.info(f"Dry run, would have +1'ed to {url} with {data}") + else: + self.github.post(url, data=data) + def head_commit(self): return self.raw["commits"]["nodes"][0]["commit"] @@ -292,6 +322,19 @@ def fetch_data(self): }, )["data"]["repository"]["pullRequest"] + def search_collaborator(self, user: str) -> List[Dict[str, Any]]: + """ + Query GitHub for collaborators matching 'user' + """ + return self.github.graphql( + query=COLLABORATORS_QUERY, + variables={ + "owner": self.owner, + "name": self.repo_name, + "user": user, + }, + )["data"]["repository"]["collaborators"]["nodes"] + def comment(self, text: str) -> None: """ Leave the comment 'text' on this PR @@ -370,70 +413,8 @@ def merge(self) -> None: self.github.put(url, data=data) - def comment_can_merge(self, comment: Dict[str, Any]) -> bool: - """ - Check if a comment was left by the PR author or by a committer - """ - if comment["author"]["login"] == self.raw["author"]["login"]: - logging.info(f"Comment {comment} was from author and is mergeable") - return True - - if comment.get("authorAssociation", "") == "CONTRIBUTOR": - logging.info(f"Comment {comment} was from committer comment and is mergeable") - return True - - if comment.get("authorCanPushToRepository", False): - logging.info(f"Comment {comment} was from a committer review comment and is mergeable") - return True - - logging.info(f"Comment {comment} was not from author or committers and is not mergeable") - return False - - def merge_requested(self) -> bool: - """ - Check if this PR has had a merge requested - """ - merge_commands = [ - "merge", - "merge this", - "merge this pr", - ] - cancel_commands = [ - "cancel", - "cancel merge", - "cancel the merge", - "stop", - "stop merge", - "stop the merge", - ] - - def parse_action(comment: Dict[str, Any]) -> Optional[str]: - if comment["author"]["login"] == "github-actions": - return "commented" - - if not self.comment_can_merge(comment): - return None - - body = comment["body"] - if any(f"@tvm-bot {c}" in body for c in merge_commands): - return "merge" - - if any(f"@tvm-bot {c}" in body for c in cancel_commands): - return "cancel" - - return None - - # Check regular comments and top-level review comments - all_comments = self.raw["comments"]["nodes"] + self.reviews() - all_comments = sorted(all_comments, key=lambda comment: comment["updatedAt"]) - actions = [parse_action(comment) for comment in all_comments] - logging.info(f"Found these tvm-bot actions: {actions}") - actions = [a for a in actions if a is not None] - - if len(actions) == 0: - return False - - return actions[-1] == "merge" + def author(self) -> str: + return self.raw["author"]["login"] def find_failed_ci_jobs(self) -> List[CIJob]: # NEUTRAL is GitHub Action's way of saying cancelled @@ -502,6 +483,49 @@ def merge_if_passed_checks(self) -> None: self.comment(f"Cannot merge, CI did not pass on on {self.head_oid()}") return + def rerun_jenkins_ci(self) -> None: + url = JENKINS_URL + f"job/tvm/job/PR-{self.number}/buildWithParameters" + logging.info(f"Rerunning ci with URL={url}") + if self.dry_run: + logging.info("Dry run, not sending POST") + else: + post(url, auth=("tvm-bot", TVM_BOT_JENKINS_TOKEN)) + + +class Merge: + triggers = [ + "merge", + "merge this", + "merge this pr", + ] + + @staticmethod + def run(pr: PR): + try: + pr.merge_if_passed_checks() + except Exception as e: + if not args.dry_run: + msg = traceback.format_exc() + pr.comment( + f"Failed to process merge request in {args.run_url}\n\n
\n\n```\n{msg}\n```\n\n
" + ) + raise e + + +class Rerun: + triggers = [ + "rerun", + "rerun ci", + "re-run", + "re-run ci", + "run", + "run ci", + ] + + @staticmethod + def run(pr: PR): + pr.rerun_jenkins_ci() + if __name__ == "__main__": help = "Check if a PR has comments trying to merge it, and do so based on reviews/CI status" @@ -509,7 +533,13 @@ def merge_if_passed_checks(self) -> None: parser.add_argument("--remote", default="origin", help="ssh remote to parse") parser.add_argument("--pr", required=True, help="pr number to check") parser.add_argument("--run-url", required=True, help="workflow run URL") + parser.add_argument( + "--trigger-comment-json", required=True, help="json of the comment that triggered this run" + ) parser.add_argument("--testing-pr-json", help="(testing only) manual data for testing") + parser.add_argument( + "--testing-collaborators-json", help="(testing only) manual data for testing" + ) parser.add_argument( "--dry-run", action="store_true", @@ -518,7 +548,27 @@ def merge_if_passed_checks(self) -> None: ) args = parser.parse_args() init_log() + comment = json.loads(args.trigger_comment_json) + body = comment["body"].strip() + + # Check that the comment was addressed to tvm-bot + if not body.startswith("@tvm-bot "): + logging.info(f"Not a bot comment, '{body}' does not start with '@tvm-bot'") + exit(0) + # Find the code to run for the command from the user + user_command = body.lstrip("@tvm-bot").strip() + command_to_run = None + for command in [Merge, Rerun]: + if user_command in command.triggers: + command_to_run = command + break + + if command_to_run is None: + logging.info(f"Command '{user_command}' did not match anything") + exit(0) + + # Find the remote for querying more data about the PR remote = git(["config", "--get", f"remote.{args.remote}.url"]) logging.info(f"Using remote remote={remote}") owner, repo = parse_remote(remote) @@ -539,21 +589,34 @@ def merge_if_passed_checks(self) -> None: else: pr = PR(number=int(args.pr), owner=owner, repo=repo, dry_run=args.dry_run) + # Acknowledge the comment with a react + pr.plus_one(comment) + + # Check the comment author + comment_author = comment["user"]["login"] + if pr.author() == comment_author: + logging.info("Comment user is PR author, continuing") + else: + logging.info("Comment is not from PR author, checking collaborators") + # Get the list of collaborators for the repo filtered by the comment + # author + if args.testing_collaborators_json: + collaborators = json.loads(args.testing_collaborators_json) + else: + collaborators = pr.search_collaborator(comment_author) + logging.info(f"Found collaborators: {collaborators}") + + if len(collaborators) > 0: + logging.info("Comment is from collaborator") + else: + logging.info("Comment is not from from PR author or collaborator, quitting") + exit(0) + state = pr.state() if state != "OPEN": logging.info(f"Ignoring event on PR, state was not OPEN, instead was state={state}") exit(0) - if pr.merge_requested(): - try: - pr.merge_if_passed_checks() - except Exception as e: - if not args.dry_run: - msg = traceback.format_exc() - pr.comment( - f"Failed to process merge request in {args.run_url}\n\n
\n\n```\n{msg}\n```\n\n
" - ) - raise e - else: - logging.info("No merge requested, exiting") + # Run the command + command_to_run.run(pr)