Skip to content

docs: Clarify setup for "Rust Components" tutorial #218

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

Merged
merged 17 commits into from
Apr 25, 2025

Conversation

mkatychev
Copy link
Contributor

This PR aims to make explicit some assumptions made in the Rust Components tutorial.

Changes made to component-model/src/language-support/rust.md:

  • Added Prerequisites section
  • Added link to WIT package naming and used explicit package/interface/world names such as example:component/example to help disambiguate between similar sounding names such as add and adder
  • Certain code is now inlined for consistency:
    ```wit
    {{#include ../../examples/example-host/add.wit}}
    ```
  • Fixed incorrect filepath in Referencing the package to import section
  • Removed some leading $ from shell commands to make them easier to copy

@vados-cosmonic
Copy link
Collaborator

Hey @mkatychev would you mind taking another look at this? One of the thing we've wanted to do for a bit was refactor to use the same WIT files everywhere, and we finally have that in now for all the guides -- I think the changes here are in conflict with that, but it should simplify your diff as well.

@mkatychev mkatychev changed the title Clarify setup for "Rust Components" tutorial docs: Clarify setup for "Rust Components" tutorial Mar 25, 2025
@mkatychev mkatychev requested a review from vados-cosmonic April 9, 2025 23:16
Copy link
Collaborator

@vados-cosmonic vados-cosmonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

One nit about an alert but this looks good, let's get this merged :)

Co-authored-by: Victor Adossi <123968127+vados-cosmonic@users.noreply.github.com>
@mkatychev
Copy link
Contributor Author

@kate-goldenring this PR should be ready for a final review & merge

Copy link
Collaborator

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mkatychev for these updates. My main suggestion is to not require the reader to clone the repository. Rather, have them just fetch the WIT file.


```toml
[package.metadata.component.target.dependencies]
"docs:adder" = { path = "../adder/wit" } # directory containing the WIT package
"docs:adder" = { path = "../wit/adder" } # directory containing the WIT package
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes they cloned the repo rather than just fetching the wit. I'd prefer to stay with the approach of fetching the one wit file rather than cloning the repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on #218 (comment), if we don't specify cloning the repo then we'd have to specify fetching the wit/adder, wit/calculator, and examples/adder directories explicitly, I don't mind doing this but it seems like that adds more complexity than a git clone.

git clone is a temporary approach regardless until #241 (comment) is done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think fetching a file is the most analogous to what the wkg experience will be like

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that that's what we should aim for, however, calling cargo run --release -- 1 2 ../add/target/wasm32-wasip1/release/adder.wasm for this step requires us to have all seven files from the example-host directory present.

Should I instead add an instruction to wget the directory and extract it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the example host is an exception -- for now, they will need to clone the repo to run it. Maybe this line should be cargo run --release -- 1 2 adder.wasm to not imply that they build the adder component from the repo.

The main thing i am thinking about is that the fact that the example host is not fetchable should not define how we instruct users to build components. We can push the host up in a container for example or release it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a great and useful example that demonstrantes using external logic to execute the user's code.
I haven't found a succinct way, other than git, to fetch the example-host logic unless something like wkg can do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to have a wit in ghcr that they can depend on? Then there would be no question about file paths because they wouldn't be needed when specifying the dependency.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is definitely the plan: upload the packages to ghcr and update the tutorial and guides to use wkg. We just need the work to be prioritized

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue to get them in GHCR: #243

Copy link
Contributor Author

@mkatychev mkatychev Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime for this PR, should I revert the path changes and references to git clone?
EDIT: that's what was done

@kate-goldenring kate-goldenring linked an issue Apr 23, 2025 that may be closed by this pull request
Copy link
Collaborator

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making this a smoother experience!

@kate-goldenring kate-goldenring merged commit 33c3ac7 into bytecodealliance:main Apr 25, 2025
1 check passed
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.

Refresh Rust language guide
4 participants