Stream: git-wasmtime

Topic: wasmtime / issue #5929 Begin implementation of wasi-http

Wasmtime GitHub notifications bot (Mar 13 2023 at 04:00):

brendandburns commented on issue #5929:

@pchickey I think this is ready for an initial high-level review.

Everything is implemented in terms of the wit-bindgen interfaces, and then an adapter is introduced which maps from the wit-bindgen to the current wasmtime::Linker implementation.

This means that it is very forward compatible with the component model linker going forward.

If you can take a quick look at the rough layout and see if this is on the path to merging that would be great.

After we get the rough layout right, we can move on to a more detailed review.

Thanks!

Wasmtime GitHub notifications bot (Mar 13 2023 at 04:01):

brendandburns commented on issue #5929:

There's also a pretty extensive example that can be used to test this implementation in crates/wasi-http/example

Wasmtime GitHub notifications bot (Mar 14 2023 at 04:44):

brendandburns commented on issue #5929:

Comments addressed except the macro vs fn comment because I'm not clear how to resolve the compile error.

Please take another look.

Wasmtime GitHub notifications bot (Mar 15 2023 at 20:52):

brendandburns edited a comment on issue #5929:

Comments addressed ~except the macro vs fn comment because I'm not clear how to resolve the compile error.~

Please take another look.

Wasmtime GitHub notifications bot (Mar 18 2023 at 05:30):

brendandburns commented on issue #5929:

Ok, this was switched to hyper. I'm not sure it really saved you very many dependencies, but if there are other ways to reduce the dependency count, let me know. Hyper doesn't implement TLS, so I had to pull in hyper_native_tls

I think this is ready to go, let me know if there is a good way to add tests, it seems the other modules don't really have tests.

Wasmtime GitHub notifications bot (Mar 21 2023 at 03:33):

brendandburns commented on issue #5929:

@pchickey

Ok, I implemented timeouts. I think that the client side implementation is now basically feature complete. Ready for a more detailed review whenever you have time.

Thanks!

Wasmtime GitHub notifications bot (Mar 21 2023 at 03:34):

brendandburns commented on issue #5929:

Oh, and I plan on starting the server side implementation in a different branch.

Wasmtime GitHub notifications bot (Mar 24 2023 at 20:27):

pchickey commented on issue #5929:

I left some fixes last night in https://github.com/brendandburns/wasmtime/pull/1 but it looks like they now conflict

Wasmtime GitHub notifications bot (Mar 24 2023 at 20:27):

pchickey edited a comment on issue #5929:

I left some fixes last night in https://github.com/brendandburns/wasmtime/pull/1 but it looks like they now conflict - I'll rebase that PR on the current state of this branch

Wasmtime GitHub notifications bot (Mar 24 2023 at 20:51):

pchickey edited a comment on issue #5929:

I left some fixes last night in https://github.com/brendandburns/wasmtime/pull/1 but it looks like they now conflict - I'll rebase that PR on the current state of this branch (edit: done now!)

Wasmtime GitHub notifications bot (Mar 24 2023 at 20:53):

pchickey commented on issue #5929:

from my point of view we still need to fix 2 architectural issues before we can merge:

• wasmtime should be running in the tokio executor, rather than using block_on inside a Func.
• rather than examples provided in C, we need to rewrite those to be test-programs written in Rust, because our CI environment doesn't have a C compiler installed.

I'll work on both of those things right now, and send them as PRs like the above:

Wasmtime GitHub notifications bot (Mar 24 2023 at 22:15):

eduardomourar commented on issue #5929:

@pchickey , I believe you can borrow code from here for your Rust example.

Wasmtime GitHub notifications bot (Mar 24 2023 at 22:22):

brendandburns commented on issue #5929:

@pchickey thanks for the PR, I merged it. Let me know if you want me to take on either of those tasks.

Wasmtime GitHub notifications bot (Mar 27 2023 at 08:26):

eduardomourar edited a comment on issue #5929:

@pchickey , I believe you can borrow code from here for your Rust example.

Wasmtime GitHub notifications bot (Mar 28 2023 at 01:27):

brendandburns commented on issue #5929:

@pchickey unless you've already started on this, I can take care of those two items.

Wasmtime GitHub notifications bot (Mar 28 2023 at 04:35):

brendandburns commented on issue #5929:

Ok, I switched the executor on the Host to use tokio::Runtime::block_on instead of executors.

