Stream: git-wasmtime

Topic: wasmtime / Issue #2169 wiggle-wasmtime: witx paths should...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:07):

peterhuene commented on Issue #2169:

Do we need to update crates/wasi/build.rs though? Should this be an argument to the macro perhaps?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:11):

pchickey commented on Issue #2169:

Nope, this macro never should have been specialized to work just with wasi - that was a bug that stuck around because the only place it is tested in-tree is with wasi.

We want to lookup witx paths relative to the crate root of the crate where the macro is invoked. Instead, it was looking up witx paths relative to the wasi crate root, which just happened to be correct, until you're using this for something besides wasi.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:12):

peterhuene commented on Issue #2169:

Right, but I mean crates/wasi/build.rs is relying on this to specify a different root. I think we can now get rid of build.rs and use witx: ["../wasi-common/phases/snapshot/witx/wasi_snapshot_preview1.witx"].

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:12):

peterhuene edited a comment on Issue #2169:

Right, but I mean crates/wasi/build.rs is relying on this to specify a different root. I think we can now get rid of build.rs and use witx: ["../wasi-common/WASI/phases/snapshot/witx/wasi_snapshot_preview1.witx"].

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:14):

pchickey commented on Issue #2169:

The macro invocation in wasi is now fixed, I hadn't run tests in the right place locally so CI caught that.

Unfortunately, wig still consumes the WASI_ROOT env var, so we need to set it in wasi-common and wasi. (My goal was to delete wig by the end of this month but I've been fighting a number of fires, unfortunately, so itll probably take another week or two)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:32):

github-actions[bot] commented on Issue #2169:

Subscribe to Label Action

cc @kubkon

<details>
This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:37):

pchickey commented on Issue #2169:

welp this fix breaks publishing to crates.io and now im puzzling how that ever worked in the first place. sigh

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 23:59):

peterhuene commented on Issue #2169:

Doh! :face_palm:

Perhaps a macro option that is the name of the environment variable to use, and if not specified, CARGO_MANIFEST_DIR?

I think the DEP_WASI_COMMON_19_WASI variable might only be available to the build script (at least according to the docs, but @alexcrichton probably knows for sure), so we'd probably still have to set WASI_ROOT to its value in build.rs still, and then pass witx_root_var = "WASI_ROOT" to the macro?

If DEP_WASI_COMMON_19_WASI is actually available to the macro, then simply witx_root_var = "DEP_WASI_COMMON_19_WASI".

view this post on Zulip Wasmtime GitHub notifications bot (Aug 28 2020 at 00:02):

peterhuene edited a comment on Issue #2169:

Doh! :face_palm:

Perhaps a wasmtime_integration! macro option that is the name of the environment variable to use, and if not specified, CARGO_MANIFEST_DIR?

I think the DEP_WASI_COMMON_19_WASI variable might only be available to the build script (at least according to the docs, but @alexcrichton probably knows for sure), so we'd probably still have to set WASI_ROOT to its value in build.rs still, and then pass witx_root_var = "WASI_ROOT" to the macro?

If DEP_WASI_COMMON_19_WASI is actually available to the macro, then simply witx_root_var = "DEP_WASI_COMMON_19_WASI"?

We'd then revert the change to the witx file path back.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 28 2020 at 00:03):

peterhuene edited a comment on Issue #2169:

Doh! :face_palm:

Perhaps a wasmtime_integration! macro option that is the name of the environment variable to use, and if not specified, CARGO_MANIFEST_DIR?

I think the DEP_WASI_COMMON_19_WASI variable might only be available to the build script (at least according to the docs, but @alexcrichton probably knows for sure), so we'd probably still have to set WASI_ROOT to its value in build.rs still, and then pass witx_root_var: "WASI_ROOT" to the macro?

If DEP_WASI_COMMON_19_WASI is actually available to the macro, then simply witx_root_var: "DEP_WASI_COMMON_19_WASI"?

We'd then revert the change to the witx file path back.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 28 2020 at 00:04):

pchickey commented on Issue #2169:

I think if I'm going to do that much I'll just do arbitrary env var interpolation in the path strings given as the argument to witx:.

Last updated: Nov 22 2024 at 17:03 UTC