Distro packaging improvements #2135

jcaesar · 2021-02-22T15:09:48Z

Description

Having installed wasmer from both gentoo and AUR (packaged by svenstaro, an Arch Maintainer), I was stumped to find that both build wasmer without compilers, not actually allowing me to run any wasm. (The AUR PKGBUILD has since been patched, but the gentoo ebuild maintainer… who knows.)

I figured that the obvious improvement would be to deny compilation if not at least one compiler is enabled, but I think more can be done to help packaging. I added

a way to set a default for WASMER_DIR at compile time
an install target for the Makefile
a PACKAGING.md with some suggestions to distro maintainers

Disclaimer: I'm not a package maintainer anywhere, so these might be entirely off the rails. Commit 1 and 2 seem fairly reasonable to me, but for the other two, I'm not so sure.

Review

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

syrusakbary

This looks great! Thanks for the addition!

syrusakbary · 2021-02-25T01:23:13Z

bors r+

2135: Distro packaging improvements r=syrusakbary a=jcaesar # Description Having installed wasmer from both gentoo and AUR (packaged by svenstaro, an Arch Maintainer), I was stumped to find that both build wasmer without compilers, not actually allowing me to run any wasm. (The AUR PKGBUILD has since been patched, but the gentoo ebuild maintainer… who knows.) I figured that the obvious improvement would be to deny compilation if not at least one compiler is enabled, but I think more can be done to help packaging. I added * a way to set a default for `WASMER_DIR` at compile time * an install target for the Makefile * a `PACKAGING.md` with some suggestions to distro maintainers Disclaimer: I'm not a package maintainer anywhere, so these might be entirely off the rails. Commit 1 and 2 seem fairly reasonable to me, but for the other two, I'm not so sure. # Review - [ ] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Julius Michaelis <[email protected]>

bors · 2021-02-25T01:29:35Z

Build failed:

Code lint

lib/cli/src/bin/wasmer.rs

syrusakbary · 2021-02-25T01:30:54Z

bors r-

Makefile

Hywan · 2021-02-25T08:08:04Z

Makefile

+	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"


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.

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.

Makefile

PACKAGING.md

Hywan · 2021-02-25T08:13:36Z

Thank you for the patch, it looks great! I made some feedbacks and found some issues, nothing difficult to fix.

Do you think we could test that in the CI?

jcaesar · 2021-02-25T08:25:13Z

(I hope to have some time to integrate the suggested fixes this evening...)
Btw, which style of PR ammendment do you prefer? Fixup the old commits, or add new ones so it's easier to review?

…ges lint-packages passes

Hywan · 2021-02-25T14:48:30Z

Just add as many commits as you want for the moment, we will see at the end :-).

jcaesar · 2021-02-26T00:22:02Z

All right. Want to give bors another spin then? I think it might pass now.

syrusakbary · 2021-02-26T09:27:31Z

bors r+

bors · 2021-02-26T09:27:33Z

👎 Rejected by code reviews

syrusakbary · 2021-02-26T09:27:50Z

bors r+

bors · 2021-02-26T09:27:51Z

👎 Rejected by code reviews

Hywan

I think it's good now!

Hywan · 2021-02-26T09:35:08Z

bors r+

2135: Distro packaging improvements r=Hywan a=jcaesar # Description Having installed wasmer from both gentoo and AUR (packaged by svenstaro, an Arch Maintainer), I was stumped to find that both build wasmer without compilers, not actually allowing me to run any wasm. (The AUR PKGBUILD has since been patched, but the gentoo ebuild maintainer… who knows.) I figured that the obvious improvement would be to deny compilation if not at least one compiler is enabled, but I think more can be done to help packaging. I added * a way to set a default for `WASMER_DIR` at compile time * an install target for the Makefile * a `PACKAGING.md` with some suggestions to distro maintainers Disclaimer: I'm not a package maintainer anywhere, so these might be entirely off the rails. Commit 1 and 2 seem fairly reasonable to me, but for the other two, I'm not so sure. # Review - [ ] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Julius Michaelis <[email protected]> Co-authored-by: Ivan Enderlin <[email protected]>

bors · 2021-02-26T09:42:59Z

Build failed:

Code lint

Hywan · 2021-02-26T09:45:22Z

Makefile

@@ -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)


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

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"?

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!).

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...

Hywan · 2021-03-01T08:37:05Z

bors try

jcaesar · 2021-03-01T11:37:38Z

Sorry, my push missed the "bors try" by about 90 seconds. Took me some force-pushing to get installing llvm right, but lint should work now.

MarkMcCaskey · 2021-03-01T14:57:17Z

bors try

bors · 2021-03-01T14:57:19Z

try

Already running a review

syrusakbary · 2021-03-03T03:49:14Z

bors try

bors · 2021-03-03T03:49:15Z

try

Already running a review

syrusakbary · 2021-03-03T03:49:23Z

bors try-

syrusakbary · 2021-03-03T03:49:33Z

bors try

jcaesar · 2021-03-03T06:08:26Z

Maybe bors will be happy if you "bors r-" "bors r+" it once? Or, if it is bugging around, I can also close this pr and reopen with the same commits...

syrusakbary · 2021-03-03T06:19:40Z

bors r-

syrusakbary · 2021-03-03T06:19:47Z

bors try-

syrusakbary · 2021-03-03T06:19:56Z

