Wasmtime GitHub notifications bot (Mar 24 2020 at 03:08): 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
wigglewill have to take awitxruntime 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
wigglewill have to take awitxruntime 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
wigglewill have to take awitxruntime 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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
wigglewill have to take awitxruntime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadatathat 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: Nov 22 2024 at 16:03 UTC