Skip to content
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.

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

Call project() after cmake_minimum_required() #191

Closed
SpaceIm opened this issue Nov 12, 2023 · 6 comments
Closed

Call project() after cmake_minimum_required() #191

SpaceIm opened this issue Nov 12, 2023 · 6 comments

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Nov 12, 2023

Could you call project() after cmake_minimum_required() please (well after include(cmake/version.cmake) actually)? It's problematic when someone has to use a toolchain, since CMakeLists of msdfgen has lot of logic between cmake_minimum_required() and project(), and stuff in toolchain files are ignored before project() is called (it's tedious for conan for example).

Moreover it's not really a good idea to hardcode vcpkg stuff in a CMakeLists. vcpkg should come as a toolchain file injected externally. CMakeLists should be package manager agnostic.

@Chlumsky
Copy link
Owner

As you can see, the logic before project needs to configure stuff for the vcpkg toolchain so this is intentional. Using vcpkg is not hardcoded, you can disable it with MSDFGEN_USE_VCPKG.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 12, 2023

Even if there is an option, this logic leads to an illformed CMakeLists, where options and some logic unrelated to vcpkg are defined before projects just because of this vcpkg intrusion (actually I realize that you could not even move other options after project() because you have vcpkg logic based on these options which has be done before project()).

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 12, 2023

Could you at least move MSDFGEN_DYNAMIC_RUNTIME and BUILD_SHARED_LIBS logic after project() please?

I mean this part:

if(MSDFGEN_DYNAMIC_RUNTIME)
    set(MSDFGEN_MSVC_RUNTIME "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
else()
    set(MSDFGEN_MSVC_RUNTIME "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()

if(BUILD_SHARED_LIBS)
    set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()

@Chlumsky
Copy link
Owner

But why?

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 13, 2023

Because it defeats conan toolchain.

Chlumsky added a commit that referenced this issue Dec 16, 2023
@Chlumsky
Copy link
Owner

Sorry, I kind of forgot about this but changed it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants