tsdl icon indicating copy to clipboard operation
tsdl copied to clipboard

Use libNAME.lib and dllNAME.dl in .cma/cmxa for MSVC

Open jonahbeckford opened this issue 1 year ago • 10 comments

This is a fix for https://github.com/dbuenzli/tsdl/issues/96

Before (1.0.0)

$ del -force -Recurse _build ; Y:\source\dksdk-coder\msys64\usr\bin\bash -lc 'PATH="/y/source/dksdk-coder/.ci/sd4/opamrun:$PATH"; opamrun exec -- ocaml pkg/pkg.ml build'

...
 ocamlfind ocamlmklib -o src/tsdl -g -LY:/source/dksdk-coder/.ci/o/dkml/bin/../lib src/tsdl_stubs.obj
...

$ with-dkml ldd .\_build\src\dlltsdl.dll
Cannot find FLEXDLL_RELOCATE
        ntdll.dll => /c/WINDOWS/SYSTEM32/ntdll.dll (0x7fff60b10000)
        KERNEL32.DLL => /c/WINDOWS/System32/KERNEL32.DLL (0x7fff5f150000)
        KERNELBASE.dll => /c/WINDOWS/System32/KERNELBASE.dll (0x7fff5e1f0000)
        msvcrt.dll => /c/WINDOWS/System32/msvcrt.dll (0x7fff5f810000)
        dlltsdl.dll => /y/source/tsdl/_build/src/dlltsdl.dll (0x60bf0000)
        ucrtbase.dll => /c/Windows/System32/ucrtbase.dll (0x7fff5de50000)
        vcruntime140.dll => /c/Windows/System32/vcruntime140.dll (0x7fff4d050000)

And see https://github.com/dbuenzli/tsdl/issues/96 for ocamlobjinfo of tsdl.cma

After (1.0.0 + PR)

Notice SDL2.dll is listed as a runtime dependency.

$ del -force -Recurse _build ; Y:\source\dksdk-coder\msys64\usr\bin\bash -lc 'PATH="/y/source/dksdk-coder/.ci/sd4/opamrun:$PATH"; opamrun exec -- ocaml pkg/pkg.ml build'

...
 ocamlfind ocamlmklib -o src/tsdl -g -LY:/source/dksdk-coder/.ci/o/dkml/bin/../lib Y:/source/dksdk-coder/.ci/o/dkml/bin/../lib/SDL2.lib src/tsdl_stubs.obj
...

$ with-dkml ldd .\_build\src\dlltsdl.dll
        ntdll.dll => /c/WINDOWS/SYSTEM32/ntdll.dll (0x7fff60b10000)
        KERNEL32.DLL => /c/WINDOWS/System32/KERNEL32.DLL (0x7fff5f150000)
        KERNELBASE.dll => /c/WINDOWS/System32/KERNELBASE.dll (0x7fff5e1f0000)
        msvcrt.dll => /c/WINDOWS/System32/msvcrt.dll (0x7fff5f810000)
        dlltsdl.dll => /y/source/tsdl/_build/src/dlltsdl.dll (0x60bf0000)
        ucrtbase.dll => /c/Windows/System32/ucrtbase.dll (0x7fff5de50000)
        vcruntime140.dll => /c/Windows/System32/vcruntime140.dll (0x7fff4d050000)
        SDL2.dll => not found
        api-ms-win-crt-environment-l1-1-0.dll => /c/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/api-ms-win-crt-environment-l1-1-0.dll (?)
        api-ms-win-crt-stdio-l1-1-0.dll => /c/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/api-ms-win-crt-stdio-l1-1-0.dll (?)
        api-ms-win-crt-runtime-l1-1-0.dll => /c/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/api-ms-win-crt-runtime-l1-1-0.dll (?)

Here is ocamlobjinfo for tsdl.cma:

File y:\source\dksdk-coder\.ci\o\dkml\lib\tsdl\tsdl.cma
Force custom: YES
Extra C object files: SDL2.lib
Extra C options: -LY:/source/dksdk-coder/.ci/o/dkml/bin/../lib
Extra dynamically-loaded libraries: dlltsdl.dll

jonahbeckford avatar Feb 16 '24 09:02 jonahbeckford

Other Testing

Tested with bogue package. With a hack to an unrelated problem, SDL graphics did show up.

jonahbeckford avatar Feb 16 '24 09:02 jonahbeckford

There seem to be quite a bit of churn here. Are you sure you are solving the problem at the right level ? I guess stub_l may be doing a platform assumption. But I'm not sure I see why pkg-config doesn't isolate us from the rest.

dbuenzli avatar Feb 16 '24 10:02 dbuenzli

"Churn" means going back and forth and back and forth. You may be using that word differently (please clarify). But I'll clarify that this PR was already part of my original set of changes (see https://github.com/jonahbeckford/tsdl/commits/v0.9.8-fix-msvc%2Bforeign%2Bnative/) from a year ago. I just ran out of time after its sister change in Dune was closed without resolution.

So ... I've re-opened the Dune sister change at https://github.com/ocaml/dune/issues/10042 ... and you might want to make sure your overarching concerns are addressed there before resolving this PR.

jonahbeckford avatar Feb 16 '24 19:02 jonahbeckford

But I'm not sure I see why pkg-config doesn't isolate us from the rest.

pkg-config is fundamentally broken on Windows and unfixable. Who the heck designs a configuration system that uses spaces as separators but does not include an escape character? To compensate, I install the pkgconf alternative to pkg-config, do a zillion hacks to convert paths with spaces to DOS 8.3 space-less paths (which is not always possible), and hand-craft .pc files for the (many) packages that have stopped using that ancient method, and often do fixups on .pc files for the packages that do.

