Fix "create-exe" for windows-x86_64 target #3299

fschutt · 2022-11-11T10:23:22Z

No description provided.

lib/cli/src/commands/create_exe.rs

On Windows, this function gets used to download zig cc releases from GitHub, but GitHub uses .zip for Windows releases and .tar.gz for everything else. So this function has to accomodate that.

fschutt · 2022-11-15T20:42:45Z

This is currently stuck on zig cc not properly linking the object files:

zig cc -target x86_64-windows-gnu -L. -l:wasmer.lib -I. "-isystem" ./python.obj ./wasmer_main.c ./volume.o -o ./python.exe

allfiles.zip
allfiles 2.zip

fschutt · 2022-11-16T09:59:48Z

Windows build to test:

x86_64-windows-gnu-python.zip

We run this test only on linux and macos, on Windows it might or might not work.

… anything

Sebastien suggested this to resolve issues on windows 32 bit, but it didn't help anything

fschutt · 2022-11-16T14:26:58Z

@syrusakbary At this point the error is fixed for every system except the x86_64-windows target: zig build-exe doesn't output a target file,but doesn't output an error either. It works for every other target (the original error of stdio.h not found is fixed by switching from zig cc to zig build-exe).

All I can do at this point is to report a bug to zig and ask for help.

zigbug.zip

fschutt · 2022-11-18T17:12:03Z

Seems like it will go through now.

fschutt · 2022-11-18T17:12:09Z

bors r+

3299: Fix "create-exe" for windows-x86_64 target r=fschutt a=fschutt Co-authored-by: Felix Schütt <[email protected]> Co-authored-by: Felix Schütt <[email protected]>

bors · 2022-11-18T17:21:00Z

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

lib/cli/src/commands/create_exe.rs

Michael-F-Bryan · 2022-11-18T18:30:56Z

lib/cli/src/commands/create_exe.rs

+                libwasmer_parent.pop();
+                cmd.arg(libwasmer_parent.join("winsdk/ADVAPI32.lib"));
+                cmd.arg(libwasmer_parent.join("winsdk/BCRYPT.lib"));
+                cmd.arg(libwasmer_parent.join("winsdk/KERNEL32.lib"));
+                cmd.arg(libwasmer_parent.join("winsdk/USERENV.lib"));
+                cmd.arg(libwasmer_parent.join("winsdk/WS2_32.lib"));


Do we have a list of these files?

It might be a good idea to pull them into a constant at the top of the file, that way we can set up tests to make sure all the *.lib files are always included with Wasmer.

No, it's just the libraries that I found out that libwasmer.a depends on.

There is no way to find out the libraries, except for using Google to lookup which DLL the missing functions belong to.

Oh no, what I meant was instead of copy-pasting the cmd.arg(libwasmer_parent.join(...)) line for every library, we should store them in a constant and use a loop.

const VENDORED_LIBRARIES: &[&str] = &[ "ADVAPI32.lib", "BCRYPT.lib", ... ];

That way we can add a test to the wasmer CLI that makes sure all the libraries are embedded in the installer. It'd be annoying if the list of libraries we tell the compiler to link against and the list of libraries bundled with wasmer got out of sync. We probably wouldn't notice until weeks later when our first user wants to do networking and finds we forgot to give them WS2_32.lib.

I've made a loop with fs::read_dir now, so we don't need to hardcode any paths, it will just link to every object in the /winsdk folder.

Michael-F-Bryan · 2022-11-18T18:31:23Z

lib/cli/src/commands/create_exe.rs

-                .arg(&output_path)
-                .output()
-                .context("Could not execute `zig`")?
+            println!("{:?}", cmd);


We can delete this one, too.

Technically we can, but I left it in on purpose so I can see which zig command gets output.

Can we use log::debug!(), then? End users probably don't want to see the full command-line we are using to generate EXEs.

yes, I migrated to log::debug

lib/cli/src/commands/create_exe.rs

lib/registry/src/lib.rs

tests/integration/cli/tests/run.rs

Add integration test for failing create-exe command

26524da

fschutt requested a review from syrusakbary as a code owner November 11, 2022 10:23

fschutt added 5 commits November 11, 2022 18:21

Debug issue on macos

b63701c

Debug issue on MacOS

6f3b356

Fix target on Linux

97b1834

Add gcc-multilib package to linux

cb6fda1

Try to fix include paths for new wasmer-windows target

ddf4b33

fschutt force-pushed the fix-create-exe-windows branch from d2af38e to ddf4b33 Compare November 14, 2022 14:20

Remove threading when dowloading release to prevent unexpected failure

18765cd

syrusakbary reviewed Nov 14, 2022

