Skip to content

ci: keep the cargo-fuzz workspace's crate patches in sync with the root - #38700

Merged
ggevay merged 2 commits into
MaterializeInc:mainfrom
ggevay:gabor/cargo-fuzz-patch-drift
Sep 25, 2026
Merged

ggevay merged 2 commits into
MaterializeInc:mainfrom
ggevay:gabor/cargo-fuzz-patch-drift

Conversation

@ggevay

@ggevay ggevay commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

The :rust: cargo-fuzz job has failed on every release-qualification run since 2026-08-28 (v26.40.0-rc.1) with build FAILED for src/transform/fuzz: mz-storage-types fails to compile with unresolved imports of TokenProvider, OAuth2TokenProvider, RequestAuthenticator and BearerTokenAuthenticator from iceberg_catalog_rest. Every release from v26.40 to v26.44 shipped without fuzz coverage.

The fuzz crates build in their own workspace, test/cargo-fuzz, which carries a copy of the root's [patch.crates-io] table. Its three iceberg-rust entries were still pinned to the fork revision for 0.9.0 after #38471 moved the root to the 0.10.1 revision, and the root has moved the revision twice more since (#38733, #38893). A patch that no longer satisfies the version requirement is not an error to Cargo: it lands in [[patch.unused]] with a warning and the crate resolves from crates.io, which lacks the fork API that #38475 started using two seconds later. The launchdarkly-server-sdk patch in the same table had drifted the same way after the root dropped it in #37026.

Description

  • test/cargo-fuzz/Cargo.toml: repin the three iceberg entries to the root's current revision, add the missing chrono-tz fork entry, and drop the stale LaunchDarkly patch.
  • bin/lint-cargo (run by CI's lint step): a new check that fails when the fuzz workspace's patch table carries an entry the root does not have, one that differs from the root's, or lacks a root entry. It fails on the pre-fix manifest naming all five drifted or missing entries, and passes after. It also caught the two later root bumps: rebased onto current main before the repin, it failed naming exactly the three iceberg entries. A missing entry gets no Cargo warning at all: the root's chrono-tz fork (sql: fork chrono-tz to deliver tzdata 2026c #38735) was absent, so the fuzz targets reaching mz-pgtz built against crates.io's older tzdata, and the check now fails naming it on the previous manifest. Root entries outside the fuzz dependency graph (duckdb, postgres_array) are listed in the check as omitted.

Alternatives considered: per-crate fuzz workspaces (more duplication), symlinking or generating the manifest (Cargo has no include mechanism; a generated file for a table that changes a few times a year is not worth the tooling), repinning without the lint (guarantees the same outage on the next root patch bump).

Verification

cargo check passes for src/transform/fuzz and for the whole test/cargo-fuzz workspace (17 crates), resolving iceberg and chrono-tz from the forks; cargo metadata shows zero [[patch.unused]] entries. bin/lint-cargo exits 1 on the old manifest and 0 on the new one; black, ruff and pyright are clean. Not run locally: the release build with sanitizer-coverage flags and cargo fuzz build itself. The errors were unresolved imports, which cargo check exercises fully; the release-qualification cargo-fuzz step is the end-to-end check.

Closes: QAR-200

🤖 Generated with Claude Code

@ggevay ggevay added the T-testing Theme: tests or test infrastructure label Sep 7, 2026
@ggevay
ggevay marked this pull request as ready for review September 10, 2026 08:18
@ggevay
ggevay marked this pull request as draft September 10, 2026 08:19
The fuzz crates build in their own workspace, test/cargo-fuzz, which
carries a copy of the root's `[patch.crates-io]` table. Its iceberg-rust
entries were still pinned to the 0.9.0 fork revision after the root moved
to the 0.10.1 revision in MaterializeInc#38471. A patch that no longer satisfies the
version requirement is not an error to Cargo: it lands in
`[[patch.unused]]` and the crate resolves from crates.io, which lacks the
fork API that MaterializeInc#38475 started using, so every fuzz target reaching
mz-storage-types has failed to build since 2026-08-28. The stale
launchdarkly-server-sdk patch in the same table had drifted the same way.

Repin the iceberg entries to the root's current revision, drop the
stale one, and add a bin/lint-cargo check that fails when the fuzz
workspace's patch table carries an entry the root lacks or one that
differs from the root's, so the next root patch bump fails lint instead
of the nightly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay
ggevay force-pushed the gabor/cargo-fuzz-patch-drift branch from 226dbd8 to 2b0e474 Compare September 25, 2026 16:16
@ggevay
ggevay marked this pull request as ready for review September 25, 2026 16:16
@ggevay
ggevay requested a review from bosconi September 25, 2026 17:43
@def-

def- commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- New lint misses a root patch that is absent from the fuzz workspace (chrono-tz is already drifted)

misc/python/materialize/cli/lint-cargo.py:170

The new check only walks the fuzz workspace's own entries, so it never notices a root [patch.crates-io] entry that the fuzz workspace lacks. That has already happened at this PR's head: the root's chrono-tz fork (added by #38735) is missing from test/cargo-fuzz/Cargo.toml, so every fuzz target that reaches mz-pgtz builds against crates.io chrono-tz 0.10.4 instead of the fork, and bin/lint-cargo still exits 0.

Details

At the PR head, cargo tree --manifest-path test/cargo-fuzz/Cargo.toml -p mz-pgtz-fuzz -i chrono-tz resolves chrono-tz from registry+https://github.com/rust-lang/crates.io-index, while the root Cargo.lock pins git+https://github.com/MaterializeInc/chrono-tz.git?rev=5999cdd…. mz-pgtz is a direct dependency of src/pgtz/fuzz and reaches most other fuzz crates through mz-repr. Its build.rs generates the zone table from chrono_tz::TZ_VARIANTS, so the fuzz binaries use the stale 2025b tzdata rather than production's.

Cargo gives no signal for this case. [[patch.unused]] warnings only appear for patches that exist but go unused, and nothing is printed when a patch is simply absent. The comment this PR edits in test/cargo-fuzz/Cargo.toml:13-19 says the omitted root entries are "currently duckdb and postgres_array", which is now wrong. chrono-tz's fork is data-only, so nothing breaks today. But the next root addition of a fork that carries API for a crate in the fuzz graph will pass this lint and break the nightly fuzz build the same way the iceberg drift did.

Suggested fix: add the chrono-tz entry to the fuzz table. Then make the check symmetric: keep an explicit set of root entries the fuzz workspace intentionally omits (duckdb, postgres_array), and fail when any other root entry is missing from test/cargo-fuzz/Cargo.toml.

@bosconi bosconi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Thank you for both putting the horses back in the stable and closing the door!

Not sure if the LLM review point about chrono-tz needs to be addressed.

A root `[patch.crates-io]` entry missing from test/cargo-fuzz/Cargo.toml
makes the fuzz build use the crates.io release without any cargo warning.
The chrono-tz fork had already gone missing that way. Add it, and make
`bin/lint-cargo` fail on any missing root entry that is not explicitly
listed as outside the fuzz dependency graph.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ggevay

ggevay commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Confirmed and fixed in 6fb3506: added the chrono-tz fork entry, and bin/lint-cargo now also fails on a root patch missing from the fuzz workspace unless it is listed as outside the fuzz dependency graph (duckdb, postgres_array). It fails on the previous head naming chrono-tz.

@ggevay
ggevay enabled auto-merge (squash) September 25, 2026 18:58

@bosconi bosconi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

@ggevay
ggevay merged commit 3497b6d into MaterializeInc:main Sep 25, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-testing Theme: tests or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants