Skip to content

Update wasi-crypto#4612

Closed
rjzak wants to merge 3 commits into
bytecodealliance:release-0.39.0from
rjzak:wasmtime39_wasi_crypto_testing
Closed

Update wasi-crypto#4612
rjzak wants to merge 3 commits into
bytecodealliance:release-0.39.0from
rjzak:wasmtime39_wasi_crypto_testing

Conversation

@rjzak
Copy link
Copy Markdown

@rjzak rjzak commented Aug 4, 2022 •
edited
Loading

@rjzak rjzak force-pushed the wasmtime39_wasi_crypto_testing branch 3 times, most recently from 641f9e0 to d2f0a36 Compare August 4, 2022 15:33
@rjzak rjzak mentioned this pull request Aug 4, 2022
Closed
@rjzak rjzak changed the title Wasmtime39 wasi crypto testing Update wasi-crypto Aug 4, 2022
@rjzak rjzak mentioned this pull request Aug 5, 2022
Closed
@npmccallum
Copy link
Copy Markdown
Member

npmccallum commented Aug 17, 2022

@sunfishcode Can we get some attention on this?

sunfishcode
sunfishcode reviewed Aug 17, 2022
View reviewed changes
Comment thread crates/wasi-crypto/Cargo.toml Outdated
[dependencies]
anyhow = "1.0"
wasi-crypto = { path = "spec/implementations/hostcalls/rust", version = "0.1.5" }
wasi-crypto = { git = "https://github.com/WebAssembly/wasi-crypto", rev="fa309ac", features = ["rcrypto"], default-features = false }
Copy link
Copy Markdown
Member

@sunfishcode sunfishcode Aug 17, 2022

Choose a reason for hiding this comment

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

wasmtime-wasi-crypto is published on crates.io, which I believe requires that all dependencies be published on crates.io. Can you publish this revision to a new crates.io release, and then depend on that?

Copy link
Copy Markdown
Author

@rjzak rjzak Aug 17, 2022

Choose a reason for hiding this comment

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

I'll ask.

Copy link
Copy Markdown
Contributor

@jedisct1 jedisct1 Aug 22, 2022

Choose a reason for hiding this comment

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

A new version has been pushed to crates.io.

@rjzak rjzak mentioned this pull request Aug 17, 2022
Closed
jedisct1
jedisct1 reviewed Aug 22, 2022
View reviewed changes
Comment thread crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs Outdated
guest_types::PublickeyEncoding::Pem => PublicKeyEncoding::Pem,
guest_types::PublickeyEncoding::Sec => PublicKeyEncoding::Sec,
guest_types::PublickeyEncoding::CompressedSec => PublicKeyEncoding::CompressedSec,
//guest_types::PublickeyEncoding::CompressedSec => PublicKeyEncoding::CompressedSec,
Copy link
Copy Markdown
Contributor

@jedisct1 jedisct1 Aug 22, 2022

Choose a reason for hiding this comment

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

Leftover from debugging code?

Copy link
Copy Markdown
Author

@rjzak rjzak Aug 22, 2022

Choose a reason for hiding this comment

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

Removed

jedisct1
jedisct1 reviewed Aug 22, 2022
View reviewed changes
Comment thread crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs Outdated
//guest_types::PublickeyEncoding::CompressedSec => PublicKeyEncoding::CompressedSec,
guest_types::PublickeyEncoding::Local => PublicKeyEncoding::Local,

_ => todo!("From guest_type::PublicKeyEncoding to PublicKeyEncoding"),
Copy link
Copy Markdown
Contributor

@jedisct1 jedisct1 Aug 22, 2022

Choose a reason for hiding this comment

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

raw, pkcs8, pem, sec and local are the only possible values. Why is this todo!() required?

Copy link
Copy Markdown
Author

@rjzak rjzak Aug 22, 2022

Choose a reason for hiding this comment

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

Removed. Previously I was getting errors about unhandled patterns in the match statement, but that seems to be resolved.

