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

[enkits] New port #34277

Closed
wants to merge 14 commits into from
50 changes: 50 additions & 0 deletions ports/enkits/fix-install.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7f8572e..8f84548 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -2,6 +2,8 @@ cmake_minimum_required(VERSION 3.0)

project( enkiTS )

+include(GNUInstallDirs)
+
option( ENKITS_BUILD_C_INTERFACE "Build C interface" ON )
option( ENKITS_BUILD_EXAMPLES "Build example applications" ON )
option( ENKITS_BUILD_SHARED "Build shared library" OFF )
@@ -9,8 +11,6 @@ option( ENKITS_INSTALL "Generate installation target" OFF )

set( ENKITS_TASK_PRIORITIES_NUM "3" CACHE STRING "Number of task priorities, 1-5, 0 for defined by defaults in source" )

-include_directories( "${PROJECT_SOURCE_DIR}/src" )
-
set( ENKITS_HEADERS
src/LockLessMultiReadPipe.h
src/TaskScheduler.h
@@ -45,6 +45,9 @@ else()
add_library( enkiTS STATIC ${ENKITS_SRC} )
endif()

+target_include_directories( enkiTS PUBLIC
+ PUBLIC $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>
+ $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> )

if( ENKITS_TASK_PRIORITIES_NUM GREATER "0" )
target_compile_definitions( enkiTS PUBLIC "ENKITS_TASK_PRIORITIES_NUM=${ENKITS_TASK_PRIORITIES_NUM}" )
@@ -60,8 +63,15 @@ if( UNIX )
endif()

if( ENKITS_INSTALL )
- install(TARGETS enkiTS DESTINATION "${CMAKE_INSTALL_PREFIX}/lib/enkiTS")
- install(FILES ${ENKITS_HEADERS} DESTINATION "${CMAKE_INSTALL_PREFIX}/include/enkiTS")
+ install(
+ TARGETS enkiTS
+ EXPORT unofficial-enkiTS-config
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
+ install(FILES ${ENKITS_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/enkiTS)
+ install(
+ EXPORT unofficial-enkiTS-config
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates an invalid file name. You musts not use upper-case with -config.cmake. CMake only looks for <lower-case-package>config.cmake. And we do want lower-case unofficial-<port> package names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of curiosity, why did you accept this PR then? I fail to see the difference with mine in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

... unexpected. I didn't even notice that it was a second PR. I only noticed that "unofficial" was "gone" again, and I found the config was added upstream.

Copy link
Contributor

Choose a reason for hiding this comment

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

The case-sensitivity problem is real: #34487, dougbinks/enkiTS#103.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thanks for the information and the fix. I used uppercase because that's what was done upstream (unreleased yet) and I hoped to be able to remove the patch on the next release, but I didn't know this could be an issue for CMake.

+ NAMESPACE unofficial::enkiTS::
Copy link
Contributor

Choose a reason for hiding this comment

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

We also want lower-case unofficial::<port>:: here.

+ DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/unofficial-enkiTS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+ DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/unofficial-enkiTS)
+ DESTINATION share/unofficial-enkits)

endif()

if( UNIX )
27 changes: 27 additions & 0 deletions ports/enkits/portfile.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
vcpkg_check_linkage(ONLY_STATIC_LIBRARY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't upstream support shared linkage on non-win32?


vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO dougbinks/enkiTS
REF v${VERSION}
SHA512 72a05058caef8d6a33cd70500aeaf05cc61521721697969d4845279b5a79b63e7a6a3f3971c5eff2776e5575720b58252e9d251ef565c2123275a3e8540948db
HEAD_REF master
PATCHES fix-install.patch
)

vcpkg_cmake_configure(
SOURCE_PATH "${SOURCE_PATH}"
OPTIONS
-DENKITS_BUILD_C_INTERFACE=OFF
-DENKITS_BUILD_EXAMPLES=OFF
-DENKITS_INSTALL=ON
)

vcpkg_cmake_install()
vcpkg_copy_pdbs()
vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/unofficial-enkiTS PACKAGE_NAME unofficial-enkiTS)
Copy link
Contributor

Choose a reason for hiding this comment

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

... lower-case ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/unofficial-enkiTS PACKAGE_NAME unofficial-enkiTS)
vcpkg_cmake_config_fixup(PACKAGE_NAME unofficial-enkits)


file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")

vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/License.txt")
file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/usage" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")
4 changes: 4 additions & 0 deletions ports/enkits/usage
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enkits provides CMake targets:

find_package(unofficial-enkits CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::enkiTS::enkiTS)
Comment on lines +3 to +4
Copy link
Contributor

@dg0yt dg0yt Oct 10, 2023

Choose a reason for hiding this comment

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

unofficial::enkits::enkiTS

17 changes: 17 additions & 0 deletions ports/enkits/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "enkits",
"version": "1.11",
"description": "A permissively licensed C and C++ Task Scheduler for creating parallel programs. Requires C++11 support.",
"homepage": "https://github.com/dougbinks/enkiTS",
"license": "Zlib",
"dependencies": [
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}
]
}
4 changes: 4 additions & 0 deletions versions/baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -2376,6 +2376,10 @@
"baseline": "1.3.17",
"port-version": 2
},
"enkits": {
"baseline": "1.11",
"port-version": 0
},
"ensmallen": {
"baseline": "2.19.1",
"port-version": 0
Expand Down
9 changes: 9 additions & 0 deletions versions/e-/enkits.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"versions": [
{
"git-tree": "e438bb0ea23cdd956552797001b2ef69bd8e36f1",
"version": "1.11",
"port-version": 0
}
]
}