Skip to content

Make the default value of E57_RELEASE_LTO dependent on E57_BUILD_SHARED #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make the default value of E57_RELEASE_LTO dependent on E57_BUILD_SHARED #313

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ option( E57_WRITE_CRAZY_PACKET_MODE "Compile library to enable reader-stressing
# CMake forces "thin" LTO (see https://gitlab.kitware.com/cmake/cmake/-/issues/23136)
# which is a problem if compiling statically for distribution (e.g. in a package manager).
# Generally you will only want to turn this off for distributing static release builds.
option( E57_RELEASE_LTO "Compile release library with link-time optimization" ON )
option( E57_RELEASE_LTO "Compile release library with link-time optimization" ${E57_BUILD_SHARED} )

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!" )
Copy link
Owner

@asmaloney asmaloney Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@SunBlack SunBlack Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

@asmaloney asmaloney Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're trying to package the lib for distribution, you can just turn it off by setting E57_RELEASE_LTO to OFF.

endif()

#########################################################################################

Expand Down