Resolve every binding build against the lockfile - #1885
Conversation
Coverage Report for CI Build 35089447950Coverage remained the same at 86.763%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
60fec4f to
26bd2fd
Compare
yoke-derive 0.8.3, published on 2026-09-15, calls str::from_utf8 and so needs Rust 1.87, but it declares no rust-version, so the MSRV-aware resolver cannot skip it on our 1.85 toolchain. Cargo-recent.lock pins 0.8.2, yet three binding build paths never see that lockfile and now fail to compile: - The Dart native wrapper is a workspace of its own, and test.sh deleted its Cargo.lock before every build. - ubrn generates the wasm crate as a workspace of its own under the gitignored rust_modules/, with no lockfile, and the JavaScript test-utils napi addon is a standalone workspace too. - The NuGet native asset script, and the pack job's bare call to the C# bindings script, never sourced the lockfile helper at all. Seed each of those workspaces from Cargo-recent.lock instead. cargo keeps every pinned version that applies and resolves only the crates specific to that workspace. Verified on a scratch crate depending on url 2.5.8: a fresh resolution picks yoke-derive 0.8.3, the seeded one keeps 0.8.2 and prunes the lockfile to the 37 packages in use. Make use_lockfile a no-op when the lockfile is already in place, so contrib/test.sh can keep calling scripts/generate_bindings.sh now that the latter takes the lockfile itself when run bare. Consumers who build the published Dart package resolve the wrapper fresh, so they still hit the yoke-derive break until it ships a rust-version or a fix; that is outside this repository's control.
The lockfile seeding in the previous commit covers this repository's CI, but anyone who builds the published Dart package resolves the native wrapper fresh on the rustup 1.85.1 toolchain its rust-toolchain.toml selects, with no lockfile at all. That build picks yoke-derive 0.8.3 today and fails, because the crate needs Rust 1.87 and declares no rust-version for the resolver to honour. A direct pin in the wrapper manifest is the only lever that reaches those consumers. It takes effect on the next pub.dev release. Drop it once yoke-derive publishes a version that declares its rust-version or builds on 1.85 again.
26bd2fd to
adb897d
Compare
Sadly from their own readme I'm not sure they will fix it.
|
|
Welp. Then this pin is just what we have to deal with methinks. |
| # Already in place, from an enclosing script or an earlier run. | ||
| if cmp -s "$src" "$LOCKFILE"; then | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
Matching contents alone also lets unrelated processes bypass an active .bak lock and skips backup/cleanup for top-level callers. For example, if Cargo.lock already matches recent, contrib/test.sh now leaves it on minimal instead of restoring recent.
One small fix is to require ${PAYJOIN_LOCKFILE_HELD:-} to equal $LOCKFILE as well as the cmp, and export PAYJOIN_LOCKFILE_HELD="$LOCKFILE" after successfully acquiring and copying the lockfile. This preserves nested C# calls while retaining restoration and mutual exclusion. Focused shell checks reproduce both regressions on this PR and pass with that change.
diff --git a/contrib/lockfile.sh b/contrib/lockfile.sh
index eae3fa10..1321a95f 100644
--- a/contrib/lockfile.sh
+++ b/contrib/lockfile.sh
@@ -16,8 +16,8 @@ _cleanup_lockfile() {
use_lockfile() {
local src="$1"
- # Already in place, from an enclosing script or an earlier run.
- if cmp -s "$src" "$LOCKFILE"; then
+ # Only reuse a lock held by this script or inherited from an enclosing script.
+ if [ "${PAYJOIN_LOCKFILE_HELD:-}" = "$LOCKFILE" ] && cmp -s "$src" "$LOCKFILE"; then
return 0
fi
if ! mkdir "$LOCKDIR" 2>/dev/null; then
@@ -29,4 +29,5 @@ use_lockfile() {
mv "$LOCKFILE" "$LOCKFILE_BAK"
fi
cp "$src" "$LOCKFILE"
+ export PAYJOIN_LOCKFILE_HELD="$LOCKFILE"
}There was a problem hiding this comment.
wat why are we depending against an "urelated process" bypassing .bak? This is just local environment that you'd have checked out on a worktree. No?
There was a problem hiding this comment.
True, a concurrency problem wouldn't be an issue in most cases, but the restoration issue can happen during local development with a single command in an isolated worktree.
If Cargo.lock already matches Cargo-recent.lock, this returns before setting up the backup and cleanup trap. contrib/test.sh then switches to Cargo-minimal.lock and leaves it there on exit, instead of restoring recent.
|
@benalleng @DanGould Seeding from the root lockfile protects the dependencies they share, but still leaves workspace-specific dependencies resolving during the build. Another upstream release could break that remaining part of the graph. Thoughts on the alternate strategy? |
|
CI is completely broken until this moves forward. It may be better to maintain a complete lockfile, but unless there is a major objection I recommend unblocking the build process for every single PR in this repo as soon as possible with some way that does not bypass CI. |
chavic
left a comment
chavic
left a comment
There was a problem hiding this comment.
cACK
Good enough for now, the alternative would bring back two issue whick occured before, stale locks and the need for lock overrides for consumers
4 participants
Fixes the dart, npm pack, and NuGet native job failures that started on 2026-09-15 and hit #1884 first.
yoke-derive 0.8.3, published that evening, callsstr::from_utf8and so needs Rust 1.87, but declares norust-version, so the MSRV-aware resolver cannot skip it on the 1.85 toolchain.Cargo-recent.lockpins 0.8.2, yet three build paths never see it: the Dart native wrapper is its own workspace andtest.shdeleted its lockfile before each build, ubrn generates the wasm crate as its own workspace underrust_modules/, and the NuGet native script (plus the pack job's bare call to the C# bindings script) never sourced the lockfile helper.Each of those workspaces is now seeded from
Cargo-recent.lock. cargo keeps every pinned version that applies and resolves only the workspace-specific remainder.use_lockfilebecomes a no-op when the caller already holds the lockfile, socontrib/test.shscripts can keep callingscripts/generate_bindings.shnow that the latter takes the lockfile when run bare.Testing: verified the mechanism on a scratch crate depending on
url 2.5.8. A fresh resolution picksyoke-derive 0.8.3; the seeded one keeps 0.8.2 and prunes the lockfile to the packages in use.bash -nandnix fmt -- --ci(shfmt and shellcheck) pass. The affected CI jobs cannot run locally, so CI on this PR is the real check.The second commit pins
yoke-derive = "=0.8.2"inpayjoin-ffi/dart/native/Cargo.toml. Consumers who build the published Dart package resolve the wrapper fresh with rustup 1.85.1 and no lockfile, so the lockfile seeding does nothing for them and a manifest pin is the only lever that reaches them. It takes effect on the next pub.dev release and should be dropped once yoke-derive declares itsrust-versionor builds on 1.85 again.For the record, 0.8.3 is a routine icu4x release (unicode-org/icu4x#8496 by a maintainer, published through the release team a minute after merge). The source change is a syn 2 to 3 bump and a
core::str::from_utf8tostr::from_utf8spelling that needs 1.87. Not a security concern, only an undeclared MSRV bump.Disclosure: co-authored by Claude Code