From fdcbe613ac152aeac8ae82830b027f90abc1c780 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 23 Oct 2023 13:57:36 +0200 Subject: [PATCH] Allow trigger try workfows using labels (#30383) This change adds and alternate method for triggering try changes. Instead of comments, changes are triggered via applying labels to pull requests. The action will remove the label from the request and start the requested jobs. This will require creating at least a few labels: - T-full - T-linux-wpt-2013 - T-linux-wpt-2020 - T-macos - T-windows More labels can be added as we support more configurations. The good thing about this change is that try jobs against the actual branch in the pull request instead of the master branch. This means that changes to CI can be tested (unlike for comment processing). One bit caveat with this change is that when adding multiple labels, a CI job is triggered for each. Only one real build will run for each label, but whether or more try jobs is triggered is a race condition. The first CI job to successfully remove the label will actually trigger the job. If the same job removes two compatible labels, then they can share a build (for instance two types of WPT linux jobs). If not there will be two. Note that this is at least as efficient as the current behavior. --- .github/workflows/linux.yml | 6 +- .github/workflows/mac.yml | 6 +- .github/workflows/main.yml | 36 ++-- .github/workflows/try.yml | 164 ++++++++++++++----- .github/workflows/windows.yml | 6 +- etc/ci/report_aggregated_expected_results.py | 5 + 6 files changed, 159 insertions(+), 64 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index f891d297c0a..4ce40e07992 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -55,15 +55,15 @@ jobs: runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v3 - if: github.event_name != 'issue_comment' + if: github.event_name != 'issue_comment' && github.event_name != 'pull_request_target' with: fetch-depth: 2 # This is necessary to checkout the pull request if this run was triggered # via an `issue_comment` action on a pull request. - uses: actions/checkout@v3 - if: github.event_name == 'issue_comment' + if: github.event_name == 'issue_comment' || github.event_name == 'pull_request_target' with: - ref: refs/pull/${{ github.event.issue.number }}/head + ref: refs/pull/${{ github.event.issue.number || github.event.number }}/head fetch-depth: 2 - name: Run sccache-cache uses: mozilla-actions/sccache-action@v0.0.3 diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index dda92fdcbdd..83d820bef9e 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -48,15 +48,15 @@ jobs: runs-on: macos-13 steps: - uses: actions/checkout@v3 - if: github.event_name != 'issue_comment' + if: github.event_name != 'issue_comment' && github.event_name != 'pull_request_target' with: fetch-depth: 2 # This is necessary to checkout the pull request if this run was triggered # via an `issue_comment` action on a pull request. - uses: actions/checkout@v3 - if: github.event_name == 'issue_comment' + if: github.event_name == 'issue_comment' || github.event_name == 'pull_request_target' with: - ref: refs/pull/${{ github.event.issue.number }}/head + ref: refs/pull/${{ github.event.issue.number || github.event.number }}/head fetch-depth: 2 - name: Run sccache-cache uses: mozilla-actions/sccache-action@v0.0.3 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cedb03b4c8f..c3eb7ef78ba 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -13,21 +13,15 @@ on: types: [checks_requested] workflow_call: inputs: - platform: + configuration: required: true type: string - layout: - required: true - type: string - unit-tests: - required: true - type: boolean workflow_dispatch: inputs: platform: required: false type: choice - options: ["none", "linux", "windows", "macos", "all", "sync"] + options: ["linux", "windows", "macos", "all"] layout: required: false type: choice @@ -48,6 +42,14 @@ jobs: uses: actions/github-script@v6 with: script: | + // If this is a workflow call with a configuration object, + // then just return it immediately. + let configuration = ${{ inputs.configuration || '""' }}; + if (configuration != "") { + console.log("Using configuration: " + JSON.stringify(configuration)); + return configuration; + } + // We need to pick defaults if the inputs are not provided. Unprovided inputs // are empty strings in this template. let platform = "${{ inputs.platform }}" || "linux"; @@ -61,18 +63,26 @@ jobs: unit_tests = true; } + let platforms = []; + if (platform == "all") { + platforms = [ "linux", "windows", "macos" ]; + } else { + platforms = [ platform ]; + } + let returnValue = { - platform, + platforms, layout, unit_tests, }; + console.log("Using configuration: " + JSON.stringify(returnValue)); return returnValue; build-win: name: Windows needs: ["decision"] - if: ${{ contains(fromJson('["windows", "all"]'), fromJson(needs.decision.outputs.configuration).platform) }} + if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'windows') }} uses: ./.github/workflows/windows.yml with: unit-tests: ${{ fromJson(needs.decision.outputs.configuration).unit_tests }} @@ -80,7 +90,7 @@ jobs: build-mac: name: Mac needs: ["decision"] - if: ${{ contains(fromJson('["macos", "all"]'), fromJson(needs.decision.outputs.configuration).platform) }} + if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'macos') }} uses: ./.github/workflows/mac.yml with: unit-tests: ${{ fromJson(needs.decision.outputs.configuration).unit_tests }} @@ -88,7 +98,7 @@ jobs: build-linux: name: Linux needs: ["decision"] - if: ${{ contains(fromJson('["linux", "all"]'), fromJson(needs.decision.outputs.configuration).platform) }} + if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'linux') }} uses: ./.github/workflows/linux.yml with: wpt: 'test' @@ -108,7 +118,7 @@ jobs: steps: - name: Mark skipped jobs as successful - if: ${{ fromJson(needs.decision.outputs.configuration).platform == 'none' }} + if: ${{ fromJson(needs.decision.outputs.configuration).platforms[0] != null }} run: exit 0 - name: Mark the job as successful if: ${{ !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled') }} diff --git a/.github/workflows/try.yml b/.github/workflows/try.yml index 88936b8f59b..f6e40cf1b55 100644 --- a/.github/workflows/try.yml +++ b/.github/workflows/try.yml @@ -1,13 +1,15 @@ name: Try on: + pull_request_target: + types: [labeled] issue_comment: types: [created] jobs: parse-comment: - name: Process Comment - if: ${{ github.event.issue.pull_request }} + name: Trigger Try (${{ github.event.name }}) + if: ${{ github.event_name == 'pull_request_target' || github.event.issue.pull_request }} runs-on: ubuntu-latest outputs: configuration: ${{ steps.configuration.outputs.result }} @@ -26,62 +28,140 @@ jobs: }) } - let tokens = context.payload.comment.body.split(/\s+/); - let tagIndex = tokens.indexOf("@bors-servo"); - if (tagIndex == -1 || tagIndex + 1 >= tokens.length) { - return { try: false }; + function combineWPTLayoutOptions(layout, newLayout) { + let has2013 = layout == "2013" || layout == "all"; + let has2020 = layout == "2020" || layout == "all"; + let adding2013 = newLayout == "2013"; + let adding2020 = newLayout == "2020"; + + if ((adding2020 && has2020) || (adding2013 && has2013)) { + return layout; + } + if (adding2020) { + return has2013 ? "all" : "2020"; + } + if (adding2013) { + return has2020 ? "all" : "2013"; + } + return layout; } - let tryString = tokens[tagIndex + 1]; - console.log("Found try string: '" + tryString + "'"); - let returnValue = { try: false }; - if (tryString == "try") { - returnValue = { try: true, platform: 'all', layout: 'all', unit_tests: true, }; - } else if (tryString == "try=wpt") { - returnValue = { try: true, platform: 'linux', layout: '2013', unit_tests: false }; - } else if (tryString == "try=wpt-2020") { - returnValue = { try: true, platform: 'linux', layout: '2020', unit_tests: false }; - } else if (tryString == "try=linux") { - returnValue = { try: true, platform: 'linux', layout: 'none', unit_tests: true }; - } else if (tryString == "try=mac") { - returnValue = { try: true, platform: 'macos', layout: 'none', unit_tests: true }; - } else if (tryString == "try=windows") { - returnValue = { try: true, platform: 'windows', layout: 'none', unit_tests: true }; - } else { - makeComment("🤔 Unknown try string '" + tryString + "'"); - return returnValue; - } - - if (returnValue.try) { - let username = context.payload.comment.user.login; - let result = await github.rest.repos.getCollaboratorPermissionLevel({ - owner: context.repo.owner, - repo: context.repo.repo, - username - }); - if (!result.data.user.permissions.push) { - makeComment('🔒 User @' + username + ' does not have permission to trigger try jobs.'); - return { try: false }; + function addPlatformToConfiguration(platform, configuration) { + if (!configuration.platforms.includes(platform)) { + configuration.platforms.push(platform); } } + function updateConfigurationFromString(tryString, configuration) { + if (tryString.includes("full")) { + configuration.platforms = ["linux", "macos", "windows"]; + configuration.unit_tests = true; + configuration.layout = "all"; + return configuration; + } + + if (tryString.includes("linux")) { + addPlatformToConfiguration("linux", configuration); + configuration.unit_tests = true; + } else if (tryString.includes("macos")) { + addPlatformToConfiguration("macos", configuration); + configuration.unit_tests = true; + } else if (tryString.includes("win")) { + addPlatformToConfiguration("windows", configuration); + configuration.unit_tests = true; + } + + if (tryString.includes("wpt")) { + addPlatformToConfiguration("linux", configuration); + if (tryString.includes("2020")) { + configuration.layout = combineWPTLayoutOptions(configuration.layout, "2020"); + } else { + configuration.layout = combineWPTLayoutOptions(configuration.layout, "2013"); + } + } + } + + let configuration = { + platforms: [], + layout: "none", + unit_tests: false, + }; + + if (context.eventName == "pull_request_target") { + let try_labels = []; + for (const label of context.payload.pull_request.labels) { + if (!label.name.startsWith("T-")) { + continue; + } + + // Try to remove the label. If that fails, it's likely that another + // workflow has already processed it or a user has removed it. + try { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + name: label.name, + }); + } catch (exception) { + console.log("Assuming '" + label.name + "' is already removed: " + exception); + continue; + } + + console.log("Found label: " + label.name); + updateConfigurationFromString(label.name, configuration); + } + } else { + let tokens = context.payload.comment.body.split(/\s+/); + let tagIndex = tokens.indexOf("@bors-servo"); + if (tagIndex == -1 || tagIndex + 1 >= tokens.length) { + return { platforms: [] }; + } + + + let tryString = tokens[tagIndex + 1]; + console.log("Found try string: '" + tryString + "'"); + if (tryString == "try") { + updateConfigurationFromString("full", configuration); + } else { + updateConfigurationFromString(tryString, configuration); + } + } + + console.log(JSON.stringify(configuration)); + + if (configuration.platforms.length == 0) { + return { platforms: [] }; + } + + let username = context.payload.sender.login; + let result = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: context.repo.owner, + repo: context.repo.repo, + username + }); + if (!result.data.user.permissions.push) { + makeComment('🔒 User @' + username + ' does not have permission to trigger try jobs.'); + return { platforms: [] }; + } + const url = context.serverUrl + "/" + context.repo.owner + "/" + context.repo.repo + "/actions/runs/" + context.runId; const formattedURL = "[#" + context.runId + "](" + url + ")"; - makeComment("🔨 Triggering try run (" + formattedURL + ") with platform=" + returnValue.platform + " and layout=" + returnValue.layout); - return returnValue; + let platformsString = configuration.platforms.toString(); + makeComment("🔨 Triggering try run (" + formattedURL + ") with platforms=" + + platformsString + " and layout=" + configuration.layout); + return configuration; run-try: name: Run Try needs: ["parse-comment"] - if: ${{ fromJson(needs.parse-comment.outputs.configuration).try}} + if: ${{ fromJson(needs.parse-comment.outputs.configuration).platforms[0] != null }} uses: ./.github/workflows/main.yml with: - platform: ${{ fromJson(needs.parse-comment.outputs.configuration).platform }} - layout: ${{ fromJson(needs.parse-comment.outputs.configuration).layout }} - unit-tests: ${{ fromJson(needs.parse-comment.outputs.configuration).unit_tests }} + configuration: ${{ needs.parse-comment.outputs.configuration }} results: name: Results diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index e416cde2707..0b7e31ecc03 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -43,15 +43,15 @@ jobs: runs-on: windows-2019 steps: - uses: actions/checkout@v3 - if: github.event_name != 'issue_comment' + if: github.event_name != 'issue_comment' && github.event_name != 'pull_request_target' with: fetch-depth: 2 # This is necessary to checkout the pull request if this run was triggered # via an `issue_comment` action on a pull request. - uses: actions/checkout@v3 - if: github.event_name == 'issue_comment' + if: github.event_name == 'issue_comment' || github.event_name == 'pull_request_target' with: - ref: refs/pull/${{ github.event.issue.number }}/head + ref: refs/pull/${{ github.event.issue.number || github.event.number }}/head fetch-depth: 2 - name: ccache uses: hendrikmuhs/ccache-action@v1.2 diff --git a/etc/ci/report_aggregated_expected_results.py b/etc/ci/report_aggregated_expected_results.py index f0046456c27..21c2d6b2431 100755 --- a/etc/ci/report_aggregated_expected_results.py +++ b/etc/ci/report_aggregated_expected_results.py @@ -173,6 +173,11 @@ def get_pr_number() -> Optional[str]: if "issue" in github_context["event"]: return str(github_context["event"]["issue"]["number"]) + # If we have an 'number' in the context, this was triggered by "pull_request" or + # "pull_request_target" event. + if "number" in github_context["event"]: + return str(github_context["event"]["number"]) + return None