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

Add WasmEntryPoint directive to change/specify entrypoints #70

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Add WasmEntryPoint directive to change/specify entrypoints #70

merged 1 commit into from
Jan 23, 2024

Conversation

dpfens
Copy link
Contributor

@dpfens dpfens commented Jan 19, 2024

Adds the WasmEntryPoint directive described in #25. The entrypoint still defaults to _start

  1. WasmEntryPoint directive sets a new entrypoint field to WasmConfig
  2. WasmConfig assigns the entrypoint field to the WasmExecutionCtx
  3. The specified entrypoint is invoked by WasmExecutionCtx::run.

wasm_runtime/src/config.rs Outdated Show resolved Hide resolved
wasm_runtime/src/config.rs Outdated Show resolved Hide resolved
@gzurl
Copy link
Contributor

gzurl commented Jan 19, 2024

Thanks @dpfens for this contribution! LGTM with minor comments.

Can we add a unit test for this new directive?

@dpfens
Copy link
Contributor Author

dpfens commented Jan 19, 2024

Can we add a unit test for this new directive?

@gzurl I added a WasmEntryPoint usage to mod_wasm/httpd.conf to verify it works in the demo/Docker container. Is there an existing way to create a unit test for a directive which would be executed during the GitHub Action? I'm not seeing it.

Hi @dpfens, it's OK to doing this way for now. We don't have yet in place a way to test the C code.

wasm_runtime/src/c_api.rs Outdated Show resolved Hide resolved
wasm_runtime/src/c_api.rs Outdated Show resolved Hide resolved
@gzurl
Copy link
Contributor

gzurl commented Jan 22, 2024

Added son minor comments. Thanks!!

@gzurl gzurl merged commit a26641d into vmware-labs:main Jan 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants