Stream: git-wasmtime

Topic: wasmtime / issue #4003 Upgrade to the high-level `ittapi`...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2022 at 13:50):

bnjbvr commented on issue #4003:

Ah, seems compilation of the ittapi crate on windows is broken. Any ideas @abrown @jlb6740?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 07 2022 at 17:58):

abrown commented on issue #4003:

I looked into this but don't know yet how to resolve it: the compilation failure is due to strnlen_s not being available in MinGW by default (it is, after all, a non-standard function). There are some thoughts out there that adding --enable-secure-api will include the headers with this function, but before I add that to ittapi-sys's build.rs, I wanted to be able to reproduce the issue.

Locally I was unable to reproduce: after installing MSYS2 and compiling the ittapi crates with cargo build --target x86_64-pc-windows-gnu, everything seemed to build as normal. When I run cargo test --target x86_64-pc-windows-gnu, I get a different error: linker x86)64-w64-mingw32-gcc not found.

I then attempted to reproduce on ittapi's CI: using a similar setup to the Wasmtime workflow, I added x86_64-pc-windows-gnu as a target in the matrix. With the windows-latest OS, however, the crates built and tests ran. With the windows-2019 OS, then I observe a completely different failure related to libclang.dll being inaccessible.

Any suggestions?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 17:02):

abrown commented on issue #4003:

@bnjbvr: since we can't really replicate this, we could just disable the ittapi dependency for MinGW builds?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 17:09):

bnjbvr commented on issue #4003:

@abrown Need to check but I think that we do use this target to cross-compile internally. If so at least I might find people who could poke at this, maybe...

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 17:15):

bnjbvr commented on issue #4003:

Seems we don't use it, so yeah we could disable ittapi on mingw then, and if someone wants to support that, they would need to contribute a patch. Does it sound acceptable?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 17:28):

abrown commented on issue #4003:

Does it sound acceptable?

It sounds acceptable to me, though others feel free to comment. I'm not sure how much energy to put into MinGW since I don't use it much (ever!) and I wonder sometimes if it has been obsoleted by WSL.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 17:56):

alexcrichton commented on issue #4003:

Personally I find managing platform-specific exceptions and such to APIs not easy to maintain over time. To that end I would prefer to see this solved. It looks to me like MinGW doesn't provide the strnlen_s function which is the source of this error. The only reason we're seeing this now is probably because something in the ittapi crate is calling a function we didn't call before in the C library and the C function requires strnlen_s or something like that.

Locally I was unable to reproduce: after installing MSYS2 and compiling the ittapi crates with cargo build --target x86_64-pc-windows-gnu, everything seemed to build as normal

That's because when you build an rlib it doesn't actually link anything, so this doesn't verify that symbols are actually present on the system.

When I run cargo test --target x86_64-pc-windows-gnu, I get a different error: linker x86)64-w64-mingw32-gcc not found.

If the ) there isn't a typo then that's a misconfigured compiler somewhere. Otherwise this may mean you're not running the command in an MSYS2 shell or something like that. Basically if the linker isn't being invoked you won't be able to reproduce this issue.

With the windows-latest OS, however, the crates built and tests ran

I don't think that the CI is configured correctly which is why this seems to work. In the logs I see:

     Running tests\bindgen-up-to-date.rs (target\debug\deps\bindgen_up_to_date-ae57c289f5ef2e2d.exe)

but if you're cross compiling the path to the binary should be target\x86_64-pc-windows-gnu\debug\deps\.... You added this to the CI configuration:

    - run: echo CARGO_BUILD_TARGET=${{ matrix.target }} >> $GITHUB_ENV
      if: matrix.target != ''

but I don't think that works because the shell you're using is windows cmd or powershell (I dunno which) and probably doesn't use $foo for environment variables. This I believe means you're not actually configuring Cargo for cross-compilation.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 13 2022 at 18:27):

bnjbvr commented on issue #4003:

Very good call, @alexcrichton! Managed to reproduce the compilation error on ittapi now: https://github.com/intel/ittapi/pull/61

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2022 at 17:19):

abrown commented on issue #4003:

This should be merge-able once GitHub's virtual environment for Windows is updated with the latest version of MinGW: https://github.com/actions/virtual-environments/pull/5729.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2022 at 23:03):

abrown commented on issue #4003:

I believe that the updated version of MinGW (which includes the missing strnlen_s definition) has been rolled out to the machines that run the windows-latest CI jobs. Over in https://github.com/intel/ittapi/pull/63 I was able to build ittapi without issues on windows-latest. I'm re-running CI here to check if we can actually merge this now.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 24 2022 at 14:23):

alexcrichton commented on issue #4003:

I believe the issue here is that we're pinned to windows-2019 (see https://github.com/bytecodealliance/wasmtime/pull/3864 for info on that) so we're not getting updates. I'd reiterate though that there's no requirement to get this working on all platforms. Disabling this on mingw is fine.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 18 2022 at 14:32):

bnjbvr commented on issue #4003:

Just added a commit that disables this on windows + mingw, and it seems to compile fine. Unfortunately there's an unrelated MacOS build error (Github says the CI run has been cancelled), and I can't request a retry, so could anyone with the right authorization bits please do that?


Last updated: Oct 23 2024 at 20:03 UTC