Make libpath an array and add more win32 paths

[mihails-strasuns]

Fixes #47

Based on the bug report, on Windows more libpaths need to be added to make libcurl.dll findable. This commit attempts to do so by turning libpath field into an array and adding some more windows paths.

Enhances test program to use std.net.curl to test this.

[mihails-strasuns]

Hm, looks like it does not work on Windows - dll is definitely in that folder but still can't be found. Currently reading how LD_LIBRARY_PATH is supposed to work on Windows :)

[mihails-strasuns]

Wait, it only fails for dub - is there anything special about it env-wise?

[mihails-strasuns]

@Geod24 @WebFreak001 can you help with the dub part? I don't have a good memory of D toolchain these days :)

[WebFreak001]

uhh I don't really have experience with curl on dub on windows and I think if I ever used it, I copied the dlls to the folder to use them

[mihails-strasuns]

@CyberShadow maybe you know? :)

[CyberShadow]

Windows does not support LD_LIBRARY_PATH, because it does not use ld for loading shared libraries.

I think the equivalent is just PATH. https://stackoverflow.com/questions/518228/is-it-possible-to-add-a-directory-to-dll-search-path-from-a-batch-file-or-cmd-sc

[mihails-strasuns]

@CyberShadow yep, figured out that part - you can see in CI that it works when you compile/run sample directly with dmd, i.e. dmd -run sample.d. But the same sample doesn't work through dub run in the same environment and I am confused why :/

[CyberShadow]

That is interesting. What I would do is to make the sample program dump its environment to stdout, and then compare the output from those two invocation methods.

[mihails-strasuns]

Okay, looks like dub automatically adds -m64 flag by the default and the issue is that 32-bit and 64-bit Windows needs to be handled as independent envs. Trying to hack something for that now..

[CyberShadow]

I see, so would that be a new parameter for the action (which architecture to use)?

[mihails-strasuns]

I hope to avoid way but it is not yet currently clear if there is any available option. From quick local testing it seems that the sample app crashes when both bin and bin64 are on path (access violation) and the first one attempted does not match the choice between -m32/-m64. I wouldn't be surprised if there is some idiomatic solution for this in Windows but I am simply not familiar enough with it to possibly guess it :)

Don't have time to debug it more right now, will get back to it later next week.

[WebFreak001]

that seems weird - is this procedure needed if you install dmd on windows normally?

The sc.ini in the bin folder also contains lib paths for Environment32, Environment64 and Environment32mscoff, which should already include everything needed, created by the installer?

[CyberShadow]

I think the lib paths only affect linking to static/import libraries. std.net.curl is loading libcurl.dll dynamically at runtime, without an import library, so it should be completely unaffected by the linker configuration.

[WebFreak001]

in that case isn't the issue that your application build step didn't copy the libcurl.dll to your application folder? I think it would induce a wrong feeling of "it just working" even though you need special configuration to run it yourself (i.e. when making deployments of the binaries)

[mihails-strasuns]

Correct. I presume if you are using curl on Windows you have to copy the dll to the application directory for distribution?

AFAICS the DMD windows installer provides separate environment bat scripts for 32-bit and 64-bit: https://github.com/dlang/installer/blob/master/windows/d2-installer.nsi#L261-L278

[CyberShadow]

Well, this doesn't seem really relevant to the problem, but either way, that depends on who your users are. If you are shipping a desktop application for non-technical users, generally you want to include all dependencies. Often enough gratis / developer-targeted software does come with instructions to first make sure that this or that C runtime / DirectX library is installed. One notable example I remember is (older versions of) mIRC, which asked you to install OpenSSL in some place where mIRC will be able to find the DLLs: https://www.mirc.com/ssl.html

In my case, this is for testing ae. Since it is a library, I can't predict or dictate how the cURL DLL will be distributed, so I rely on the mechanism used by std.net.curl to work, and I do think it should work if the DLL is added to PATH. I was hoping that the action's maintainers would have a better understanding of the problems at hand. Maybe Windows gives up upon seeing that the first DLL it finds is of the wrong bitness, and therefore 64-bit programs need the path to the 64-bit DLL to come first in PATH, and same for 32-bit DLLs?

[mihails-strasuns]

That would be optimal, the problem is that at the action setup time we can't possibly know if user will use -m32 or -m64 (or both), thus can't know the right order to use. Adding an extra action argument to explicitly select bitness for Windows would be the easiest of course.

[CyberShadow]

What I would do is to make the sample program dump its environment to stdout, and then compare the output from those two invocation methods.

Has this produced any illuminating information?

Edit: oh, it was because the process bitness was different.

[mihails-strasuns]

Yeah, the same issue can be reproduced by calling dmd -m32 and dmd -m64 directly.

[mihails-strasuns]

I'll give a try to an action argument approach. In most cases it should not be needed to specify (i.e. if no runtime loading is involved), so that will only be a small extra burden for devs using curl.

[CyberShadow]

Sounds great. Ideally it would also control the dmd -m and Dub --arch settings (maybe by editing sc.ini), but if that's not possible, maybe the setting should have value names that match one of the existing settings' value names to control the architecture in Dub or DMD.

[mihails-strasuns]

It is possible but I am not sure if this is a good idea. For the most CI scenarios the user can already freely use and mix different -m and --arch configuration and compiler/linker config will take care of correct paths. I don't really want to complicate that, was thinking about introducing a very dedicated (like win-dll-bitness) flag just to handle just the curl case. As far as I can see libcurl.dll is literally the only such library in dmd distribution so the use case is very niche.

[Geod24]

To me, it sounds like a failure of what is trying to load the library. IIUC, the loader finds a DLL matching the name, loads it, then error out before it's the wrong bitness. In such a case, it should keep on searching other directories.

But looking at the code, it relies on LoadLibraryA only and there's not much Phobos can do: https://github.com/dlang/phobos/blob/9ec24190b1f657d594b5b675aff090dcd6c93782/std/net/curl.d#L4227-L4239

But, if my assumption is correct and LoadLibraryA bails as soon as it fails to load a library with the provided name, it looks to me that a possible simple solution would be to copy the two libraries in a folder, under two different name, and add that to the path. IOW:

• bin64/libcurl.dll => somepath/libcurl.dll;
• bin32/libcurl.dll => somepath/curl.dll;

EDIT: You mentioned access violation in this comment. In this case, Phobos is probably the one that needs to be fixed.

[mihails-strasuns]

Thanks all for the input. I'll try few approaches and get back with an update :)

[mihails-strasuns]

Okay, an update. So, the first and most important bit - the initial approach works. LoadLibraryA is perfectly fine with wrong bitness libraries on path and is expected to ignore them.

The problem that confused me earlier was that Phobos crashes afterwards for some reason for older DMD releases - and CI was using specifically 2.089 for a fixed version test. After I have noticed it passes for dmd-latest a simple update to 2.097 has resulted in fully green CI.

So the current fix is conceptually a correct one but it may need a workaround for older dmd releases. Unfortunately, I can't reproduce the crash even with 2.089 on my local Windows machine, so still no clue about what causes it. A further debugging path would be to build that Phobos tag with debug symbols and get the exact backtrace, however, I am close to the limit of time I can spend on this topic right now. So I'd propose to merge the fix as it is right now and create a separate issue to investigate older issues later - whoever will first get it. Is that suitable?

[CyberShadow]

Sounds great to me! The crash only happens when loading libcurl.dll (i.e. this change won't affect programs that don't use std.net.curl), right?

[mihails-strasuns]

Correct.

[mihails-strasuns]

https://github.com/dlang-community/setup-dlang/releases/tag/v1.1.1