Update wasm related dependencies #102

benbrandt · 2024-09-04T21:17:29Z

This updates all of the WASM related dependencies I could.

I think there is one failing test still, which I don't understand, so it is possible I messed something up in addressing the changes. Would appreciate a lookover.

Also, I couldn't go to the latest for everything because some of the sub git deps are pinned at wasm-tools 0.209.1, so I had to keep them there as well as a wasmtime version that also used those versions.

Hopefully this is a helpful start though, and if not, no worries :)

benbrandt

I didn't touch pyo3 because that might be a bigger change that I didn't get to.
I also was having issues in the runtime crate updating wit-bindgen, but I'm not sure if that needs to be updated or not

benbrandt · 2024-09-04T21:21:19Z

src/bindings.rs

        EntityType::Global(GlobalType {
            val_type: ValType::I32,
            mutable: false,
+            shared: false,


The docs all had this as false, but I don't know what the correct value should be.

false is correct in this context. true would mean the global can be accessed from more than one thread, but WASIp2 and the component model are strictly single threaded for the time being.

benbrandt · 2024-09-04T21:21:30Z

src/bindings.rs

            maximum: None,
            memory64: false,
            shared: false,
+            page_size_log2: None,


Same here. I assumed the default

Yeah, we have no use for non-default page sizes at this point. That's primarily meant for embedded systems.

benbrandt · 2024-09-04T21:21:53Z

src/bindings.rs

-                nullable: true,
-                heap_type: HeapType::Func,
-            },
+            element_type: RefType::FUNCREF,


It seemed that this constant was a convenience wrapper around the same thing, but I could be wrong

benbrandt · 2024-09-04T21:22:01Z

src/bindings.rs

                .try_into()
                .unwrap(),
            maximum: None,
+            table64: false,


Followed the doc example here as well

benbrandt · 2024-09-04T21:22:24Z

src/lib.rs

-            "bundled",
-        );
+        .preopened_dir(stdlib.path(), "python", DirPerms::all(), FilePerms::all())?
+        .preopened_dir(bundled.path(), "bundled", DirPerms::all(), FilePerms::all())?;


It seemed that this now just wanted paths. I think I got the arguments in the right order...

Pretty much none of the tests would pass if you used the wrong order, so we're good :)

benbrandt · 2024-09-04T21:22:54Z

src/lib.rs

    linker: &mut Linker<Ctx>,
 ) -> Result<()> {
-    wasi_command::add_to_linker(linker)?;
+    wasmtime_wasi::add_to_linker_async(linker)?;


I was having trouble finding if the old command was just replaced by the top-level linker. it is possible this is linking too much, I couldn't tell.

benbrandt · 2024-09-04T21:23:30Z

src/test/echoes.rs

    world: "echoes-test",
-    async: true
+    async: true,
+    trappable_imports: true,


I added this to all of them, since the previous implementations allowed for Result in the return values.

benbrandt · 2024-09-04T21:23:41Z

src/test/tests.rs

        "componentize-py:test/resource-alias1/thing": ThingString,
        "componentize-py:test/resource-floats/float": MyFloat,
        "resource-floats-imports/float": MyFloat,
-        "resource-floats-exports/float": MyFloat,


These came up as "unused"

benbrandt · 2024-09-04T21:25:55Z

@dicej I added some comments where I was unsure on things to help focus the review. I don't claim to be an expert, and the changelogs are hard to follow/nonexistent for these crates

dicej · 2024-09-04T21:51:57Z

Thanks, @benbrandt ! BTW, you probably noticed I was using forks of wasmtime etc. with patches to support isyswasfa. We can drop all that and remove the isyswasfa code -- it was a proof-of-concept that has served its purpose; we don't need it anymore.

BTW, I'd be fine with ripping off the band-aid and upgrading to wasmtime 24 and whatever wasm-tools it's using. Might as well jump to the latest of everything while we're doing this.

benbrandt · 2024-09-04T22:02:14Z

Ok I can take a look at it tomorrow. I created a few more PRs for downstream deps so that wasmtime and wasm-tools can be updated.

https://github.com/dicej/wasm-convert/pull/1/files
dicej/component-init#2
dicej/isyswasfa-transform#1

Though the last one isn't necessary if we rip it out.

benbrandt · 2024-09-05T16:22:01Z

@dicej I removed the isyswasfa, but I am really struggling with some of the wasmtime changes, especially in the test generators. I'll see if I can manage to get this in an ok state

benbrandt · 2024-09-05T16:46:37Z

@dicej ok I pushed up what I did. It compiles, but tests fail horribly. Maybe you might have some ideas of what I did wrong here 🙈

benbrandt · 2024-09-05T16:56:16Z

src/lib.rs

-            .exports(&mut self.store)
-            .root()
-            .typed_func::<(), (i32,)>(function)?;
+            .get_typed_func::<(), (i32,)>(&mut self.store, function)?;


These all seem to fail, which I suspect is because somehow I am not retrieving the function from the correct level.

benbrandt · 2024-09-05T16:57:03Z

src/lib.rs

-                    Init::instantiate_async(&mut store, component, &linker).await?;
+                let instance = linker.instantiate_async(&mut store, component).await?;
+                let init = instance
+                    .get_typed_func::<(&str, &Symbols, bool), (Result<(), String>,)>(


I had to replicate some of what was happening in the bindgen because the bindgen code no longer returns the instance

benbrandt · 2024-09-05T16:59:30Z

src/test.rs

-        store: &mut Store<Ctx>,
-        pre: &InstancePre<Ctx>,
-    ) -> Result<(Self::World, Instance)>;
+    async fn instantiate_pre(store: &mut Store<Ctx>, pre: InstancePre<Ctx>) -> Result<Self::World>;


Since the instance doesn't get returned anymore, I removed it from this return as well.
Also, the only methods I could find that did roughly the same thing needed to consume the InstancePre. Since this is holding Arcs, I am hoping it is ok to clone and consume...

benbrandt · 2024-09-05T16:59:52Z

test-generator/src/lib.rs

            writeln!(
                &mut typed_function_inits,
-                r#"echo{test_index}: instance.typed_func::<({params}), ({result_type},)>("echo{test_index}")?,"#
+                r#"echo{test_index}: instance.get_typed_func::<({params}), ({result_type},)>(&mut *store, "componentize-py:test/echoes-generated/echo{test_index}")?,"#


I'm almost positive this isn't correct....

benbrandt · 2024-09-05T17:00:18Z

test-generator/src/lib.rs

        Ok((Self::World {{
           {typed_function_inits}
-        }}, guest_instance))
+        }}))