I don't think it is required to build out the full async support in this first PR, but if that's really a blocker, I will.

I also added an example implemented in rust for use as a test or just another example.

Please take another look.

Wasmtime GitHub notifications bot (Mar 28 2023 at 04:36):

brendandburns edited a comment on issue #5929:

Ok, I switched the executor on the Host to use tokio::Runtime::block_on instead of executors.

I don't think it is required to build out the full async support in this first PR, but if that's really a blocker, I will.

We can't just use spawn_task and await in that function because it is not declared async

I also added an example implemented in rust for use as a test or just another example.

Please take another look.

Wasmtime GitHub notifications bot (Mar 28 2023 at 05:14):

pchickey commented on issue #5929:

I just finished writing basically the same rust example this morning but I didn't push it yet because I need to hook its build and execution into the test-programs machinery. I'll wrap that up tomorrow and push it to you.

And yeah lets just do async after we land this PR. We'll have to string async through everywhere, including into the wasmtime cli to run things under tokio when the http module is enabled.

Wasmtime GitHub notifications bot (Mar 28 2023 at 15:38):

brendandburns commented on issue #5929:

@pchickey sounds good, I'm happy to delete my Rust example and just use yours if that is easier.

Wasmtime GitHub notifications bot (Mar 29 2023 at 18:06):

pchickey commented on issue #5929:

https://github.com/brendandburns/wasmtime/pull/2 finishes up the testing changes. I am working on the supply chain stuff today, I will send a PR with any changes needed here but most of those I'll have to submit to wasmtime in a separate PR that will merge before this one.

Wasmtime GitHub notifications bot (Mar 29 2023 at 20:39):

brendandburns commented on issue #5929:

Cool. I squashed/merged that PR. Let me know if you need anything else from me.

Wasmtime GitHub notifications bot (Mar 30 2023 at 00:23):

pchickey commented on issue #5929:

https://github.com/bytecodealliance/wasmtime/pull/6121 will need to land and merge to make the deny and vet CI steps pass. Then, we expect the rest of the steps will pass, since they did locally, but windows and mac can surprise me sometimes.

We will need to remove the contents of crates/wasi-http/example/, and at that point I think we are good to go, I'll make one pass with fresh eyes tomorrow.

Wasmtime GitHub notifications bot (Mar 30 2023 at 00:35):

pchickey edited a comment on issue #5929:

https://github.com/bytecodealliance/wasmtime/pull/6121 will need to land and merge to make the deny and vet CI steps pass. Then, we expect the rest of the steps will pass, since they did locally, but windows and mac can surprise me sometimes.

We will need to remove the contents of crates/wasi-http/example/: we don't want any code in the tree which isn't checked by CI, because it is bound to get out of date. At that point I think we are good to go, I'll make one more pass with fresh eyes tomorrow.

Wasmtime GitHub notifications bot (Mar 30 2023 at 15:51):

pchickey commented on issue #5929:

https://github.com/brendandburns/wasmtime/pull/3 will make the cargo vet step pass

Wasmtime GitHub notifications bot (Mar 30 2023 at 16:01):

brendandburns commented on issue #5929:

• PRs merged (thanks!)
• example directory removed

I think this is ready to go.

Wasmtime GitHub notifications bot (Mar 30 2023 at 16:16):

pchickey commented on issue #5929:

It looks like I missed the vet of some stuff I introduced in #2, I'll get those if you can fix the warnings I sloppily left in the test

Wasmtime GitHub notifications bot (Mar 30 2023 at 21:43):

pchickey commented on issue #5929:

in order to make CI faster, we don't run every step until its time to go in the merge queue. https://github.com/brendandburns/wasmtime/pull/4 fixes the publishing issues.

Wasmtime GitHub notifications bot (Mar 30 2023 at 22:14):

eduardomourar commented on issue #5929:

@pchickey , I know this is a little bit late, but can we change this package name to wasmtime-wasi-http, please? I feel that the current name there will eventually be used for the guest bindings and its high-level basic implementation.

Wasmtime GitHub notifications bot (Mar 30 2023 at 23:24):

pchickey commented on issue #5929:

@eduardomourar I think that is a good idea but lets get things merged and then rename it - we have a few more days before the release gets cut.

Wasmtime GitHub notifications bot (Mar 31 2023 at 00:05):

