b-fuze / deno-dom

Browser DOM & HTML parser in Deno
https://jsr.io/@b-fuze/deno-dom
MIT License
425 stars 47 forks source link

consider building windows plugin.dll without dependency on vcruntime140.dll #124

Closed jjallaire closed 2 years ago

jjallaire commented 2 years ago

We are using deno_dom in software we distribute on Windows and just had a user report that they couldn't load within Windows Sandbox due to a missing vcruntime140.dll. I traced that dependency through all of our binaries and found it here in deno_dom:

$ ldd deno_dom/plugin.dll
        ntdll.dll => /c/WINDOWS/SYSTEM32/ntdll.dll (0x7ffced6d0000)
        KERNEL32.DLL => /c/WINDOWS/System32/KERNEL32.DLL (0x7ffcecda0000)
        KERNELBASE.dll => /c/WINDOWS/System32/KERNELBASE.dll (0x7ffceb010000)
        ADVAPI32.dll => /c/WINDOWS/System32/ADVAPI32.dll (0x7ffcec990000)
        msvcrt.dll => /c/WINDOWS/System32/msvcrt.dll (0x7ffced210000)
        sechost.dll => /c/WINDOWS/System32/sechost.dll (0x7ffcec130000)
        RPCRT4.dll => /c/WINDOWS/System32/RPCRT4.dll (0x7ffcec1d0000)
        ucrtbase.dll => /c/WINDOWS/System32/ucrtbase.dll (0x7ffceb4a0000)
        VCRUNTIME140.dll => /c/WINDOWS/SYSTEM32/VCRUNTIME140.dll (0x7ffcd3db0000)
        CRYPTBASE.DLL => /c/WINDOWS/SYSTEM32/CRYPTBASE.DLL (0x7ffcea750000)

I believe that the Rust linker is adding this dependency as a matter of course when using the VC toolchain. It looks like it is possible to build with a static dependency though: https://crates.io/crates/static_vcruntime

It would great to build w/o this dependency as I'm sure this may snag others.

b-fuze commented 2 years ago

Yeah, I'm not super familiar with Rust on Windows, but I can look into it. If you want to contribute PRs as well I'm open to that

jjallaire commented 2 years ago

This is all we had to do to enable this in our project: https://github.com/quarto-dev/quarto-cli/commit/a8dfb3c694b1e608605e7154368994b85ab76dfc

Note that this will no-op on other platforms.

b-fuze commented 2 years ago

Okay, I'll try that 🤔

b-fuze commented 2 years ago

@jjallaire I added your proposed changes to the latest binary builds. I assume this problem should be solved now, but I'm not sure how to test, so you can reopen if it still poses issues

jjallaire commented 2 years ago

Yes, that does indeed do the trick -- thanks!!!