boostorg / boost_install

8 stars 31 forks source link

Improve CMake Windows support with DLLs #56

Closed 3Descape closed 2 years ago

3Descape commented 2 years ago

This commit sets the IMPORTED_LOCATION property of the imported target to the .dll. This allows developers to consume the runtime dependend .dlls via the $ generator expression on windows.

An open question is, wheter or not we should set the IMPORTED_LOCATION for all platforms, not only windows, as this doesn't seem to have negative side-effects. I am no CMake nor b2/jam expert though, so I'd be happy about some feedback.

Signed-off-by: Michael Lackner mlackner@student.tugraz.at

pdimov commented 2 years ago

I suspect that this isn't going to work correctly for MinGW (where import libraries are called foo.dll.a, or were they libfoo.dll.a) or Cygwin, which had its own convention, but I'm not quite sure what's the right b2 way to get the actual DLL name.

grisumbras commented 2 years ago

These can be used to get proper suffixes and prefixes: https://github.com/boostorg/build/blob/develop/src/build/type.jam#L275 https://github.com/boostorg/build/blob/develop/src/build/type.jam#L305

The target types are defined here: https://github.com/boostorg/build/blob/develop/src/tools/types/lib.jam

Something like this will probably work

local prefix = [ type.generated-target-prefix IMPORT_LIB : $(ps) ] ;
local suffix = [ type.generated-target-suffix IMPORT_LIB : $(ps) ] ;
local dllname = "$(prefix)$(fname:BS=$(suffix))" ;
3Descape commented 2 years ago

Thank you very much for the replies! @grisumbras These were some great ressources!

I did try out your solution and adapted it(currently for MSCV only) to a functional equivalent solution to my previous commit. I used

local dllname = [ sequence.join $(prefix) $(fname:BS=) "." $(suffix) ] ;

, as $(prefix) and $(suffix) can be undefined and it seems like the variable exansion then causes the dllname to be empty as well.

Furthermore we need the SHARED_LIB type to get the .dll suffix and prefix.

I will try to improve the implementation to the best of my abilities within the next few day to make it work for multiple toolchains/platforms.

Thanks for the support ;)

pdimov commented 2 years ago

I don't think this is going to work correctly either, because fname already has a prefix and a suffix, appropriate for its type, and those need to be taken into account.

pdimov commented 2 years ago

Implemented in https://github.com/boostorg/boost_install/commit/f23b121b3da409ff6b2b959ee4ce716812b29ad9. Let me know if something's not right.

3Descape commented 2 years ago

Looking good :) Thank you very much!

Have a nice weekend and a happy new year.