brendandburns commented on issue #5929:

Merged the PR. I will send a couple of follow on PRs (including the rename) this evening.

Wasmtime GitHub notifications bot (Mar 31 2023 at 01:03):

eduardomourar commented on issue #5929:

Here is an initial stab at the guest binding: https://github.com/bytecodealliance/wasi/pull/69

Wasmtime GitHub notifications bot (Mar 31 2023 at 03:33):

jeffparsons commented on issue #5929:

Is it easy/possible to play with these WASI module implementations from Rust as they land? E.g. if I copy a snapshot of the relevant wit files locally and then build a component using cargo-component and its adapter magic, could I then run that in Wasmtime and expect something to happen?

Just having a bit of trouble understanding what pieces already work together, and which need other pieces (new WASI target in rustc, completed component model support in Wasmtime) to land first.

Wasmtime GitHub notifications bot (Mar 31 2023 at 03:34):

jeffparsons edited a comment on issue #5929:

Is it easy/possible to play with these WASI module implementations from Rust as they land? E.g. if I copy a snapshot of the relevant wit files locally and then build a component using cargo-component and its adapter magic (edit: that attempts to use wasi-http), could I then run that in Wasmtime and expect something to happen?

Just having a bit of trouble understanding what pieces already work together, and which need other pieces (new WASI target in rustc, completed component model support in Wasmtime) to land first.

Wasmtime GitHub notifications bot (Mar 31 2023 at 15:26):

brendandburns commented on issue #5929:

@jeffparsons yes, it's pretty easy to play with the modules.

You need to:

