-
Notifications
You must be signed in to change notification settings - Fork 64
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
Project modernization #33
base: master
Are you sure you want to change the base?
Conversation
include_directories(${GKLIB_PATH}/include) change the directory of files
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
e3b23da
to
3379e9f
Compare
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
3379e9f
to
1ba74b3
Compare
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this, I am testing this on MSVC and clang on Windows and I ran into a couple of issues. My only other request would be that a .pc file would be nice, otherwise the cmake works just fine!
# win32/adapt.h | ||
# ) | ||
if (GKLIB_INSTALL) | ||
install(FILES win32/adapt.h ${CMAKE_INSTALL_INCLUDEDIR}/win32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here, should be
install(FILES win32/adapt.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/win32
add_feature_info(GKLIB_OpenMP GKLIB_OpenMP "OpenMP support") | ||
option(GKLIB_PCRE "GKlib: Enable PCRE support" OFF) | ||
add_feature_info(GKLIB_PCRE GKLIB_PCRE "PCRE support") | ||
cmake_dependent_option(GKLIB_GKREGEX "GKlib: Enable GKREGEX support" OFF "NOT GKLIB_PCRE" OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error here, it should be
Need to add `-GKLIB_GKREGEX=ON`
```cmake
# Second "OFF" should be "ON"
cmake_dependent_option(GKLIB_GKREGEX "GKlib: Enable GKREGEX support" OFF "NOT GKLIB_PCRE" ON)
include(FetchContent) | ||
if (GKLIB_INSTALL) | ||
include(CMakePackageConfigHelpers) | ||
if (UNIX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be unconditional, it's not an issue for this on Windows. And also, you use e.g. CMAKE_INSTALL_BINDIR
unconditionally later on which expands to nothing on Windows currently.
This is the counterpart of KarypisLab/METIS#79