grpc-node icon indicating copy to clipboard operation
grpc-node copied to clipboard

Compiled protoc.exe depends on debug DLL

Open pdugas opened this issue 2 years ago • 11 comments

Problem description

On Windows, in an NPM project dependent on [email protected], the installed protoc.exe is dependent on the debug version of a DLL that isn't available in the normal redistributable package from Microsoft.

$ ls /c/Windows/System32/*.dll | grep vcruntime
/c/Windows/System32/vcruntime140.dll*
/c/Windows/System32/vcruntime140_1.dll*
/c/Windows/System32/vcruntime140d.dll*

$ ldd node_modules/grpc-tools/bin/protoc.exe
        ntdll.dll => /c/Windows/SYSTEM32/ntdll.dll (0x7ffa8f4d0000)
        KERNEL32.DLL => /c/Windows/System32/KERNEL32.DLL (0x7ffa8c9b0000)
        KERNELBASE.dll => /c/Windows/System32/KERNELBASE.dll (0x7ffa8b910000)
        VCRUNTIME140D.dll => /c/Windows/SYSTEM32/VCRUNTIME140D.dll (0x7ffa7ab00000)
        MSVCP140D.dll => /c/Windows/SYSTEM32/MSVCP140D.dll (0x7ffa6d770000)
        ucrtbased.dll => /c/Windows/SYSTEM32/ucrtbased.dll (0x7ffa65630000)
        ucrtbased.dll => /c/Windows/System32/ucrtbased.dll (0x261b7e80000)
        ucrtbased.dll => /c/Windows/System32/ucrtbased.dll (0x261b7e80000)
        VCRUNTIME140_1D.dll => not found

Running protoc.exe results in 3221225781 as the exit code.

Reproduction steps

npm install --save-dev grpc-tools in an NPM project on Windows then npx grpc_tools_node_protoc.

Environment

Additional context

Downloading a copy of VCRUNTIME140_1D.dll from another machine with the dev tools installed resolves the issue.

Can the binary be built to depend on the non-debug version of the DLL?

pdugas avatar Feb 14 '23 01:02 pdugas

Existence of this issue is probably the best OSX advertisement ever. Looks like nobody is using Windows for software development anymore :shrug:

abuinitski avatar Mar 20 '23 18:03 abuinitski

Will add that installing the VS Community edition with the C++ tools supplies the libs but OMG a surprising amount of other disk space is required. Had to relearn how to expand VM drives. Annoying.

pdugas avatar Mar 20 '23 22:03 pdugas

Will add that installing the VS Community edition with the C++ tools supplies the libs but OMG a surprising amount of other disk space is required. Had to relearn how to expand VM drives. Annoying.

This is still happening. Why, for the love of all, is this package depending on debug MSVC DLLs? Is there any rationale for it? For non-debug ones, you can easily use vcredist. For the debug ones, as pointed out, VS >= 2019 (Community or other) installation is required, at least of the CLI part. This makes no sense. Core protoc binaries don't have this problem.

@murgatroid99 do you know if there is any maintainer/dev here that could chime in on this?

FyiurAmron avatar Mar 13 '25 13:03 FyiurAmron

some additional research:

  • precompiled version used by grpc-tools <= v1.11.3 are OK, e.g. http://node-precompiled-binaries.grpc.io/grpc-tools/v1.11.3/win32-x64.tar.gz has protoc 3.15.6 which was compiled with MSVC 19.33.31630.0 (Visual Studio 2022 17.3.6 probably) and is fully compatible with everything, the only dep there is Win32 kernel :) - getting this and extracting manually over the new ones is IMVHO the easiest workaround ATM
  • the switch to debug VS deps started to happen from grpc-tools v1.12.0 - that's protoc 3.19.1.0 compiled with MSVC 19.34.31933.0 (Visual Studio 2022 17.4.0 probably) - however, the official protoc 3.19.1.0 from https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.19.1/protoc-3.19.1-windows-x86_64.exe doesn't exhibit this behaviour - its only dependency is likewise Win32 kernel (it or its newer versions can be used as a drop-in replacement for grpc-tools' protoc.exe, YMMV)

As such, I think the custom-built grpc-node protoc binaries (or rather, the build process used for creating them) are to blame. Probably they are built in debug mode, which previously (pre-1.12) just added the symbols, and now also switch the libs and linkage? 🤔

Source links:

https://github.com/grpc/grpc-node/blob/master/packages/grpc-tools/CMakeLists.txt#L35 https://github.com/grpc/grpc-node/blob/master/packages/grpc-tools/build_binaries.ps1#L69

those should be amended to publish release binaries instead of the debug ones for Windows.

FyiurAmron avatar Mar 13 '25 15:03 FyiurAmron

Honestly, I don't work on Windows myself, and the current grpc-tools Windows build script was largely contributed by others, so I don't really understand what caused the reported problem. If you have a specific fix in mind, please send a PR with the change.

murgatroid99 avatar Mar 13 '25 18:03 murgatroid99

@murgatroid99 IIUC it's actually your commit(s) that caused this, although indirectly (because it took years for M$ to change the build behaviour this far, and people refactoring the code later kept the intent that was before them): https://github.com/grpc/grpc-node/commit/35c257c019fa1826ed6d1e877ee8a12f23285ba6 & https://github.com/grpc/grpc-node/commit/9a629f9b76c40baffeed69c61ea2ee6fec8fa039 (14 Feb 2019) ... and that's why I'm asking you in particular: "can you recall why you changed the build from release to debug on M$ systems?" - because it's unlikely that any other human knows the answer at this point in time :D (and the commit message provides no further enlightenment here for me as well : ) ... and if there's any tribal knowledge that explains this change, I'm all ears :D

The PR AFAIK would be basically reverting that change, with slightly updated syntax probably for new MSVC, but still I'm not eager to revert things that were intentionally changed the way there are... because there might be actual reasons for the change? : )

FyiurAmron avatar Mar 13 '25 21:03 FyiurAmron

The --config Release argument was actually added in https://github.com/grpc/grpc-node/pull/732/commits/90233c965fa68c38d0d31ff8170d3301d8e48425, earlier in the same PR. So it's not that we were previously doing a release build in that way and then switched to the debug build, but rather that we tried to explicitly do the release build, and then concluded that it didn't work, and then undid that change.

I don't remember any reasoning for the changes we made in that PR. I think we were just trying whatever changes we could think of, until something worked as far as we could tell.

murgatroid99 avatar Mar 13 '25 21:03 murgatroid99

@murgatroid99 thanks for the context. If/when I have some spare time in my job, I'll try and see if I can fork and reproduce the build in release vs debug settings and spot the differences or problems there. I hope that even if there was a problem with release builds back in the days, a way exists nowadays to fix it (or maybe the problem/bug is even gone). Anyway, I'll get back when I find something more.

FyiurAmron avatar Mar 13 '25 22:03 FyiurAmron

is there any update on this issue?

questflow-kyle avatar Jul 19 '25 06:07 questflow-kyle

@MipaChan unfortunately, first I got assigned other projects to work on in my job, now I'm on vacation, and then probably I'm going on an extended leave. Nothing new from me, highly unlikely I'll have enough time to go back to this (taking into account we still have a semi-usable workaround at least).

FyiurAmron avatar Jul 19 '25 10:07 FyiurAmron

@FyiurAmron Thanks for the update! I'll take a look at this issue and see if I can contribute or help brainstorm some solutions. Enjoy your vacation!

questflow-kyle avatar Jul 20 '25 15:07 questflow-kyle