From 27a5680e62eb96b75758cb836469ed5255f026e2 Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Sun, 24 Aug 2025 14:12:16 -0700 Subject: [PATCH 1/2] Revise workflows to fix GHA syntax & rename files The values from workflow calls turn out _always_ to be strings, even when you try to make sure to produce Booleans. This means tests of values output by workflows called by other workflows have to be of the form `${{variable == 'true'}}` rather than (e.g.) `${{variable}}` or even `${{variable == true}}`. This is poorly documented in GitHub's documentation, IMHO. Along with fixing the conditionals, this PR renames the reusable workflow files to start with leading underscore characters. That seems to be more conventional in other GitHub repos. --- ...e_build_wheels.yaml => _build_wheels.yaml} | 0 ...e_find_changes.yaml => _find_changes.yaml} | 45 +++++++++++++------ .github/workflows/ci_build_library.yaml | 8 ++-- .github/workflows/ci_build_wheels.yaml | 8 ++-- .github/workflows/ci_docker_tests.yaml | 6 ++- .github/workflows/ci_format_checks.yml | 14 +++--- .github/workflows/ci_hardware_options.yaml | 10 +++-- .github/workflows/ci_sanitizer_tests.yaml | 10 +++-- .github/workflows/ci_tcmalloc_test.yaml | 8 ++-- .github/workflows/pr-labeler.yaml | 2 +- .github/workflows/release_wheels.yml | 2 +- 11 files changed, 73 insertions(+), 40 deletions(-) rename .github/workflows/{reusable_build_wheels.yaml => _build_wheels.yaml} (100%) rename .github/workflows/{reusable_find_changes.yaml => _find_changes.yaml} (58%) diff --git a/.github/workflows/reusable_build_wheels.yaml b/.github/workflows/_build_wheels.yaml similarity index 100% rename from .github/workflows/reusable_build_wheels.yaml rename to .github/workflows/_build_wheels.yaml diff --git a/.github/workflows/reusable_find_changes.yaml b/.github/workflows/_find_changes.yaml similarity index 58% rename from .github/workflows/reusable_find_changes.yaml rename to .github/workflows/_find_changes.yaml index 87803e599..3c70ab3f1 100644 --- a/.github/workflows/reusable_find_changes.yaml +++ b/.github/workflows/_find_changes.yaml @@ -21,8 +21,8 @@ # protection rules work is: "If a workflow is skipped due to path filtering # [...] the checks associated with that workflow will remain in a Pending # state. A PR that requires those checks to be successful will be blocked from -# merging." This makes path filters unusable with merge queues. We forgo the -# use of path filters, and instead check file changes using our own workflow. +# merging." This makes path filters unusable with merge queues. So, we forgo the +# use of path filters and instead check file changes using our own workflow. # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # The tilde is only to make our reusable workflows be listed last in the UI. @@ -31,13 +31,16 @@ run-name: Determine which files have been changed on: workflow_call: + # N.B.: GitHub Actions workflow_call output values are ALWAYS strings, even + # for (what you think are) Booleans. Consequently, calling workflows have to + # test values using, e.g., ${{foo = 'false'}}, and not ${{foo}} or similar. outputs: code: - description: 'True if any code files were changed' - value: jobs.test.outputs.only-noncode-changes == 'false' + description: 'True if any potential code file was changed' + value: ${{jobs.test.outputs.nondoc-file-changes == 'true'}} python: - description: 'True if Python files were changed' - value: jobs.test.outputs.have-python-changes == 'true' + description: 'True if any Python file was changed' + value: ${{jobs.test.outputs.python-changes == 'true'}} permissions: read-all @@ -47,24 +50,34 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 5 outputs: - only-noncode-changes: steps.only-noncode-files.outputs.matched - python-changes: steps.python-files.outputs.matched + nondoc-file-changes: ${{steps.nondoc-files.outputs.matched}} + python-changes: ${{steps.python-files.outputs.matched}} steps: - name: Check out a copy of the git repository uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - - name: Test whether only docs or similar files were changed + - name: Test whether the changes involved one or more non-doc files uses: tomi/paths-filter-action@32c62f5ca100c1110406e3477d5b3ecef4666fec # v3.0.2 - id: only-noncode-files + id: nondoc-files with: base: ${{github.ref_name}} - predicate-quantifier: 'every' + # Note: when editing the patterns below, always make sure to negate + # the condition (i.e., use a leading '!'). See the next comment. filters: | matched: - - '!**/*.md' - - '!docs/**' - '!.github/ISSUE_TEMPLATE/**' + - '!.github/dependabot.yaml' + - '!.github/problem-matchers/**' - '!CITATION.cff' + - '!docs/**' + - '!**/*.md' + - '!**/*.png' + # The default paths-filter-action behavior is "match at least one" + # pattern. We change the behavior to be "match every" pattern. + # Combined with negating every pattern above, it means the final output + # will be non-empty only if there is at least one file that is NOT a + # documentation file, or in other words, is a potential code file. + predicate-quantifier: 'every' - name: Test whether Python files are among the changed files uses: tomi/paths-filter-action@32c62f5ca100c1110406e3477d5b3ecef4666fec # v3.0.2 @@ -74,3 +87,9 @@ jobs: filters: | matched: - '**/*.py' + + - name: Summary of test results + run: | + set -x + echo steps.nondoc-files.outputs.matched = │${{steps.nondoc-files.outputs.matched}}│ + echo steps.python-files.outputs.matched = │${{steps.python-files.outputs.matched}}│ diff --git a/.github/workflows/ci_build_library.yaml b/.github/workflows/ci_build_library.yaml index ae5006219..06774c321 100644 --- a/.github/workflows/ci_build_library.yaml +++ b/.github/workflows/ci_build_library.yaml @@ -45,12 +45,14 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit build-wheels: # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + if: >- + ${{needs.find-changes.outputs.code == 'true' + || github.event_name == 'workflow_dispatch'}} name: ${{matrix.conf.os}}/${{matrix.conf.pyarch}}/py3${{matrix.conf.py}} needs: find-changes runs-on: ${{matrix.conf.os}} @@ -121,7 +123,7 @@ jobs: - if: matrix.conf.os != 'windows-2025' name: Run the build and test script (non-Windows case) env: - # SHELLOPTS is used by Bash. Add xtrace when doing manual debug runs. + # Add xtrace to SHELLOPTS for all Bash scripts when doing debug runs. SHELLOPTS: ${{inputs.debug && 'xtrace' || '' }} run: dev_tools/test_libs.sh ${{env.use-verbose && '--config=verbose'}} diff --git a/.github/workflows/ci_build_wheels.yaml b/.github/workflows/ci_build_wheels.yaml index 205d4be18..83c0d3a33 100644 --- a/.github/workflows/ci_build_wheels.yaml +++ b/.github/workflows/ci_build_wheels.yaml @@ -45,15 +45,17 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit build-wheels: # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + if: >- + ${{needs.find-changes.outputs.code == 'true' + || github.event_name == 'workflow_dispatch'}} name: Build & test wheels needs: find-changes - uses: ./.github/workflows/reusable_build_wheels.yaml + uses: ./.github/workflows/_build_wheels.yaml secrets: inherit with: debug: ${{inputs.debug == true}} diff --git a/.github/workflows/ci_docker_tests.yaml b/.github/workflows/ci_docker_tests.yaml index 77bf452c4..79de56871 100644 --- a/.github/workflows/ci_docker_tests.yaml +++ b/.github/workflows/ci_docker_tests.yaml @@ -45,12 +45,14 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit build-and-test: # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + if: >- + ${{needs.find-changes.outputs.code == 'true' + || github.event_name == 'workflow_dispatch'}} name: Build and test Docker images needs: find-changes runs-on: ubuntu-24.04 diff --git a/.github/workflows/ci_format_checks.yml b/.github/workflows/ci_format_checks.yml index 820a39a49..d52e78648 100644 --- a/.github/workflows/ci_format_checks.yml +++ b/.github/workflows/ci_format_checks.yml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: 'CI: lint & format checks' +name: 'CI: check code format' run-name: Test with linters and formatters on: @@ -45,16 +45,21 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit check-format: # For efficiency, skip this workflow if there were no Python file changes. - if: needs.find-changes.outputs.python || github.event_name == 'workflow_dispatch' + if: >- + ${{needs.find-changes.outputs.python == 'true' + || github.event_name == 'workflow_dispatch'}} name: Python format check needs: find-changes runs-on: ubuntu-24.04 timeout-minutes: 30 + env: + # Add xtrace to SHELLOPTS for all Bash scripts when doing debug runs. + SHELLOPTS: ${{inputs.debug && 'xtrace' || '' }} steps: - name: Check out a copy of the git repository uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 @@ -77,7 +82,4 @@ jobs: pip install -r dev-requirements.txt - name: Check Python file format - env: - # SHELLOPTS is used by Bash. Add xtrace when doing manual debug runs. - SHELLOPTS: ${{inputs.debug && 'xtrace' || '' }} run: check/format-incremental diff --git a/.github/workflows/ci_hardware_options.yaml b/.github/workflows/ci_hardware_options.yaml index dbea2d0ee..ddae57b16 100644 --- a/.github/workflows/ci_hardware_options.yaml +++ b/.github/workflows/ci_hardware_options.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: 'CI: hardware options tests' +name: 'CI: test with different hardware options' run-name: Test with instruction set extensions and parallelism on: @@ -45,12 +45,14 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit test-options: - # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + # For efficiency, skip this workflow if there were no Python file changes. + if: >- + ${{needs.find-changes.outputs.python == 'true' + || github.event_name == 'workflow_dispatch'}} name: Test ${{matrix.hardware_opt}} + ${{matrix.parallel_opt}} needs: find-changes runs-on: ubuntu-24.04 diff --git a/.github/workflows/ci_sanitizer_tests.yaml b/.github/workflows/ci_sanitizer_tests.yaml index 8ec0c982f..6d8b6223d 100644 --- a/.github/workflows/ci_sanitizer_tests.yaml +++ b/.github/workflows/ci_sanitizer_tests.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: 'CI: sanitizer tests' +name: 'CI: run sanitizer tests' run-name: Test with address and memory sanitizers on: @@ -45,12 +45,14 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit sanitizer-tests: - # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + # For efficiency, skip this workflow if there were no Python file changes. + if: >- + ${{needs.find-changes.outputs.python == 'true' + || github.event_name == 'workflow_dispatch'}} name: Test with sanitizers needs: find-changes runs-on: ubuntu-24.04 diff --git a/.github/workflows/ci_tcmalloc_test.yaml b/.github/workflows/ci_tcmalloc_test.yaml index 8998a5060..3090a43d2 100644 --- a/.github/workflows/ci_tcmalloc_test.yaml +++ b/.github/workflows/ci_tcmalloc_test.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -name: 'CI: TCMalloc test' +name: 'CI: run TCMalloc test' run-name: Test with TCMalloc (thread-caching malloc) on: @@ -45,12 +45,14 @@ concurrency: jobs: find-changes: name: Find changed files - uses: ./.github/workflows/reusable_find_changes.yaml + uses: ./.github/workflows/_find_changes.yaml secrets: inherit tcmalloc-test: # For efficiency, skip this workflow if there were no code file changes. - if: needs.find-changes.outputs.code || github.event_name == 'workflow_dispatch' + if: >- + ${{needs.find-changes.outputs.code == 'true' + || github.event_name == 'workflow_dispatch'}} name: Test with TCMalloc needs: find-changes runs-on: ubuntu-24.04 diff --git a/.github/workflows/pr-labeler.yaml b/.github/workflows/pr-labeler.yaml index 13b865a2c..2d3424634 100644 --- a/.github/workflows/pr-labeler.yaml +++ b/.github/workflows/pr-labeler.yaml @@ -19,7 +19,7 @@ # getting applied, check the run summary page for errors. # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -name: Pull request labeler +name: Label pull requests run-name: >- Label pull request ${{github.event.pull_request.number}} by ${{github.actor}} diff --git a/.github/workflows/release_wheels.yml b/.github/workflows/release_wheels.yml index 68eb5fa53..7f3f7ca43 100644 --- a/.github/workflows/release_wheels.yml +++ b/.github/workflows/release_wheels.yml @@ -31,7 +31,7 @@ permissions: read-all jobs: build-wheels: name: Build and save wheels - uses: ./.github/workflows/reusable_build_wheels.yaml + uses: ./.github/workflows/_build_wheels.yaml secrets: inherit with: upload: true From 0fa053de3c4a7b827eb7e384adaaca1fdcad60ed Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Sun, 24 Aug 2025 16:03:04 -0700 Subject: [PATCH 2/2] Fix tests -- should be against code, not just python --- .github/workflows/ci_hardware_options.yaml | 4 ++-- .github/workflows/ci_sanitizer_tests.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci_hardware_options.yaml b/.github/workflows/ci_hardware_options.yaml index ddae57b16..e2cc83976 100644 --- a/.github/workflows/ci_hardware_options.yaml +++ b/.github/workflows/ci_hardware_options.yaml @@ -49,9 +49,9 @@ jobs: secrets: inherit test-options: - # For efficiency, skip this workflow if there were no Python file changes. + # For efficiency, skip this workflow if there were no code file changes. if: >- - ${{needs.find-changes.outputs.python == 'true' + ${{needs.find-changes.outputs.code == 'true' || github.event_name == 'workflow_dispatch'}} name: Test ${{matrix.hardware_opt}} + ${{matrix.parallel_opt}} needs: find-changes diff --git a/.github/workflows/ci_sanitizer_tests.yaml b/.github/workflows/ci_sanitizer_tests.yaml index 6d8b6223d..0bc2383f7 100644 --- a/.github/workflows/ci_sanitizer_tests.yaml +++ b/.github/workflows/ci_sanitizer_tests.yaml @@ -49,9 +49,9 @@ jobs: secrets: inherit sanitizer-tests: - # For efficiency, skip this workflow if there were no Python file changes. + # For efficiency, skip this workflow if there were no code file changes. if: >- - ${{needs.find-changes.outputs.python == 'true' + ${{needs.find-changes.outputs.code == 'true' || github.event_name == 'workflow_dispatch'}} name: Test with sanitizers needs: find-changes