From 4069c5fba9441ef24814ef8043f1c4b68fbcd0cf Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 16 Jul 2024 13:34:27 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A7=AA=F0=9F=9A=91=20Fix=20using=20`c?= =?UTF-8?q?heck=5Fsource`=20flags=20as=20bool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, those flags would sometimes end up having empty string values, which tends to break evaluating them as JSON. This patch adds `false` fallbacks to all such outputs. This allows feeding them to `fromJSON()` without a fear of them causing surprising internal behaviors in the GitHub Actions CI/CD workflows platform itself [[1]]. The behavior observed was that some skipped jobs wouldn't show up in the workflow sidebar view at all, would display in the graph view as `Waiting for pending jobs` and in the `${{ needs }}` context, they would have a `result: failure` entry [[2]]. This should help make PRs like #121831 mergeable again. [1]: https://github.com/python/cpython/pull/121766#issuecomment-2230023214 [2]: https://github.com/python/cpython/actions/runs/9950331379/job/27501637459?pr=121831#step:2:244 --- .github/workflows/build.yml | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e1d9e1632f136c..0298d0467fe35f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,11 +27,31 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 outputs: + # Some of the referenced steps set outputs conditionally and there may be + # cases when referencing them evaluates to empty strings. It is nice to + # work with proper booleans so they have to be evaluated through JSON + # conversion in the expressions. However, empty strings used like that + # may trigger all sorts of undefined and hard-to-debug behaviors in + # GitHub Actions CI/CD. To help with this, all of the outputs set here + # that are meant to be used as boolean flags (and not arbitrary strings), + # MUST have fallbacks with default values set. A common pattern would be + # to add ` || false` to all such expressions here, in the output + # definitions. They can then later be safely used through the following + # idiom in job conditionals and other expressions. Here's some examples: + # + # if: fromJSON(needs.check_source.outputs.run-docs) + # + # ${{ + # fromJSON(needs.check_source.outputs.run_tests) + # && 'truthy-branch' + # || 'falsy-branch' + # }} + # run-docs: ${{ steps.docs-changes.outputs.run-docs || false }} - run_tests: ${{ steps.check.outputs.run_tests }} - run_hypothesis: ${{ steps.check.outputs.run_hypothesis }} - run_cifuzz: ${{ steps.check.outputs.run_cifuzz }} - config_hash: ${{ steps.config_hash.outputs.hash }} + run_tests: ${{ steps.check.outputs.run_tests || false }} + run_hypothesis: ${{ steps.check.outputs.run_hypothesis || false }} + run_cifuzz: ${{ steps.check.outputs.run_cifuzz || false }} + config_hash: ${{ steps.config_hash.outputs.hash }} # str steps: - uses: actions/checkout@v4 - name: Check for source changes From d42c6f6a5b1ee291ce651ae8da38bf156bac0e66 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 16 Jul 2024 14:14:01 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A7=AA=F0=9F=93=9D=20Change=20reusabl?= =?UTF-8?q?e-windows=20input=20description?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This updates it to avoid referencing to `no-GIL` in favor of the official term `free-threading`. --- .github/workflows/reusable-windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-windows.yml b/.github/workflows/reusable-windows.yml index 94a3abe530cb9b..e9c3c8e05a801c 100644 --- a/.github/workflows/reusable-windows.yml +++ b/.github/workflows/reusable-windows.yml @@ -6,7 +6,7 @@ on: required: true type: string free-threading: - description: Whether to use no-GIL mode + description: Whether to compile CPython in free-threading mode required: false type: boolean default: false