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

Support relaxed kebab case for filenames in compose #1440

Open
nuke-web3 opened this issue Mar 8, 2024 · 7 comments
Open

Support relaxed kebab case for filenames in compose #1440

nuke-web3 opened this issue Mar 8, 2024 · 7 comments

Comments

@nuke-web3
Copy link

Rust [lib] artifacts generated by cargo component are not compatible with kebab case, see bytecodealliance/cargo-component#250 (comment) 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. 🙏

@alexcrichton
Copy link
Member

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?

@nuke-web3
Copy link
Author

nuke-web3 commented Mar 12, 2024

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

@alexcrichton
Copy link
Member

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?

@nuke-web3
Copy link
Author

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 rust-lang/cargo#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?

@peterhuene
Copy link
Member

peterhuene commented Mar 13, 2024

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.

@nuke-web3 nuke-web3 changed the title Support relaxed kebab case in compose Support relaxed kebab case for filenames in compose Mar 13, 2024
@nuke-web3
Copy link
Author

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. 😁

@peterhuene
Copy link
Member

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!

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

No branches or pull requests

3 participants