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

Feature/headless wasmer revamp #2011

Merged
merged 8 commits into from
Jan 12, 2021
Merged

Feature/headless wasmer revamp #2011

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d63490e
Move HOST_TARGET up
syrusakbary Jan 12, 2021
0d40d52
Merge branch 'feature/headless-wasmer' of github.com:wasmerio/wasmer …
syrusakbary Jan 12, 2021
8994a06
Merge branch 'feature/headless-wasmer' of github.com:wasmerio/wasmer …
syrusakbary Jan 12, 2021
a9bba97
Improved strip command
syrusakbary Jan 12, 2021
1aa6da0
Improved Linux strip command
syrusakbary Jan 12, 2021
59de326
Merge branch 'feature/headless-wasmer' into feature/headless-wasmer-r…
syrusakbary Jan 12, 2021
35dee40
Fix flags
syrusakbary Jan 12, 2021
74a7162
Removed wasmer-headless bin
syrusakbary Jan 12, 2021
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
30 changes: 13 additions & 17 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ endif
# Using filter as a logical OR
# https://stackoverflow.com/questions/7656425/makefile-ifeq-logical-or
use_system_ffi =
cross_compiling_to_mac_aarch64 =
ifneq (,$(filter $(ARCH),aarch64 arm64))
test_compilers_engines += cranelift-jit
ifneq (, $(findstring llvm,$(compilers)))
Expand All @@ -63,7 +62,6 @@ ifneq (,$(filter $(ARCH),aarch64 arm64))
# if we are in macos arm64, we use the system libffi for the capi
ifeq ($(UNAME_S), Darwin)
use_system_ffi = yes
cross_compiling_to_mac_aarch64 = yes
endif
endif

Expand All @@ -90,6 +88,9 @@ endif
compiler_features_spaced := $(foreach compiler,$(compilers),$(compiler))
compiler_features := --features "$(compiler_features_spaced)"

HOST_TARGET=$(shell rustup show | grep 'Default host: ' | cut -d':' -f2 | tr -d ' ')

$(info Host target: $(bold)$(green)$(HOST_TARGET)$(reset))
$(info Available compilers: $(bold)$(green)${compilers}$(reset))
$(info Compilers features: $(bold)$(green)${compiler_features}$(reset))
$(info Available compilers + engines for test: $(bold)$(green)${test_compilers_engines}$(reset))
Expand All @@ -103,10 +104,10 @@ bench:
cargo bench $(compiler_features)

build-wasmer:
cargo build --release --manifest-path lib/cli/Cargo.toml $(compiler_features)
cargo build --release --manifest-path lib/cli/Cargo.toml $(compiler_features) --bin wasmer

build-wasmer-debug:
cargo build --manifest-path lib/cli/Cargo.toml $(compiler_features)
cargo build --manifest-path lib/cli/Cargo.toml $(compiler_features) --bin wasmer

# For best results ensure the release profile looks like the following
# in Cargo.toml:
Expand All @@ -121,10 +122,12 @@ build-wasmer-debug:
# codegen-units = 1
# rpath = false
build-wasmer-headless-minimal:
HOST_TARGET=$$(rustup show | grep 'Default host: ' | cut -d':' -f2 | tr -d ' ') ;\
echo $$HOST_TARGET ;\
RUSTFLAGS="-C panic=abort" xargo build --target $$HOST_TARGET --release --manifest-path=lib/cli/Cargo.toml --no-default-features --features headless-minimal --bin wasmer-headless ;\
strip -s required_symbols.txt target/$$HOST_TARGET/release/wasmer-headless
RUSTFLAGS="-C panic=abort" xargo build -v --target $(HOST_TARGET) --release --manifest-path=lib/cli/Cargo.toml --no-default-features --features headless-minimal --bin wasmer-headless
ifeq ($(UNAME_S), Darwin)
strip -u target/$(HOST_TARGET)/release/wasmer-headless
else
strip --strip-unneeded target/$(HOST_TARGET)/release/wasmer-headless
endif

WAPM_VERSION = master # v0.5.0
get-wapm:
Expand Down Expand Up @@ -329,16 +332,9 @@ endif
endif

package-minimal-headless-wasmer:
ifdef cross_compiling_to_mac_aarch64
if [ -f "target/aarch64-apple-darwin/release/wasmer-headless" ]; then \
cp target/aarch64-apple-darwin/release/wasmer-headless package/bin/ ;\
if [ -f "target/$(HOST_TARGET)/release/wasmer-headless" ]; then \
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we still need the special casing for aarch64 macos here? if we're not building on actual aarch64 macos machines, I believe this will not work correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if I follow why.
HOST_TARGET will be aarch64-apple-darwin when cross compiling to macos-arm64 since we setup the target in cargo config:
https://github.com/wasmerio/wasmer/blob/master/.github/workflows/main.yaml#L138-L144

Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would assume

 HOST_TARGET=$(shell rustup show | grep 'Default host: ' | cut -d':' -f2 | tr -d ' ')

Will not give the correct results there, but I'm just assuming

Copy link
Member Author

Choose a reason for hiding this comment

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

I just re-checked and you are right, however I just found out the previous logic was also wrong.

cross_compiling_to_mac_aarch64 =
ifneq (,$(filter $(ARCH),aarch64 arm64))
	test_compilers_engines += cranelift-jit
	ifneq (, $(findstring llvm,$(compilers)))
		test_compilers_engines += llvm-native
	endif
	# if we are in macos arm64, we use the system libffi for the capi
	ifeq ($(UNAME_S), Darwin)
		cross_compiling_to_mac_aarch64 = yes
		use_system_ffi = yes
	endif
endif

This can never be true. cross_compiling_to_mac_aarch64 will never be yes unless the host is a macOS M1 itself

Copy link
Member Author

@syrusakbary syrusakbary Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Re-checking this:

HOST_TARGET=$(shell rustup show | grep 'Default host: ' | cut -d':' -f2 | tr -d ' ')

Should return aarch64-apple-darwin when cross-compiling. Since it's the default (and only) one installed for that.
So we should be good!

Note: I haven't tried so that speculation is on the theorical field!

cp target/$(HOST_TARGET)/release/wasmer-headless package/bin ;\
fi
else
HOST_TARGET=$$(rustup show | grep 'Default host: ' | cut -d':' -f2 | tr -d ' ') ;\
if [ -f "target/$$HOST_TARGET/release/wasmer-headless" ]; then \
cp target/$$HOST_TARGET/release/wasmer-headless package/bin/ ;\
fi
endif

package-wasmer:
mkdir -p "package/bin"
Expand Down