Skip to content
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

deps: update proxy-wasm-cpp-host #23460

Closed

Conversation

keith
Copy link
Member

@keith keith commented Oct 12, 2022

Fixes #23390

Signed-off-by: Keith Smiley [email protected]

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Oct 12, 2022
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @htuch

🐱

Caused by: #23460 was opened by keith.

see: more, trace.

@keith
Copy link
Member Author

keith commented Oct 12, 2022

not sure what I'm in for here

@phlax
Copy link
Member

phlax commented Oct 12, 2022

cc @kfaseela

@kfaseela
Copy link
Contributor

@phlax @keith did we finish updating proxy-wasm-cpp-host first?

@keith keith force-pushed the ks/deps-update-proxy-wasm-cpp-host branch from 923df42 to 4f0b5bd Compare October 12, 2022 18:36
@dio
Copy link
Member

dio commented Oct 12, 2022

@phlax do we want to land this before #23434?

@keith keith force-pushed the ks/deps-update-proxy-wasm-cpp-host branch from 4f0b5bd to af5a750 Compare October 12, 2022 21:26
@keith
Copy link
Member Author

keith commented Oct 12, 2022

I removed the v8 version bump from this one to only take that bazel fix, if we can land yours instead w/o other blockers we should do that and close this one

@phlax
Copy link
Member

phlax commented Oct 12, 2022

@phlax @keith did we finish updating proxy-wasm-cpp-host first?

im not sure - previously i believe there was a need to update these deps together (and do some cargo-fu)

it seems like they are not in sync now - altho maybe the important deps are

Envoy's version of wasmtime:

com_github_wasmtime = dict(
project_name = "wasmtime",
project_desc = "A standalone runtime for WebAssembly",
project_url = "https://github.com/bytecodealliance/wasmtime",
version = "1.0.0",
sha256 = "af9906ce0c30f3de2bc967342735809817e74875cf8b3f509efd297887f54a1c",
strip_prefix = "wasmtime-{version}",
urls = ["https://github.com/bytecodealliance/wasmtime/archive/v{version}.tar.gz"],
release_date = "2022-09-20",
use_category = ["dataplane_ext"],
extensions = ["envoy.wasm.runtime.wasmtime"],
cpe = "cpe:2.3:a:bytecodealliance:wasmtime:*",
license = "Apache-2.0",
license_url = "https://github.com/bytecodealliance/wasmtime/blob/v{version}/LICENSE",
),

which is using a recent hash version (maybe we should switch to a releaese version there)

current wasmtime upstream

https://github.com/bytecodealliance/wasmtime/releases/tag/v1.0.1

but it doesnt seem like the wasmtime crates have been updated:

https://github.com/proxy-wasm/proxy-wasm-cpp-host/blob/master/bazel/cargo/wasmtime/BUILD.bazel#L44

@keith
Copy link
Member Author

keith commented Oct 12, 2022

There are only 2 commits being pulled in with this change proxy-wasm/proxy-wasm-cpp-host@4fcf895...25d6a99 so if the other change is blocked on anything I would think updating this would be pretty safe

Copy link
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

it seems like they are not in sync now - altho maybe the important deps are

They are not. Wasmtime is out-of-sync and it only works by accident because there were no breaking changes in Wasmtime's C API (updated in #23232), but Envoy is now using C API from a version different than Wasmtime core.

@RiverPhillips is working on fixing that in proxy-wasm/proxy-wasm-cpp-host#309, and hopefully we can land it and merge into Envoy before the next release.

Regarding this PR, it can be either merged as-is or it could be dropped in favor of #23434, which also contains those changes and was opened first.

@htuch
Copy link
Member

htuch commented Oct 13, 2022

Maybe just do #23434?

@keith
Copy link
Member Author

keith commented Oct 25, 2022

#23434

@keith keith closed this Oct 25, 2022
@keith keith deleted the ks/deps-update-proxy-wasm-cpp-host branch October 25, 2022 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visibility failure on Bazel 6.0
6 participants