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

CMake configuration is too platform-dependent and potentially broken #6

Open
JayFoxRox opened this issue Aug 6, 2020 · 2 comments
Open

Comments

@JayFoxRox
Copy link

This differentiates MSVC and non-MSVC, when, for the most part, it probably shouldn't have to be this way:

if(MSVC) # assumes vcpkg
find_package(jansson CONFIG REQUIRED)
find_package(PNG REQUIRED)
find_package(embree 3 REQUIRED)
find_package(assimp CONFIG REQUIRED)
link_directories(${EMBREE_ROOT_DIR}/lib)
set(JANSSON jansson::jansson)
add_definitions(/D_USE_MATH_DEFINES /DNOMINMAX /wd4305 /wd4244 /wd4996)
else()
set(JANSSON jansson)
endif()

Most (all?) packages are probably also required on non-MSVC.
This will hinder portability.


In addition, none of those packges are actually used / linked within this projects CMakeLists.txt.

I assume that the dependencies will take care of linking them, but in that case, this project shouldn't even have to be aware of these libraries (instead, the dependency should pull them in).

@X123M3-256
Copy link
Owner

The MSVC stuff was added by @janisozaur. I can't say I understand CMake well enough to address this issue, but I do know that all of those packages are required by this project.

@JayFoxRox
Copy link
Author

I might send a PR about this (and unresolved CMake stuff from #5) in the future; remind me if I don't. However, I probably won't get to it before January, so hopefully someone else picks it up before then.

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