So please: no to pkg-config as the only way to interact with tsdl. I am quite fine with providing build and linking configuration to tsdl in an alternative fashion ... but the minimum bar needs to be to use a method that can at least allow for spaces. Do you have any suggestions?

jonahbeckford avatar Feb 16 '24 19:02 jonahbeckford

"Churn" means going back and forth and back and forth. You may be using that word differently (please clarify)

My dictionary has that:

3 [no object] have an unpleasant disturbed feeling: her stomach was churning at the thought of the ordeal | you never feel rested because your mind is always churning.

pkg-config is fundamentally broken on Windows and unfixable. […] So please: no to pkg-config as the only way to interact with tsdl.

I don't have the expertise to say whether this is accurate or not but in any case I'm quite unsympathetic to this kind of thinking "this is broken so let's add stuff in every other build system or package out there" rather than fix the root cause.

Whether you like it or not .pc files is the only "standard" and cross-platform way we have to interact with C libraries. I'd rather have something that simply works with that.

Do you have any suggestions?

At worst sdl2-config but I'd rather not.

dbuenzli avatar Feb 16 '24 21:02 dbuenzli

I'm quite unsympathetic to this kind of thinking "this is broken so let's add stuff in every other build system or package out there" rather than fix the root cause.

You think that the build system in tsdl that

  1. Emits an incompatible GCC/clang linking option -ltsdl for a MSVC linker:

    Extra dynamically-loaded libraries: -ltsdl
    

    and then

  2. Ignores the SDL2.lib entry in the provided MSVC-compatible sdl2.pc

is not broken and should unsympathetically not be modified? Huh? This PR is precisely in line with your words about pkg-config: "I'd rather have something that simply works with that.".

Whether you like it or not .pc files is the only "standard" and cross-platform way we have to interact with C libraries.

The majority of C developers and C libraries use CMake, and the canonical way to interact with C libraries for the past decade has been CMake export targets. https://www.jetbrains.com/lp/devecosystem-2022/cpp/#Which-project-models-or-build-systems-do-you-regularly-use and other data say the same thing. And there is an active effort by a couple people on the C standards committees to use https://cps-org.github.io/cps/history.html rather than CMake export targets; I may help out with that.

I'd encourage you to skim the first link for 15 seconds, and the second link for two minutes. Then we can properly talk about root causes, and a way to fix it cleanly and permanently.

jonahbeckford avatar Feb 17 '24 00:02 jonahbeckford

is not broken and should unsympathetically not be modified? Huh?

Not sure where you read that. Please read what I wrote here, especially the bit about stub_l.

This PR is precisely in line with your words about pkg-config: "I'd rather have something that simply works with that.".

If you need to add 60 lines to the build system here to achieve that which includes munging its output in platform dependent way and testing for the existence of files I suggest this is not the case.

I know Windows always likes to be special and work in incredibly idiosyncratic ways but I'm not sure why pkg-config on windows can't simply emit the right flags on --cflags, --libs-only-L and --libs-only-l that can be readily used with the platform compiler.

The majority of C developers and C libraries use CMake, and the canonical way to interact with C libraries for the past decade has been CMake export target […]

I don't care about what build system other people use or what new cool technology might come near me in ten years and whether you are involved with it or not. What I see in system install bases now is .pc files. If something better emerges in the future I'm happy to switch to it but that's not a reality for now.

dbuenzli avatar Feb 17 '24 08:02 dbuenzli

Finally, there is something actionable ("add 60 lines to the build system here"). I'll have a look at the line count over the next few days.

jonahbeckford avatar Feb 17 '24 20:02 jonahbeckford

Line count is down to 15 with absolute paths in .pc.

jonahbeckford avatar Feb 27 '24 06:02 jonahbeckford

Could you maybe show me the actual *.pc file one gets on Windows in a SDL install ? (me) Line count is down to 15 with absolute paths in .pc.

I already linked to sdl2.pc, although it certainly is not what "one gets on Windows" (more on this later). I made the absolute path change below so your pkgconfig parsing code did not need to be complex, although that should not have been necessary because the earlier .pc was perfectly valid and was relocatable:

prefix=${pcfiledir}/../..
# sdl pkg-config source file

exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: sdl2
Description: Simple DirectMedia Layer is a cross-platform multimedia library designed to provide low level access to audio, keyboard, mouse, joystick, 3D hardware via OpenGL, and 2D video framebuffer.
Version: 2.30.0
Requires.private: 
Conflicts:
#Libs: "-L${libdir}" SDL2.lib
Libs: Y:/source/dksdk-coder/.ci/o/dkml/bin/../lib/SDL2.lib
Libs.private:    kernel32.lib user32.lib gdi32.lib winmm.lib imm32.lib ole32.lib oleaut32.lib version.lib uuid.lib advapi32.lib setupapi.lib shell32.lib dinput8.lib
Cflags: "-I${includedir}" "-I${includedir}/SDL2" 

The .pc comes from slightly modified vcpkg which exports some lib/pkgconfig/*.pc for legacy (Unix) systems and always lib/cmake/*.cmake export sets for its own use. sdl luckily exported a .pc. The tsdl dependency libffi (used by ctypes) did not export a .pc so I manually wrote that by hand. In general, vcpkg package maintainers do not export .pc files on Windows, for reasons I have already provided evidence for.

jonahbeckford avatar Feb 27 '24 16:02 jonahbeckford

Closing this for now, see https://github.com/dbuenzli/tsdl/issues/96#issuecomment-2343653672

dbuenzli avatar Sep 11 '24 13:09 dbuenzli