plan(v0.38): make v0.38.0 a YAML maintenance and technical-debt release - #931
Merged
Conversation
Maintainer decision after varve reported serde_yaml is unmaintained (#930). The review-surface trio moves to v0.39.0 so v0.38.0 has one theme. WHAT #930 GOT RIGHT, AND WHAT I GOT WRONG. serde_yaml 0.9 resolves to 0.9.34+deprecated, archived upstream, and it reads EVERY artifact — rivet.yaml declares artifacts, safety/stpa-sec and safety/tool-qualification as generic-yaml, which routes to GenericYamlAdapter and serde_yaml::from_str at formats/generic.rs:191. I initially claimed rivet loaded artifacts through its own rowan CST and posted that as a correction on the issue. It was wrong, and has been retracted there. yaml_hir::extract_schema_driven is reached from import_with_schema, a separate route the configured sources do not take. My own reproduction printed `[WARN rivet_core::formats::generic]`, naming the module, and I read it as confirmation because the conclusion was already formed. THREE IMPLEMENTATIONS, NOT TWO, and rivet already owns the write path. Reading artifacts goes through serde_yaml; editing and writing go through yaml_edit.rs, rivet's own line-oriented editor at 2514 lines and 49 tests, whose only serde_yaml use is one flow-mapping case; and yaml_cst.rs, a rowan CST at 1419 lines and 34 tests, serves stpa-yaml sources only. Measured split: 265 artifacts (27%) through the rowan path, 711 (73%) through serde_yaml. An earlier count said 0% for the rowan path and was wrong — STPA files use `hazards:` and similar as top-level keys, so a counter looking only for `artifacts:` missed all 265. That narrows the work considerably. The one thing serde_yaml does that rivet does not already do itself is deserialize into typed structs for the store — a serde Deserializer over an existing CST, not a parser project. CORRECTED SUPPLY-CHAIN FINDING. #930 recommends setting `unmaintained = deny`. That would NOT surface serde_yaml: there is no unmaintained advisory for it, only RUSTSEC-2018-0005, a crash patched in 0.8.4. Testing that revealed something larger — CI runs `cargo deny check bans licenses sources`, so the advisories check is not run at all and deny.toml's thirteen reasoned RUSTSEC ignores are dead configuration, while cargo-audit runs outside CI Gate's needs. Locally `cargo deny check advisories` exits 1 today on bitmaps, im and sized-chunks. v0.38.0 scope: REQ-332, REQ-334, REQ-343, REQ-345, REQ-346, REQ-347, REQ-348. Moved to v0.39.0: REQ-333, REQ-335, REQ-336, and REQ-252 out of backlog since publishing the rowan fork is taken as the next piece of work. REQ-349 records the varve handoff. The blocker found while planning it — that rivet's CST is built on a permanent maintenance fork of rowan, so a handoff would hand varve the fork it declined — was resolved by the maintainer: the fork is ours, which is a different proposition from forking an abandoned third-party crate, so the handoff does not wait on crates.io. What survives is the crate boundary: consolidate behind a boundary with no rivet-CLI types crossing it, so extraction is later a move rather than a rewrite. Confirmed with fmt 0, clippy 1.97.0 --all-targets -D warnings 0, cargo test --workspace 0 (2368 passed), cargo test -p rivet-cli --test cli_commands 0 (213 passed), rivet validate 0, rivet docs check 0, yamllint 0, diagnose_test.sh 0. 318 artifacts, no duplicate ids, provenance present on all four new artifacts. Refs: FEAT-001 Trace: skip
📐 Rivet artifact delta
Graphgraph LR
REQ_346["REQ-346"]:::added
REQ_347["REQ-347"]:::added
REQ_348["REQ-348"]:::added
REQ_349["REQ-349"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Posted by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintainer decision after varve reported serde_yaml unmaintained (#930). The review-surface trio moves to v0.39.0 so v0.38.0 has one theme.
What #930 got right, and what I got wrong
serde_yaml 0.9→0.9.34+deprecated, archived, and it reads every artifact:artifacts/,safety/stpa-secandsafety/tool-qualificationare allgeneric-yaml→GenericYamlAdapter→serde_yaml::from_str(formats/generic.rs:191).I initially claimed artifacts loaded through rivet's own rowan CST and posted that as a correction on the issue. It was wrong and is retracted there.
extract_schema_drivenis reached fromimport_with_schema, a route the configured sources don't take. My own reproduction printed[WARN rivet_core::formats::generic]— naming the module — and I read it as confirmation because the conclusion was already formed.Three implementations, and rivet already owns the write path
yaml_cst.rsrowan CSTyaml_edit.rs— rivet's own, 2514 lines, 49 tests(An earlier count said the rowan path was 0% — wrong; STPA files use
hazards:as top-level keys and my counter only looked forartifacts:.)That narrows the work: the one thing serde_yaml does that rivet doesn't already do is deserialize into typed structs. A serde
Deserializerover an existing CST, not a parser project.Corrected supply-chain finding
#930 recommends
unmaintained = "deny". It would not surface serde_yaml — there's no unmaintained advisory, onlyRUSTSEC-2018-0005(patched in 0.8.4). Testing that revealed something larger: CI runscargo deny check bans licenses sources— the advisories check isn't run at all, so deny.toml's thirteen reasoned ignores are dead config, andcargo-auditsits outsideCI Gate's needs. Locallycheck advisoriesexits 1 today onbitmaps,im,sized-chunks.Scope
v0.38.0 — REQ-332, 334, 343, 345, 346, 347, 348
→ v0.39.0 — REQ-333, 335, 336, plus REQ-252 out of backlog (rowan publication taken next)
REQ-349 records the varve handoff. The blocker found while planning it — rivet's CST sits on a permanent maintenance fork of rowan, so a handoff hands varve the fork it declined — was resolved by the maintainer: the fork is ours, a different proposition from forking an abandoned third-party crate. The handoff doesn't wait on crates.io. What survives is the crate boundary: consolidate with no rivet-CLI types crossing it, so extraction is later a move rather than a rewrite.
fmt/clippy -D warnings(1.97.0)cargo test --workspacecargo test -p rivet-cli --test cli_commandsvalidate/docs check/yamllint/diagnose_test.shRefs: FEAT-001