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

[TESTING/WIP] deps/rust: Rename wasm32-wasi -> wasm32-wasip1 #36285

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

phlax
Copy link
Member

@phlax phlax commented Sep 23, 2024

No description provided.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 23, 2024
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 @moderation

🐱

Caused by: #36285 was opened by phlax.

see: more, trace.

@phlax
Copy link
Member Author

phlax commented Sep 23, 2024

requires resolution to bazelbuild/rules_rust#2782

/wait

@moderation
Copy link
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Sep 23, 2024
@repokitteh-read-only repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Oct 22, 2024
mattklein123
mattklein123 previously approved these changes Oct 23, 2024
@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Oct 23, 2024
@mattklein123
Copy link
Member

/retest

Signed-off-by: Ryan Northey <[email protected]>
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Oct 25, 2024
@phlax phlax changed the title deps/rust: Rename wasm32-wasi -> wasm32-wasip1 [TESTING/WIP] deps/rust: Rename wasm32-wasi -> wasm32-wasip1 Oct 25, 2024
@phlax phlax marked this pull request as draft October 25, 2024 08:45
@phlax
Copy link
Member Author

phlax commented Oct 25, 2024

seems like this needs a fix in proxy-wasm-rust-sdk

ive tested proxy-wasm/proxy-wasm-rust-sdk#277 but that still gives the same warnings so probs more is needed there

@PiotrSikora
Copy link
Contributor

PiotrSikora commented Oct 26, 2024

proxy-wasm/proxy-wasm-rust-sdk#277 only changes its own CI and text in examples, when using Cargo and not Bazel, so it won't have any effect on Envoy.

But what are you trying to fix? I don't see any Wasm or Rust tests failing in this PR.

If you're trying to get rid of warnings about wasm32-wasi being deprecated, than that needs to be fixed in rules_rust, since it maps @rules_rust//rust/platform:wasi to wasm32-wasi and not wasm32-wasip1.

@phlax
Copy link
Member Author

phlax commented Oct 28, 2024

But what are you trying to fix? ...

yeah - trying to resolve the warnings

i initially came to the conclusion that it needed to be resolved in rules_rust - but after updating that with what i thought should fix still seeing the same

@PiotrSikora
Copy link
Contributor

PiotrSikora commented Oct 28, 2024

i initially came to the conclusion that it needed to be resolved in rules_rust - but after updating that with what i thought should fix still seeing the same

Do you mean bazelbuild/rules_rust#2894? I have no idea what that PR was trying to do, but you need significantly more changes to actually support a new target, and even more to modify existing platform.

@phlax
Copy link
Member Author

phlax commented Oct 28, 2024

Do you mean ...

yeah - mostly as it closed bazelbuild/rules_rust#2782 which i thought was the issue

@PiotrSikora
Copy link
Contributor

Try bazelbuild/rules_rust#2967?

Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented Oct 29, 2024

requires a cpp-host update at least i think

Signed-off-by: Ryan Northey <[email protected]>
@PiotrSikora
Copy link
Contributor

requires a cpp-host update at least i think

I don't think so? There is nothing host-related in there, at least if you ignore Wasmtime.

In any case, you forgot to update checksum, which is why the tests are failing.

@phlax
Copy link
Member Author

phlax commented Oct 31, 2024

apologies - i didnt push my latest local change - this was the error about cpp-host

ERROR: Error computing the main repository mapping: at /src/workspace/envoy/bazel/repositories_extra.bzl:4:6: at /home/worker/.cache/bazel/_bazel_worker/f704bab1b165ed1368cb88f9f49e7532/external/proxy_wasm_cpp_host/bazel/cargo/wasmtime/remote/crates.bzl:15:6: Every .bzl file must have a corresponding package, but '@rules_rust//crate_universe/private:crates_vendor.bzl' does not have one. Please create a BUILD file in the same or any parent directory. Note that this BUILD file does not need to do anything except exist.
Loading:

i didnt look too far what caused it

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.

4 participants