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

Check for Rust and wasm32-wasi on spin new and spin build #1432

Closed

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Apr 28, 2023

This adds a Rhai scripting facility to Spin templates and to the spin build command. The scripting facility is used to implement a check that the user has Rust and the wasm32-wasi target installed when they install the http-rust template, and when they build an application based on that template and the build fails.

Some questions for reviewers:

  1. The current draft ships scripts to the user as part of template install and instantiation. I can already envisage problems with this: as Rust 1.71 will change the wasm32-wasi target to wasm32-wasi-preview1, users on Rust 1.71 whose templates predate Rust 1.71 will get the wrong advice. I would kind of like to be able to point at, say, magic:rust instead of a local path, and have that fetch the latest script(s) - but I don't want to slow down template instantiation or build, and definitely not require the user to be online! We can snapshot the magic scripts at template install time (which is an inherently online activity), but how do we painlessly update them? [ETA: Incidentally, this would largely obviate question 5.]

  2. There is duplication across the templates and build crates. Would it be reasonable to create a scripting crate to encapsulate this? (That in turn raises whether we should have an interaction or ui crate, since scripting alone would have duplication with templates::interaction.)

  3. Currently templates cite scripts via the manifest (e.g. [scripts] after_instantiate = "after_instantiate.rhai". This feels like there's redundancy there. How do people feel about just using a naming convention instead? (E.g. if the template includes a script file called after_instantiation.rhai then we run it after instantiation, no need for a manifest entry.)

  4. I put the component checker in a directory under the component called .spinbuild. I hate this. I considered .spin/build (and excluding that from .gitignore) but at the moment the .spin directory is kind of transient, and I suspect people are already learning to blow it away when you want to reset an app to its clean state. Any ideas?

  5. If a component build fails, should a script be allowed to signal to retry it? This would fix a bit of ugliness/potential confusion where we go "okay we have installed wasm32-wasi for you but your build failed." But I am wondering if it would cause coordination issues in multi-component applications, e.g. if different scripts signal different things.

  6. Is it worth being able to mark a template script as "advisory" - that is, success or failure does not affect the overall outcome of template instantiation? The current Rust script probably should be advisory, because you still have an app if it fails. So maybe "advisory" should be the default, and we can add "critical" later if we need to?

Some to-dos:

  1. I haven't yet done the redis-rust template

  2. I haven't done any languages other than Rust

  3. Uh probably a bunch of other things I have lost track of, yay

@itowlson
Copy link
Contributor Author

  1. The current scripting host assumes we can just run the script top to bottom. If, later on, we wanted to change the interface, e.g. to call specific entry points, this would break all existing scripts. Do we need more versioning around the host-script interface?

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson force-pushed the build-prerequisites-yes-again-i-know branch from 6ebb685 to 1710319 Compare April 28, 2023 01:41
@mikkelhegn
Copy link
Member

A went through the experience of not having wasm32-wasi target installed, and I really like it.

This is the output

✅ > ./spin new http-rust test
Description:
HTTP base: /
HTTP path: /...
This template requires the Rust `wasm32-wasi` target.
You do not have this target installed. Would you like Spin to install it for you? yes

info: downloading component 'rust-std' for 'wasm32-wasi'
info: installing component 'rust-std' for 'wasm32-wasi'

Small nits:

  1. Could we run the check before prompting? That could save some keystrokes if a user want to opt-out.
  2. Can we format this as a warning or advisory (like you propose)?
    advisory: This template requires the Rust `wasm32-wasi` target.
    advisory: You do not have this target installed. Would you like Spin to install it for you?
    
  3. Can this somehow be tied in to the Spin doctor infrastructure which Lann has proposed 8be0102? That might help getting a consistent experience for e.g. "Advises, Info, Errors, etc."

@lann
Copy link
Collaborator

lann commented Apr 28, 2023

I am somewhat wary of tying specific toolchain scripts to individual templates, which I think you're getting at with:

point at, say, magic:rust instead of a local path

Would we better off pushing toolchain setup into plugins? Then templates could declare soft dependencies on plugins (which would also be helpful for e.g. py2wasm).

Can this somehow be tied in to the Spin doctor infrastructure which Lann has proposed

Indeed I had envisioned "toolchain health checks" as part of spin doctor. If we ran those immediately after spin new I think we could get a similar experience as template-specific toolchains, especially if combined with spin doctor plugin template dependencies (!).

@itowlson
Copy link
Contributor Author

Closing in favour of #1435

@itowlson itowlson closed this Apr 30, 2023
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.

3 participants