diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 7ac8185f1b4..cacf4ed7b5e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -11,11 +11,11 @@ jobs: env: PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Bootstrap Pants, test and lint Rust (Linux-x86_64) needs: - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -147,11 +147,11 @@ jobs: env: PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Bootstrap Pants, test Rust (macOS11-x86_64) needs: - - docs_only_check + - classify_changes runs-on: - macos-11 steps: @@ -274,11 +274,11 @@ jobs: env: PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Build wheels and fs_util (Linux-x86_64) needs: - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -359,11 +359,11 @@ jobs: env: PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Build wheels and fs_util (macOS10-15-x86_64) needs: - - docs_only_check + - classify_changes runs-on: - macOS-10.15-X64 steps: @@ -452,11 +452,11 @@ jobs: ARCHFLAGS: -arch arm64 PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Bootstrap Pants, build wheels and fs_util (macOS11-ARM64) needs: - - docs_only_check + - classify_changes runs-on: - macOS-11-ARM64 steps: @@ -582,11 +582,11 @@ jobs: env: PANTS_REMOTE_CACHE_READ: 'false' PANTS_REMOTE_CACHE_WRITE: 'false' - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Build wheels and fs_util (macOS11-x86_64) needs: - - docs_only_check + - classify_changes runs-on: - macos-11 steps: @@ -688,11 +688,15 @@ jobs: labels: category:new feature, category:user api change, category:plugin api change, category:performance, category:bugfix, category:documentation, category:internal mode: exactly - docs_only_check: + classify_changes: if: github.repository_owner == 'pantsbuild' - name: Check for docs-only change + name: Classify changes outputs: - docs_only: ${{ steps.docs_only_check.outputs.docs_only }} + ci_config: ${{ steps.classify.outputs.ci_config }} + docs: ${{ steps.classify.outputs.docs }} + docs_only: ${{ steps.classify.outputs.docs_only }} + release: ${{ steps.classify.outputs.release }} + rust: ${{ steps.classify.outputs.rust }} runs-on: - ubuntu-20.04 steps: @@ -702,21 +706,25 @@ jobs: fetch-depth: 10 - id: files name: Get changed files - uses: tj-actions/changed-files@v32.0.0 + uses: tj-actions/changed-files@v32 with: - files_ignore: docs/**|build-support/bin/generate_user_list.py - files_ignore_separator: '|' - - id: docs_only_check - if: steps.files.outputs.any_changed != 'true' - name: Check for docs-only changes - run: echo '::set-output name=docs_only::DOCS_ONLY' + separator: '|' + - id: classify + name: Classify changed files + run: " affected=$(python build-support/bin/classify_changed_files.py\ + \ '${{ steps.files.outputs.all_modified_files }}')\n\ + \ echo \"Affected:\n${affected}\"\n \ + \ if [[ \"${affected}\" == \"docs\" ]]; then\n \ + \ echo '::set-output name=docs_only::true'\n \ + \ fi\n for i in ${affected}; do\n \ + \ echo \"::set-output name=${i}::true\"\n done\n" lint_python: - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Lint Python and Shell needs: - bootstrap_pants_linux_x86_64 - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -793,10 +801,10 @@ jobs: \ == \"true\" ]]; then\n echo \"Merge OK\"\n exit 0\nelse\n echo\ \ \"Merge NOT OK\"\n exit 1\nfi\n" set_merge_ok_docs_only: - if: needs.docs_only_check.outputs.docs_only == 'DOCS_ONLY' + if: needs.classify_changes.outputs.docs_only == true name: Set Merge OK needs: - - docs_only_check + - classify_changes - check_labels outputs: merge_ok: ${{ steps.set_merge_ok.outputs.merge_ok }} @@ -806,10 +814,10 @@ jobs: - id: set_merge_ok run: echo '::set-output name=merge_ok::true' set_merge_ok_not_docs_only: - if: needs.docs_only_check.outputs.docs_only != 'DOCS_ONLY' + if: needs.classify_changes.outputs.docs_only != true name: Set Merge OK needs: - - docs_only_check + - classify_changes - check_labels - bootstrap_pants_linux_x86_64 - bootstrap_pants_macos11_x86_64 @@ -818,7 +826,7 @@ jobs: - build_wheels_macos11_arm64 - build_wheels_macos11_x86_64 - check_labels - - docs_only_check + - classify_changes - lint_python - test_python_linux_x86_64_0 - test_python_linux_x86_64_1 @@ -832,12 +840,12 @@ jobs: - id: set_merge_ok run: echo '::set-output name=merge_ok::true' test_python_linux_x86_64_0: - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Test Python (Linux-x86_64) Shard 0/3 needs: - bootstrap_pants_linux_x86_64 - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -923,12 +931,12 @@ jobs: - '3.7' timeout-minutes: 90 test_python_linux_x86_64_1: - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Test Python (Linux-x86_64) Shard 1/3 needs: - bootstrap_pants_linux_x86_64 - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -1014,12 +1022,12 @@ jobs: - '3.7' timeout-minutes: 90 test_python_linux_x86_64_2: - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Test Python (Linux-x86_64) Shard 2/3 needs: - bootstrap_pants_linux_x86_64 - - docs_only_check + - classify_changes runs-on: - ubuntu-20.04 steps: @@ -1107,12 +1115,12 @@ jobs: test_python_macos11_x86_64: env: ARCHFLAGS: -arch x86_64 - if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only - != 'DOCS_ONLY') + if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only + != true) name: Test Python (macOS11-x86_64) needs: - bootstrap_pants_macos11_x86_64 - - docs_only_check + - classify_changes runs-on: - macos-11 steps: diff --git a/build-support/bin/classify_changed_files.py b/build-support/bin/classify_changed_files.py new file mode 100644 index 00000000000..d6651a78b8e --- /dev/null +++ b/build-support/bin/classify_changed_files.py @@ -0,0 +1,67 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import enum +import fnmatch +import sys +from collections import defaultdict + +# This script may be run in CI before Pants is bootstrapped, so it must be kept simple +# and runnable without `./pants run`. + + +class Affected(enum.Enum): + docs = "docs" + rust = "rust" + release = "release" + ci_config = "ci_config" + other = "other" + + +_docs_globs = ["docs/*", "build-support/bin/generate_user_list.py"] +_rust_globs = ["src/rust/engine/*", "rust-toolchain", "build-support/bin/rust/*"] +_release_globs = [ + "pants.toml", + "src/python/pants/VERSION", + "src/python/pants/notes/*", + "src/python/pants/init/BUILD", + "build-support/bin/release.sh", + "build-support/bin/release_helper.py", +] +_ci_config_globs = [ + "build-support/bin/classify_changed_files.py", + "build-support/bin/generate_github_workflows.py", +] + + +_affected_to_globs = { + Affected.docs: _docs_globs, + Affected.rust: _rust_globs, + Affected.release: _release_globs, + Affected.ci_config: _ci_config_globs, +} + + +def classify(changed_files: list[str]) -> set[Affected]: + classified: dict[Affected, set[str]] = defaultdict(set) + for affected, globs in _affected_to_globs.items(): + for pattern in globs: + classified[affected].update(fnmatch.filter(changed_files, pattern)) + ret = {k for k, v in classified.items() if v} + if set(changed_files) - set().union(*classified.values()): + ret.add(Affected.other) + return ret + + +def main() -> None: + if len(sys.argv) < 2: + return + affecteds = classify(sys.argv[1].split("|")) + for affected in sorted([a.name for a in affecteds]): + print(affected) + + +if __name__ == "__main__": + main() diff --git a/build-support/bin/classify_changed_files_test.py b/build-support/bin/classify_changed_files_test.py new file mode 100644 index 00000000000..82c98c1aa8f --- /dev/null +++ b/build-support/bin/classify_changed_files_test.py @@ -0,0 +1,24 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import pytest +from classify_changed_files import Affected, classify + + +@pytest.mark.parametrize( + ["changed_files", "expected"], + ( + [["docs/path/to/some/doc", "docs/path/to/some/other/doc"], {Affected.docs}], + [["src/rust/engine/path/to/file.rs"], {Affected.rust}], + [["src/python/pants/VERSION"], {Affected.release}], + [["build-support/bin/generate_github_workflows.py"], {Affected.ci_config}], + [["src/python/pants/whatever.py"], {Affected.other}], + [["docs/path/to/some/doc", "rust-toolchain"], {Affected.docs, Affected.rust}], + [ + ["docs/path/to/some/doc", "rust-toolchain", "src/python/pants/whatever.py"], + {Affected.docs, Affected.rust, Affected.other}, + ], + ), +) +def test_classification(changed_files, expected): + assert classify(changed_files) == expected diff --git a/build-support/bin/generate_github_workflows.py b/build-support/bin/generate_github_workflows.py index f371ce4af2c..723c13cccbf 100644 --- a/build-support/bin/generate_github_workflows.py +++ b/build-support/bin/generate_github_workflows.py @@ -87,45 +87,56 @@ def hashFiles(path: str) -> str: # Actions # ---------------------------------------------------------------------- -_DOCS_ONLY_TEXT = "DOCS_ONLY" - -def _docs_only_cond(docs_only: bool) -> str: - op = "==" if docs_only else "!=" - return f"needs.docs_only_check.outputs.docs_only {op} '{_DOCS_ONLY_TEXT}'" - - -def is_docs_only() -> Jobs: - """Check if this change only involves docs.""" +def classify_changes() -> Jobs: linux_x86_64_helper = Helper(Platform.LINUX_X86_64) - docs_files = ["docs/**", "build-support/bin/generate_user_list.py"] return { - "docs_only_check": { - "name": "Check for docs-only change", + "classify_changes": { + "name": "Classify changes", "runs-on": linux_x86_64_helper.runs_on(), "if": IS_PANTS_OWNER, - "outputs": {"docs_only": gha_expr("steps.docs_only_check.outputs.docs_only")}, + "outputs": { + "docs_only": gha_expr("steps.classify.outputs.docs_only"), + "docs": gha_expr("steps.classify.outputs.docs"), + "rust": gha_expr("steps.classify.outputs.rust"), + "release": gha_expr("steps.classify.outputs.release"), + "ci_config": gha_expr("steps.classify.outputs.ci_config"), + }, "steps": [ *checkout(get_commit_msg=False), { "id": "files", "name": "Get changed files", - "uses": "tj-actions/changed-files@v32.0.0", - "with": {"files_ignore_separator": "|", "files_ignore": "|".join(docs_files)}, + "uses": "tj-actions/changed-files@v32", + "with": {"separator": "|"}, }, { - "id": "docs_only_check", - "name": "Check for docs-only changes", - # Note that if no changes were detected in the step above, the string - # may be empty, not 'false', so we check for != 'true'. - "if": "steps.files.outputs.any_changed != 'true'", - "run": f"echo '::set-output name=docs_only::{_DOCS_ONLY_TEXT}'", + "id": "classify", + "name": "Classify changed files", + "run": dedent( + """\ + affected=$(python build-support/bin/classify_changed_files.py \ + '${{ steps.files.outputs.all_modified_files }}') + echo "Affected:\n${affected}" + if [[ "${affected}" == "docs" ]]; then + echo '::set-output name=docs_only::true' + fi + for i in ${affected}; do + echo "::set-output name=${i}::true" + done + """ + ), }, ], }, } +def _docs_only_cond(docs_only: bool) -> str: + op = "==" if docs_only else "!=" + return f"needs.classify_changes.outputs.docs_only {op} true" + + def ensure_category_label() -> Sequence[Step]: """Check that exactly one category label is present on a pull request.""" return [ @@ -918,7 +929,7 @@ def set_merge_ok(needs: list[str], docs_only: bool) -> Jobs: # If in the future we have any docs-related checks, we can make both "Set Merge OK" # jobs depend on them here (it has to be both since some changes may modify docs # as well as code, and so are not "docs only"). - "needs": ["docs_only_check", "check_labels"] + sorted(needs), + "needs": ["classify_changes", "check_labels"] + sorted(needs), "outputs": {"merge_ok": f"{gha_expr('steps.set_merge_ok.outputs.merge_ok')}"}, "steps": [ { @@ -971,14 +982,14 @@ def generate() -> dict[Path, str]: not_docs_only = _docs_only_cond(docs_only=False) pr_jobs = test_workflow_jobs([PYTHON37_VERSION], cron=False) - pr_jobs.update(**is_docs_only()) + pr_jobs.update(**classify_changes()) for key, val in pr_jobs.items(): - if key in {"check_labels", "docs_only_check"}: + if key in {"check_labels", "classify_changes"}: continue needs = val.get("needs", []) if isinstance(needs, str): needs = [needs] - needs.extend(["docs_only_check"]) + needs.extend(["classify_changes"]) val["needs"] = needs if_cond = val.get("if") val["if"] = not_docs_only if if_cond is None else f"({if_cond}) && ({not_docs_only})"