Skip to content

zisk/sp1: make guest crates standalone workspace roots - #520

Merged
samuelburnham merged 1 commit into
mainfrom
sb/zisk-sp1-guest-workspace-roots
Jul 27, 2026
Merged

zisk/sp1: make guest crates standalone workspace roots#520
samuelburnham merged 1 commit into
mainfrom
sb/zisk-sp1-guest-workspace-roots

Conversation

@samuelburnham

@samuelburnham samuelburnham commented Jul 27, 2026

Copy link
Copy Markdown
Member

The zkVM guest crates (zisk/guest, zisk/agg-guest, sp1/guest) are compiled out-of-tree for the riscv target by the zkVM build scripts, never as part of the host workspace. They carried no [workspace] of their own and relied purely on the ancestor manifests' exclude lists to be treated as standalone.

That breaks under a NESTED checkout. The benchmark base-run (bench-pr.yml) checks the base repo out under base/ INSIDE the PR checkout, so the base guest lives at base/zisk/guest. The root manifest's exclude = ["zisk", "sp1"] is relative, so it covers ./zisk but not ./base/zisk; cargo's workspace search for the base guest walks up past base/zisk (which excludes it) to the outer PR-checkout root, which neither lists nor excludes it, and ziskbuild's cargo metadata call panics with "current package believes it's in a workspace when it's not". The base-side zisk-host build then fails and the main column of the bench comparison comes up empty (seen on the zisk-InitStd-execute base run).

Give each guest its own empty [workspace] table so cargo pins its workspace to the crate itself and never walks up to an ancestor, regardless of nesting — exactly what the cargo error recommends. Verified by reproducing the nested layout: without the table cargo emits the exact error; with it, resolution no longer reaches the outer workspace. The guests use no workspace = true inheritance, so a standalone root changes nothing else.

Fixes the n/a errors for the base branch in the !benchmark output, e.g. #442 (comment) (workflow logs)

The zkVM guest crates (zisk/guest, zisk/agg-guest, sp1/guest) are compiled
out-of-tree for the riscv target by the zkVM build scripts, never as part
of the host workspace. They carried no `[workspace]` of their own and
relied purely on the ancestor manifests' `exclude` lists to be treated as
standalone.

That breaks under a NESTED checkout. The benchmark base-run (bench-pr.yml)
checks the base repo out under `base/` INSIDE the PR checkout, so the base
guest lives at `base/zisk/guest`. The root manifest's `exclude = ["zisk",
"sp1"]` is relative, so it covers `./zisk` but not `./base/zisk`; cargo's
workspace search for the base guest walks up past `base/zisk` (which
excludes it) to the outer PR-checkout root, which neither lists nor
excludes it, and `ziskbuild`'s `cargo metadata` call panics with "current
package believes it's in a workspace when it's not". The base-side
`zisk-host` build then fails and the main column of the bench comparison
comes up empty (seen on the zisk-InitStd-execute base run).

Give each guest its own empty `[workspace]` table so cargo pins its
workspace to the crate itself and never walks up to an ancestor,
regardless of nesting — exactly what the cargo error recommends. Verified
by reproducing the nested layout: without the table cargo emits the exact
error; with it, resolution no longer reaches the outer workspace. The
guests use no `workspace = true` inheritance, so a standalone root changes
nothing else.
@samuelburnham
samuelburnham force-pushed the sb/zisk-sp1-guest-workspace-roots branch from d0ece22 to ea477b8 Compare July 27, 2026 12:49
@samuelburnham
samuelburnham marked this pull request as ready for review July 27, 2026 14:45
@samuelburnham
samuelburnham enabled auto-merge (squash) July 27, 2026 14:48
@samuelburnham
samuelburnham merged commit cadf5f2 into main Jul 27, 2026
10 checks passed
@samuelburnham
samuelburnham deleted the sb/zisk-sp1-guest-workspace-roots branch July 27, 2026 15:47
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.

2 participants