OSGeo / libgeotiff

Official repository of the libgeotiff project
183 stars 70 forks source link

TIFF_LIBRARIES are private #76

Closed dg0yt closed 2 years ago

dg0yt commented 2 years ago

The public linkage of TIFF_LIBRARIES exposes TIFF to downstream compilation stages, even when linking to the shared libgeotiff. This isn't needed, and the exposure of TIFF::TIFF in dynamic builds is not handled in the exported CMake config.

dg0yt commented 2 years ago

FTR this bug was discovered in dynamic builds of PDAL 2.4 with GDAL 3.5.0rc2 CMake builds.

hobu commented 2 years ago

The linkages for the applications will need to be adjusted in support of this change too.

rouault commented 2 years ago

The linkages for the applications will need to be adjusted in support of this change too.

what do you mean exactly ?

rouault commented 2 years ago

hum actually I'm wondering if this change is correct. libtiff is a public dependency of libgeotiff because of the xtiffio.h header

hobu commented 2 years ago
    Creating library D:/a/libgeotiff/libgeotiff/libgeotiff/build/lib/Release/makegeo.lib and object D:/a/libgeotiff/libgeotiff/libgeotiff/build/lib/Release/makegeo.exp
makegeo.obj : error LNK2019: unresolved external symbol TIFFClose referenced in function main [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2001: unresolved external symbol TIFFClose [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
makegeo.obj : error LNK2019: unresolved external symbol TIFFSetField referenced in function SetUpTIFFDirectory [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
makegeo.obj : error LNK2019: unresolved external symbol TIFFWriteScanline referenced in function WriteImage [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
makegeo.obj : error LNK2019: unresolved external symbol TIFFError referenced in function WriteImage [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2019: unresolved external symbol TIFFOpen referenced in function XTIFFOpen [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2019: unresolved external symbol TIFFFdOpen referenced in function XTIFFFdOpen [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2019: unresolved external symbol TIFFClientOpen referenced in function XTIFFClientOpen [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2019: unresolved external symbol TIFFSetTagExtender referenced in function XTIFFClientOpen [D:\a\libgeotiff\libgeotiff\libgeotiff\build\bin\makegeo.vcxproj]
xtiff.lib(xtiff.obj) : error LNK2019: unresolved external symbol TIFFMergeFieldInfo referenced in function 

libtiff is a public dependency of libgeotiff because of the xtiffio.h

It would be nice to vendor that into libgeotiff. PDAL's kind of special though as we're one of the few direct users of external libgeotiff AND GDAL, but this has been annoyance.

dg0yt commented 2 years ago

Well, of course it is possible to stay with PUBLIC, but then it takes another change to the exported CMake config.

dg0yt commented 2 years ago

PDAL's kind of special though as we're one of the few direct users of external libgeotiff AND GDAL, but this has been annoyance.

It is not a PDAL issue but a libgeotiff issue:

Given the header dependency, it must be PUBLIC instead.

abellgithub commented 2 years ago

@hobu asked for my comment.

It's not clear to me what the actual complaint is -- whether this is about symbol visibility from a DLL or simply about link-time requirements for downstream libraries.

If you really don't want the tiff symbols visible from the GDAL DSO, you need to change their visibility when building that shared library. Changing the PUBLIC/PRIVATE interface in cmake won't fix this. Like Windows, where you must explicitly choose the functions you want exported from a DLL interface, the same can be done on Linux by using __attribute__((visibility("default")) or __declspec(dllexport) along with setting default visibility to hidden when compiling. See here

Since PDAL was mentioned, I'll note that I'm almost finished with limiting symbol visibility from PDAL with this technique and I would think it would be in the next release.

dg0yt commented 2 years ago

Hm, another project asking for vcpkg updates?

dg0yt commented 2 years ago

We seem to post in parallel. But there is no hurry. I can prepare another, tested PR later tonight (CEST).

rouault commented 2 years ago

superseded per #77