wasm-tools icon indicating copy to clipboard operation
wasm-tools copied to clipboard

Support relaxed kebab case for filenames in `compose`

Open nuke-web3 opened this issue 5 months ago • 7 comments

Rust [lib] artifacts generated by cargo component are not compatible with kebab case, see https://github.com/bytecodealliance/cargo-component/issues/250#issuecomment-1979608104 for context:

# Wasm generated by `cargo component` in `target/wasm32-wasi/<mode>/ 
error: `some_lib_package.wasm` is not in kebab case (at offset 0x0)

It's a real unfortunate hack to need to do something like this in a build or make script:

ln -fs some_lib_package.wasm target/wasm32-wasi/release/some-lib-package.wasm

I am all ears on how to better manage this case mismatch, other than the hack above. :pray:

nuke-web3 avatar Mar 08 '24 04:03 nuke-web3

Thanks for the report! I fear though I may be missing the context necessary to reproduce this. This repository has libraries used in a bunch of other places so errors from these libraries may require code/reproductions from other places to investigate. Would you happen to have a repository and/or instructions of how to reproduce this?

alexcrichton avatar Mar 08 '24 15:03 alexcrichton

Sorry, should have given a MRE:

cargo component new bin-foo
cd bin-foo
cargo component build
ls target/wasm32-wasi/debug/ 

# bin-foo.wasm is created

cd ..
cargo component new --lib lib-foo
cd lib-foo
cargo component build
ls target/wasm32-wasi/debug/ 

# lib_foo.wasm is created... but we need `-` not `_` 😭

cd ..
wasm-tools compose bin-foo/target/wasm32-wasi/debug/bin-foo.wasm -d lib-foo/target/wasm32-wasi/debug/lib_foo.wasm

# error: `lib_foo` is not in kebab case (at offset 0x0)

So without modifying somethings along the chain here to compose, you must manually edit the name to use it (or make a soft link to it with the right name)


Confirmed not to be fixed with cargo-component 0.10.0

nuke-web3 avatar Mar 12 '24 02:03 nuke-web3

Thanks! The issue here looks like it's inferring the name of the component from lib_foo.wasm by taking the file stem by default, which in this case is with _ instead of -, as you've seen, and it's failing.

Could you detail a bit more about how this component is used within your build though? For example with composing components and -d I think that the expectation is bin-foo.wasm imports the lib_foo.wasm component, but how is that configured? For example how does bin-foo get configured for lib-foo instead of lib_foo?

alexcrichton avatar Mar 12 '24 22:03 alexcrichton

In that MRE there isn't the correct imports/exports configured of course, the issue here is that wasm-tools rejects any filename not in kebab-case completely, hence I cannot use any lib.rs generated output as AFAIK there is no way to insist that cargo output that case (per the OP issue as well as https://github.com/rust-lang/cargo/issues/13541).

Here is a justfile that has a ln workaround to compose: https://github.com/NukeManDan/wasi-demo/blob/9ddcb136541e4ed5ae078e2b201ffe1fcf4f0466/justfile#L24-L30

AFAIK, there is no reason to enforce kebab-case for file names (of course there is for symbols in wit)... is there?

nuke-web3 avatar Mar 13 '24 18:03 nuke-web3

wasm-compose can be configured to import the dependency in the output composition rather than defining it inline; the import name it uses is based on the filename stem, which is why it has that restriction.

Probably the simplest fix here is to simply have it kebab-case the stem so that any file name will work.

peterhuene avatar Mar 13 '24 18:03 peterhuene

I am guessing that filename parsing is in a few subcommands, so there could be a few places where some conversion should happen?

I see https://github.com/rutrum/convert-case might be a reasonable dependency add if you wanted to support arbitrary filenames -> import name used. Alternatively, just some patch like "this_filename_is_nice.wasm".replace("_", "-"); where needed.

Happy to make a PR if pointed to the right place(s) that should include the conversion and approach desired. :grin:

nuke-web3 avatar Mar 13 '24 19:03 nuke-web3

We already use heck for conversion to different casings, so it should be a matter of just applying to_kebab_case on the stem at the appropriate point (which I suspect to be here).

Happy to review a PR if you feel inclined to submit one!

peterhuene avatar Mar 13 '24 22:03 peterhuene