@rjzak rjzak force-pushed the wasmtime39_wasi_crypto_testing branch 2 times, most recently from cf93a0d to ac8e09b Compare August 22, 2022 20:03
@jedisct1
Copy link
Copy Markdown
Contributor

jedisct1 commented Sep 14, 2022

@rjzak -- Looking good now! Thanks!

Would you mind resolving the conflict so it can be merged?

@rjzak
Copy link
Copy Markdown
Author

rjzak commented Sep 14, 2022 •
edited
Loading

@jedisct1 I'd like to resolve the conflict, but I changed the Cargo.toml file and need it to have my version, not what this thing is that Git seems to be trying to merge. How can I force it to be my version?
Resolved!

@rjzak rjzak force-pushed the wasmtime39_wasi_crypto_testing branch 2 times, most recently from b44f576 to e4199eb Compare September 15, 2022 14:49
@rjzak rjzak requested a review from jedisct1 September 15, 2022 14:50
@rjzak
chore: update wasi-crypto
a323c96 
Signed-off-by: Richard Zak <richard@profian.com>
@rjzak rjzak force-pushed the wasmtime39_wasi_crypto_testing branch 2 times, most recently from 38dabff to e10bc64 Compare September 15, 2022 16:04
@rjzak
chore: use nightly for wasi-crypto
977a14f 
Signed-off-by: Richard Zak <richard@profian.com>
@rjzak rjzak force-pushed the wasmtime39_wasi_crypto_testing branch from e10bc64 to 977a14f Compare September 15, 2022 18:29
@rjzak
Copy link
Copy Markdown
Author

rjzak commented Sep 19, 2022

@jedisct1 I could use some advice regarding some of the CI errors. A lot of these are not related to wasi-crypto.

@rjzak
Copy link
Copy Markdown
Author

rjzak commented Sep 19, 2022

I added the nightly requirement for wasmtime-wasi-crypto since wasi-crypto uses Cargo's "per-package-target" WebAssembly/wasi-crypto@67d9821...fa309ac#diff-9c249d46387da48fcc9d73b78e7a99a00bb825ae6b5fa222b60f6b27f9a4d2a0. Is this ok @sunfishcode?

@jedisct1
Copy link
Copy Markdown
Contributor

jedisct1 commented Sep 19, 2022

@rjzak I don't think nightly is required (at least I hope it's not...)

However, the errors about dead code were likely due to the example main.rs file in wasi-crypto/.../bindings/rust/src.

This has been fixed upstream, so an update to a more recent commit should fix the issues we are seeing here as well.

@rjzak
Copy link
Copy Markdown
Author

rjzak commented Sep 19, 2022

Nightly was only needed for running the test case for Wasi-Crypto, not for compiling the Wasmtime-Wasi-Crypto module.

I'll look at updating the Wasi-Crypto submodule.

@jedisct1
Copy link
Copy Markdown
Contributor

jedisct1 commented Sep 19, 2022

Yes, but the test case depends on the Rust example bindings.

@rjzak
chore: update wasi-crypto submodule
15b784d 
Signed-off-by: Richard Zak <richard@profian.com>
@rjzak
Copy link
Copy Markdown
Author

rjzak commented Oct 7, 2022 •
edited
Loading

I've updated the Wasi-Crypto git submodule, may I have a re-run of CI? @sunfishcode or @jedisct1
This should fix one of the CI issues about unused import.

Edit: nevermind, CI just decided to run as soon as I made this comment?

@rjzak
Copy link
Copy Markdown
Author

rjzak commented Oct 18, 2022

Abandoning in favour of future PR against the main branch.

@rjzak rjzak closed this Oct 18, 2022
@jedisct1 jedisct1 mentioned this pull request Nov 15, 2022
Closed
@alexcrichton alexcrichton mentioned this pull request Jul 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunfishcode sunfishcode sunfishcode left review comments

@jedisct1 jedisct1 Awaiting requested review from jedisct1

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rjzak @npmccallum @jedisct1 @sunfishcode