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(wasm): remove wasm crate from monorepo #3772

Merged
merged 11 commits into from
Feb 14, 2024
Merged

refactor(wasm): remove wasm crate from monorepo #3772

merged 11 commits into from
Feb 14, 2024

Conversation

TalDerei
Copy link
Collaborator

@TalDerei TalDerei commented Feb 8, 2024

References #3819 and penumbra-zone/web#218

@TalDerei TalDerei self-assigned this Feb 8, 2024
@cratelyn cratelyn added A-wasm Area: Related to WebAssembly components C-refactor Category: refactors and other related improvements labels Feb 8, 2024
@cratelyn cratelyn removed the C-refactor Category: refactors and other related improvements label Feb 8, 2024
@TalDerei
Copy link
Collaborator Author

TalDerei commented Feb 9, 2024

@grod220 the existing wasm crate is split into penumbra-wasm and penumbra-extension. The parts of the wasm crate that have logic live in penumbra-extension, and wasm-bindgen wrappers are in penumbra-wasm. Ideally, the dependency relation should be a one-way relationship where the extension crate in the web can depend on the wasm crate in the monorepo or any other crate, but the wasm crate shouldn't depend on the extension crate. Currently, the penumbra-wasm can't live in isolation since the dependency chain is a two-way relationship, where the wasm crate relies on dependencies from the extension crate.

@TalDerei
Copy link
Collaborator Author

TalDerei commented Feb 9, 2024

Additionally, the wasm unit testing suite is currently broken from changes in a previous PR. I'll bisect to isolate the breakage.

@grod220
Copy link
Contributor

grod220 commented Feb 9, 2024

Ah, thiserror (crates/wasm/src/error.rs) saved us when we refactored to handle errors and not unwrap everything. I have a feeling we'll want this code to stay with the wasm crate in the core repo and the extension crate will import it.

@TalDerei
Copy link
Collaborator Author

TalDerei commented Feb 10, 2024

The wasm crate has been removed from the penumbra monorepo, and the onus to maintain the wasm stack explicitly and exclusively falls to the web team. The wasm code, existing CI workflows and testing infrastructure will be ported accordingly to the web side in penumbra-zone/web#478.

cc @grod220 @turbocrime @Valentine1898 @Vadim121197 @jessepinho

Signed-off-by: Tal Derei <70081547+TalDerei@users.noreply.github.com>
@TalDerei TalDerei marked this pull request as ready for review February 10, 2024 08:26
@TalDerei TalDerei requested review from grod220 and conorsch February 10, 2024 08:26
@TalDerei TalDerei changed the title refactor(wasm): split penumbra-wasm and penumbra-extension refactor(wasm): remove wasm crate from monorepo Feb 10, 2024
@TalDerei
Copy link
Collaborator Author

We'll wait to merge this until penumbra-zone/web#478 is in

Signed-off-by: Tal Derei <70081547+TalDerei@users.noreply.github.com>
@TalDerei
Copy link
Collaborator Author

@conorsch ready to merge on your green light

Signed-off-by: Tal Derei <70081547+TalDerei@users.noreply.github.com>
@conorsch
Copy link
Contributor

This looks complete to my eye. Will check post-merge that workflows on main still pass, and clean up anything missed. Thanks for your liberal cross-linking, @TalDerei. Really helps to follow the history of discussion around this change, particularly in penumbra-zone/web#218.

@TalDerei TalDerei merged commit d710c42 into main Feb 14, 2024
6 checks passed
@TalDerei TalDerei deleted the wasm-refactor branch February 14, 2024 23:23
@TalDerei TalDerei restored the wasm-refactor branch February 14, 2024 23:23
@TalDerei TalDerei deleted the wasm-refactor branch February 14, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-wasm Area: Related to WebAssembly components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants