Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64 new-backend CI: use Cargo.lock in build. #2352

Merged
merged 1 commit into from
Nov 2, 2020

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Nov 2, 2020

We do a cargo fetch --locked for most of our CI builds, but
run-experimental-x64-ci.sh was not doing this. As a result, some CI
runs seem to fail depending on which versions of crates they download. A
common failure mode is that two different versions of the syn crate
get into the build somehow, resulting in errors in wiggle/witx.

This change simply adds the --locked flag to the cargo test run for
the new x64 backend.

We do a `cargo fetch --locked` for most of our CI builds, but
`run-experimental-x64-ci.sh` was not doing this. As a result, some CI
runs seem to fail depending on which versions of crates they download. A
common failure mode is that two different versions of the `syn` crate
get into the build somehow, resulting in errors in wiggle/witx.

This change simply adds the `--locked` flag to the `cargo test` run for
the new x64 backend.
@cfallin cfallin requested a review from alexcrichton November 2, 2020 19:17
@cfallin
Copy link
Member Author

cfallin commented Nov 2, 2020

I think this should fix the CI failure in #2345.

Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Ah nice!

@cfallin cfallin merged commit 59a2ce4 into bytecodealliance:main Nov 2, 2020
@alexcrichton
Copy link
Member

Hm so in theory --locked has no user-visible effect other than returning errors instead of downloading new things, but if a Cargo.lock exists and works then --locked-or-not in theory should be the same. I've seen this issue crop up a few times (the one on #2345), and I suspect it may be a bug in Cargo, so if it shows up even with this added let me know and I can try to dig in more.

@cfallin
Copy link
Member Author

cfallin commented Nov 2, 2020

Ah, I see, I had misunderstood what the flag does, then; though I suppose it doesn't hurt to use it here if we use it for other runs?

Of note is that this build does turn on the new crate-version resolver (?) with -Zpackage-features -- if it is a bug I would guess that could be the reason we see it. Perhaps we could instead create a root-level feature to enable the new x64 backend and get this working without the Cargo feature -- or do you suspect the bug would still be hit?

@alexcrichton
Copy link
Member

Yeah if anything I suspect a bug with -Zpackage-features rather than in Wasmtime or our CI, I just haven't ever dug in!

@alexcrichton
Copy link
Member

Ah after some testing locally this is a bug that was fixed in Cargo, but we're pinned to an older nightly so we don't have it fixed yet. I'll send a PR update.

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.

3 participants