Skip to content

ci: improve artifact verification flow#741

Open
Haricharanpanjwani wants to merge 1 commit into
apache:mainfrom
Haricharanpanjwani:issue-720-verify-artifacts
Open

ci: improve artifact verification flow#741
Haricharanpanjwani wants to merge 1 commit into
apache:mainfrom
Haricharanpanjwani:issue-720-verify-artifacts

Conversation

@Haricharanpanjwani
Copy link
Copy Markdown

@Haricharanpanjwani Haricharanpanjwani commented Apr 18, 2026

Closes #720.

Improve the Apache release artifact verification flow used by voters and release managers.

Summary

  • add structured pass/fail/skip result collection and summary rendering in scripts/verify_apache_artifacts.py
  • verify LICENSE, NOTICE, and DISCLAIMER contents in release artifacts
  • verify wheel-specific license files, including LICENSE-wheel
  • add reproducible rebuild verification by rebuilding from the source artifact and comparing outputs against release artifacts
  • integrate Apache RAT results into the verifier summary flow
  • support RAT 0.18 output quirks, including log-line preambles, trailing summary lines, and relative-target invocation from extracted temp directories
  • fix .rat-excludes patterns so RAT 0.18 applies the intended exclusions during end-to-end verification
  • align release-wheel and rebuild-wheel packaging so reproducible comparison passes against RC-style artifacts
  • add optional vote email generation via --vote-email and --vote-email-output
  • add tests covering metadata verification, wheel license checks, rebuild comparison flow, vote email rendering, and RAT parser/invocation compatibility
  • address review feedback by converting the verifier tests to pytest, making wheel metadata requirements explicit, removing the generic build_tool path, and isolating rebuild environment changes to subprocess execution

How I tested this

  • ran python -m pytest tests/test_verify_apache_artifacts.py
  • ran PYTHONPYCACHEPREFIX=/tmp/pycache python -m py_compile scripts/apache_release.py scripts/verify_apache_artifacts.py tests/test_verify_apache_artifacts.py
  • ran python scripts/verify_apache_artifacts.py --help
  • ran python scripts/verify_apache_artifacts.py all --help
  • created a clean worktree from the PR commit and set up a fresh .venv
  • built RC-style artifacts with python scripts/apache_release.py archive 0.41.0 0, python scripts/apache_release.py sdist 0.41.0 0, and python scripts/apache_release.py wheel 0.41.0 0
  • ran python scripts/verify_apache_artifacts.py signatures --artifacts-dir dist
  • ran python scripts/verify_apache_artifacts.py artifacts --artifacts-dir dist
  • ran python scripts/verify_apache_artifacts.py licenses --artifacts-dir dist --rat-jar /Users/hpanjwani/Downloads/apache-rat-0.18/apache-rat-0.18.jar
  • ran python scripts/verify_apache_artifacts.py reproducible --artifacts-dir dist
  • ran python scripts/verify_apache_artifacts.py all --artifacts-dir dist --rat-jar /Users/hpanjwani/Downloads/apache-rat-0.18/apache-rat-0.18.jar --vote-email --vote-email-output /tmp/burr-vote-email.txt

Notes

  • this PR is intentionally scoped to issue ci: improve voter verification script #720 and covers the requested acceptance criteria: reproducible rebuild verification, RAT integration, artifact metadata checks, clear pass/fail summaries, and optional vote email generation
  • end-to-end verification was done against the RC-style artifact set produced by scripts/apache_release.py
  • the main reviewer-facing areas are scripts/verify_apache_artifacts.py, tests/test_verify_apache_artifacts.py, .rat-excludes, and the release/rebuild alignment in scripts/apache_release.py

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal
  • Code passed local verification for this change
  • Any change in functionality is tested
  • Project documentation/help text was updated where applicable

@github-actions github-actions Bot added the area/ci Workflows, build, release scripts label Apr 18, 2026
@Haricharanpanjwani
Copy link
Copy Markdown
Author

I ran the new verifier flow locally in a clean clone to exercise the issue #720 implementation paths.

What the local run confirmed:

  • LICENSE / NOTICE / DISCLAIMER checks pass on built artifacts
  • wheel license-file checks pass, including LICENSE-wheel
  • structured pass/fail summary output is working
  • vote email draft generation is working
  • the reproducible rebuild verification path is active: it rebuilt from source and compared rebuilt outputs against the artifacts under test

Caveats from the local run:

  • the local artifact set was unsigned, so .asc and .sha512 checks failed as expected
  • Apache RAT was not exercised in that run unless a local --rat-jar was provided
  • the reproducible checksum comparison failed against locally built artifacts, which shows the check is active rather than silently passing

