Compare commits

..
Author SHA1 Message Date
Niels Lohmann 4b538d80f6 Merge branch 'develop' into ci/path-filters
Keep ci_test_documentation in repository_checks.yml and apply the
harden-runner 2.22.2 bump from #5962 there.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-30 15:35:39 +01:00
Niels Lohmann 7a4e6b7a5d Merge develop into ci/path-filters
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-28 22:09:53 +01:00
Niels Lohmann fd37b04813 Merge origin/develop into ci/path-filters
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-28 20:32:10 +01:00
Niels Lohmann bd442d5d07 Merge develop
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-28 07:11:43 +01:00
Niels Lohmann eae3c8217b Split the checks of check_path_filters.py into smaller functions
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-27 19:39:20 +01:00
Niels Lohmann 960c234cc6 Do not report the excluded files that check_path_filters.py lists itself
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-27 18:58:36 +01:00
Niels Lohmann b6e24d19a9 Skip the compiler workflows for pull requests that change no build input
ubuntu.yml, macos.yml and windows.yml run about 155 jobs on every pull
request, including those that change only the documentation. They now
have a pull_request path filter: it starts with '**' and excludes the
documentation, Markdown files, GitHub metadata, other workflows and the
license files, so a new file runs them unless it is excluded on purpose.
Pushes to develop, master and release/* are not filtered.

The documentation jobs and the REUSE check move from ubuntu.yml to the
new repository_checks.yml, which has no path filter and also runs
tools/check_path_filters. That check keeps the three filters identical
and fails when a file the compiler jobs keep mentions an excluded file.
It found two: tools/check_build_options reads the Meson section of
package_managers.md, and the swiftpm job builds the Swift example from
the documentation; both are included in the filter.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-27 18:35:03 +01:00
10 changed files with 450 additions and 220 deletions

No files matched your search

@@ -1,194 +0,0 @@
---
name: review-like-gregmarr
description: Review a diff, branch, or PR of nlohmann/json the way long-time contributor gregmarr does — a line-by-line read focused on C++11 correctness, reference invalidation, backward/source compatibility, ODR/ABI, duplicated logic, hidden costs, repo conventions (amalgamation, astyle, macros) and precise docs, phrased as short courteous questions. Use when asked to "review like Greg", "gregmarr review", or for a careful maintainer-style review of changes to this library.
---
# Review like gregmarr
Distilled from ~1,400 inline review comments and ~1,300 issue/PR comments by
@gregmarr on nlohmann/json. He is a careful, courteous, line-by-line reviewer
who ("just a contributor") defers final decisions to @nlohmann.
## Workflow
1. Determine the target: a PR number (`gh pr diff <n>`), a branch
(`git diff $(git merge-base HEAD develop)...HEAD`), or the working tree
(`git diff`). Read the full diff, and open surrounding code in `include/nlohmann/`
whenever a hunk's context is not enough to judge it.
2. Walk the diff **file by file, hunk by hunk**, applying the checklist below.
Comment on specific lines; don't write a big essay.
3. Write the review in his voice (see "Voice"). Group nothing artificially —
emit one comment per location, in file/line order, with `path:line`.
4. If the same issue repeats, comment fully once, then "As above" /
"Same here (and N other places in this file)."
5. End with a 1–3 line summary only if useful (e.g. "Just this remaining and
then LGTM." or "@nlohmann thoughts on the naming?"). Don't post to GitHub
unless the user asks.
## Checklist (roughly in order of how often he raises them)
### Duplication and simplification (most frequent)
- Copy-pasted bodies, duplicated `#if/#elif` blocks, or "a copy of this function
that can get out of sync" → propose a shared helper or alias, with a code sketch.
- **Diverging limits/constants**: a new depth/recursion limit or magic number
where one already exists → "Should this reuse the existing recursion limit?
Different limits per operation invites them to diverge over time."
- Unneeded variables, parameters that can be deduced, values passed to a
function that the constructor already has, output parameter == return value.
- Verbose type spellings → `using` alias / alias template; `decltype()` where the
type is known.
- Disabling a lint warning instead of fixing the code ("why not just add the parens?").
### Lifetime, invalidation, UB
- A reference/pointer/iterator into a container (`stack.back()`,
`container_stack.back()`, `ordered_map` elements) held across `push_back`,
`pop_back`, `emplace`, or reallocation → "This invalidates `top`." Prefer a
`const` copy, or at least a comment warning future editors.
- Dangling refs from rvalue arguments stored in adapters; views outliving documents.
- Wrong logic operators (`&&` vs `||`), lost `!`, unreachable code, `continue`
that should be `break`, missing `return`, off-by-one in counters
(`chars_read` incremented twice).
- Uninitialized members → default member initializers; parameters that
"should not have a default so it's never left off by accident".
- Exception safety: leaks when `to_json` is called on a non-null value, partial
state, destructors that allocate, swallowed exceptions (`noexcept` that hides
throws — "The library shouldn't try to swallow the exception").
- Thread-unsafe calls (`setlocale`), specializations that redefine `std::` things
(ODR / UB).
- Narrowing/truncation: `static_cast<std::make_unsigned<...>>`, signed/unsigned
`char` cases, `n == 0` edge cases, buffer overruns guarded only by `JSON_ASSERT`.
### C++11 floor and portability
- The library requires only **C++11**: flag `if constexpr`, C++14 generic
lambdas/variable templates, `std::clamp`, C++17 library features, etc. unless
gated.
- Gate on the library's macros (`JSON_HAS_CPP_17`, `JSON_HAS_CPP_20`,
`JSON_HAS_THREE_WAY_COMPARISON`, `JSON_HAS_FILESYSTEM`) or feature-test macros
with `defined()` checks and exact values
(`#if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811`), not raw
`__cplusplus`. Don't silently disable a feature for an existing config.
- User-overridable macros: if externally defined, leave as-is — never clobber to
`1` (breaks `#define X 0`). `#if X` vs `#ifdef X` must match the convention.
- New macros start with `JSON_` / `NLOHMANN_JSON_` consistently with neighbors.
- `throw` must be `JSON_THROW` (and `try/catch` → `JSON_TRY/JSON_CATCH`) so
`JSON_NOEXCEPTION` builds work.
- `(std::min)(a, b)` / `(std::max)` for Windows macros; MSVC warnings (C4100
etc.); `string_t` not being `std::string`/`char`.
- Code relying on operations not required of user-supplied `array_t`/`object_t`
(range ctor, `emplace_hint`, allocator) → ask whether that's allowed.
- Does it work with `JSON_USE_IMPLICIT_CONVERSIONS=0`? With `JSON_DIAGNOSTICS`?
### Compatibility, ABI, API design
- Behavior changes to existing APIs/macros (e.g. `NLOHMANN_DEFINE_TYPE_*`,
released in 3.11.0) are breaking → needs opt-in macro or wait for 4.0; must be
prominently documented in release notes. Cite when the behavior shipped.
- Header-only: source compatibility matters, binary doesn't — adding a
**defaulted** parameter is fine; new overloads add ambiguity risk.
- Anything that changes layout or semantics per-config → should it be part of the
ABI tag (`json_abi_diag...`)? Mismatched config macros across TUs = ODR violation.
- Brand-new API shouldn't be added as deprecated — use `= delete` or omit.
- Naming consistency with existing options (`ignore_trailing_commas` vs
`ignore_comments`; singular "diagnostic"); names that read like functions;
single-letter or inconsistently `m_`-prefixed members.
- Prefer a type-safe template over passing a `value_t` that could be invalid.
- "Why would this belong in this library?" for very niche features.
### Hidden costs
- Extra copies (lost `std::move`, removed C++17 path), temporaries
(`push_back(std::string(...))` vs two `push_back`s), repeated `.back()` calls,
casting more than once, `reserve()` inside loops, quadratic recomputation for
deep nesting, dynamic allocation replacing stack allocation.
- `push_back` → `emplace_back`; `rbegin()`, `.data()`, `std::array`, `pop_back()`
to keep counters in sync.
- Struct member order/padding (put `bool` after `int`; "store value before type"),
`sizeof(basic_json)` growth, per-node pointer cost. "Don't pay for what you
don't use."
- Compile-time impact of new includes/templates.
- Ask for measurement instead of asserting a win.
### Style and repo conventions
- Formatting must match `make pretty` (astyle): spaced `template <`, `!` in traits
→ suggest `detail::negation<>`; 4-space indents; brace style; blank lines.
- Redundant keywords: `inline` inside class bodies or on templates (full
specializations do need it), `noexcept` on destructors, `public` in `struct`,
`{}` after defaulted init, C-style casts → `static_cast`.
- Sorted includes (cpplint: system before others), sorted lists in docs/CMake.
- **Amalgamation**: changes must be in `include/nlohmann/...` and
`single_include/` regenerated via `make amalgamate` — flag edits made only in
`single_include`, or amalgamated output that doesn't match / contains unrelated
changes.
- Merge debris, stray files at repo root, accidental changes to vendored files
(doctest.h), Makefile tweaks from local testing, missing trailing newline,
whitespace churn mixed with real changes ("separate commit or PR?").
- Comments: explain non-obvious code ("Comment about why this iterates
backwards?"); no change-log wording like "still" in live code; comment
blocks must stay attached to their function.
### Docs (`docs/mkdocs/docs/...`, README)
- Typos as `s/varry/vary/` or "typo: interal".
- Accuracy vs. actual behavior; state preconditions ("array is not empty"),
costs (one pointer per value with `JSON_DIAGNOSTICS`), defaults, which overloads
a sentence applies to, version numbers ("3.13.0").
- Consistency with sibling pages; unexplained claims ("Why is that a problem?
It's not obvious here."); markdown table column alignment.
- New macros/features need a docs page and an example.
### Tests
- Not his main focus, but: new macro/feature needs tests in `tests/src/unit-*.cpp`
that always build with the option (coverage build); test both with and
without the option; tests should use different values than the
initial ones; put tests in the existing file instead of duplicating a SAX class;
static_asserts where possible.
### Usually NOT commented on
Commit messages/PR titles, licensing, local variable naming bikeshedding,
demanding benchmarks, rejecting the overall feature idea. CI YAML gets
comments only for merge debris.
## Voice
- **Mostly questions**: "Is there a reason…?", "Should this…?", "Would it make
sense to…?", "Why not…?", "Any reason not to…?", "Is this needed?",
"Intentional?"
- **Flat statements only for clear bugs**, with the reason: "This `if` will
never be true. You want `||` not `&&`." "Remove the noexcept. The library
shouldn't try to swallow the exception."
- **Terse when trivial**: "`emplace_back`", "typo: alse", "astyle, negation",
"as above", "Lost a blank line here."
- **Concrete sketches** in plain fenced code blocks introduced with "Maybe like
this?" / "Something like this:" / "Untested:". Rarely ```suggestion blocks.
- **Hedge taste**: "minor nit:", "Personally I prefer…", "I'm on the fence…",
"not something for which I would hold up the PR", "(If this was just moved,
ignore this.)", "not the fault of this PR".
- Cite evidence: cppreference, godbolt, the standard, related issues/PRs by
number, existing code ("see `NLOHMANN_JSON_ABI_TAG_DIAGNOSTICS`").
- Light humor allowed sparingly (":)", "astyle strikes again", "Ugh").
- Praise is rare and short ("Good change.", "Looks reasonable.").
- Defer design calls: "@nlohmann has the final say here."
- Never invent facts about the code: if unsure, phrase it as a question rather than a claim.
## Example output
```
include/nlohmann/detail/input/parser.hpp:212
This invalidates `top`. Make it a `const` copy so it can't be read after the
`push_back` by a future change?
include/nlohmann/detail/input/parser.hpp:240
Should this reuse the existing recursion limit rather than adding a new one?
Different limits per operation invites them to diverge over time.
include/nlohmann/detail/macro_scope.hpp:88
If this has been externally defined, we should leave it as-is. This will break
if someone has `#define JSON_HAS_FOO 0`.
include/nlohmann/json.hpp:1534
`throw` → `JSON_THROW`, or this can't be used by people that disable exceptions.
single_include/nlohmann/json.hpp:9001
Looks like this was edited directly. Changes go in `include/` and then
`make amalgamate`.
docs/mkdocs/docs/api/basic_json/back.md:12
Should this list the precondition that the array is not empty?
```
+5
View File
@@ -154,6 +154,11 @@ and each page's title (H1) and declaration by
[`docs/docset/generate_docset.py`](https://github.com/nlohmann/json/blob/develop/docs/docset/generate_docset.py);
`make build` reports API pages that cannot be classified.
A pull request that changes only the documentation, Markdown files, GitHub metadata, or license files skips the
compiler workflows (Ubuntu, macOS, Windows); the documentation and license checks still run. See
[`tools/check_path_filters`](https://github.com/nlohmann/json/tree/develop/tools/check_path_filters) for the list of
skipped files and what to do when a compiler job starts reading one of them.
### Amalgamate the source code
The single-header files
+19
View File
@@ -7,6 +7,25 @@ on:
- master
- release/*
pull_request:
# Skip pull requests that change only files no job here reads. The list is checked by
# tools/check_path_filters/check_path_filters.py (repository_checks.yml); change both together.
paths:
- '**'
- '!docs/**'
- '!**.md'
- '!.github/*'
- '!.github/ISSUE_TEMPLATE/**'
- '!.github/external_ci/**'
- '!.github/workflows/**'
- '!.reuse/**'
- '!LICENSES/**'
- '!CITATION.cff'
- '!REUSE.toml'
# read by tools/check_build_options in the ci_meson_install job (ubuntu.yml)
- 'docs/mkdocs/docs/integration/package_managers.md'
# built by the swiftpm job (macos.yml)
- 'docs/mkdocs/docs/integration/swift/**'
- '.github/workflows/macos.yml'
workflow_dispatch:
concurrency:
+78
View File
@@ -0,0 +1,78 @@
name: Repository checks
# Checks that concern files the compiler workflows (ubuntu.yml, macos.yml,
# windows.yml) skip with their path filter: the documentation, the license
# information, and the path filter itself. This workflow has no path filter.
on:
push:
branches:
- develop
- master
- release/*
pull_request:
workflow_dispatch:
permissions:
contents: read
concurrency:
group: ${{ github.workflow }}-${{ github.ref || github.run_id }}
cancel-in-progress: true
jobs:
check_path_filters:
runs-on: ubuntu-26.04
steps:
- name: Harden Runner
uses: step-security/harden-runner@dc2769abcc0fcfbadfc1df54081c1608579ef6b2 # v2.22.2
with:
egress-policy: audit
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: Check the path filters of the compiler workflows
run: python3 tools/check_path_filters/check_path_filters.py
ci_reuse_compliance:
runs-on: ubuntu-26.04
steps:
- name: Harden Runner
uses: step-security/harden-runner@dc2769abcc0fcfbadfc1df54081c1608579ef6b2 # v2.22.2
with:
egress-policy: audit
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: Get latest CMake and ninja
uses: lukka/get-cmake@a89e43e22a96abafcd7a62db652a65ecd33052c9 # v4.4.4
- name: Run CMake
run: cmake -S . -B build -DJSON_CI=On
- name: Build
run: cmake --build build --target ci_reuse_compliance
ci_test_documentation:
runs-on: ubuntu-26.04
strategy:
matrix:
target: [ci_test_examples, ci_test_build_documentation, ci_test_documentation_mermaid]
steps:
- name: Harden Runner
uses: step-security/harden-runner@dc2769abcc0fcfbadfc1df54081c1608579ef6b2 # v2.22.2
with:
egress-policy: audit
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
# the git-revision-date-localized plugin needs the history; a shallow clone makes the strict build fail
fetch-depth: ${{ matrix.target == 'ci_test_build_documentation' && '0' || '1' }}
- uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
if: matrix.target == 'ci_test_documentation_mermaid'
with:
node-version: 24
- name: Run CMake
run: cmake -S . -B build -DJSON_CI=On
- name: Build
run: cmake --build build --target ${{ matrix.target }}
+20 -26
View File
@@ -7,6 +7,25 @@ on:
- master
- release/*
pull_request:
# Skip pull requests that change only files no job here reads. The list is checked by
# tools/check_path_filters/check_path_filters.py (repository_checks.yml); change both together.
paths:
- '**'
- '!docs/**'
- '!**.md'
- '!.github/*'
- '!.github/ISSUE_TEMPLATE/**'
- '!.github/external_ci/**'
- '!.github/workflows/**'
- '!.reuse/**'
- '!LICENSES/**'
- '!CITATION.cff'
- '!REUSE.toml'
# read by tools/check_build_options in the ci_meson_install job (ubuntu.yml)
- 'docs/mkdocs/docs/integration/package_managers.md'
# built by the swiftpm job (macos.yml)
- 'docs/mkdocs/docs/integration/swift/**'
- '.github/workflows/ubuntu.yml'
workflow_dispatch:
permissions:
@@ -109,7 +128,7 @@ jobs:
runs-on: ubuntu-26.04
strategy:
matrix:
target: [ci_test_single_header, ci_cppcheck, ci_cpplint, ci_reproducible_tests, ci_non_git_tests, ci_offline_testdata, ci_reuse_compliance, ci_test_valgrind]
target: [ci_test_single_header, ci_cppcheck, ci_cpplint, ci_reproducible_tests, ci_non_git_tests, ci_offline_testdata, ci_test_valgrind]
steps:
- name: Harden Runner
uses: step-security/harden-runner@dc2769abcc0fcfbadfc1df54081c1608579ef6b2 # v2.22.2
@@ -449,31 +468,6 @@ jobs:
- name: Build
run: cmake --build build
ci_test_documentation:
runs-on: ubuntu-26.04
strategy:
matrix:
target: [ci_test_examples, ci_test_build_documentation, ci_test_documentation_mermaid]
steps:
- name: Harden Runner
uses: step-security/harden-runner@dc2769abcc0fcfbadfc1df54081c1608579ef6b2 # v2.22.2
with:
egress-policy: audit
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
# the git-revision-date-localized plugin needs the history; a shallow clone makes the strict build fail
fetch-depth: ${{ matrix.target == 'ci_test_build_documentation' && '0' || '1' }}
- uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
if: matrix.target == 'ci_test_documentation_mermaid'
with:
node-version: 24
- name: Run CMake
run: cmake -S . -B build -DJSON_CI=On
- name: Build
run: cmake --build build --target ${{ matrix.target }}
# Native Linux ARM64 runner with the latest GCC. The ubuntu-24.04-arm label is
# only available for public repositories.
ci_test_arm64:
+19
View File
@@ -7,6 +7,25 @@ on:
- master
- release/*
pull_request:
# Skip pull requests that change only files no job here reads. The list is checked by
# tools/check_path_filters/check_path_filters.py (repository_checks.yml); change both together.
paths:
- '**'
- '!docs/**'
- '!**.md'
- '!.github/*'
- '!.github/ISSUE_TEMPLATE/**'
- '!.github/external_ci/**'
- '!.github/workflows/**'
- '!.reuse/**'
- '!LICENSES/**'
- '!CITATION.cff'
- '!REUSE.toml'
# read by tools/check_build_options in the ci_meson_install job (ubuntu.yml)
- 'docs/mkdocs/docs/integration/package_managers.md'
# built by the swiftpm job (macos.yml)
- 'docs/mkdocs/docs/integration/swift/**'
- '.github/workflows/windows.yml'
workflow_dispatch:
permissions:
+3
View File
@@ -766,6 +766,9 @@ class basic_json_view
template<typename, typename> friend class detail::view::view_side;
friend iterator;
/// whether the view is one of an editable document
static constexpr bool is_editable = Editable;
basic_json_view(const document_data* d, const node* n) noexcept
: m_doc(d), m_node(n)
{}
+3
View File
@@ -9807,6 +9807,9 @@ class basic_json_view
template<typename, typename> friend class detail::view::view_side;
friend iterator;
/// whether the view is one of an editable document
static constexpr bool is_editable = Editable;
basic_json_view(const document_data* d, const node* n) noexcept
: m_doc(d), m_node(n)
{}
+34
View File
@@ -0,0 +1,34 @@
# check_path_filters
The compiler workflows [`ubuntu.yml`](../../.github/workflows/ubuntu.yml),
[`macos.yml`](../../.github/workflows/macos.yml), and [`windows.yml`](../../.github/workflows/windows.yml) skip pull
requests that change only files no compiler job reads: the documentation, Markdown files, GitHub metadata, other
workflows, and the license files. Pushes to `develop`, `master`, and `release/*` always run them. The documentation,
license, and path-filter checks run in [`repository_checks.yml`](../../.github/workflows/repository_checks.yml), which
has no path filter.
The filter starts with `'**'` and excludes these files explicitly, so a new file runs the compiler workflows unless it
is excluded on purpose. [`check_path_filters.py`](check_path_filters.py) holds the filter (`PATHS`) and checks that
- each of the three workflows has exactly this filter, with its own workflow file included,
- the CI targets that read excluded files (`UNFILTERED_TARGETS`) do not run in these workflows, and
- no file the filter keeps (sources, tests, build files, tools, workflows) mentions an excluded file or directory
outside a comment line, unless the mention is listed in `KNOWN_REFERENCES` with a reason.
When the check reports a mention, decide whether a compiler job reads that file:
- If it does, include the file in `PATHS` after the exclusions (as `package_managers.md` and the Swift example are) and
copy the new filter into the three workflows.
- If it does not, add the mention to `KNOWN_REFERENCES` with the reason.
Run the check with
```shell
python3 tools/check_path_filters/check_path_filters.py
```
To see which workflows a pull request would run, pass the changed files:
```shell
python3 tools/check_path_filters/check_path_filters.py --changed $(git diff --name-only origin/develop)
```
+269
View File
@@ -0,0 +1,269 @@
#!/usr/bin/env python3
"""Check the path filters of the compiler workflows.
ubuntu.yml, macos.yml and windows.yml skip pull requests that change only files
no compiler job reads (documentation, Markdown files, GitHub metadata, license
files). The filter starts with '**' and excludes these files explicitly, so a
new file runs the workflows unless it is excluded on purpose.
The script checks that
- each of the three workflows has exactly the filter defined in PATHS below,
with its own workflow file included, and
- no tracked file that the filter keeps (sources, tests, build files, tools,
workflows) mentions a file the filter excludes, unless the mention is listed
in KNOWN_REFERENCES with the reason why it does not matter.
A mention found this way must either be added to PATHS as an included path
(the file is read by a compiler job) or to KNOWN_REFERENCES (it is not).
With --changed FILE..., the script prints instead which workflows a pull
request changing these files would run.
"""
import argparse
import os
import re
import subprocess # nosec B404
import sys
REPO_ROOT = os.path.normpath(os.path.join(sys.path[0], '..', '..'))
SELF = 'tools/check_path_filters/check_path_filters.py'
WORKFLOWS = ['.github/workflows/ubuntu.yml', '.github/workflows/macos.yml', '.github/workflows/windows.yml']
# The pull_request paths filter of the workflows above; '{workflow}' is replaced
# with the workflow's own file. GitHub applies the patterns in order: a file
# matching a later pattern is included ('pattern') or excluded ('!pattern').
PATHS = [
'**',
'!docs/**',
'!**.md',
'!.github/*',
'!.github/ISSUE_TEMPLATE/**',
'!.github/external_ci/**',
'!.github/workflows/**',
'!.reuse/**',
'!LICENSES/**',
'!CITATION.cff',
'!REUSE.toml',
# read by tools/check_build_options in the ci_meson_install job (ubuntu.yml)
'docs/mkdocs/docs/integration/package_managers.md',
# built by the swiftpm job (macos.yml)
'docs/mkdocs/docs/integration/swift/**',
'{workflow}',
]
# Mentions of excluded files that no job of the three workflows depends on:
# (file that mentions it, the mention, reason)
KNOWN_REFERENCES = [
('cmake/ci.cmake', 'docs/', 'ci_test_examples; runs in repository_checks.yml, which has no path filter'),
('cmake/ci.cmake', 'docs/mkdocs/', 'ci_test_build_documentation and ci_test_documentation_mermaid; they run in '
'repository_checks.yml, which has no path filter'),
('cmake/ci.cmake', 'docs/mkdocs/docs/examples/', 'indentation check of ci_test_amalgamation; the same check runs in '
'check_amalgamation.yml, which has no path filter'),
('Makefile', 'docs/mkdocs/docs/examples/', 'make pretty; runs in check_amalgamation.yml, which has no path filter'),
('Makefile', 'ChangeLog.md', 'make ChangeLog.md writes the file; no CI job runs it'),
('Makefile', 'tests/fuzzing.md', 'help text'),
('Makefile', 'tools/generate_natvis/README.md', 'help text'),
('tests/src/make_test_data_available.hpp', 'README.md', 'a file of the external test data repository'),
('tools/check_build_options/check_build_options.py', 'docs/', 'builds the path of package_managers.md, which '
'PATHS includes'),
]
# CI targets that read excluded files; they must run in a workflow without a path filter
UNFILTERED_TARGETS = ['ci_test_examples', 'ci_test_build_documentation', 'ci_test_documentation_mermaid',
'ci_test_amalgamation', 'ci_reuse_compliance']
def git_files(root):
# fixed command, no user input; includes new files not yet added, so a local run
# sees what CI sees after the commit
out = subprocess.check_output(['git', 'ls-files', '-z', '--cached', '--others', '--exclude-standard'], cwd=root) # nosec B603, B607
return [f for f in out.decode('utf-8').split('\0') if f]
def glob_to_regex(pattern):
"""Translate a GitHub Actions path pattern to a regular expression."""
regex = ''
i = 0
while i < len(pattern):
if pattern.startswith('**', i):
regex += '.*'
i += 2
elif pattern[i] == '*':
regex += '[^/]*'
i += 1
elif pattern[i] == '?':
regex += '[^/]'
i += 1
else:
regex += re.escape(pattern[i])
i += 1
return re.compile(regex + r'\Z')
def compile_filter(paths, workflow):
return [(p.startswith('!'), glob_to_regex(p.lstrip('!').replace('{workflow}', workflow))) for p in paths]
def included(compiled, path):
result = False
for negated, regex in compiled:
if regex.match(path):
result = not negated
return result
def read_filter(root, workflow):
"""Return the pull_request paths list of a workflow file."""
with open(os.path.join(root, workflow), encoding='utf-8') as f:
lines = f.read().splitlines()
paths = None
in_pull_request = False
for line in lines:
if re.match(r'^ pull_request:', line):
in_pull_request = True
continue
if in_pull_request:
if re.match(r'^ \S', line) or re.match(r'^\S', line):
break
if re.match(r'^ paths:\s*$', line):
paths = []
continue
m = re.match(r"^ - '(.*)'\s*(#.*)?$", line)
if paths is not None and m:
paths.append(m.group(1))
elif paths is not None and line.strip() and not line.strip().startswith('#'):
break
return paths
TOKEN = re.compile(r'(["\'/]?)([\w.-]+(?:/[\w.-]+)*)')
COMMENT_LINE = re.compile(r'^[ \t]*(//|#|/?\*|--).*$', re.MULTILINE)
class Mentions:
"""Finds the words and paths in a text that name an excluded file or directory."""
def __init__(self, files, excluded, is_excluded):
self.files = set(files)
self.excluded = excluded
self.dirs = {'/'.join(f.split('/')[:n]) for f in files for n in range(1, f.count('/') + 1)}
# an excluded directory: a new file in it would be excluded
self.excluded_dirs = {d for d in self.dirs if is_excluded(d + '/new_file')}
# base names that only excluded files have, e.g. 'ChangeLog.md' but not 'CMakeLists.txt'
self.names = {os.path.basename(f) for f in excluded} - {os.path.basename(f) for f in files if f not in excluded}
def find(self, text):
"""Yield (offset, mention) for every word or path in text that names an excluded file or directory."""
for m in TOKEN.finditer(text):
parts = m.group(2).rstrip('/').split('/')
# a single word names a directory only in a path context, e.g. 'docs' in os.path.join('docs', ...)
path_context = len(parts) > 1 or m.group(1)
for i in range(len(parts)):
path = '/'.join(parts[i:])
if path in self.files:
if path in self.excluded:
yield m.start(2), path
break
if path in self.dirs and path_context:
if path in self.excluded_dirs:
yield m.start(2), path + '/'
break
else:
if parts[-1] in self.names and '.' in parts[-1]:
yield m.start(2), parts[-1]
def known(path, mention):
return any(path == file and mention == known_mention for file, known_mention, _ in KNOWN_REFERENCES)
def check_workflows(root):
"""Return the errors in the filters of the workflows and in the targets they run."""
errors = []
for workflow in WORKFLOWS:
expected = [p.replace('{workflow}', workflow) for p in PATHS]
actual = read_filter(root, workflow)
if actual != expected:
errors.append('{}: the pull_request paths filter differs from tools/check_path_filters/check_path_filters.py\n'
' expected: {}\n actual: {}'.format(workflow, expected, actual))
with open(os.path.join(root, workflow), encoding='utf-8') as f:
text = f.read()
errors += ['{}: runs {}, which reads excluded files; run it in repository_checks.yml'.format(workflow, target)
for target in UNFILTERED_TARGETS if re.search(r'\b' + target + r'\b', text)]
return errors
def scanned_text(root, path):
"""Return the text of a kept file without comment lines and without the filter itself, or None to skip it."""
# third-party code and Markdown files read nothing; this script lists the excluded files
if path.startswith('tests/thirdparty/') or path.endswith('.md') or path == SELF:
return None
try:
with open(os.path.join(root, path), encoding='utf-8') as f:
# a mention in a comment line reads nothing; keep the line numbers
text = COMMENT_LINE.sub('', f.read())
except (UnicodeDecodeError, OSError):
return None
if path in WORKFLOWS:
# the filter itself lists the excluded files
for p in PATHS:
text = text.replace(" - '{}'".format(p.replace('{workflow}', path)), '')
return text
def check_mentions(root, files, excluded, finder):
"""Return an error for every mention of an excluded file in a kept file that KNOWN_REFERENCES does not list."""
errors = []
for path in sorted(set(files) - excluded):
text = scanned_text(root, path)
reported = set()
for offset, mention in finder.find(text or ''):
if mention not in reported and not known(path, mention):
reported.add(mention)
line = text.count('\n', 0, offset) + 1
errors.append('{}:{}: mentions excluded {}; include it in PATHS if a compiler job reads it, '
'or add it to KNOWN_REFERENCES'.format(path, line, mention))
return errors
def check(root):
files = git_files(root)
union = [compile_filter(PATHS, w) for w in WORKFLOWS]
def is_excluded(path):
return not any(included(c, path) for c in union)
excluded = {f for f in files if is_excluded(f)}
errors = check_workflows(root) + check_mentions(root, files, excluded, Mentions(files, excluded, is_excluded))
for e in errors:
print(e)
if not errors:
print('path filters OK: {} of {} tracked files skip the compiler workflows'.format(len(excluded), len(files)))
return 1 if errors else 0
def explain(changed):
for workflow in WORKFLOWS:
compiled = compile_filter(PATHS, workflow)
hits = [f for f in changed if included(compiled, f)]
print('{}: {}'.format(workflow, 'runs ({})'.format(', '.join(hits[:3]) + (' ...' if len(hits) > 3 else ''))
if hits else 'skipped'))
return 0
def main():
parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
parser.add_argument('--root', default=REPO_ROOT, help='repository root')
parser.add_argument('--changed', nargs='+', metavar='FILE', help='show which workflows these changed files run')
args = parser.parse_args()
return explain(args.changed) if args.changed else check(args.root)
if __name__ == '__main__':
sys.exit(main())