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

Distro packaging improvements #2135

Merged
merged 9 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,18 @@ jobs:
toolchain: 1.49
override: true
components: rustfmt, clippy
- name: Install LLVM (Linux)
run: |
curl --proto '=https' --tlsv1.2 -sSf https://github.com/wasmerio/llvm-custom-builds/releases/download/11.x/linux-amd64.tar.gz -L -o /opt/llvm.tar.xz
mkdir -p /opt/llvm-11
tar xf /opt/llvm.tar.xz --strip-components=1 -C /opt/llvm-11
echo '/opt/llvm-11/bin' >> $GITHUB_PATH
echo 'LLVM_SYS_110_PREFIX=/opt/llvm-11' >> $GITHUB_ENV
- run: make lint
env:
ENABLE_CRANELIFT: "1"
ENABLE_LLVM: "1"
ENABLE_SINGLEPASS: "1"
- name: Assert no files have changed
run: |
git status
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- [#2123](https://github.com/wasmerio/wasmer/pull/2123) Use `ENABLE_{{compiler_name}}=(0|1)` to resp. force to disable or enable a compiler when running the `Makefile`, e.g. `ENABLE_LLVM=1 make build-wasmer`.
- [#2123](https://github.com/wasmerio/wasmer/pull/2123) `libwasmer` comes with all available compilers per target instead of Cranelift only.
- [#2118](https://github.com/wasmerio/wasmer/pull/2118) Add an unstable non-standard C API to query available engines and compilers.
- [#2135](https://github.com/wasmerio/wasmer/pull/2135) [Documentation](./PACKAGING.md) for linux distribution maintainers

### Changed
- [#2113](https://github.com/wasmerio/wasmer/pull/2113) Bump minimum supported Rust version to 1.49
Expand Down
45 changes: 43 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ $(info )
# Building #
############

# Not really "all", just the default target that builds enough so make install will go through
all: build-wasmer build-capi

bench:
cargo bench $(compiler_features)

Expand Down Expand Up @@ -493,7 +496,7 @@ test-packages:
cargo test -p wasmer-engine-native --release --no-default-features
cargo test -p wasmer-engine-jit --release --no-default-features
cargo test -p wasmer-compiler --release
cargo test -p wasmer-cli --release
cargo test --manifest-path lib/cli/Cargo.toml $(compiler_features) --release
cargo test -p wasmer-cache --release
cargo test -p wasmer-engine --release
cargo test -p wasmer-derive --release
Expand Down Expand Up @@ -657,6 +660,44 @@ endif
tar -C package -zcvf wasmer.tar.gz bin lib include LICENSE ATTRIBUTIONS
mv wasmer.tar.gz dist/

########################
# (Distro-) Installing #
########################

DESTDIR ?= /usr/local

install: install-wasmer install-capi-headers install-capi-lib install-capi-staticlib install-pkgconfig install-misc

install-wasmer:
install -Dm755 target/release/wasmer $(DESTDIR)/bin/wasmer

install-capi-headers:
for header in lib/c-api/*.{h,hh}; do install -Dm644 "$$header" $(DESTDIR)/include/$$(basename $$header); done
install -Dm644 lib/c-api/README.md $(DESTDIR)/include/wasmer-README.md

# Currently implemented for linux only. TODO
install-capi-lib:
pkgver=$$(target/release/wasmer --version | cut -d\ -f2) && \
shortver="$${pkgver%.*}" && \
majorver="$${shortver%.*}" && \
install -Dm755 target/release/libwasmer_c_api.so "$(DESTDIR)/lib/libwasmer.so.$$pkgver" && \
ln -sf "libwasmer.so.$$pkgver" "$(DESTDIR)/lib/libwasmer.so.$$shortver" && \
ln -sf "libwasmer.so.$$pkgver" "$(DESTDIR)/lib/libwasmer.so.$$majorver" && \
ln -sf "libwasmer.so.$$pkgver" "$(DESTDIR)/lib/libwasmer.so"
Comment on lines +683 to +686
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a mention that install-capi-lib is for Linux distros only, which excludes Darwin and Windows. A simple documentation line dropped before install-capi-lib would be OK for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I could also migrate some parts over from package-capi to support Windows/Darwin, but I have little in the way of testing it.

I'm also not exactly happy with the duplication between package-* and install-*, but I don't think I can smoothly migrate whatever is using package-* to install-*. I'll leave that for someone with more inside knowledge.


install-capi-staticlib:
install -Dm644 target/release/libwasmer_c_api.a "$(DESTDIR)/lib/libwasmer.a"

install-misc:
install -Dm644 LICENSE "$(DESTDIR)"/share/licenses/wasmer/LICENSE

install-pkgconfig:
unset WASMER_DIR # Make sure WASMER_INSTALL_PREFIX is set during build
target/release/wasmer config --pkg-config | install -Dm644 /dev/stdin "$(DESTDIR)"/lib/pkgconfig/wasmer.pc

install-wasmer-headless-minimal:
install -Dm755 target/release/wasmer-headless $(DESTDIR)/bin/wasmer-headless

#################
# Miscellaneous #
#################
Expand All @@ -678,7 +719,7 @@ lint-packages:
RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-compiler
RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-compiler-cranelift
RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-compiler-singlepass
RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-cli
RUSTFLAGS=${RUSTFLAGS} cargo clippy --manifest-path lib/cli/Cargo.toml $(compiler_features)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since LLVM is likely to be part of $(compiler_features) set, the Lint workflow in the CI must install LLVM now.

Copy link
Contributor Author

@jcaesar jcaesar Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I guess I'll add that to the CI setup. There actually was one problem in the code that would have been caught by linting with all compilers on. Should I additionally keep the "lint with all off"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon we should keep the Lints as they are in master for the moment, and open another PR or a new issue to update them so that they include $(compiler_features) (which makes sense!).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not an option, as compiling without any compiler_features will fail. But I guess I could temporarily remove linting wasmer-cli and do another PR? Dangerous...

RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-cache
RUSTFLAGS=${RUSTFLAGS} cargo clippy -p wasmer-engine

Expand Down
18 changes: 18 additions & 0 deletions PACKAGING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Wasmer distro packaging notes

* Where possible, do not directly invoke cargo, but use the supplied Makefile
* wasmer has several compiler backends and the Makefile autodetects whether to enable llvm and singlepass.
Set `ENABLE_{CRANELIFT,LLVM,SINGLEPASS}=1` to build the full set or fail trying
* Set `WASMER_CAPI_USE_SYSTEM_LIBFFI=1` to force dynamic linking of libffi on the shared library
* `make install` respects `DESTDIR`, but `prefix` must be configured as e.g. `WASMER_INSTALL_PREFIX=/usr make all`
* In case you must build/install directly with cargo, make sure to enable at least one compiler backend feature
* Beware that compiling with `cargo build --workspace/--all --features ...` will not enable features on the subcrates in the workspace and result in a headless wasmer binary that can not run wasm files directly.
* If you split the package into several subpackages, beware that the create-exe command of wasmer requires `libwasmer.a` to be installed at `$WASMER_INSTALL_PREFIX/lib/libwasmer.a`.
Suggestion for splitting:
* `wasmer` and `wasmer-headless`, containing the respective executables
* `wasmer-headless` contains a subset of `wasmer`'s functionality and should only be packaged when splitting - it must be built explicitly with `make build-wasmer-headless-minimal install-wasmer-headless-minimal`
* `libwasmer`, containing `libwasmer.so*`
* `libwasmer-dev`, containing the header files and a `.pc` file
* `libwasmer-static`, containing `libwasmer.a`

The wasmer distro packaging story is still in its infancy, so feedback is very welcome.
1 change: 1 addition & 0 deletions lib/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license = "MIT"
readme = "README.md"
edition = "2018"
default-run = "wasmer"
build = "build.rs"

[[bin]]
name = "wasmer"
Expand Down
4 changes: 4 additions & 0 deletions lib/cli/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-env-changed=WASMER_INSTALL_PREFIX");
}
5 changes: 5 additions & 0 deletions lib/cli/src/bin/wasmer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use wasmer_cli::cli::wasmer_main;

#[cfg(not(any(feature = "cranelift", feature = "singlepass", feature = "llvm")))]
compile_error!(
"Either enable at least one compiler, or compile the wasmer-headless binary instead"
);
jcaesar marked this conversation as resolved.
Show resolved Hide resolved

fn main() {
wasmer_main();
}
14 changes: 10 additions & 4 deletions lib/cli/src/commands/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ impl Config {
}
fn inner_execute(&self) -> Result<()> {
let key = "WASMER_DIR";
let wasmer_dir = env::var(key).context(format!(
"failed to retrieve the {} environment variables",
key
))?;
let wasmer_dir = env::var(key)
.or_else(|e| {
option_env!("WASMER_INSTALL_PREFIX")
.map(str::to_string)
.ok_or(e)
})
.context(format!(
"failed to retrieve the {} environment variables",
key
))?;

let prefix = PathBuf::from(wasmer_dir);

Expand Down
10 changes: 8 additions & 2 deletions lib/cli/src/commands/create_exe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,20 @@ fn generate_header(header_file_src: &[u8]) -> anyhow::Result<()> {
.open(&header_file_path)?;

use std::io::Write;
header.write(header_file_src)?;
header.write_all(header_file_src)?;

Ok(())
}

fn get_wasmer_dir() -> anyhow::Result<PathBuf> {
Ok(PathBuf::from(
env::var("WASMER_DIR").context("Trying to read env var `WASMER_DIR`")?,
env::var("WASMER_DIR")
.or_else(|e| {
option_env!("WASMER_INSTALL_PREFIX")
.map(str::to_string)
.ok_or(e)
})
.context("Trying to read env var `WASMER_DIR`")?,
))
}

Expand Down