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

Refactor installation of wasmtime/wasm-tools on CI #387

Conversation

alexcrichton
Copy link
Collaborator

@alexcrichton alexcrichton commented Mar 2, 2024

We've got official actions for installation Wasmtime and wasm-tools in the bytecodealliance organization which should help make this installation step a bit more readable.

This additionally updates a few versions:

  • For Wasmtime its 18.0.2 release is made now that a dev tag is no longer needed.
  • For wasm-tools its latest release of 1.201.0 is used.
  • For the adapter the version from 18.0.2 is used now that dev is no longer needed.

We've got official actions for installation Wasmtime and wasm-tools in
the bytecodealliance organization which should help make this
installation step a bit more readable.
Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

It seem like this change also updates the version of wasm-tools and wasmtime and the adapter?

If that desirable/indented can you mention the version bumps (old and and new version numbers) in the PR descrption?

tar xf wasm-tools-1.0.54-x86_64-macos.tar.gz
cp wasm-tools-1.0.54-x86_64-macos/wasm-tools ~/.wasmtime/bin/
fi
- name: Setup `wasmtime`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe keep the "for tests" part of the old title, or include it in a comment so its clear that these things are only used for testing and not part of wasi-sdk itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, updated.

@alexcrichton
Copy link
Collaborator Author

I can switch back to the old versions if desired, but it seems like they were arbitrarily chosen, so I took this opportunity to update them to the latest version of releases. They additionally no longer use dev tags meaning that everything should be pinned for CI and breaking shouldn't happen due to development on main in the wasmtime repo.

@sbc100
Copy link
Member

sbc100 commented Mar 6, 2024

I can switch back to the old versions if desired, but it seems like they were arbitrarily chosen, so I took this opportunity to update them to the latest version of releases. They additionally no longer use dev tags meaning that everything should be pinned for CI and breaking shouldn't happen due to development on main in the wasmtime repo.

sgtm, maybe just mention that in the PR description when this lands?

@alexcrichton
Copy link
Collaborator Author

Sure, I've updated the description.

@alexcrichton
Copy link
Collaborator Author

alexcrichton commented Mar 8, 2024

(I'll note I don't have write permissions on this repo so I'll need someone else to hit merge, also sorry if this is already known, I'm not sure what the desired etiquette is on this repo for merging PRs)

@sunfishcode sunfishcode merged commit 73dbb2f into WebAssembly:main Mar 8, 2024
5 checks passed
@sbc100
Copy link
Member

sbc100 commented Mar 8, 2024

@alexcrichton I added you as a contributor

@alexcrichton alexcrichton deleted the refactor-wasmtime-wasm-tools-install branch March 8, 2024 15:58
@alexcrichton
Copy link
Collaborator Author

Oh awesome, thank you!

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.

4 participants