search

hobuinc / laz-perf

Alternative LAZ implementation for C++ and JavaScript
Apache License 2.0
74 stars 44 forks source link

Improvement of the export/import statement and the distinction between shared and static for MSVC. #112

Closed OgreTransporter closed 10 months ago

abellgithub commented 2 years ago

It's unclear what problem this is solving. Please open an issue with a good description.

OgreTransporter commented 2 years ago

It is actually not a problem, but an improvement. The control of import/export of static and shared library. Comparable with https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zconf.h.cmakein#L337

abellgithub commented 2 years ago

That's not really helpful. What does it improve, exactly?

On Sun, Jan 2, 2022, 11:46 AM OgreTransporter @.***> wrote:

It is actually not a problem, but an improvement. The control of import/export of static and shared library. Comparable with https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zconf.h.cmakein#L337

— Reply to this email directly, view it on GitHub https://github.com/hobu/laz-perf/pull/112#issuecomment-1003743610, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKBMMEVRFE4T3GSC5EAXN3UUB6MXANCNFSM5LD2A5SA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

OgreTransporter commented 2 years ago

As it is now, it is not so easy to switch between the static and the shared library under MSVC because shared is fixed. With the changes I can compile a program like PDAL with LAZPERF_LIB_STATIC, then static is used. If the switch is missing, shared is used automatically. In addition, only the header lazperf_base.hpp has to be changed in case of a version change. The second header lazperf_user_base.hpp is no longer needed. Actually this file should be generated by a template and the version of CMake should be entered there, but I can still change that.

The solution for zlib is similar. If you specify ZLIB_DLL shared is used otherwise static. For non-MSVC users, of course, this is not an improvement.

abellgithub commented 2 years ago

The current licence for lazperf does not permit static linking, though we expect this will be changed soon.

On Sun, Jan 2, 2022, 1:03 PM OgreTransporter @.***> wrote:

As it is now, it is not so easy to switch between the static and the shared library under MSVC because shared is fixed. With the changes I can compile a program like PDAL with LAZPERF_LIB_STATIC, then static is used. If the switch is missing, shared is used automatically. In addition, only the header lazperf_base.hpp has to be changed in case of a version change. The second header lazperf_user_base.hpp is no longer needed. Actually this file should be generated by a template and the version of CMake should be entered there, but I can still change that.

The solution for zlib is similar. If you specify ZLIB_DLL shared is used otherwise static. For non-MSVC users, of course, this is not an improvement.

— Reply to this email directly, view it on GitHub https://github.com/hobu/laz-perf/pull/112#issuecomment-1003753347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKBMMFXRKZB5Y7JMI4M7FTUUCHOLANCNFSM5LD2A5SA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

OgreTransporter commented 2 years ago

The current licence for lazperf does not permit static linking, though we expect this will be changed soon.

For PDAL, this is true. However, other LGPL projects can also be linked statically.

If the license is changed anyway, then you can wait with the merge until the license change.

hobu commented 2 years ago

@OgreTransporter I think this entire issue has gone away for PDAL now that we've simply vendored the laz-perf code in it. Are you still wanting to merge this given that?