View reviewed changes

lib/cli/src/commands/create_exe.rs Outdated Show resolved Hide resolved

syrusakbary reviewed Nov 14, 2022

View reviewed changes

lib/cli/src/commands/create_exe.rs Outdated Show resolved Hide resolved

fschutt marked this pull request as draft November 15, 2022 10:36

fschutt added 7 commits November 15, 2022 12:51

Completely rework downloading logic, add -isystem to zig cc

cf77524

Ignore non- .tar.gz files in cache dir

b0cee0e

Debug why autoinstalling zig isn't working on Windows

748fe29

Debug why create-exe is not downloading Zig on Windows part 2

c6bd018

Add unpack_zip to try_unpack_targz

a1c919a

On Windows, this function gets used to download zig cc releases from GitHub, but GitHub uses .zip for Windows releases and .tar.gz for everything else. So this function has to accomodate that.

Download .tar.gz files into tempdir instead of path-relative dir

852475b

Windows: use zig.exe instead of "zig" command

2c604e8

Debug why python.obj doesn't get linked correctly

e3b317a

fschutt added 8 commits November 16, 2022 11:27

Fix cross-compile test by switching to build-exe

f73cf54

We run this test only on linux and macos, on Windows it might or might not work.

Update debug output to debug why create-exe on Windows doesn't output…

493f393

… anything

Add more debug to the zig autoinstallation

c60a04c

Remove try_autoinstall_zig function

ec13f47

Remove windows_underscore specialcase

e97118a

Sebastien suggested this to resolve issues on windows 32 bit, but it didn't help anything

Remove unnecessary configuration

6375a8d

Remove unpack zip functionality from unpack_tar_gz

aafd57c

Revert renaming wasm.o / wasm.obj differences

0778ddf

fschutt added 3 commits November 16, 2022 14:04

Revert more .o / .obj renaming

676e1cc

Last .o / .obj renaming

a854775

Panic in integration test

87e00f7

fschutt and others added 3 commits November 18, 2022 11:11

Merge branch 'master' into fix-create-exe-windows

64d4e60

Fix merge issues

9bedc0a

Fix windows-gnu create-exe

a81344e

fschutt marked this pull request as ready for review November 18, 2022 10:38

fschutt and others added 6 commits November 18, 2022 11:50

Download -gnu64.tar.gz on Windows, fix make lint

195ec84

Remove windows target for the -rc.5 release

a6acb6b

Fix make lint

b5d0896

Add zig to integration-tests

be6f63f

Merge branch 'master' into fix-create-exe-windows

43c94ff

Turn error into warning for failing GH API fetch, print response

71563bc

fschutt enabled auto-merge November 18, 2022 16:02

Authenticate requests to /release page with GITHUB_TOKEN in CI

db299b8

bors bot added a commit that referenced this pull request Nov 18, 2022

Merge #3299

03c5e55

3299: Fix "create-exe" for windows-x86_64 target r=fschutt a=fschutt Co-authored-by: Felix Schütt <[email protected]> Co-authored-by: Felix Schütt <[email protected]>

fschutt merged commit 58d8415 into master Nov 18, 2022

fschutt deleted the fix-create-exe-windows branch November 18, 2022 17:20

Michael-F-Bryan reviewed Nov 18, 2022

View reviewed changes

fschutt added a commit that referenced this pull request Nov 18, 2022

Address review comments from #3299 (review)

3daec87

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix "create-exe" for windows-x86_64 target #3299

Fix "create-exe" for windows-x86_64 target #3299

fschutt commented Nov 11, 2022

fschutt commented Nov 15, 2022 •

edited

Loading

fschutt commented Nov 16, 2022

fschutt commented Nov 16, 2022

fschutt commented Nov 18, 2022

fschutt commented Nov 18, 2022

bors bot commented Nov 18, 2022

Michael-F-Bryan Nov 18, 2022

fschutt Nov 18, 2022

Michael-F-Bryan Nov 18, 2022

fschutt Nov 18, 2022

Michael-F-Bryan Nov 18, 2022

fschutt Nov 18, 2022

Michael-F-Bryan Nov 18, 2022

fschutt Nov 18, 2022

Fix "create-exe" for windows-x86_64 target #3299

Fix "create-exe" for windows-x86_64 target #3299

Conversation

fschutt commented Nov 11, 2022

fschutt commented Nov 15, 2022 • edited Loading

fschutt commented Nov 16, 2022

fschutt commented Nov 16, 2022

fschutt commented Nov 18, 2022

fschutt commented Nov 18, 2022

bors bot commented Nov 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fschutt commented Nov 15, 2022 •

edited

Loading