This too. All of this has changed a lot, and I don't think I am accessing the functions correctly here.

dicej · 2024-09-05T17:38:48Z

@benbrandt Thanks for putting so much effort into this. I'm having a busy week, so apologies for not helping much so far. Hopefully I'll have some time tomorrow to take a look.

benbrandt · 2024-09-05T18:02:50Z

No worries. If you want you can also just go commit by commit and if I went wrong somewhere I can maybe cherry pick or revert if the others are ok

Signed-off-by: Joel Dice <[email protected]>

dicej · 2024-09-06T20:28:27Z

@benbrandt I just pushed an update to fix the tests. I'll merge your PRs on the other repos, then update the Cargo.toml here to match, and then we should be ready to merge this.

Signed-off-by: Joel Dice <[email protected]>

dicej · 2024-09-06T20:52:43Z

@benbrandt Thanks again for doing this; it saved me a lot of time!

benbrandt · 2024-09-06T21:17:06Z

@dicej glad I could help!

dicej · 2024-09-06T22:05:25Z

FYI, I'm planning to publish a new release to pypi.org, but having CI issues I may need an administrator to help with, and that may need to wait until Monday.

dicej · 2024-09-06T23:00:45Z

Update: the release is live: https://pypi.org/project/componentize-py/

benbrandt · 2024-09-07T05:48:56Z

Amazing thank you!

Update wasm related dependencies

75804bb

benbrandt mentioned this pull request Sep 4, 2024

Can't use @since in WIT world #101

Closed

benbrandt commented Sep 4, 2024

View reviewed changes

Remove isyswasfa related code

36db195

wip: attempt to upgrade wasmtime

1b5399a

benbrandt marked this pull request as draft September 5, 2024 16:55

benbrandt commented Sep 5, 2024

View reviewed changes

dicej added 2 commits September 6, 2024 14:19

fix tests; bump version to 0.14.0

0c3264d

Signed-off-by: Joel Dice <[email protected]>

address clippy's latest concerns

2801483

Signed-off-by: Joel Dice <[email protected]>

switch back to upstream component-init and wasm-convert deps

9cf3f20

Signed-off-by: Joel Dice <[email protected]>

dicej marked this pull request as ready for review September 6, 2024 20:42

dicej merged commit d477aa0 into bytecodealliance:main Sep 6, 2024
1 check passed

benbrandt deleted the update-deps branch September 6, 2024 21:25

Update wasm related dependencies #102

Update wasm related dependencies #102

Uh oh!

Conversation

benbrandt commented Sep 4, 2024

Uh oh!

benbrandt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benbrandt commented Sep 4, 2024

Uh oh!

dicej commented Sep 4, 2024

Uh oh!

benbrandt commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benbrandt commented Sep 5, 2024

Uh oh!

benbrandt commented Sep 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dicej commented Sep 5, 2024

Uh oh!

benbrandt commented Sep 5, 2024

Uh oh!

dicej commented Sep 6, 2024

Uh oh!

dicej commented Sep 6, 2024

Uh oh!

Uh oh!

benbrandt commented Sep 6, 2024

Uh oh!

dicej commented Sep 6, 2024

Uh oh!

dicej commented Sep 6, 2024

Uh oh!

benbrandt commented Sep 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

benbrandt commented Sep 4, 2024 •

edited

Loading