Make the default value of E57_RELEASE_LTO dependent on E57_BUILD_SHARED #313
Conversation
| if ( E57_RELEASE_LTO AND NOT E57_BUILD_SHARED ) | ||
| message( WARNING "E57_RELEASE_LTO is set to ON, but ${PROJECT_NAME} will be build as static library. This may lead to linker issues!" ) | ||
| elseif ( NOT E57_RELEASE_LTO AND E57_BUILD_SHARED ) | ||
| message( WARNING "E57_RELEASE_LTO is set to OFF, but ${PROJECT_NAME} will be build as dynamic library. This may lead issues if compiling statically for distribution!" ) |
There was a problem hiding this comment.
Having it OFF for a dynamic lib isn't a problem. If they are compiling a dynamic lib, then they are are not "compiling statically for distribution".
Maybe I'm misunderstanding what you're trying to warn about here?
There was a problem hiding this comment.
I have only copied text from the comment, as it only says that there are problems, but nothing more. From #254 I also couldn't figure out why the option really has to be set as a default and couldn't just be added from outside if needed.
There was a problem hiding this comment.
If you're trying to package the lib for distribution, you can just turn it off by setting E57_RELEASE_LTO to OFF.
It's actually talking specifically about release static builds for distribution.
If you're using a static lib in your own code (i.e. including this library as a static lib) it's fine. |
Sadly not. See the linker issue in the initial post of me. |
|
I use it as a static lib on macOS and Windows (MSYS2) without issues so this sounds like maybe a compiler difference. What OS and compiler are you using? |
On MSVC everything is fine with 3.2.0. On our CI we are using a docker image based on |
|
I've never seen "plugin needed to handle lto object" - sounds like something isn't installed? Are you getting the same message from both clang and gcc on Ubuntu? Any more info about it when you search for it? |
Tried again. On GCC I'm getting (build libE57 with GCC 11.3, but our project than with GCC 12.0): So basically LTO seems to break the use case, to build a dependency with the default OS compiler and then compile it with a different compiler. Since we test our project on our CI with different GCC versions (standard OS compiler when we build a release so that we don't have to install another standard lib at the customer's site) and the latest compiler for more compiler warnings, you have to build multiple versions of libE57 (if you have LTO enabled) so that you can build with multiple compilers at the same time. In principle, I have the feeling that you really don't want LTO as default, unless you build a release where you want to have a better optimization again - and in that case you can also activate it manually.
Here for example |
|
OK thanks. I'll consider it. In the meantime, you should be able to set |
By default
BUILD_SHARED_LIBSis not defined, therefore the default ofE57_BUILD_SHAREDis alsoOFF. Therefore this lib will be build as static library by default. This leads by default to linker issues later:As the comment about
E57_RELEASE_LTOsays, this option should be OFF when building a static library. Therefore the default value is now adjusted accordingly. As changingE57_BUILD_SHAREDlater will not change the default value ofE57_RELEASE_LTOI have added a warning so that there is a hint that the combination may cause issues.GDAL has IPO disabled by default:
https://github.com/OSGeo/gdal/blob/305b23f5a90bf6d86c7fb895dfc745321def0aab/gdal.cmake#L27
Qt seems to also not set this value, but in case
CMAKE_INTERPROCEDURAL_OPTIMIZATIONis set they enforce it toOFFfor MSVC 3rd party: qt/qtbase@0d708a9.So basically I would argue that
E57_RELEASE_LTOshould be alwaysOFFor removed and in case someone needs this, he can build it by passingCMAKE_INTERPROCEDURAL_OPTIMIZATION(in case canon has an issue with it, they can add it to their build script as it seems it was not necessary for vcpkg when checking the port scripts from before libE57Format 3.1.0 release)