wasi-adapter: Implement provider crate that embeds the adapter binaries [v2]
Follow-up to #8792, which was reverted in #8856. The new design follows @alexcrichton's suggestions in https://github.com/bytecodealliance/wasmtime/pull/8856#issuecomment-2182798604.
The provider repo is materialised into the workspace to ensure that it is built with the same version, lints, etc as the other crates.
- [x] repo contains a template for the provider crate
- [x] template crate is materialised in CI to be tested
- [x] template crate is materialised just before publishing
I absolutely understand your concerns.
I like the idea of downloading the adapter binaries as artifacts rather than rebuilding them as it guarantees they're the same as the published ones. Downloading from the tag though feels like it might be brittle and run a risk of racing with the job that's uploading artifacts per-tag. Could the artifact instead be downloaded with the actions/download-artifact action? Also, could the logic to download this and package it be shared with the per-PR CI too?
That's a good idea! I don't have experience with workflow artefacts yet but I'll see how they work.
Could publish.rs perhaps be leveraged for this? That way delta in logic-on-each PR and logic-on-publish is as small as possible.
That would be one option. Alternatively, we could have a common CI script, which is invoked in the normal CI and during publish, which downloads the artefact and does rustfmt+check+clippy+package checks (skipped during publish, so this would be a codepath that's not exercised during publish)? Then the publish action would call the same script as CI, and only the cargo publish to crates.io would only happen in the publish action.
If cargo publish --dry-run can execute without credentials, we could also run that in CI as well.
Also ideally this could be done with a "dry run" of sorts to ensure that everything actually works once we hit the publication of all this. Would you be up for getting this running on your fork of Wasmtime for example? You'll probably have to comment/tweak some other things to not publish to crates.io for example, though.
I hadn't thought of this, but yes of course! In theory, adding --dry-run into the publish.rs script (and to the provider crate publish step) should be sufficient? Once we have the scripts in a good state, I'll do a run-through.
I hope I can implement your ideas tomorrow and then do a test run on Friday, so there is enough time before the next publish cycle kicks off (if things take longer, we'll just take the next train)
I tried to run the full publish cycle in my own repo but have come across some issues (I'm yet at the stage where I can actually test the adapter):
- the publish artefact actions always fail: e.g. https://github.com/juntyr/wasmtime/actions/runs/9695441124/job/26755206220
- my latest release attempt failed without any logs at all: https://github.com/juntyr/wasmtime/actions/runs/9695801920/job/26756378300
The tests were run on top of https://github.com/juntyr/wasmtime/pull/10, which includes the extra commit https://github.com/juntyr/wasmtime/pull/10/commits/cbb0a0a074a54781bcccdb2d4cf4ba9e6397fe37 to run the full tests / publish cycle in my repo and without actually executing any cargo publish.
Do you have any ideas? Also, what do you think of the current implementation?
@alexcrichton I've slowly been getting further and further into the try runs and I now have one running that I hope will work.
The extra commit to enable the try is https://github.com/juntyr/wasmtime/pull/19/commits/f1ad3a341bca34957dc543119a8046048269ebe9 - the issue with the artifact action was the wasmtime repo seems to have the artifacts under the hash of the merge commit (maybe because of the merge queue), which I cannot reproduce in my repo, so I just pick a run since it doesn't matter for the try right now.
I'll report back once https://github.com/juntyr/wasmtime/actions/runs/9715657045 (and the follow-up publish action has been completed) - does the implementation look good already or is there anything else that should be changed?
[Just using any run id for testing perhaps wasn't such a good idea (I think the workflow should work but right now it chooses a run id without artifacts ... I'll try to see if I can get lucky, otherwise I need a better way to filter the runs so that we can demonstrate that the publish action works)]
None of this of course affects the code in this PR, just the extra commit in my own repo to test the full publish pipeline
I feel like GitHub has started to rate limit my repo as the CI action just no longer even starts (e.g. https://github.com/juntyr/wasmtime/pull/23), without any notice. I'll keep an eye on this tonight, but I'm not sure my test run can do much more tonight, I'm sorry
@alexcrichton I got a full release process run to succeed:
- https://github.com/juntyr/wasmtime/pull/22 is the PR which includes this PR and the extra commit https://github.com/juntyr/wasmtime/pull/22/commits/0f9c4516adabc9d65cef0da7d45699a86044f260 to allow CI to run in my repo (and to pick any run id instead of the sha of the merged commit one since that one doesn't work in my repo, my guess is because it doesn't have a merge queue, but we already know it works in the main repo and at the latest this will be caught immediately after merging the PR when the publish artifacts workflow runs)
- https://github.com/juntyr/wasmtime/pull/24 is the release cut PR (full CI success in the PR; publish artifact action after merge fails in https://github.com/juntyr/wasmtime/actions/runs/9721774907/job/26834892676 after successfully completing my code, since I don't have GitHub pages set up in my fork)
- https://github.com/juntyr/wasmtime/pull/25 is the release latest PR (full CI success in the PR)
- https://github.com/juntyr/wasmtime/actions/runs/9722005042 is the full release CI run on the to-be-released branch (all succeed, needed a 2nd run since I forgot to remove the release tag from a previous run and so it succeeded but didn't trigger the following steps in my first attempt)
- https://github.com/juntyr/wasmtime/actions/runs/9722304795 is the publish artifacts run on the released branch (all succeed)
- https://github.com/juntyr/wasmtime/actions/runs/9722304796 is the publish to crates.io run on the released branch (all succeed, of course this one is modified to only do dry runs)
I hope the implementation and full try-run of the release process are now good to go :)
Is there any chance this PR could still be reviewed before the v23 cutoff?
@juntyr Alex is on vacation this week, so the review will take a bit longer still. I can't really comment on what that'll do to the timing relative to the release, however
@juntyr Alex is on vacation this week, so the review will take a bit longer still. I can't really comment on what that'll do to the timing relative to the release, however
Ok thanks - I hope Alex has a refreshing holiday :) I'll be on holiday myself for the two weeks after that, so perhaps we can finalize and merge the PR in the v24 cycle
@alexcrichton Thank you so much for your guidance and I'm excited to (hopefully) be released with v23 soon :D