mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
572 stars 177 forks source link

meson: pass -DCMS_DLL_BUILD=1 on Windows, not only msvc #292

Closed vtorri closed 2 years ago

vtorri commented 2 years ago

there are 2 remarks :

so actually, windows detection should be done, then visibility, not the contrary.

so something like (also line 66 to 69 are managed)

if host_machine.system() == 'windows'
  win = import('windows')
  lcms2_srcs += win.compile_resources('Projects/VC2019/lcms2.rc')
  cargs += '-DCMS_DLL_BUILD=1'
elif cc.has_function_attribute('visibility:hidden')
  cargs +='-DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1'
endif
mm2 commented 2 years ago

Both are (should be) independent. visibility:hidden means only API functions are accessible, DLL is a windows .so with special export calling convention. If you are compiling in windows with CMS_DLL_BUILD, DHAVE_FUNC_ATTRIBUTE_VISIBILITY is ignored. So, I am using two different detection steps and let lcms2.h decide which one to pick.

if cc.has_function_attribute('visibility:hidden')
  cargs +='-DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1'
endif

if host_machine.system() == 'windows'
  win = import('windows')
  lcms2_srcs += win.compile_resources('Projects/VC2019/lcms2.rc')
  cargs += '-DCMS_DLL_BUILD=1'
endif

But still, you may want to use a static library on windows... well, I left this for next release.

vtorri commented 2 years ago

btw is HAVE_FUNC_ATTRIBUTE_VISIBILITY really used in code ?

vtorri commented 2 years ago

I mean that you should not use a "configure time" macro in public header for unix visibility. Instead you should use something like

#if defined __GNUC__ && __GNUC__ >= 4
mm2 commented 2 years ago

Actually this is only used to build .so, where the autotools sets the symbol and calls the compiler with -fvisibility=hidden or whatever the different compiler would need. Clients does not need to set anything.

mm2 commented 2 years ago

No further activity, so I close the issue