Skip to content

Commit 9e204dd

Browse files
mhuckamhucka
authored
Vastly overhaul the CI workflow for file checks (#847)
Vastly overhaul the CI workflow for file checks This is a complete rewrite of the file checks portion of the CI workflow. It incorporates many new features and efficiency gains. It also serves as a framework for adding additional file linters/format checkers, such as for YAML, Markdown, shell scripts, and more. Features: - Serious use of caching of the Python installation and environment, so that repeated runs don't have to reinstall everything every time. - Concurrency detection: if a new event such as a push to a PR branch comes in while a workflow is already executing, that workflow is cancelled and a new one restarted. (The existing workflow keeps the existing one running and starts a second one, which is almost never useful.) - File-aware execution: the workflow tests which files have been modified in a given event, and invokes only the relevant lint/format-checker jobs. This reduces compute cycles needed and speeds up the overall execution. - Use of GitHub [Problem Matchers](https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md) for better error reporting. - Addition of a manual run interface, allowing the workflow to be invoked manually from the GitHub GUI with different values for program versions – useful for debugging, testing, exploring behaviors with different versions of linters, etc. - Compatibility with GitHub [merge queues](https://github.blog/news-insights/product-news/github-merge-queue-is-generally-available/). For Python, this implements the equivalent of `scripts/lint_all.sh` (which, despite its name, actually only lints Python files) and the Python-specific format checks of `scripts/format_check.sh`. For C++, this runs clang-format with input limited to the files changed in a given PR.
2 parents 9bb36aa + a02d3f6 commit 9e204dd

File tree

3 files changed

+357
-23
lines changed

3 files changed

+357
-23
lines changed

‎.github/problem-matchers/README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Problem Matchers
2+
3+
GitHub [Problem Matchers](https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md) are a mechanism that enable workflow steps to scan the outputs of GitHub Actions for regex patterns and automatically write annotations in the workflow summary page. Using Problem Matchers allows information to be displayed more prominently in the GitHub user interface.
4+
5+
This directory contains Problem Matchers used by the GitHub Actions workflows in the [`workflows`](./workflows) subdirectory.
6+
7+
The following problem matcher JSON files found in this directory were copied from the [Home Assistant](https://github.com/home-assistant/core) project on GitHub. The Home Assistant project is licensed under the Apache 2.0 open-source license. The version of the files at the time they were copied was 2025.1.2.
8+
9+
- [`pylint.json`](https://github.com/home-assistant/core/blob/dev/.github/workflows/matchers/pylint.json)
10+
- [`yamllint.json`](https://github.com/home-assistant/core/blob/dev/.github/workflows/matchers/yamllint.json)

‎.github/problem-matchers/pylint.json

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"problemMatcher": [
3+
{
4+
"owner": "pylint-error",
5+
"severity": "error",
6+
"pattern": [
7+
{
8+
"regexp": "^(.+):(\\d+):(\\d+):\\s(([EF]\\d{4}):\\s.+)$",
9+
"file": 1,
10+
"line": 2,
11+
"column": 3,
12+
"message": 4,
13+
"code": 5
14+
}
15+
]
16+
},
17+
{
18+
"owner": "pylint-warning",
19+
"severity": "warning",
20+
"pattern": [
21+
{
22+
"regexp": "^(.+):(\\d+):(\\d+):\\s(([CRW]\\d{4}):\\s.+)$",
23+
"file": 1,
24+
"line": 2,
25+
"column": 3,
26+
"message": 4,
27+
"code": 5
28+
}
29+
]
30+
}
31+
]
32+
}

‎.github/workflows/ci-file-checks.yaml

Lines changed: 315 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,326 @@
1+
# Summary: TFQ continuous integration workflow for static code analysis.
2+
#
3+
# This workflow runs linters and code format/style checkers on certain events
4+
# such as pull requests and merge-queue merges. It tries to be as efficient as
5+
# possible by only running jobs when specific types of files were affected by
6+
# a PR, and by caching the Python installation so that it doesn't have to be
7+
# re-installed on every run. It reads the requirements.txt file to find out
8+
# the required versions of some program like pylint and yapf, to adhere to DRY
9+
# principles. It uses GitHub "problem matchers" to write error outputs to the
10+
# workflow summary to make it easier to learn the outcome. Finally, It can be
11+
# invoked manually using the "Run workflow" button on the page at
12+
# https://github.com/tensorflow/quantum/actions/workflows/ci-file-checks.yaml
13+
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
14+
115
name: CI file checks
16+
run-name: Continuous integration file checks
17+
18+
on:
19+
pull_request:
20+
types: [opened, synchronize]
21+
branches:
22+
- main
23+
24+
push:
25+
branches:
26+
- main
27+
28+
merge_group:
29+
types:
30+
- checks_requested
31+
32+
# Allow manual invocation, with options that can be useful for debugging.
33+
workflow_dispatch:
34+
inputs:
35+
files:
36+
description: "Files or directories to check:"
37+
type: string
38+
python_ver:
39+
description: 'Python version:'
40+
type: string
41+
pylint_ver:
42+
description: 'Pylint version:'
43+
type: string
44+
yapf_ver:
45+
description: 'Yapf version:'
46+
type: string
47+
clang_format_ver:
48+
description: 'clang-format version:'
49+
type: string
250

3-
on: [pull_request]
51+
env:
52+
# Set some default versions that we can't get from files in the repo (they're
53+
# not in requirements.txt). These can be overridden via workflow dispatch.
54+
python_ver: '3.10'
55+
# Note: as of 2025-01-16, clang-format v. 18 is the latest available on
56+
# GitHub, and you have to use Ubuntu 24 to get it.
57+
clang_format_ver: '18'
58+
59+
concurrency:
60+
# Cancel any previously-started but still active runs on the same branch.
61+
cancel-in-progress: true
62+
group: ${{github.workflow}}-${{github.event.pull_request.number||github.ref}}
463

564
jobs:
6-
lint:
7-
name: Lint check
8-
runs-on: ubuntu-20.04
65+
changed-files:
66+
runs-on: ubuntu-24.04
67+
outputs:
68+
python: ${{steps.changes.outputs.python}}
69+
python_files: ${{steps.changes.outputs.python_files}}
70+
cc: ${{steps.changes.outputs.cc}}
71+
cc_files: ${{steps.changes.outputs.cc_files}}
72+
steps:
73+
- name: Check out a copy of the TFQ git repository
74+
uses: actions/checkout@v4
75+
76+
- name: Determine which files were changed
77+
uses: dorny/paths-filter@v3
78+
id: changes
79+
with:
80+
list-files: 'shell'
81+
# The outputs will be variables named "foo_files" for a filter "foo".
82+
filters: |
83+
python:
84+
- added|modified:
85+
- '**/*.py'
86+
cc:
87+
- added|modified:
88+
- '**/*.cc'
89+
- '**/*.h'
90+
- '**/*.proto'
91+
92+
python-setup:
93+
if: needs.changed-files.outputs.python == 'true'
94+
needs: changed-files
95+
runs-on: ubuntu-22.04
96+
timeout-minutes: 5
97+
outputs:
98+
cache_key: ${{steps.parameters.outputs.cache_key}}
99+
cache_paths: ${{steps.parameters.outputs.cache_paths}}
100+
files: ${{steps.files.outputs.files_to_check}}
101+
steps:
102+
- name: Check out a copy of the TFQ git repository
103+
uses: actions/checkout@v4
104+
105+
# Note: setup-python has a cache facility, but we don't use it here
106+
# because we want to cache more Python things than setup-python does.
107+
- name: Set up Python
108+
uses: actions/setup-python@v5
109+
with:
110+
python-version: ${{inputs.python_ver || env.python_ver}}
111+
112+
- name: Set cache parameters
113+
id: parameters
114+
run: |
115+
key="${{github.workflow_ref}}-${{hashFiles('requirements.txt')}}"
116+
echo "cache_key=$key" >> "$GITHUB_OUTPUT"
117+
# The paths used for actions/cache need to be on separate lines.
118+
# Constructing a multiline value for an output variable is awkward.
119+
# shellcheck disable=SC2005
120+
{
121+
echo 'cache_paths<<EOF'
122+
echo "$(pip cache dir)"
123+
echo "${{env.pythonLocation}}"
124+
echo 'EOF'
125+
} >> "$GITHUB_OUTPUT"
126+
127+
- name: Test if the cache already exists
128+
uses: actions/cache@v4
129+
id: check_cache
130+
with:
131+
lookup-only: true
132+
key: ${{steps.parameters.outputs.cache_key}}
133+
path: ${{steps.parameters.outputs.cache_paths}}
134+
135+
- if: steps.check_cache.outputs.cache-hit != 'true'
136+
name: Set up the cache
137+
uses: actions/cache@v4
138+
id: restore_cache
139+
with:
140+
key: ${{steps.parameters.outputs.cache_key}}
141+
path: ${{steps.parameters.outputs.cache_paths}}
142+
143+
- if: github.event_name != 'workflow_dispatch'
144+
name: Get the list of Python files changed in the triggering event
145+
id: changes
146+
uses: tj-actions/changed-files@v45
147+
# Although this workflow is triggered only if Python files are changed,
148+
# they might not be the *only* files changed; hence, we have to filter
149+
# the set of files down to just the Python files.
150+
with:
151+
files: |
152+
**.py
153+
154+
- name: Determine the files to be checked
155+
id: files
156+
run: |
157+
# Files/dirs given during manual invocation take precedence.
158+
if [[ "${{github.event_name}}" == "workflow_dispatch" ]]; then
159+
if [[ -n "${{inputs.files}}" ]]; then
160+
files="${{inputs.files}}"
161+
else
162+
echo "::error title=Error::No file names or directories provided."
163+
exit 1
164+
fi
165+
else
166+
files="${{steps.changes.outputs.all_changed_files}}"
167+
fi
168+
echo "files_to_check=$files" >> "$GITHUB_OUTPUT"
169+
170+
- if: steps.check_cache.outputs.cache-hit != 'true'
171+
name: Determine the versions of Pylint and Yapf to use
172+
id: req
173+
run: |
174+
# Get the pylint & yapf versions from requirements.txt.
175+
# In case of multiple values in the file, the last one will win.
176+
echo "pylint_version=" >> "$GITHUB_OUTPUT"
177+
echo "yapf_version=" >> "$GITHUB_OUTPUT"
178+
number_regex='[0-9]+(\.[0-9]+)?(\.[0-9]+)?'
179+
while IFS= read -r line; do
180+
if [[ $line =~ ^pylint.* ]]; then
181+
if [[ $line =~ $number_regex ]]; then
182+
version="${BASH_REMATCH[0]}"
183+
echo "pylint_version=$version" >> "$GITHUB_OUTPUT"
184+
continue
185+
fi
186+
fi
187+
if [[ $line =~ ^yapf.* ]]; then
188+
if [[ $line =~ $number_regex ]]; then
189+
version="${BASH_REMATCH[0]}"
190+
echo "yapf_version=$version" >> "$GITHUB_OUTPUT"
191+
continue
192+
fi
193+
fi
194+
done < "requirements.txt"
195+
# If given versions in manual invocation, use them instead.
196+
if [[ "${{inputs.pylint_ver}}" != "" ]]; then
197+
echo "pylint_version=${{inputs.pylint_ver}}" >> "$GITHUB_OUTPUT"
198+
fi
199+
if [[ "${{inputs.yapf_ver}}" != "" ]]; then
200+
echo "yapf_version=${{inputs.yapf_ver}}" >> "$GITHUB_OUTPUT"
201+
fi
202+
203+
- if: steps.check_cache.outputs.cache-hit != 'true'
204+
name: Install Pylint and Yapf
205+
run: |
206+
python -m pip install pylint==${{steps.req.outputs.pylint_version}}
207+
python -m pip install yapf==${{steps.req.outputs.yapf_version}}
208+
209+
cc-format:
210+
if: needs.changed-files.outputs.cc == 'true'
211+
name: C++ and Protobuf coding style
212+
needs: changed-files
213+
runs-on: ubuntu-24.04
214+
env:
215+
changed_files: ${{needs.changed-files.outputs.cc_files}}
9216
steps:
10-
- uses: actions/checkout@v1
11-
- uses: actions/setup-python@v1
217+
- name: Check out a copy of the TFQ git repository
218+
uses: actions/checkout@v4
219+
220+
- name: Run clang-format on C++ and Protobuf files
221+
run: |
222+
set -x
223+
version=${{inputs.clang_format_ver || env.clang_format_ver}}
224+
clang-format-$version --verbose --style google --dry-run \
225+
${{env.changed_files}} > diff.out 2>&1
226+
exit_code=$?
227+
if (( exit_code == 1 )); then
228+
# Write output both here and to the job summary.
229+
TERM=xterm-color
230+
bo=$'\e[1m'
231+
bl=$'\e[38;5;117m'
232+
rs=$'\e[0m'
233+
hi="👋🏻"
234+
u="https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}"
235+
echo "$hi ${bl}Visit $bo$u${rs}$bl for formatted diff output$rs $hi"
236+
echo "::group::clang-format output"
237+
cat diff.out
238+
echo "::endgroup::"
239+
# shellcheck disable=SC2006
240+
{
241+
echo "## Output from <code>clang-format</code> version $version"
242+
echo ""
243+
echo '```diff'
244+
echo "$(< diff.out)"
245+
echo '```'
246+
} >> "$GITHUB_STEP_SUMMARY"
247+
fi
248+
exit $exit_code
249+
250+
python-lint:
251+
if: needs.changed-files.outputs.python == 'true'
252+
name: Lint Python files
253+
needs: [changed-files, python-setup]
254+
runs-on: ubuntu-22.04
255+
steps:
256+
- name: Check out a copy of the TFQ git repository
257+
uses: actions/checkout@v4
258+
259+
- name: Set up Python
260+
uses: actions/setup-python@v5
261+
with:
262+
python-version: ${{inputs.python_ver || env.python_ver}}
263+
264+
- name: Restore the Python cache
265+
uses: actions/cache@v4
12266
with:
13-
python-version: '3.10'
14-
architecture: 'x64'
15-
- name: Install Lint tools
16-
run: pip install --upgrade pip setuptools; pip install -r requirements.txt;
17-
- name: Lint All
18-
run: ./scripts/lint_all.sh
267+
key: ${{needs.python-setup.outputs.cache_key}}
268+
path: ${{needs.python-setup.outputs.cache_paths}}
269+
fail-on-cache-miss: true
19270

20-
format:
21-
name: Formatting check
22-
runs-on: ubuntu-20.04
271+
- name: Set up pylint output problem matcher
272+
run: echo "::add-matcher::.github/problem-matchers/pylint.json"
23273

274+
- name: Lint the changed Python files
275+
run: |
276+
pylint ${{needs.changed-files.outputs.python_files}}
277+
278+
python-format:
279+
if: needs.changed-files.outputs.python == 'true'
280+
name: Check the format of Python files
281+
needs: [changed-files, python-setup]
282+
runs-on: ubuntu-22.04
24283
steps:
25-
- uses: actions/checkout@v1
26-
- uses: actions/setup-python@v1
284+
- name: Check out a copy of the TFQ git repository
285+
uses: actions/checkout@v4
286+
287+
- name: Set up Python
288+
uses: actions/setup-python@v5
289+
with:
290+
python-version: ${{inputs.python_ver || env.python_ver}}
291+
292+
- name: Restore the Python cache
293+
uses: actions/cache@v4
27294
with:
28-
python-version: '3.10'
29-
architecture: 'x64'
30-
- name: Install Format tools
31-
run: pip install --upgrade pip setuptools; pip install -r requirements.txt; sudo apt-get install -y clang-format-6.0
32-
- name: Format Check
33-
run: ./scripts/format_check.sh
295+
key: ${{needs.python-setup.outputs.cache_key}}
296+
path: ${{needs.python-setup.outputs.cache_paths}}
297+
fail-on-cache-miss: true
34298

299+
- name: Run Yapf on the Python changed files
300+
run: |
301+
set +e
302+
yapf --parallel --diff --style=google \
303+
"${{needs.changed-files.outputs.python_files}}" > diff.out 2>&1
304+
exit_code=$?
305+
if [[ -s ./diff.out ]]; then
306+
# Write output both here and to the job summary.
307+
TERM=xterm-color
308+
bo=$'\e[1m'
309+
bl=$'\e[38;5;117m'
310+
rs=$'\e[0m'
311+
hi="👋🏻"
312+
u="https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}"
313+
echo "$hi ${bl}Visit $bo$u${rs}$bl for formatted diff output$rs $hi"
314+
echo "::group::Yapf output"
315+
cat diff.out
316+
echo "::endgroup::"
317+
# shellcheck disable=SC2006
318+
{
319+
echo "## Output from <code>yapf</code>"
320+
echo ""
321+
echo '```diff'
322+
echo "$(< diff.out)"
323+
echo '```'
324+
} >> "$GITHUB_STEP_SUMMARY"
325+
fi
326+
exit $exit_code

0 commit comments

Comments
 (0)