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

Set CMake module path #335

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Conversation

arvid-norlander
Copy link
Contributor

This is a redo/update of PR #182, as that was by a different author I cannot update that PR, but have to do a separate one.

Compared to the previous PR, this adds testing of the cmake setup as well.

Fixes: #181

@arvid-norlander arvid-norlander changed the title Cmake module path Set CMake module path Jul 12, 2023
@arvid-norlander
Copy link
Contributor Author

I seem to have an unrelated failure in:

===== Testing with -O0 =====
Testing abort.c...
Testing abort.c...
--- abort.c.stderr.expected     2023-07-07 13:13:40.708014588 +0200
+++ abort.c.-O0.stderr.observed.filtered        2023-07-12 10:19:53.021388399 +0200
@@ -1,6 +1,2 @@
-Error: failed to run main module `abort.c.---.wasm`
-
-Caused by:
-    0: failed to invoke command default
-    1: error while executing at wasm backtrace:
-    2: wasm trap: wasm `unreachable` instruction executed
+error: RuntimeError: unreachable
+╰─▶ 1: RuntimeError: unreachable
make: *** [Makefile:54: check] Error 1

I'm using wasmer as my wasm runtime. This error is unrelated to my changes, and hopefully will not be a blocker.

@arvid-norlander
Copy link
Contributor Author

arvid-norlander commented Jul 13, 2023

I don't have windows, but it looks like CMake generates Ninja files instead of Makefiles on Windows. I have nothing against ninja, in fact I prefer it. Can we just default to always using ninja for the test across the board (with -GNinja). Or is it not part of the Linux/OS X setup?

tests/run.sh Show resolved Hide resolved
@abrown
Copy link
Collaborator

abrown commented Jul 14, 2023

This error is unrelated to my changes, and hopefully will not be a blocker

Yeah, just looks like a different error message. If you use Wasmtime, which the original author probably did, I would expect the error to go away.

@arvid-norlander
Copy link
Contributor Author

This should fix it. Unfortunately I can't start the CI.

tests/run.sh Show resolved Hide resolved
tests/run.sh Outdated Show resolved Hide resolved
wasi-sdk.cmake Outdated Show resolved Hide resolved
@arvid-norlander
Copy link
Contributor Author

Hi, I have been on vacation, but I'll look into your feedback in the coming days.

@arvid-norlander
Copy link
Contributor Author

All requested changes have been implemented, please allow the workflow again (this is really annoying with github)

@arvid-norlander
Copy link
Contributor Author

Now also rebased onto tip

@abrown abrown force-pushed the cmake_module_path branch from a609387 to f29bf18 Compare August 29, 2023 20:00
@arvid-norlander
Copy link
Contributor Author

Not sure what that push from @sbc100 was about (rebase?)

Is this waiting on anything else at this point?

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2023

It looks like you still have three commits on top of main somehow (your new one and two existing ones).

@sbc100
Copy link
Member

sbc100 commented Sep 6, 2023

It looks like you still have three commits on top of main somehow (your new one and two existing ones).

Sorry, ignore me. I forgot that you took #182 as part of this change.

@sbc100 sbc100 merged commit 8d4dbb1 into WebAssembly:main Sep 6, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set(CMAKE_SYSTEM_NAME WASI) issues on macOS host
3 participants