bors r+

syrusakbary · 2021-03-03T06:22:24Z

I'll merge manually and see if tests pass later or not. Bors seems broken

jcaesar added 4 commits February 22, 2021 23:39

wasmer-cli: refuse to build if not at least one compiler is enabled

8041f03

Allow preconfiguring WASMER_DIR at build time

80ec06f

Makefile: add install target

a9537c9

Document some suggestions for distro packaging

a0800cb

jcaesar requested review from MarkMcCaskey and syrusakbary as code owners February 22, 2021 15:09

syrusakbary approved these changes Feb 25, 2021

View reviewed changes

syrusakbary reviewed Feb 25, 2021

View reviewed changes

lib/cli/src/bin/wasmer.rs Show resolved Hide resolved

Hywan suggested changes Feb 25, 2021

View reviewed changes

Hywan self-assigned this Feb 25, 2021

Hywan added 🎉 enhancement New feature! 📚 documentation Do you like to read? 🤖 bot Bip bip labels Feb 25, 2021

jcaesar and others added 2 commits February 25, 2021 18:10

Fix 8041f03: enable compilers during tests, make sure make test-packa…

59b5544

…ges lint-packages passes

Fix a0800cb typos

637351c

jcaesar force-pushed the distro branch from 2d7eff6 to b8b35dc Compare February 25, 2021 09:18

jcaesar added 2 commits February 25, 2021 20:46

Fix a9537c9: install wasmer-headless

53c4d4f

Update changelog

1cd42ce

jcaesar force-pushed the distro branch from b8b35dc to 1cd42ce Compare February 25, 2021 11:46

syrusakbary approved these changes Feb 26, 2021

View reviewed changes

Hywan approved these changes Feb 26, 2021

View reviewed changes

Hywan reviewed Feb 26, 2021

View reviewed changes

jcaesar force-pushed the distro branch 4 times, most recently from 5284dc4 to 4885b7c Compare March 1, 2021 08:34

Fix 59b5544: Install llvm in lint workflow

7eef4b6

jcaesar force-pushed the distro branch from 4885b7c to 7eef4b6 Compare March 1, 2021 08:38

syrusakbary merged commit 1ee7b4a into wasmerio:master Mar 3, 2021

jcaesar deleted the distro branch March 3, 2021 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distro packaging improvements #2135

Distro packaging improvements #2135

jcaesar commented Feb 22, 2021

syrusakbary left a comment

syrusakbary commented Feb 25, 2021

bors bot commented Feb 25, 2021

syrusakbary commented Feb 25, 2021

Hywan Feb 25, 2021

jcaesar Feb 25, 2021

Hywan commented Feb 25, 2021

jcaesar commented Feb 25, 2021

Hywan commented Feb 25, 2021

jcaesar commented Feb 26, 2021 •

edited

Loading

syrusakbary commented Feb 26, 2021

bors bot commented Feb 26, 2021

syrusakbary commented Feb 26, 2021

bors bot commented Feb 26, 2021

Hywan left a comment

Hywan commented Feb 26, 2021

bors bot commented Feb 26, 2021

Hywan Feb 26, 2021

jcaesar Mar 1, 2021 •

edited

Loading

Hywan Mar 1, 2021

jcaesar Mar 1, 2021

Hywan commented Mar 1, 2021

jcaesar commented Mar 1, 2021

MarkMcCaskey commented Mar 1, 2021

bors bot commented Mar 1, 2021

syrusakbary commented Mar 3, 2021

bors bot commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

jcaesar commented Mar 3, 2021 •

edited

Loading

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

Distro packaging improvements #2135

Distro packaging improvements #2135

Conversation

jcaesar commented Feb 22, 2021

Description

Review

syrusakbary left a comment

Choose a reason for hiding this comment

syrusakbary commented Feb 25, 2021

bors bot commented Feb 25, 2021

syrusakbary commented Feb 25, 2021

Hywan Feb 25, 2021

Choose a reason for hiding this comment

jcaesar Feb 25, 2021

Choose a reason for hiding this comment

Hywan commented Feb 25, 2021

jcaesar commented Feb 25, 2021

Hywan commented Feb 25, 2021

jcaesar commented Feb 26, 2021 • edited Loading

syrusakbary commented Feb 26, 2021

bors bot commented Feb 26, 2021

syrusakbary commented Feb 26, 2021

bors bot commented Feb 26, 2021

Hywan left a comment

Choose a reason for hiding this comment

Hywan commented Feb 26, 2021

bors bot commented Feb 26, 2021

Hywan Feb 26, 2021

Choose a reason for hiding this comment

jcaesar Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

Hywan Mar 1, 2021

Choose a reason for hiding this comment

jcaesar Mar 1, 2021

Choose a reason for hiding this comment

Hywan commented Mar 1, 2021

jcaesar commented Mar 1, 2021

MarkMcCaskey commented Mar 1, 2021

bors bot commented Mar 1, 2021

try

syrusakbary commented Mar 3, 2021

bors bot commented Mar 3, 2021

try

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

jcaesar commented Mar 3, 2021 • edited Loading

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

syrusakbary commented Mar 3, 2021

jcaesar commented Feb 26, 2021 •

edited

Loading

jcaesar Mar 1, 2021 •

edited

Loading

jcaesar commented Mar 3, 2021 •

edited

Loading