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

Default path for a versioned Wasm dependency omits patch version #137

Closed
itowlson opened this issue Jul 30, 2024 · 2 comments · Fixed by #138
Closed

Default path for a versioned Wasm dependency omits patch version #137

itowlson opened this issue Jul 30, 2024 · 2 comments · Fixed by #138

Comments

@itowlson
Copy link
Contributor

itowlson commented Jul 30, 2024

Consider this WAC file:

package root:[email protected] targets fermyon:spin/[email protected];

let c = new root:[email protected] { ... };

export c...;

Per #136, it's not currently possible to give a path to the root:[email protected] Wasm file on the CLI. However, it is possible to omit it from the CLI and instead supply it via the deps folder. The file name wac looks for in this case is ./deps/root/component/0.0.wasm.

Note that this does not include the patch version! This is because of this line:

path.set_extension("wasm");

The preceding lines derive a path of deps/root/component/0.0.1, but set_extension replaces the .1 with .wasm.

This seems unintentional, although perhaps I am misunderstanding the intended layout of the deps directory. I did try having a directory called 0.0.1 (which would bypass the set_extension call) with the Wasm file in it, but that just produced a "no package header was found in any WIT file for this package" error.

@rylev
Copy link
Collaborator

rylev commented Jul 31, 2024

The current behavior is definitely not intended. It's unfortunate that set_extension is more like a replace_extension when an extension already exists. I think the fix is to not rely on set_extension but instead implement the logic ourselves.

@itowlson were you interested in tackling this yourself or would you like me to submit a patch?

@itowlson
Copy link
Contributor Author

Patch sent! Thanks for the clarification @rylev

@rylev rylev closed this as completed in #138 Aug 1, 2024
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 a pull request may close this issue.

2 participants