Skip to content

fix(release): realign the stale Cargo.lock entry - #330

Open
YuanYuYuan wants to merge 1 commit into
mainfrom
fix/stale-cargo-lock-entry
Open

fix(release): realign the stale Cargo.lock entry#330
YuanYuYuan wants to merge 1 commit into
mainfrom
fix/stale-cargo-lock-entry

Conversation

@YuanYuYuan

Copy link
Copy Markdown
Collaborator

Summary

The 0.2.0 release bumped 11 of the workspace's 12 members in Cargo.lock and left protobuf_demo at 0.1.0:

member Cargo.lock
hiroz, hiroz-cdr, hiroz-codegen, hiroz-derive, hiroz-protocol, hiroz-py, rmw-zenoh-rs, hiroz-schema, hiroz-msgs, hiroz-tests, hiroz-union 0.2.0
protobuf_demo 0.1.0

Its manifest says version.workspace = true, so cargo rewrites that one line on every invocation and every build leaves the tree dirty. Observed on three separate jobs, all of which only read main.

This realigns the line and adds the checks that would have caught it.

Why nothing noticed

Two independent reasons, and the second is the more interesting one.

No job passes --locked, so the rewrite is silent — cargo fixes the lock, the build succeeds, and the modification is only visible to someone who runs git status afterwards.

The check that polices versions could not see the crate. scripts/test-release-version-semantics.sh already asserts that no crate carries a literal version instead of version.workspace. It found them by globbing:

for f in "$ROOT"/crates/*/Cargo.toml; do

That glob is one level deep. protobuf_demo lives at crates/hiroz/examples/protobuf_demo, so it was never examined — measured directly: the old glob matches the nested member no.

That blindness is the same shape as the release's own miss. Both walked a crates/* pattern; both skipped the one member that is not directly under crates/. The crate happens to inherit correctly today, so the gate reported a pass on a crate it had not looked at.

What changed

All three changes land in test-release-version-semantics.sh, which already runs on every pull request and needs no cargo, no network and no build — the same properties as the rest of that suite.

change why
the inheritance check reads the members list from Cargo.toml instead of globbing a check that cannot see a member cannot vouch for it
a new check compares every member's Cargo.lock version against [workspace.package] version this defect, directly
the members parser guards itself (>= 10 members parsed) an awk change that stopped matching would leave both loops iterating over nothing and reporting a clean pass

The third row is not decoration. Both new loops are driven by the same parser, so a silent parse failure would turn two checks green simultaneously — zero iterations and zero failures look identical.

Before and after

today with this change
protobuf_demo in Cargo.lock 0.1.0 0.2.0
cargo build on a clean checkout rewrites Cargo.lock, leaves the tree dirty leaves the tree clean
a member's lock entry drifting from the workspace version ships, silently fails Release workflow version semantics
a nested member carrying a literal version invisible to the gate fails the gate
a broken members parser would report a clean pass fails the gate
checks in the suite 59 62

What fails without this

Run locally — the suite needs no toolchain. Each red case is reverted before the next.

direction result
unmodified pass62 passed, 0 failed; 12 members parsed, all at 0.2.0
Cargo.lock line put back to 0.1.0 failCargo.lock disagrees with the workspace version 0.2.0 for: protobuf_demo(0.1.0)
version.workspace = true → a literal, in the nested member failthese crates carry a literal version instead of version.workspace: crates/hiroz/examples/protobuf_demo
the members parser broken failparsed only 0 workspace members -- the members parser is broken

The third row is the one that matters most, because it is the case the previous gate silently passed. Confirmed separately that the old glob does not match that path, so this is a real gap being closed rather than a restatement of existing coverage.

On the lock edit being hand-made

It is one line, and it is byte-identical to what cargo produces. The resulting blob is 5148407d2 — the same git object a cargo invocation wrote on a clean checkout of main before this branch existed. That is why the edit is safe to make without running cargo here.

Not addressed

Adding --locked to CI's cargo invocations would catch this class at the build rather than in a bespoke check. It is a larger change — it makes every dependency update an explicit lock commit, and it would need each job audited for jobs that legitimately resolve — so it belongs in its own change, not folded into a one-line fix.

Breaking changes

None. No manifest, no dependency resolution and no version number changes — Cargo.lock is brought into agreement with what the manifests already declared.

…aligned

The 0.2.0 release bumped 11 of the workspace's 12 members in Cargo.lock and
left protobuf_demo at 0.1.0. Its manifest inherits version.workspace, so cargo
rewrote that one line on every invocation and every build left the tree dirty.
Observed on three separate jobs, all of which only read main.

Nothing caught it because nothing looks. No job passes --locked, so the rewrite
is silent, and the check that does police versions could not see the crate at
all: it globbed crates/*/Cargo.toml, which is one level deep, while
protobuf_demo lives at crates/hiroz/examples/protobuf_demo. A check that cannot
see a member cannot vouch for it -- and that blindness is the same shape as the
release's own miss.

Three changes, in the gate that already runs on every pull request and needs no
cargo, no network and no build:

- the inheritance check now reads the members list from Cargo.toml instead of
  globbing, so nested members are covered;
- a new check compares every member's Cargo.lock version against
  [workspace.package] version;
- the members parser guards itself, because an awk change that stopped matching
  would leave both loops iterating over nothing and reporting a clean pass.

The lock edit is one line and byte-identical to what cargo produces: the
resulting blob is 5148407, the same object a cargo invocation on a clean
checkout of main wrote.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant