-
Notifications
You must be signed in to change notification settings - Fork 829
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
fix(c-api) Rename lib's name to wasmer
#2456
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch does several things. 1. For the crate `wasmer-c-api`, the library name is modified from `wasmer_c_api` to `wasmer` in `Cargo.toml`. That way, the new library files are named `libwasmer.*` rather than `libwasmer_c_api.*`. That's the primaly goal of this patch. The rest is a consequence of this point. Why do we want that? Because the `build.rs` script of the `wasmer-c-api` crate will configure the `soname` (on Linux), the `install_name` + `current_version` + `compatibility_version` (on macOS), and the `out-implib` + `output-def` (on Windows) for a library named `libwasmer`, which is the name we provide in the Wasmer package for the Wasmer libraries. If we want everything to be testable, we cannot use `libwasmer` in `soname` for a file named `libwasmer_c_api` for example. If we rename the file when packaging (as it's done prior this patch), we would need to re-update all those information in the `Makefile`. It implies to duplicate the code in 2 places. So let's do things right directly and only once: We want the library to be named `libwasmer`, let's do that. 2. For the crate `wasmer-c-api`, since the library name has been renamed to `wasmer`, it creates a conflict with the `wasmer` crate which is a dependency. Consequently, this patch also updates the `Cargo.toml` file to modifiy the dependency name with the special `package` attribute (see https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml to learn more). So now, the `wasmer` refers to the `wasmer_c_api` crate, and `wasmer-api` refers to the `wasmer` crate. 3. The code of the `wasmer-c-api` crate is updated accordingly. The `WasmerEnv` derive procedural macro fails because it expects a crate named `wasmer` (which is now `wasmer_api`), so we implement the `WasmerEnv` trait by hand. 4. The patch updates the `build.rs` script of the `wasmer-c-api` crate: 1. In the `build_cdylib_link_arg` function: The dependency to the `cdylib-link-lines` crate has been removed because the output is not exactly the one we expect. So we compute all the `cargo:rustc-cdylib-link-arg=…` lines by hand. The version number no longer appears in the library file name for example. 2. In the `build_inline_c_env_vars` function: Values passed to `LDFLAGS` have been updated to be `libwasmer` rather than `libwasmer_c_api`. 3. A new `shared_object_dir` helper function has been created because it's used in `build_inline_c_env_vars` and in `build_cdylib_link_arg`. 5. The `Makefile` has been updated: 1. In `package-capi`, we no longer rename `libwasmer_c_api` to `libwasmer` since the name is correctly defined since the beginning now. Calling `install_name_tool` on macOS is no longer required since `install_name` is correctly set by the linker in the `build.rs` script of `wasmer-c-api`. 2. In `package-docs`, some stuffs have been fixed, like the `SOURCE_VERSION` variable that didn't exist, so removed, or the `mkdir` command that was incorrect etc. 3. In `build-docs`, the `wasmer-c-api` crate is excluded from the list of crates to generate the documentation for. Mostly because the `build-docs-capi` recipe exists, and we must use it to generate the documentation of `wasmer-c-api` now. 4. In `build-docs-capi`, we generate the documentation for the `wasmer-c-api` crate. But `rustdoc` uses the library name for the directory name in the `target/doc/` directory. Since the library name is now `wasmer`, it creates a conflict with the `wasmer` crate. Consequently, we update the library name by using `sed` on the `Cargo.toml` file before running `cargo doc`, to finally restore `Cargo.toml` as it was previously.
Hywan
added
🎉 enhancement
New feature!
📦 lib-c-api
About wasmer-c-api
🧪 tests
I love tests
📚 documentation
Do you like to read?
🤖 bot
Bip bip
labels
Jul 8, 2021
bors try |
Closed in favor of #2457. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
🤖 bot
Bip bip
📚 documentation
Do you like to read?
🎉 enhancement
New feature!
📦 lib-c-api
About wasmer-c-api
🧪 tests
I love tests
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This patch does several things.
wasmer-c-api
, the library name is modified fromwasmer_c_api
towasmer
inCargo.toml
. That way, the newlibrary files are named
libwasmer.*
rather thanlibwasmer_c_api.*
. That's the primaly goal of this patch. Therest is a consequence of this point. Why do we want that? Because
the
build.rs
script of thewasmer-c-api
crate will configurethe
soname
(on Linux), theinstall_name
+current_version
+compatibility_version
(on macOS), and theout-implib
+output-def
(on Windows) for a library namedlibwasmer
, which isthe name we provide in the Wasmer package for the Wasmer
libraries. If we want everything to be testable, we cannot use
libwasmer
insoname
for a file namedlibwasmer_c_api
forexample. If we rename the file when packaging (as it's done prior
this patch), we would need to re-update all those information in
the
Makefile
. It implies to duplicate the code in 2 places. Solet's do things right directly and only once: We want the library
to be named
libwasmer
, let's do that.wasmer-c-api
, since the library name has beenrenamed to
wasmer
, it creates a conflict with thewasmer
cratewhich is a dependency. Consequently, this patch also updates the
Cargo.toml
file to modifiy the dependency name with the specialpackage
attribute (seehttps://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml
to learn more). So now, the
wasmer
refers to thewasmer_c_api
crate, and
wasmer-api
refers to thewasmer
crate.wasmer-c-api
crate is updated accordingly. TheWasmerEnv
derive procedural macro fails because it expects acrate named
wasmer
(which is nowwasmer_api
), so we implementthe
WasmerEnv
trait by hand.build.rs
script of thewasmer-c-api
crate:
build_cdylib_link_arg
function: The dependency to thecdylib-link-lines
crate has been removed because the output isnot exactly the one we expect. So we compute all the
cargo:rustc-cdylib-link-arg=…
lines by hand. The version numberno longer appears in the library file name for example.
build_inline_c_env_vars
function: Values passed toLDFLAGS
have been updated to belibwasmer
rather thanlibwasmer_c_api
.shared_object_dir
helper function has been createdbecause it's used in
build_inline_c_env_vars
and inbuild_cdylib_link_arg
.Makefile
has been updated:package-capi
, we no longer renamelibwasmer_c_api
tolibwasmer
since the name is correctly defined since thebeginning now.
Calling
install_name_tool
on macOS is no longer required sinceinstall_name
is correctly set by the linker in thebuild.rs
script of
wasmer-c-api
.package-docs
, some stuffs have been fixed, like theSOURCE_VERSION
variable that didn't exist, so removed, or themkdir
command that was incorrect etc.build-docs
, thewasmer-c-api
crate is excluded from thelist of crates to generate the documentation for. Mostly because
the
build-docs-capi
recipe exists, and we must use it togenerate the documentation of
wasmer-c-api
now.build-docs-capi
, we generate the documentation for thewasmer-c-api
crate. Butrustdoc
uses the library name for thedirectory name in the
target/doc/
directory. Since the libraryname is now
wasmer
, it creates a conflict with thewasmer
crate. Consequently, we update the library name by using
sed
onthe
Cargo.toml
file before runningcargo doc
, to finallyrestore
Cargo.toml
as it was previously.Fixes #2429.
Review