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

doc(c-api) Remove inline documentation in wasmer_wasm.h, clarification about stability… #2083

Merged
merged 8 commits into from
Feb 2, 2021

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Feb 1, 2021

Description

As suggested in #2053 (comment), this PR removes the automatically generated documentation when building wasmer_wasm.h.

It also clarifies our position regarding stability, and add a section about the documentation.

This PR takes also the opportunity to mark wasi_env_set_(instance|memory) as deprecated functions.

Review

  • Add a short description of the the change to the CHANGELOG.md file

@Hywan Hywan added 📦 lib-c-api About wasmer-c-api 📚 documentation Do you like to read? labels Feb 1, 2021
@Hywan Hywan self-assigned this Feb 1, 2021
@Hywan Hywan changed the title Doc c api clarify doc(c-api) Remove inline documentation in wasmer_wasm, clarification about stability… Feb 1, 2021
@Hywan Hywan changed the title doc(c-api) Remove inline documentation in wasmer_wasm, clarification about stability… doc(c-api) Remove inline documentation in wasmer_wasm.h, clarification about stability… Feb 1, 2021
lib/c-api/src/wasm_c_api/wasi/mod.rs Outdated Show resolved Hide resolved
@Hywan
Copy link
Contributor Author

Hywan commented Feb 1, 2021

bors r+

bors bot added a commit that referenced this pull request Feb 1, 2021
2074: feat(package) Be consistent on Windows regarding `libwasmer` name. r=jubianchi a=Hywan

# Description

Windows package libraries aren't named consistently regarding other platforms. This patch fixes that.

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


2083: doc(c-api) Remove inline documentation in `wasmer_wasm.h`, clarification about stability… r=Hywan a=Hywan

# Description

As suggested in #2053 (comment), this PR removes the automatically generated documentation when building `wasmer_wasm.h`.

It also clarifies our position regarding stability, and add a section about the documentation.

This PR takes also the opportunity to mark `wasi_env_set_(instance|memory)` as deprecated functions.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 1, 2021

Build failed (retrying...):

bors bot added a commit that referenced this pull request Feb 1, 2021
2053: feat(c-api) Implement `wasi_get_unordered_imports` r=Hywan a=Hywan

Depend on #2083.

# Description

The problem I've with the C API for WASI is the following:

```c
// At some point, we write this:
bool success = wasi_get_imports(store, module, wasi_env, &imports_to_collect);
```

This function iterates over the module imports, and for each entry, it looks inside the (inner generated) WASI import object if a matching entry exists. If it doesn't, it fails. See:

https://github.com/wasmerio/wasmer/blob/6b028410c23da088d62a6b6919e2c086931ad101/lib/c-api/src/wasm_c_api/wasi/mod.rs#L333-L356

So… if a module requires WASI and WASI only, it works like a charm of course.
But, if a module requires WASI + other imports, it will fail.

And I think it's a common pattern to use WASI + some other imports (like `math.random` or anything related to I/O etc etc.).

What we need is an API to collect all the WASI imports, in no particular order, by module/namespace and name, so that we can implement an API like `ImportObject` to re-order the imports when needed (i.e. when passing a vec of externs to `wasm_instance_new` for example).

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
bors bot added a commit that referenced this pull request Feb 1, 2021
2083: doc(c-api) Remove inline documentation in `wasmer_wasm.h`, clarification about stability… r=Hywan a=Hywan

# Description

As suggested in #2053 (comment), this PR removes the automatically generated documentation when building `wasmer_wasm.h`.

It also clarifies our position regarding stability, and add a section about the documentation.

This PR takes also the opportunity to mark `wasi_env_set_(instance|memory)` as deprecated functions.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 1, 2021

Build failed:

@Hywan
Copy link
Contributor Author

Hywan commented Feb 2, 2021

bors r+

bors bot added a commit that referenced this pull request Feb 2, 2021
2083: doc(c-api) Remove inline documentation in `wasmer_wasm.h`, clarification about stability… r=Hywan a=Hywan

# Description

As suggested in #2053 (comment), this PR removes the automatically generated documentation when building `wasmer_wasm.h`.

It also clarifies our position regarding stability, and add a section about the documentation.

This PR takes also the opportunity to mark `wasi_env_set_(instance|memory)` as deprecated functions.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 2, 2021

Build failed:

@Hywan
Copy link
Contributor Author

Hywan commented Feb 2, 2021

bors r+

bors bot added a commit that referenced this pull request Feb 2, 2021
2083: doc(c-api) Remove inline documentation in `wasmer_wasm.h`, clarification about stability… r=Hywan a=Hywan

# Description

As suggested in #2053 (comment), this PR removes the automatically generated documentation when building `wasmer_wasm.h`.

It also clarifies our position regarding stability, and add a section about the documentation.

This PR takes also the opportunity to mark `wasi_env_set_(instance|memory)` as deprecated functions.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 2, 2021

Build failed:

@Hywan
Copy link
Contributor Author

Hywan commented Feb 2, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 2, 2021

@bors bors bot merged commit c4c7b2b into wasmerio:master Feb 2, 2021
bors bot added a commit that referenced this pull request Feb 2, 2021
2053: feat(c-api) Implement `wasi_get_unordered_imports` r=Hywan a=Hywan

Depend on #2083.

# Description

The problem I've with the C API for WASI is the following:

```c
// At some point, we write this:
bool success = wasi_get_imports(store, module, wasi_env, &imports_to_collect);
```

This function iterates over the module imports, and for each entry, it looks inside the (inner generated) WASI import object if a matching entry exists. If it doesn't, it fails. See:

https://github.com/wasmerio/wasmer/blob/6b028410c23da088d62a6b6919e2c086931ad101/lib/c-api/src/wasm_c_api/wasi/mod.rs#L333-L356

So… if a module requires WASI and WASI only, it works like a charm of course.
But, if a module requires WASI + other imports, it will fail.

And I think it's a common pattern to use WASI + some other imports (like `math.random` or anything related to I/O etc etc.).

What we need is an API to collect all the WASI imports, in no particular order, by module/namespace and name, so that we can implement an API like `ImportObject` to re-order the imports when needed (i.e. when passing a vec of externs to `wasm_instance_new` for example).

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Do you like to read? 📦 lib-c-api About wasmer-c-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants