Stream: git-wasmtime

Topic: wasmtime / PR #1387 wiggle: emit a metadata module contai...

Wasmtime GitHub notifications bot (Mar 24 2020 at 03:08):

pchickey opened PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.

<!--

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 (Mar 24 2020 at 03:13):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.

<!--

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 (Mar 24 2020 at 03:38):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.

<!--

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 (Mar 24 2020 at 03:40):

pchickey edited PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 14:20):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Mar 24 2020 at 17:32):

pchickey closed without merge PR #1387.

Wasmtime GitHub notifications bot (Mar 24 2020 at 17:32):

pchickey reopened PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 17:59):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 18:13):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 18:23):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 18:40):

alexcrichton updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 20:08):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 24 2020 at 23:11):

pchickey updated PR #1387 from pch/wiggle_add_document to master:

Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.

The biggest issue I have with this PR is that it means all users of wiggle will have to take a witx runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flag wiggle_metadata that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.

<!--

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 (Mar 25 2020 at 15:57):

pchickey requested alexcrichton for a review on PR #1387.

Wasmtime GitHub notifications bot (Mar 25 2020 at 19:57):

alexcrichton merged PR #1387.

Last updated: Oct 23 2024 at 20:03 UTC