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.

Sign up for GitHub

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

Jump to bottom

Fix WAVM support. #47

Merged
merged 8 commits into from
Aug 5, 2020
Merged

Fix WAVM support. #47

merged 8 commits into from
Aug 5, 2020

Conversation

jplevyak
Copy link
Contributor

@jplevyak jplevyak commented Aug 4, 2020

Signed-off-by: John Plevyak jplevyak@gmail.com

@jplevyak
Fix WAVM support.
295dcc7 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak requested a review from PiotrSikora August 4, 2020 00:07
jplevyak added 4 commits August 3, 2020 17:10
@jplevyak
Fix NDEBUG ASSERT.
5262959 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
Merge branch 'master' of github.com:proxy-wasm/proxy-wasm-cpp-host in…
22feef9 
…to wavm
@jplevyak
Merge branch 'master' of github.com:proxy-wasm/proxy-wasm-cpp-host in…
0cd68a5 
…to wavm

Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
Fix getAbiVersion.
f2ac720 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
PiotrSikora
PiotrSikora reviewed Aug 4, 2020
View reviewed changes
Copy link
Member

@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.

This looks fine, but is the any way to actually test it?

Shouldn't this be linked against WAVM in this repo? (Same for V8, really)

@jplevyak
Copy link
Contributor Author

jplevyak commented Aug 4, 2020

yes, we should test in this repo. Currently the tests are in envoy-wasm and I have a PR I am working on which will enable those tests for WAVM. Created issue: #49

@jplevyak jplevyak requested a review from PiotrSikora August 4, 2020 19:52
PiotrSikora
PiotrSikora reviewed Aug 4, 2020
View reviewed changes
Copy link
Member

@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.

Could you at least open the PR in envoy-wasm so that we can verify that this builds on the CI, before merging this? Right now I cannot even tell if this compiles.

@jplevyak
Copy link
Contributor Author

jplevyak commented Aug 4, 2020

Will do.

@jplevyak
Cache ABI version.
fff03bc 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak
Copy link
Contributor Author

jplevyak commented Aug 4, 2020

envoyproxy/envoy-wasm#600

@jplevyak jplevyak requested a review from PiotrSikora August 4, 2020 20:44
@jplevyak
Copy link
Contributor Author

jplevyak commented Aug 4, 2020

The attached PR uses these changes to allow WAVM to be used in envoy-wasm.

PiotrSikora
PiotrSikora requested changes Aug 5, 2020
View reviewed changes
src/wavm/wavm.cc Show resolved Hide resolved
jplevyak added 2 commits August 5, 2020 10:31
@jplevyak
Merge branch 'master' of github.com:proxy-wasm/proxy-wasm-cpp-host in…
ba34aff 
…to wavm
@jplevyak
Add 0.2.1 test.
83dd01f 
Signed-off-by: John Plevyak <jplevyak@gmail.com>
@jplevyak jplevyak requested a review from PiotrSikora August 5, 2020 17:34
@jplevyak jplevyak merged commit 5afe9bd into proxy-wasm:master Aug 5, 2020
@mathetake mathetake mentioned this pull request Oct 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PiotrSikora PiotrSikora PiotrSikora approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jplevyak @PiotrSikora