Stream: git-wasmtime

Topic: wasmtime / PR #3271 More flexible versioning for module s...

Wasmtime GitHub notifications bot (Sep 01 2021 at 01:35):

pchickey opened PR #3271 from pch/flexible_ser_module_versioning to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Sep 01 2021 at 01:41):

pchickey updated PR #3271 from pch/flexible_ser_module_versioning to main.

Wasmtime GitHub notifications bot (Sep 01 2021 at 17:51):

pchickey updated PR #3271 from pch/flexible_ser_module_versioning to main.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:07):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:07):

peterhuene created PR review comment:

:bike: shed: perhaps WasmtimeVersion to remove "cargo" from the identifier if we want to use this 1:1 from other language bindings (they don't know what cargo is).

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:07):

peterhuene edited PR review comment.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:19):

pchickey submitted PR review.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:19):

pchickey created PR review comment:

Thanks, I like this idea.

The other thing to figure out is whether to sanitize custom strings to be less than 256 bytes somehow, or to change the length field in the binary to be a u64 instead of a byte.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:22):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Sep 01 2021 at 18:22):

peterhuene created PR review comment:

I don't think it's unreasonable to document a length limit, personally.

We have other settings that return Result<&mut Self> too, so we could verify it at the Config level.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:09):

pchickey updated PR #3271 from pch/flexible_ser_module_versioning to main.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:11):

pchickey edited PR #3271 from pch/flexible_ser_module_versioning to main:

replace Config::deserialize_check_wasmtime_version with Config::module_version, which is more expressive.

Instead of just configuring Module::deserialize to ignore version
information, we can configure Module::serialize to emit a custom version
string, and Module::deserialize to check for that string. A new enum
ModuleVersionStrategy is declared, and
Config::deserialize_check_wasmtime_version:bool is replaced with
Config::module_version:ModuleVersionStrategy.

For custom versioning, it used to be reasonable to wrap up a serialized module in your own container with its own version information. Now that we have mmap-based module loading, this is no longer as attractive, since it means giving up Module::deserialize_file.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:11):

pchickey requested peterhuene for a review on PR #3271.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:11):

pchickey has marked PR #3271 as ready for review.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:12):

pchickey updated PR #3271 from pch/flexible_ser_module_versioning to main.

Wasmtime GitHub notifications bot (Sep 02 2021 at 00:44):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Sep 02 2021 at 16:31):

pchickey updated PR #3271 from pch/flexible_ser_module_versioning to main.

Wasmtime GitHub notifications bot (Sep 02 2021 at 19:51):

pchickey merged PR #3271.

Last updated: Dec 23 2024 at 12:05 UTC