Reproducibility-specific caveats encountered:

  • local flit build artifacts do not necessarily match the Apache release-script naming/layout without additional release-style handling
  • a manual rename was needed to exercise the source-artifact path in the verifier outside the full release flow
  • a passing reproducible checksum comparison depends on reproducible release artifact generation, including the work tracked in ci: reproducible builds with SOURCE_DATE_EPOCH #716
  • in other words, this local run validates the new verification logic, but a fully green reproducibility result still depends on the release artifacts themselves being reproducible

So this comment is not claiming a fully green release verification from local demo artifacts. It is documenting that the new verifier checks are implemented, executable, and correctly detect both success and failure conditions.

@Haricharanpanjwani
Copy link
Copy Markdown
Author

Follow-up update pushed in 9c8417dc (ci: support modern Apache RAT output).

This came out of local validation with Apache RAT 0.18:

  • RAT emits INFO/WARN lines before the XML payload
  • RAT 0.18 uses a newer <license ...> XML shape than the parser originally expected
  • rerunning the verifier could also pick up generated rat-report-* files as artifacts

This follow-up makes the verifier more robust and backward compatible by:

  • stripping log preamble lines before XML parsing
  • supporting both the older RAT XML form and the RAT 0.18 license element shape
  • ignoring generated rat-report-* files during artifact enumeration
  • adding regression tests for those cases

@Haricharanpanjwani Haricharanpanjwani force-pushed the issue-720-verify-artifacts branch from 9c8417d to 965f087 Compare April 18, 2026 22:07
@Haricharanpanjwani Haricharanpanjwani changed the title ci: improve voter verification script ci: improve artifact verification flow Apr 18, 2026
@Haricharanpanjwani Haricharanpanjwani force-pushed the issue-720-verify-artifacts branch 7 times, most recently from f5b34ac to e47df20 Compare April 19, 2026 18:54
Comment thread tests/test_verify_apache_artifacts.py Outdated
Copy link
Copy Markdown
Contributor

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement to the release verification flow. The structured VerificationSummary / CheckResult approach is clean and makes the output much more useful for voters. The RAT 0.18 compatibility work (log-line parsing, relative-target invocation, new XML format handling) and reproducible build verification are both valuable additions.

One question on _build_reproducible_wheel (line 772): the os.environ.clear() followed by os.environ.update(original_env) pattern is risky if anything goes wrong between those two calls — the process would be left with an empty environment. Could you pass the modified env through to the subprocess calls instead of mutating os.environ directly?

A few smaller suggestions inline. All 9 tests pass locally and the overall structure is solid.

Comment thread scripts/verify_apache_artifacts.py Outdated
Comment thread scripts/verify_apache_artifacts.py Outdated
@andreahlert
Copy link
Copy Markdown
Collaborator

andreahlert commented Apr 23, 2026

tks @Haricharanpanjwani !

looked at this. the CI failure is just a missing init.py, easy fix one thing tho, get_first_matching_hook means only one interceptor per action right? might want to document that EOF

@elijahbenizzy
Copy link
Copy Markdown
Contributor

MInd rebasing? Looks reasonable. I added some changes so can you rebase? Then I'll look again.

@Haricharanpanjwani Haricharanpanjwani force-pushed the issue-720-verify-artifacts branch from a0ac346 to 20c34a4 Compare April 25, 2026 23:39
@Haricharanpanjwani Haricharanpanjwani force-pushed the issue-720-verify-artifacts branch from 20c34a4 to e184f6f Compare April 27, 2026 14:29
@Haricharanpanjwani
Copy link
Copy Markdown
Author

tks @Haricharanpanjwani !

looked at this. the CI failure is just a missing init.py, easy fix one thing tho, get_first_matching_hook means only one interceptor per action right? might want to document that EOF

Rebased onto the latest main and force-pushed. The branch now includes the missing tests/__init__.py fix for the CI failure as well.

Copy link
Copy Markdown
Collaborator

@andreahlert andreahlert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

just _render_template still has a lazy from jinja2 import ... inside the function body — @skrawcz asked to just make it required, so might as well move it to the top-level imports

@Haricharanpanjwani
Copy link
Copy Markdown
Author

just _render_template still has a lazy from jinja2 import ... inside the function body — @skrawcz asked to just make it required, so might as well move it to the top-level imports

@andreahlert I checked the current branch tip after the rebase and that _render_template helper / lazy jinja2 import is no longer present in the PR changes, so there isn’t anything left to move to top-level imports here.

@Haricharanpanjwani Haricharanpanjwani requested a review from skrawcz May 2, 2026 07:32
@github-actions github-actions Bot added the pr/stale No activity for 14+ days after review label May 18, 2026
@github-actions
Copy link
Copy Markdown

This PR has been inactive for 16 days after receiving review feedback.
Converting to draft.

Please mark it as ready for review when you have addressed the comments.
If no activity occurs within 90 days, it will be closed automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci Workflows, build, release scripts pr/stale No activity for 14+ days after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: improve voter verification script

4 participants