Run wit-bindgen rust wasmtime/crates/wasi-http/wit this will generate the Rust client side bindings (you can substitute wit-bindgen c ... if you want the c bindings instead.

Once you have that, here's a small example program that you can compile:

use crate::proxy::types::new_fields;
use crate::proxy::types::new_outgoing_request;
use crate::proxy::types::MethodParam;
use crate::proxy::types::SchemeParam;

pub mod proxy;

fn request(method: MethodParam, scheme: Option<SchemeParam>, host: &str, path: &str) {
    let headers = new_fields(&[
        ("Content-type", "text/plain"),
        ("User-agent", "wasm32-wasi-rust"),
    ]);

    let req = new_outgoing_request(method, path, "", scheme, host, headers);
    let fut = crate::proxy::default_outgoing_http::handle(req, None);
    let res = crate::proxy::types::future_incoming_response_get(fut)
        .unwrap()
        .unwrap();
    let code = crate::proxy::types::incoming_response_status(res);
    let response_headers = crate::proxy::types::incoming_response_headers(res);
    let stream = crate::proxy::types::incoming_response_consume(res).unwrap();
    let body = crate::proxy::streams::read(stream, 60 * 1024).unwrap().0;

    println!("Status is {}", code);
    println!("Headers are:");
    let entries = crate::proxy::types::fields_entries(response_headers);
    for tuple in entries.iter() {
        println!("{}: {}", tuple.0, tuple.1);
    }
    println!("{}", String::from_utf8(body).unwrap());
}

fn main() {
    request(
        MethodParam::Get,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/get",
    );
    request(
        MethodParam::Post,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/post",
    );
    request(
        MethodParam::Put,
        Some(SchemeParam::Http),
        "postman-echo.com",
        "/put",
    );
}

Wasmtime GitHub notifications bot (Mar 31 2023 at 15:26):

brendandburns edited a comment on issue #5929:

@jeffparsons yes, it's pretty easy to play with the modules, it doesn't require any of the component stuff (yet)

You need to:

Run wit-bindgen rust wasmtime/crates/wasi-http/wit this will generate the Rust client side bindings (you can substitute wit-bindgen c ... if you want the c bindings instead.

Once you have that, here's a small example program that you can compile:

use crate::proxy::types::new_fields;
use crate::proxy::types::new_outgoing_request;
use crate::proxy::types::MethodParam;
use crate::proxy::types::SchemeParam;

pub mod proxy;

fn request(method: MethodParam, scheme: Option<SchemeParam>, host: &str, path: &str) {
    let headers = new_fields(&[
        ("Content-type", "text/plain"),
        ("User-agent", "wasm32-wasi-rust"),
    ]);

    let req = new_outgoing_request(method, path, "", scheme, host, headers);
    let fut = crate::proxy::default_outgoing_http::handle(req, None);
    let res = crate::proxy::types::future_incoming_response_get(fut)
        .unwrap()
        .unwrap();
    let code = crate::proxy::types::incoming_response_status(res);
    let response_headers = crate::proxy::types::incoming_response_headers(res);
    let stream = crate::proxy::types::incoming_response_consume(res).unwrap();
    let body = crate::proxy::streams::read(stream, 60 * 1024).unwrap().0;

    println!("Status is {}", code);
    println!("Headers are:");
    let entries = crate::proxy::types::fields_entries(response_headers);
    for tuple in entries.iter() {
        println!("{}: {}", tuple.0, tuple.1);
    }
    println!("{}", String::from_utf8(body).unwrap());
}

fn main() {
    request(
        MethodParam::Get,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/get",
    );
    request(
        MethodParam::Post,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/post",
    );
    request(
        MethodParam::Put,
        Some(SchemeParam::Http),
        "postman-echo.com",
        "/put",
    );
}

Wasmtime GitHub notifications bot (Mar 31 2023 at 15:27):

brendandburns edited a comment on issue #5929:

@jeffparsons yes, it's pretty easy to play with the modules, it doesn't require any of the component stuff (yet)

You need to:

Run wit-bindgen rust wasmtime/crates/wasi-http/wit this will generate the Rust client side bindings (you can substitute wit-bindgen c ... if you want the c bindings instead.

Once you have that, here's a small example program that you can compile:

use crate::proxy::types::new_fields;
use crate::proxy::types::new_outgoing_request;
use crate::proxy::types::MethodParam;
use crate::proxy::types::SchemeParam;

pub mod proxy;

fn request(method: MethodParam, scheme: Option<SchemeParam>, host: &str, path: &str) {
    let headers = new_fields(&[
        ("Content-type", "text/plain"),
        ("User-agent", "wasm32-wasi-rust"),
    ]);

    let req = new_outgoing_request(method, path, "", scheme, host, headers);
    let fut = crate::proxy::default_outgoing_http::handle(req, None);
    let res = crate::proxy::types::future_incoming_response_get(fut)
        .unwrap()
        .unwrap();
    let code = crate::proxy::types::incoming_response_status(res);
    let response_headers = crate::proxy::types::incoming_response_headers(res);
    let stream = crate::proxy::types::incoming_response_consume(res).unwrap();
    let body = crate::proxy::streams::read(stream, 60 * 1024).unwrap().0;

    println!("Status is {}", code);
    println!("Headers are:");
    let entries = crate::proxy::types::fields_entries(response_headers);
    for tuple in entries.iter() {
        println!("{}: {}", tuple.0, tuple.1);
    }
    println!("{}", String::from_utf8(body).unwrap());
}

fn main() {
    request(
        MethodParam::Get,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/get",
    );
    request(
        MethodParam::Post,
        Some(SchemeParam::Https),
        "postman-echo.com",
        "/post",
    );
    request(
        MethodParam::Put,
        Some(SchemeParam::Http),
        "postman-echo.com",
        "/put",
    );
}

once you compile that to main.wasm (or whatever) you can run:

wasmtime --wasi-modules=experimental-wasi-http main.wasm` and it should work correctly.
~~~

Wasmtime GitHub notifications bot (Mar 31 2023 at 16:23):

pchickey commented on issue #5929:

@jeffparsons we still need to land the preview 1 adapter and a component model implementation of the rest of wasi before this becomes usable from components. the preview 1 adapter means we don't have to worry about a new wasi target in the rust toolchain at this time.

Wasmtime GitHub notifications bot (Mar 31 2023 at 17:04):

brendandburns commented on issue #5929:

@pchickey looks like the merge failed b/c libssl isn't installed on some of the test environments :(

How do you want to handle fixing this?

Wasmtime GitHub notifications bot (Mar 31 2023 at 17:57):

pchickey commented on issue #5929:

I am a bit stumped on how to fix this.

To maximize linux libc compatibility, we perform our linux release builds on centos 7 (https://github.com/bytecodealliance/wasmtime/blob/main/ci/docker/x86_64-linux/Dockerfile). If we were to install the default libssl from centos 7 in the builder, it would mean all of our wasmtime linux users would need openssl 1.0.2 installed, which has been out of support since the start of 2020, so that isn't an acceptable solution.

I don't know enough about the linux and openssl ecosystem to know what minimum version is acceptable, but clearly it reduce our binary compatibility to take on this dep. I also don't know if there are any knobs we can tweak in the build to lazily load the openssl dylib, so users are opting in to needing it by passing the --wasi-modules=experimental-wasi-http flag.

Finally, I don't know if there are alternatives we can swap in for needing openssl on linux. I am aware that rustls exists, but I don't know if it is a complete alternative. I'll ask some of my colleagues who have spent more time in this space.

Wasmtime GitHub notifications bot (Mar 31 2023 at 18:18):

rvolosatovs commented on issue #5929:

I am a bit stumped on how to fix this.

To maximize linux libc compatibility, we perform our linux release builds on centos 7 (main/ci/docker/x86_64-linux/Dockerfile). If we were to install the default libssl from centos 7 in the builder, it would mean all of our wasmtime linux users would need openssl 1.0.2 installed, which has been out of support since the start of 2020, so that isn't an acceptable solution.

I don't know enough about the linux and openssl ecosystem to know what minimum version is acceptable, but clearly it reduce our binary compatibility to take on this dep. I also don't know if there are any knobs we can tweak in the build to lazily load the openssl dylib, so users are opting in to needing it by passing the --wasi-modules=experimental-wasi-http flag.

Finally, I don't know if there are alternatives we can swap in for needing openssl on linux. I am aware that rustls exists, but I don't know if it is a complete alternative. I'll ask some of my colleagues who have spent more time in this space.

FWIW, we have used rustls in https://github.com/enarx/enarx to provide transparent TLS to guests by wrapping the wasmtime WASI TCP traits and it was perfectly sufficient for that particular use case, which seems quite related.

Depending on rustls would be best for both compatibility and simplicity of development. Definitely would vote for depending on rustls instead of openssl

This would also make (perhaps eventually) static builds much easier

Wasmtime GitHub notifications bot (Mar 31 2023 at 18:18):

rvolosatovs edited a comment on issue #5929:

I am a bit stumped on how to fix this.

To maximize linux libc compatibility, we perform our linux release builds on centos 7 (main/ci/docker/x86_64-linux/Dockerfile). If we were to install the default libssl from centos 7 in the builder, it would mean all of our wasmtime linux users would need openssl 1.0.2 installed, which has been out of support since the start of 2020, so that isn't an acceptable solution.

I don't know enough about the linux and openssl ecosystem to know what minimum version is acceptable, but clearly it reduce our binary compatibility to take on this dep. I also don't know if there are any knobs we can tweak in the build to lazily load the openssl dylib, so users are opting in to needing it by passing the --wasi-modules=experimental-wasi-http flag.

Finally, I don't know if there are alternatives we can swap in for needing openssl on linux. I am aware that rustls exists, but I don't know if it is a complete alternative. I'll ask some of my colleagues who have spent more time in this space.

FWIW, we have used rustls in https://github.com/enarx/enarx to provide transparent TLS to guests by wrapping the wasmtime WASI TCP traits and it was perfectly sufficient for that particular use case, which seems quite related.

Depending on rustls would be best for both compatibility and simplicity of development. Definitely would vote for depending on rustls instead of openssl

This would also make (perhaps eventually) static builds much easier

The only drawback I see is that rustls has not been through the same audits as openssl has, but IMO that is acceptable

Wasmtime GitHub notifications bot (Mar 31 2023 at 18:20):

rvolosatovs edited a comment on issue #5929:

I am a bit stumped on how to fix this.

To maximize linux libc compatibility, we perform our linux release builds on centos 7 (main/ci/docker/x86_64-linux/Dockerfile). If we were to install the default libssl from centos 7 in the builder, it would mean all of our wasmtime linux users would need openssl 1.0.2 installed, which has been out of support since the start of 2020, so that isn't an acceptable solution.

I don't know enough about the linux and openssl ecosystem to know what minimum version is acceptable, but clearly it reduce our binary compatibility to take on this dep. I also don't know if there are any knobs we can tweak in the build to lazily load the openssl dylib, so users are opting in to needing it by passing the --wasi-modules=experimental-wasi-http flag.

Finally, I don't know if there are alternatives we can swap in for needing openssl on linux. I am aware that rustls exists, but I don't know if it is a complete alternative. I'll ask some of my colleagues who have spent more time in this space.

FWIW, we have used rustls in https://github.com/enarx/enarx to provide transparent TLS to guests by wrapping the wasmtime WASI TCP traits and it was perfectly sufficient for that particular use case (both incoming and outbound), which seems quite related.

Depending on rustls would be best for both compatibility and simplicity of development. Definitely would vote for depending on rustls instead of openssl

This would also make (perhaps eventually) static builds much easier

The only drawback I see is that rustls has not been through the same audits as openssl has, but IMO that is acceptable

Wasmtime GitHub notifications bot (Mar 31 2023 at 18:32):

pchickey commented on issue #5929:

Thanks, I asked around and we don't use rustls inside fastly for reasons that aren't relevant to this case. @brendandburns are you up for switching things to use rustls? I can do whatever supply chain audits that requires as soon as we know which crates it will take.

I guess it is a good time to rename the wasi-http crate to wasmtime-wasi-http in this PR since we'll be squeaking in before the release is cut next week.

Wasmtime GitHub notifications bot (Mar 31 2023 at 18:37):

pchickey commented on issue #5929:

I was just told we use rustls in https://github.com/fastly/viceroy and it has worked great for that use case.

Wasmtime GitHub notifications bot (Mar 31 2023 at 20:28):

brendandburns commented on issue #5929:

I will try switching to rustls I believe that it should be a pretty easy switch. I have a commit for the rename queued up also, so I will add that to this PR too.

Wasmtime GitHub notifications bot (Mar 31 2023 at 21:24):

brendandburns commented on issue #5929:

@pchickey this is switched to rustls and the rename (and a few minor cleanups) are added also.

Please take another look. I think you may have to vet a couple more packages :(

Wasmtime GitHub notifications bot (Apr 02 2023 at 21:56):

pchickey commented on issue #5929:

I'm not a license lawyer at all but it appears the config our team selected for cargo-deny doesn't permit the use of webpki-roots:
https://github.com/bytecodealliance/wasmtime/actions/runs/4579627092/jobs/8087593223?pr=5929#step:5:160
Could we swap it out to use https://github.com/rustls/rustls-native-certs instead?

Wasmtime GitHub notifications bot (Apr 02 2023 at 22:03):

pchickey edited a comment on issue #5929:

I'm not a license lawyer at all but it appears the config our team selected for cargo-deny doesn't permit the use of webpki-roots:
https://github.com/bytecodealliance/wasmtime/actions/runs/4579627092/jobs/8087593223?pr=5929#step:5:160
Could we swap it out to use https://github.com/rustls/rustls-native-certs instead?

There is a related warning about ring, but it appears from the license file that it contains code with MIT, ISC, and the OpenSSL license https://github.com/briansmith/ring/blob/main/LICENSE. The only one there we don't currently permit is OpenSSL. I will have to seek guidance from the Bytecode Alliance TSC or board to see if we can allow that.

Wasmtime GitHub notifications bot (Apr 02 2023 at 22:03):

pchickey edited a comment on issue #5929:

I'm not a license lawyer at all but it appears the config our team selected for cargo-deny doesn't permit the use of webpki-roots:
https://github.com/bytecodealliance/wasmtime/actions/runs/4579627092/jobs/8087593223?pr=5929#step:5:160
Could we swap it out to use https://github.com/rustls/rustls-native-certs instead?

There is a related warning about ring, but it appears from the license file that it contains code with MIT, ISC, and the OpenSSL license https://github.com/briansmith/ring/blob/main/LICENSE. The only one there we don't currently permit is OpenSSL. I will have to seek guidance from the Bytecode Alliance TSC or board to see if we can allow that, cc @tschneidereit @ricochet @fitzgen

Wasmtime GitHub notifications bot (Apr 02 2023 at 22:23):

tschneidereit commented on issue #5929:

If our cargo deny config doesn't accept MPL I'd argue we should change it. The MPL 2.0 is one of the most well designed and carefully legally vetted licenses out there, and has all the key properties we need, in particular only per-file copyleft and a patent grant

Wasmtime GitHub notifications bot (Apr 03 2023 at 22:56):

pchickey commented on issue #5929:

@brendandburns please merge main and this should pass CI, fingers crossed

Wasmtime GitHub notifications bot (Apr 04 2023 at 17:14):

brendandburns commented on issue #5929:

@pchickey thanks, rebased on main this looks like it's ready to go (again :))

Wasmtime GitHub notifications bot (Apr 04 2023 at 19:01):

brendandburns commented on issue #5929:

@pchickey I missed the rename in scripts/publish.rs so this got bounced out of the queue.

This is now fixed, hopefully it will pass the merge queue this time...

Wasmtime GitHub notifications bot (Apr 04 2023 at 22:41):

pchickey commented on issue #5929:

Unfortunately, it appears ring does not support riscv64. I think we will have to juggle some cargo feature flags around to disable including the wasmtime-wasi-http crate into wasmtime-cli on that platform.

Also, I just learned that you can use the magic string prtest:full in a commit message, and github will run the complete CI instead of just the automatically-estimated subset. That will at least get these issues to show up more eagerly, rather than waiting to enter the main merge queue.

Wasmtime GitHub notifications bot (Apr 05 2023 at 02:31):

brendandburns commented on issue #5929:

@pchickey ok, I will see about flipping the feature flags the right way (and thanks for the tip on the prtest:full, that will definitely enable quicker iteration.

Wasmtime GitHub notifications bot (Apr 05 2023 at 03:29):

brendandburns commented on issue #5929:

@pchickey I conditionalized tls to disable it on riscv64gc

I tried the prtest:full thing but it didn't seem to work for me, maybe I did it wrong...

Anyhow, perhaps we can give this another try? If there's a way for me to trigger the full test runs w/o you adding it to the merge queue, let me know what I need to change in my commit message.

Thanks!

Wasmtime GitHub notifications bot (Apr 05 2023 at 03:34):

brendandburns edited a comment on issue #5929:

@pchickey I conditionalized tls to disable it on riscv64gc

~I tried the prtest:full thing but it didn't seem to work for me, maybe I did it wrong...~

~Anyhow, perhaps we can give this another try? If there's a way for me to trigger the full test runs w/o you adding it to the merge queue, let me know what I need to change in my commit message.~

Turns out the gh status was just cached and lagging, prtest:full appears to work correctly.

Turns out that ring doesn't work on s390x either, so I conditionalized that also. We may want to investigate how we can go back to native SSL, but that's for another PR...

Wasmtime GitHub notifications bot (Apr 05 2023 at 04:26):

brendandburns commented on issue #5929:

cc @pchickey

Ok, next issue hit:

The wasmtime::component::bindgen! macro expects a folder named wit in the same directory as the Cargo.toml

The challenge is that if we want to pull the wasi-http repository in as a git submodule, then the directory structure is messed up, so I used a symbolic link to link wit -> wasi-http/wit but it looks like those symbolic links don't work properly for Windows.

I can get rid of the git submodule, and just check in the wit files directly into this repo, it will make it a little more annoying to keep the wit files in-sync but it's do-able.

Let me know what you think.

`

Wasmtime GitHub notifications bot (Apr 05 2023 at 04:29):

brendandburns commented on issue #5929:

It looks like _maybe_ setting git config --global core.symlinks true in our builds would fix things? But I'm not certain where those builds are configured, I can't seem to find them in main.yml

Wasmtime GitHub notifications bot (Apr 05 2023 at 04:54):

cfallin commented on issue #5929:

Is it possible to modify the macro to have a configurable path? I think we'd probably prefer that over most any directory-structure or build-time hack (symlinks, copying files, or the like).

Wasmtime GitHub notifications bot (Apr 05 2023 at 06:55):

eduardomourar commented on issue #5929:

I believe the macro does accept a path property and you can check it here: https://github.com/bytecodealliance/wasmtime/blob/main/crates/component-macro/tests/codegen.rs

Wasmtime GitHub notifications bot (Apr 05 2023 at 13:56):

brendandburns commented on issue #5929:

@eduardomourar thanks for the pointer (I don't think that's documented anywhere :() I updated this to use path and I removed the symlink.

crosses fingers

Wasmtime GitHub notifications bot (Apr 05 2023 at 17:48):

brendandburns commented on issue #5929:

@pchickey This passed complete CI, but it needed a rebase. I'm pretty sure that it will pass the merge queue this time.

Thanks for the patience!

Wasmtime GitHub notifications bot (Apr 05 2023 at 21:35):

brendandburns commented on issue #5929:

:clink: :confetti: :clink:

Thanks for the patience here, I'll send in some follow-on PRs.

Last updated: Oct 23 2024 at 20:03 UTC