Repository navigation
ci(validate_dialect): validate the dialect on every pull request - #11
Conversation
c337498 to
e9a8481
Compare
caea0ae to
3797060
Compare
3797060 to
157da87
Compare
|
Could this PR also skip publishing when only the build date changed, and narrow the generate triggers? Both touch files it already rewrites. Merging #15 (the contributing guidelines) published Assisted-by: Claude-Code:claude-opus-5-5 |
|
@mrpollo done, in c3515c4 and 70337f8. The publish check is The generate triggers are |
mrpollo
left a comment
There was a problem hiding this comment.
General rule for the workflows and the setup action: a run: block longer than 3 lines goes into .github/scripts/ as its own script, following generate_c_headers.sh and publish_c_library.sh: POSIX sh, a usage header, inputs as arguments. A single command wrapped over several lines, like the mavgen and gcc calls, can stay inline. Scripts can be shellchecked, tested like the Python checkers, and run locally. I've marked each block that needs it.
The c-library environment is set up, holds C_LIBRARY_DEPLOY_KEY and is limited to main.
Assisted-by: Claude-Code:claude-opus-5-5
2ef6cb6 to
16c9417
Compare
|
@mrpollo thanks for setting up |
.github/scripts holds every helper in one flat folder, and this series adds a dozen scripts and their tests. A flat folder hides which workflow runs which script, while CONTRIBUTING.md scopes a script change by the workflow that runs it. Each workflow's scripts go to .github/scripts/<workflow>/: check_commits.sh to commit_checks/, generate_c_headers.sh and publish_c_library.sh to generate_c_lib/. Their tests go to a tests/ folder beside them, which the same workflow runs. The files move byte for byte; the workflows, the CONTRIBUTING.md examples and the commit and pr skills name the new paths, and CONTRIBUTING.md states the layout. generate_c_lib.yml keeps triggering on .github/scripts/**, which matches the new folders too. Validation: git records the three moves as 100% renames, actionlint 1.7.12 reports nothing, shellcheck 0.11.0 finds nothing in the moved scripts, and check_commits.sh passes every subject of this series from its new path. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The conventions job checks out the pull request head with actions/checkout@v4, a tag that can move, and keeps the token in .git/config for the rest of the job, although nothing after the checkout pushes. It runs without a timeout, so a hung step holds a runner for the default six hours. The checkout is pinned to v4.4.0 by SHA with persist-credentials false, and the job is bounded at five minutes. The job keeps its id and gets no name, so a required status check that names it still matches. Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1 finds nothing in commit_checks.yml. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
MAVLINK_REF and PYMAVLINK_REF sit in generate_c_lib.yml's env, and docs/scripts/generate_messages.py digs MAVLINK_REF out of that YAML with a regular expression, so the docs depend on the workflow's layout, and docs.yml misses a pin bump unless it also triggers on the generate workflow. The generate job sets up the generator in four steps of its own, which every other job that generates code would repeat. The pins live in .github/mavlink-pins.env as plain KEY=value lines. read_pins.sh checks that each is set once, to a full 40-character commit SHA, and prints them in the form GITHUB_OUTPUT takes. The setup-mavlink-tooling action reads them through it, checks out mavlink/mavlink (definitions only) and pymavlink at those commits with pinned actions and no persisted credentials, installs the generator's dependencies and places the dialect beside the upstream definitions; the generate job runs it from a root checkout. publish_c_library.sh records the pins in its commit from the same file, generate_messages.py parses the file, and docs.yml and generate_c_lib.yml trigger on it, so a bump touches one file. The CONTRIBUTING.md example for a pin bump names it. A test job runs the tests of the generate_c_lib scripts: test_read_pins.py, and test_publish_c_library.py, which runs the publish script against local git repositories. .gitignore skips the __pycache__ folders that test runs leave. Validation: 14 tests pass on Python 3.10.12 and 3.14.5: for read_pins.sh valid pins, the repository's own file, a short SHA, a missing pin, a pin set twice, a missing file and the usage error; for the publish script unchanged headers, removed headers, the commit message with both pins, merge commit titles, manual runs and the usage error. generate_messages.py reads 87da370 from the new file, and actionlint 1.7.12 and shellcheck 0.11.0 report nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
publish_c_library.sh syncs the headers into the C library checkout with rsync -a, whose quick check skips a file whose size and mtime, in whole seconds, match. A regenerated version.h keeps its size, since only the build date in it changes, so headers written in the same second as the checkout are never published. This pull request's CI rehearsal of the publish reported no change for a version.h that differed. rsync runs with --checksum and copies every file whose content differs, at the cost of reading both copies of each of the 288 generated headers. Validation: a new test gives a generated and a published header of the same size the same mtime; it passes with --checksum and fails without it. 15 tests pass on Python 3.10.12 and 3.14.5, and shellcheck 0.11.0 reports nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
mavgen stamps the generation date into every version.h, so headers regenerated on another day differ from the published ones in that line alone, and with --checksum each such run publishes a commit that changes nothing else. Merging the contributing guidelines published 3c168c6 to the C library, which only bumps MAVLINK_BUILD_DATE. The publish check is git diff --cached --quiet -I '^#define MAVLINK_BUILD_DATE ', so a change made only of build-date lines publishes nothing, while any other change carries the new date with it. Validation: a new test publishes headers that differ from the library's only in the build date and expects no commit; it fails without -I. The same-second test rests on a same-size CRC change, so it keeps covering --checksum. 16 tests pass on Python 3.10.12 and 3.14.5. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The generate job checks out the C library with the deploy key and pushes to it after running the generator: pip installs future and lxml from PyPI at whatever version is current, and pymavlink's code runs, all in the job that holds a key able to push to the C library. Its actions are tags that can move, the checkout keeps its token in .git/config, and the job has no timeout. generate builds the headers and uploads its out/ directory as the c-headers artifact, kept for a day, since it only has to outlive the run. publish runs only on main, in the c-library environment, which holds the deploy key and is limited to main: it checks out this repository, downloads the artifact, checks out the C library with the key and runs publish_c_library.sh, so no generator and no third-party Python package runs there, and the published headers are the ones generate built. It waits for the test job, so a publish script whose tests fail never runs with the key. Every action is pinned by SHA, download-artifact at v8.0.1, which fails on a digest mismatch by default; the checkouts drop their credentials except the C library's, which the push needs; and the jobs are bounded at 15, 5 and 10 minutes. Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1 reports no unpinned action and no persisted credential in the workflow beyond the C library checkout's, waived inline for the push. This pull request's generate job runs the upload; the download and the push run on main. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The workflow triggers on .github/scripts/**, so a change to any script under it, whichever workflow runs it, regenerates the C headers and reruns the publish tests for nothing. It triggers on what its jobs read: military.xml, the workflow itself, .github/mavlink-pins.env, the setup-mavlink-tooling action and .github/scripts/generate_c_lib/**, which holds the scripts it runs and their tests. Validation: actionlint 1.7.12 reports nothing, and every file the generate, test and publish jobs read lies under one of those paths. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The site's build and deploy jobs use actions by tags that can move, keep the checkout token in .git/config, have neither timeouts nor names, and grant pages: write and id-token: write without saying what needs them. Every action keeps its major version, pinned by SHA: checkout v4.4.0, setup-python v5.6.0, setup-node v4.4.0, upload-pages-artifact v3.0.1 and deploy-pages v4.0.5. The checkout drops its credentials, the jobs are named and bounded at 15 and 10 minutes, and each permission says what uses it. Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1 finds nothing in docs.yml. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
Nothing checks the workflows themselves: a wrong expression type, a shell mistake in a run block, an unpinned action or a template injection shows only when the workflow runs, if at all. workflow_lint.yml runs on changes under .github, in two jobs. The first fetches actionlint 1.7.12 and shellcheck 0.11.0 from their releases with fetch_release.sh, which checks each archive against its SHA-256 before extracting it, so every runner gets the same versions. actionlint checks every workflow and runs shellcheck on each run block, shellcheck checks .github/scripts/*/*.sh, and the job runs fetch_release.sh's tests. The second runs zizmor 1.30.1 from PyPI with the job token for its online audits (impostor commits, known-vulnerable versions). .github/zizmor.yml disables the self-repository rule: actionlint 1.7.12 rejects the $/ form that rule asks for, and would stop checking those steps. Validation: actionlint, shellcheck on the 5 scripts and zizmor, online with the regular persona, report nothing on this commit. The 6 fetch_release.sh tests, gzip and xz archives, a checksum mismatch and a missing release that extract nothing, the download removed either way, and the usage error, pass on Python 3.10.12 and 3.14.5. Run as the workflow runs them, against the real releases, both installs verify and extract, and actionlint passes with the pinned shellcheck. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
An action pinned by SHA stays at that commit until someone bumps it, including past a release that fixes a vulnerability. dependabot.yml checks the github-actions ecosystem monthly, for the workflows and for the setup-mavlink-tooling composite action, whose directory dependabot scans on its own. Each opens one grouped pull request, at least seven days after a release. The mavlink and pymavlink pins in .github/mavlink-pins.env and the release pins in workflow_lint.yml are not action references, so they move by hand. Validation: GitHub validates dependabot.yml once it lands on the default branch; nothing in this repository can run it. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The publish job runs only on main, so a change to publish_c_library.sh or to what it publishes shows only after merge, as a failed or wrong publish. On pull requests a rehearse-publish job downloads the c-headers artifact and runs rehearse_publish.sh, which clones the C library anonymously into a local bare repository, runs publish_c_library.sh through a clone of that, and prints a Markdown summary for the step summary: the commit a publish would push, with its files, or that the library already holds these headers. The publish log goes to stderr. The job holds no credentials, and fails if the C library cannot be cloned. Validation: test_rehearse_publish.py covers a change, whose summary names the commit and its files while the library itself stays untouched, headers the library already holds, an unreachable library and the usage error; the 20 generate_c_lib tests pass on Python 3.10.12 and 3.14.5. Against the live C library at 3c168c6, with headers generated from this tree at the pinned commits, it reports no change. shellcheck 0.11.0 and actionlint 1.7.12 report nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The docs workflow deploys whatever VitePress builds and never looks at the result: a dead link, a dead anchor or a sitemap URL outside the site's base path goes live, and nothing confirms that the deployed site serves its pages. Before the upload, npm run docs:check runs docs/scripts/check_site.py on the built site: every sitemap URL lies under the site address, and no hand-written page has a dead link or anchor; dead anchors in the generated message reference only warn, since the upstream generator writes them. After the deploy, the deploy job checks out .github/scripts/docs sparsely and without credentials, which needs contents: read, and runs check_deployed_site.sh on the page URL: the main pages and the sitemap load, with retries while the CDN catches up, and the sitemap's first URL lies under the site and loads too. The build job runs both scripts' tests, and the workflow triggers on .github/scripts/docs/**. Validation: 17 check_site.py tests, at 100% line and branch coverage, and 5 check_deployed_site.sh tests against a site served from a temporary directory pass on Python 3.10.12 and 3.14.5. check_deployed_site.sh passes against https://dronecode.github.io/mavlink-military/, and actionlint 1.7.12, shellcheck 0.11.0 and zizmor 1.30.1 report nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
A dialect change gets one check before merge: the C headers still generate. Nothing validates military_extensions.xml, which is never generated, nothing compiles the headers, and nothing loads the Python bindings. validate_dialect.yml runs on every pull request and push to main, and writes and publishes nothing. Its jobs validate both XML files against pymavlink's mavschema.xsd with xmllint, generate C with --strict-units and type-check the headers with gcc at -Wall -Wextra -Werror, and generate Python with --strict-units and check the module with check_python_bindings.py: it loads, and every message military.xml defines is in its mavlink_map, without which a frame decodes as MAVLink_unknown. A script-tests job runs the tests of the workflow's scripts. xmllint installs only when the runner lacks it, through sudo sh -c, so a failed apt-get stops the step under bash -e. Validation: on this tree both files validate, the headers compile at -Werror, and check_python_bindings.py reports "261 messages in the generated map, all 26 dialect messages present" with the pinned pymavlink 19880422. Its 7 tests pass at 100% line and branch coverage on Python 3.10.12 and 3.14.5. With stub sudo and apt-get, the xmllint install runs only when xmllint is missing and exits 100 when apt-get fails. actionlint 1.7.12 and zizmor 1.30.1 report nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
The allocation rules live in README.md, IDMAPPING.md and the military.xml header, and nothing enforces them: a message outside the shared window, a shared ID in the private block, a misnamed field or an IDMAPPING.md that disagrees with the XML passes unless a reviewer spots it. check_dialect_policy.py checks the documented policy on the Python standard library alone: the 53000-53899 shared window and the 53900-53999 private block, MAVLink naming conventions, duplicate IDs and names, collisions between the template and the shared dialect, and IDMAPPING.md against military.xml. Every message is listed once, in "Shared messages" or in a family's own "## <family> messages" table, at the ID military.xml gives it (New ID in a table that maps development IDs, ID in a table of new messages); every MAV_CMD entry is in "MAV_CMD entries"; nothing listed is missing from the XML; and no ID in use lies inside a reserved block. Each finding names its file and line, and under GitHub Actions it is an error annotation on that line. Validation: 24 tests cover each rule and its findings, a family table, a message listed twice and a family table without its columns among them, at 100% line and branch coverage on Python 3.10.12 and 3.14.5. The check passes on this repository, and on a dialect whose IDMAPPING.md lists a family of new messages in its own table. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
A peer whose CRC_EXTRA for a message differs from the sender's drops
that message without an error, and nothing shows a reviewer which
messages a change breaks.
check_wire_compat.py parses the base and head military.xml with
pymavlink's generator and compares, per message ID, the entry row
mavgen writes into MAVLINK_MESSAGE_CRCS: {msgid, CRC_EXTRA, min and
max length, flags, target_system and target_component offsets}.
Removed and renamed messages and CRC_EXTRA changes are breaking; row
changes with the same CRC_EXTRA, which extension fields make, and
additions are listed for information, each with its old and new row,
in the log and the step summary. The report only informs until
message IDs are frozen; WIRE_COMPAT_ENFORCE=1 makes it fail. The
script-tests job sets up pymavlink, which these tests import.
Validation: 13 tests at 100% line and branch coverage pass on Python
3.10.12 and 3.14.5. Against main this tree reports 0 changes and 26
messages untouched; against a revision that adds target fields to
ESAD_ARMING it reports CRC_EXTRA 153 -> 3 with the rows
{53031, 153, 15, 15, 0, 0, 0} -> {53031, 3, 33, 33, 3, 12, 13}.
Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
CRC_EXTRA leaves out changed enum values, removed enum entries and extension fields that are removed, retyped or moved, so the wire report cannot see them. The wire-compat job runs mavlink's own scripts/check_api_break.py at the pinned MAVLINK_REF, which setup-mavlink-tooling checks out with the definitions, through run_api_break_check.sh: a break is a warning, or an error under WIRE_COMPAT_ENFORCE=1, and an exit without the checker's "Name removals/changes detected" line is the tool failing, an error either way. The checker's output goes to the log and the step summary. Validation: 8 tests with stand-in checkers, a clean run, a break with and without enforcement, a crash under both settings, stderr outside GitHub Actions, the summary and its pointer, and the usage error, pass on Python 3.10.12 and 3.14.5. The real checker at 87da370, on one enum value moved 4 -> 40, gives the warning with exit 0, and the error with exit 1 when enforcing; on this pull request it prints "No XML files changed.". shellcheck 0.11.0, actionlint 1.7.12 and zizmor 1.30.1 report nothing. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: TSC21 <nmarques@riis.com>
3668d62 to
b2b9176
Compare
|
@mrpollo I regrouped the history so each commit is either a fix to what main has or one new check, and moved the scripts into a folder per workflow, with their tests in |
On main, a dialect change gets one check before it merges: the C headers still generate. This adds the checks a reviewer needs on every pull request. Both XML files are validated against the MAVLink schema, the documented ID allocation and naming rules are checked with each finding annotated on its line, and
IDMAPPING.mdhas to match the XML. A wire-compatibility report shows every CRC_EXTRA change with the generated entry row of each changed message, and mavlink's owncheck_api_break.pyadds what CRC_EXTRA can't see, such as changed enum values; it only reports until message IDs are frozen, when settingWIRE_COMPAT_ENFORCEto"1"makes it blocking. C and Python are generated with strict units and the headers compiled at-Werror, the built docs site is checked for broken links, anchors and sitemap URLs, and the workflows themselves are linted with actionlint, shellcheck and zizmor. Their scripts live in a folder per workflow under.github/scripts/, with tests that the same workflow runs, so every check runs locally the way CI runs it.It also changes how the C library and the site are published. The publish job pushes the headers the generate job built, so no generator code runs next to the deploy key, and it runs in the
c-libraryenvironment, which holds that key and is limited to main. Pull requests rehearse the publish against a local copy of the C library; the first rehearsal found that rsync skipped a regeneratedversion.hwritten in the same second as the checkout, so the publish script compares contents, and it skips a publish whose only change is the build date. The workflow runs only when its inputs change. Each docs deploy is followed by a check of the live site. The mavlink and pymavlink pins live in.github/mavlink-pins.env, which the checks, the publish and the docs reference all read, so a bump touches one file. Every action is pinned by SHA, the commit-checks workflow included, jobs have timeouts and least-privilege permissions, and dependabot groups action updates monthly.There is no change to the dialect or the generated headers.
With the pinned mavlink 87da370 and pymavlink 19880422 every check passes on main's dialect, 83 script tests and 17 site tests pass with full line and branch coverage of the four Python checkers, and the linters report nothing.
Assisted-by: Claude-Code:claude-opus-5-5