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

Auto download Wasmer libs for module use #22

Closed
ashtonmeuser opened this issue May 3, 2023 · 13 comments
Closed

Auto download Wasmer libs for module use #22

ashtonmeuser opened this issue May 3, 2023 · 13 comments

Comments

@ashtonmeuser
Copy link
Owner

ashtonmeuser commented May 3, 2023

Should download Wasmer libs (if required) when Godot Wasm used as a module i.e. in SCsub. Spawned from this comment. This may be a good opportunity to abstract the Wasmer lib download functionality already present in SConstruct.

This was referenced May 3, 2023
@fire
Copy link

fire commented May 3, 2023

You got the description of downloading Wasmer libraries correct.

Also note that windows supports both mingw and mvsc which corresponds to .a and .lib.

@fire
Copy link

fire commented May 3, 2023

@ashtonmeuser
Copy link
Owner Author

ashtonmeuser commented May 4, 2023

Pushed a fix in aa4b49e (Godot 3.x) and e6bb0ee (Godot 4.x). The issue was that the Wasmer library wasn't being downloaded for the linuxbsd platform specified by Godot modules. The library download and default version are also now kept in a common file (utils.py) which should simplify maintaining the two branches.

Also addresses using a .a library suffix for MinGW. Note that the MVSC suffix is actually .dll.lib which seems to be specific for Wasmer.

@fire Lemme know if this will suffice and I'll close this issue.

Edit: For posterity's sake, here's the workflow being discussed.

@ashtonmeuser
Copy link
Owner Author

ashtonmeuser commented May 4, 2023

This made me realize that the paths for Linux in the actual GDExtension file will probably be incorrect (should be .../linuxbsd/... instead of .../linux/...). Should compile but won't run. Away from my computer and can't fix right now. Just a heads up.

Ignore this. GDNative and GDExtension use linux platform and no library is created for Godot modules.

@fire
Copy link

fire commented May 4, 2023

@fire
Copy link

fire commented May 4, 2023

Some errors. I added a fix for the lib download but failing elsewhere.

image

@fire
Copy link

fire commented May 4, 2023

Modified:

#ifndef WASM_API_EXTERN
#if defined(_WIN32) && !defined(__MINGW32__)
#define WASM_API_EXTERN __declspec(dllimport)
#else
#define WASM_API_EXTERN
#endif
#endif

In wasm.h to have a check for mingw.

@fire
Copy link

fire commented May 4, 2023

> git diff
diff --git a/modules/wasm/SCsub b/modules/wasm/SCsub
index 0f8a3342402..918495c0a99 100644
--- a/modules/wasm/SCsub
+++ b/modules/wasm/SCsub
@@ -17,6 +17,7 @@ elif env["platform"] in ["osx", "macos"]:
     env.Append(LINKFLAGS=["-framework", "Security"])
 elif env["platform"] == "windows":
     module_env["LIBWASMERSUFFIX"] = ".a" if env.get("use_mingw") else ".dll.lib"
+    env.Append(LIBS=["userenv"])
 
 # Explicit static libraries
 wasmer_library = env.File(
diff --git a/modules/wasm/utils.py b/modules/wasm/utils.py
index 725da531c20..dcc2d089011 100644
--- a/modules/wasm/utils.py
+++ b/modules/wasm/utils.py
@@ -47,4 +47,7 @@ def download_wasmer(env, force=False, version=VERSION_DEFAULT):
     elif env["platform"] in ["linux", "linuxbsd"]:
         download_tarfile(BASE_URL.format(version, "linux-amd64"), "wasmer")
     elif env["platform"] == "windows":
-        download_tarfile(BASE_URL.format(version, "windows-amd64"), "wasmer")
+        if env.get("use_mingw"):
+            download_tarfile(BASE_URL.format(version, "windows-gnu64"), "wasmer")
+        else:
+            download_tarfile(BASE_URL.format(version, "windows-amd64"), "wasmer")

I was able to launch on windows with the wasm.h changes.

@ashtonmeuser
Copy link
Owner Author

Created #26 to capture failing MinGW build.

@ashtonmeuser
Copy link
Owner Author

@fire Huge thanks for all of the feedback you've provided! I think I've addressed everything you mentioned. All outstanding tasks have been rolled into their own issues seeing as we've gone on a bit of a tangent here. Let me know if anything's been missed.

@fire
Copy link

fire commented May 5, 2023

I'm a bit tired, so I'll let you know when I do an update.

I believe you can use a hot patch on the wasmer includes. Like literally apply a diff.

@fire
Copy link

fire commented May 5, 2023

We can close this one though.

@ashtonmeuser
Copy link
Owner Author

No rush at all! Yeah I'm sure there are plenty of workarounds but figured I'd open a PR in case others faced the same issue. Closing this.

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

No branches or pull requests

2 participants