Hi Carel,

On 11/17/25 12:33, Carel Combrink wrote:
Hi Mario,

    tests on my local branch successfully. However, the PR is not sufficient
    for a shared library build, at least not on MSVC, because the library
    does not export any symbols. Maybe I'm going into old news here?


This is curious since the the CI tests do not show this issue nor did the 
previous iterations of the conan package.
If I understand correctly if none were exported then at least the conan 
packages should fail.
The tests here are also expected to fail but I can think of many reasons why it 
might work.
I will see if I can't find anything.
PS: Conan has a very basic test package that I am lead to believe does work with previous versions: https://github.com/conan-io/conan-center-index/blob/master/ recipes/thrift/all/test_package/test_package.cpp <https://github.com/conan-io/conan-center-index/blob/master/recipes/thrift/all/test_package/test_package.cpp>

Ok, I finally got around to checking this, and actually, the cmake build does
set `CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS` in 
`build/cmake/DefinePlatformSpecifc.cmake`.
This is then sufficient - but it will be cmake-only. There is a complex logic
behind this flag, that is not easily reproduced with autotools.


    In cmake, this could be combined with WINDOWS_EXPORT_ALL_SYMBOLS (see
    https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html 
<https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html>).

I would probably vote for `WINDOWS_EXPORT_ALL_SYMBOLS ` since it does not seem like Windows has a very large user base and to touch all the files for a smaller user base does not look productive, and it then has at least the same behaviour on Linux and Windows.

    To continue automake support, one could also combine it with "real" export
    macros like for example https://wiki.tcl-lang.org/page/DLLEXPORT+and+DLLIMPORT 
<https://wiki.tcl-lang.org/page/DLLEXPORT+and+DLLIMPORT>

cmake has the ability to generate the import/export macros but thrift is not 
built only by cmake which makes it less ideal.
See https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html 
<https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html>

Yes, the import/export macros would be the cleaner way. What cmake generates
is actually easily reproduced in autotools, it just boils down to a new
header, and a few defines. And thrift contains such a header already in
thrift/thrift_export.h, however it is currently not (or sparsely) used.


    Did you consider this?

No, as mentioned above

Thank you for taking the time to look at this,
Regards,
Carel



Viele Grüße,

    Mario Emmenlauer

Reply via email to