[tbb, pagmo2] Update TBB to 2021.5 and update pagmo2 to 2.18.0 #26284
Conversation
Co-authored-by: Thomas1664 <[email protected]>
Co-authored-by: Thomas1664 <[email protected]>
Co-authored-by: Thomas1664 <[email protected]>
Co-authored-by: Thomas1664 <[email protected]>
ports/tbb/portfile.cmake
Outdated
| file(READ "${CURRENT_PACKAGES_DIR}/share/tbb/TBBConfig.cmake" _contents) | ||
| file(WRITE "${CURRENT_PACKAGES_DIR}/share/tbb/TBBConfig.cmake" " | ||
| file(READ "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" _contents) | ||
| file(WRITE "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" " |
There was a problem hiding this comment.
No need to change the case of the tbb directory. CMake finds the directory case-insensitively.
There was a problem hiding this comment.
I'm not sure I understand, this isn't changing the case? (Or rather, if there is a case change, it was done by TBB itself.
There was a problem hiding this comment.
You mean upstream changed the installation directory? Even if this is the case, do we want to continue keeping the files together in share/${PORT}? If yes, ...
There was a problem hiding this comment.
I mean, I don't see anywhere where vcpkg asked for TBB in all caps.
There was a problem hiding this comment.
Definitely in vcpkg_cmake_config_fixup(PACKAGE_NAME TBB <-----
There was a problem hiding this comment.
Definitely in
vcpkg_cmake_config_fixup(PACKAGE_NAME TBB
That's an input though, not an output. Example:
bion@BION-TR:~/vcpkg$ git diff
diff --git a/ports/tbb/portfile.cmake b/ports/tbb/portfile.cmake
index 6bfa28251..4ab2243b8 100644
--- a/ports/tbb/portfile.cmake
+++ b/ports/tbb/portfile.cmake
@@ -16,7 +16,6 @@ vcpkg_cmake_configure(
)
vcpkg_cmake_install()
-vcpkg_cmake_config_fixup(PACKAGE_NAME TBB CONFIG_PATH "lib/cmake/TBB")
vcpkg_fixup_pkgconfig()
vcpkg_copy_pdbs()
@@ -29,10 +28,4 @@ file(REMOVE_RECURSE
"${CURRENT_PACKAGES_DIR}/debug/lib/tbb_debug.lib"
)
-file(READ "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" _contents)
-file(WRITE "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" "
-include(CMakeFindDependencyMacro)
-find_dependency(Threads)
-${_contents}")
-
file(INSTALL "${SOURCE_PATH}/LICENSE.txt" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}" RENAME copyright)bion@BION-TR:~/vcpkg$ ./vcpkg install tbb
Computing installation plan...
The following packages will be built and installed:
tbb[core]:x64-linux -> 2021.5.0
* vcpkg-cmake[core]:x64-linux -> 2022-07-18
* vcpkg-cmake-config[core]:x64-linux -> 2022-02-06#1
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x64-linux...
Restored 2 package(s) from /home/bion/.cache/vcpkg/archives in 37.87 ms. Use --debug to see more details.
Installing 1/3 vcpkg-cmake:x64-linux...
Elapsed time to handle vcpkg-cmake:x64-linux: 499.3 us
Installing 2/3 vcpkg-cmake-config:x64-linux...
Elapsed time to handle vcpkg-cmake-config:x64-linux: 296.4 us
Installing 3/3 tbb:x64-linux...
Building tbb[core]:x64-linux...
-- Using cached oneapi-src-oneTBB-v2021.5.0.tar.gz.
-- Extracting source /mnt/c/Dev/vcpkg-downloads/oneapi-src-oneTBB-v2021.5.0.tar.gz
-- Using source at /home/bion/vcpkg/buildtrees/tbb/src/v2021.5.0-808f1ff7af.clean
-- Configuring x64-linux
-- Building x64-linux-dbg
-- Building x64-linux-rel
-- Fixing pkgconfig file: /home/bion/vcpkg/packages/tbb_x64-linux/lib/pkgconfig/tbb.pc
-- Fixing pkgconfig file: /home/bion/vcpkg/packages/tbb_x64-linux/debug/lib/pkgconfig/tbb.pc
-- Installing: /home/bion/vcpkg/packages/tbb_x64-linux/share/tbb/copyright
-- Performing post-build validation
The /lib/cmake folder should be merged with /debug/lib/cmake and moved to /share/tbb/cmake.
Please use the helper function `vcpkg_cmake_config_fixup()` from the port vcpkg-cmake-config.`
The following cmake files were found outside /share/tbb. Please place cmake files in /share/tbb.
/home/bion/vcpkg/packages/tbb_x64-linux/lib/cmake/TBB/TBBTargets-release.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/lib/cmake/TBB/TBBConfig.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/lib/cmake/TBB/TBBTargets.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/lib/cmake/TBB/TBBConfigVersion.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/debug/lib/cmake/TBB/TBBConfig.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/debug/lib/cmake/TBB/TBBTargets.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/debug/lib/cmake/TBB/TBBTargets-debug.cmake
/home/bion/vcpkg/packages/tbb_x64-linux/debug/lib/cmake/TBB/TBBConfigVersion.cmake
The /debug/lib/cmake folder should be merged with /lib/cmake into /share/tbb
Found 3 post-build check problem(s). To submit these ports to curated catalogs, please first correct the portfile:
/home/bion/vcpkg/ports/tbb/portfile.cmake
-- Performing post-build validation done
Stored binary cache: "/home/bion/.cache/vcpkg/archives/ac/ac8bd29b609bbc52f74540ceef7da7a4634d8ebfd518a4ead473a6eb8c39261c.zip"
Elapsed time to handle tbb:x64-linux: 6.446 s
Total elapsed time: 12.73 s
bion@BION-TR:~/vcpkg$
ports/tbb/portfile.cmake
Outdated
| file(READ "${CURRENT_PACKAGES_DIR}/share/tbb/TBBConfig.cmake" _contents) | ||
| file(WRITE "${CURRENT_PACKAGES_DIR}/share/tbb/TBBConfig.cmake" " | ||
| file(READ "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" _contents) | ||
| file(WRITE "${CURRENT_PACKAGES_DIR}/share/TBB/TBBConfig.cmake" " |
There was a problem hiding this comment.
You mean upstream changed the installation directory? Even if this is the case, do we want to continue keeping the files together in share/${PORT}? If yes, ...
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/usd/portfile.cmake
You have modified or added at least one vcpkg.json where you should check the license field.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/usd/vcpkg.json
Valid values for the license field can be found in the documentation
| include(CMakeFindDependencyMacro) | ||
|
|
||
| set(OpenCV_DIR @OpenCV_DIR@) | ||
| +find_dependency(TBB) |
There was a problem hiding this comment.
This should match the find_package() in the project, minus REQUIRED. However, I think it would be better to delete FindTBB.cmake entirely than to patch every consumption site to use CONFIG.
| +find_dependency(TBB) | |
| +find_dependency(TBB CONFIG) |
There was a problem hiding this comment.
We were already deleting FindTBB.cmake so I just removed this part of the patch.
ports/cctag/0002-find-tbb.patch
Outdated
|
|
||
| # TBB | ||
| if(NOT TBB_FOUND) | ||
| - set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${PROJECT_SOURCE_DIR}/cmake") |
There was a problem hiding this comment.
Can we just delete cmake/FindTBB.cmake instead of patching this file?
ports/cctag/0002-find-tbb.patch
Outdated
| # With MVSC, CMAKE_BUILD_TYPE will always be None, so TBB_USE_DEBUG_BUILD will always be false. | ||
| -string(COMPARE EQUAL "${CMAKE_BUILD_TYPE}" Debug TBB_USE_DEBUG_BUILD) | ||
| -find_package(TBB REQUIRED) | ||
| +find_package(TBB CONFIG REQUIRED) |
There was a problem hiding this comment.
Deleting FindTBB.cmake removes the need for this hunk.
ports/cctag/0002-find-tbb.patch
Outdated
| # INCLUDE_DIRECTORIES -- "$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}>" | ||
| include_directories(${CMAKE_CURRENT_LIST_DIR} | ||
| ${OpenCV_INCLUDE_DIRS} ${Eigen_INCLUDE_DIR} | ||
| - ${TBB_INCLUDE_DIRS} |
There was a problem hiding this comment.
This hunk isn't needed -- TBB_INCLUDE_DIRS will evaluate to empty
| #include <list> | ||
| #include <utility> | ||
| #include <memory> | ||
| +#include <mutex> |
There was a problem hiding this comment.
Can we pull https://github.com/alicevision/CCTag/commit/1840f4224ef6f2ba78aaa2cd0b311d0a5bbd6710.diff instead of including it inline here?
There was a problem hiding this comment.
- I don't think the change is big enough to trigger licensing concerns, and
- fetching additional content like that is worse UX, and
- this change isn't exactly that because that was made against a different branch
I think we should be doing things like that only when the patch wouldn't be de-minimis.
ports/tbb/vcpkg.json
Outdated
| "description": "Intel's Threading Building Blocks.", | ||
| "homepage": "https://github.com/01org/tbb", | ||
| "supports": "!(uwp | arm | arm64) | linux | osx" | ||
| "license": "Apache-2.0", | ||
| "supports": "!(windows & uwp) | linux | osx", |
There was a problem hiding this comment.
| "supports": "!(windows & uwp) | linux | osx", | |
| "supports": "(windows & !uwp) | linux | osx", |
emscripten passes !(windows & uwp), but this is clearly an allow list that intends to exclude it.
ports/usd/portfile.cmake
Outdated
| @@ -1,6 +1,14 @@ | |||
| # Don't file if the bin folder exists. We need exe and custom files. | |||
| SET(VCPKG_POLICY_EMPTY_PACKAGE enabled) | |||
|
|
|||
| message(STATUS [=[ | |||
| The usd port does not work the the version of Threading Building Blocks (tbb) currently chosen by vcpkg's baselines, | |||
| and does not expect to be updated to work with current versions soon. | |||
There was a problem hiding this comment.
I think we should point users towards the upstream issue: PixarAnimationStudios/OpenUSD#1600
|
Additionally, the diff --git a/ports/pagmo2/disable-werror.patch b/ports/pagmo2/disable-werror.patch
index 92d930a04..fbbad4785 100644
--- a/ports/pagmo2/disable-werror.patch
+++ b/ports/pagmo2/disable-werror.patch
@@ -2,21 +2,19 @@ diff --git a/cmake_modules/yacma/YACMACompilerLinkerSettings.cmake b/cmake_modul
index 7d7aa1b..21613a2 100644
--- a/cmake_modules/yacma/YACMACompilerLinkerSettings.cmake
+++ b/cmake_modules/yacma/YACMACompilerLinkerSettings.cmake
-@@ -95,7 +95,7 @@ if(NOT _YACMACompilerLinkerSettingsRun)
+@@ -95,7 +95,6 @@ if(NOT _YACMACompilerLinkerSettingsRun)
# NOTE: enable unconditionally, as it seems like the CMake
# machinery for detecting this fails. Perhaps the source code
# used for checking the flag emits warnings?
- list(APPEND _YACMA_CXX_FLAGS_DEBUG "-Werror")
-+ #list(APPEND _YACMA_CXX_FLAGS_DEBUG "-Werror")
# New warnings in clang 8.
# NOTE: a few issues with macros here, let's disable for now.
# _YACMA_CHECK_ENABLE_DEBUG_CXX_FLAG(-Wextra-semi-stmt)
-@@ -180,7 +180,7 @@ if(NOT _YACMACompilerLinkerSettingsRun)
+@@ -180,7 +179,6 @@ if(NOT _YACMACompilerLinkerSettingsRun)
# Enable higher warning level than usual.
_YACMA_CHECK_ENABLE_DEBUG_CXX_FLAG(/W4)
# Treat warnings as errors.
- _YACMA_CHECK_ENABLE_DEBUG_CXX_FLAG(/WX)
-+ #_YACMA_CHECK_ENABLE_DEBUG_CXX_FLAG(/WX)
endif()
# Set the cache variables.
diff --git a/ports/pagmo2/doxygen.patch b/ports/pagmo2/doxygen.patch
index 1a59a9110..a95cc47b3 100644
--- a/ports/pagmo2/doxygen.patch
+++ b/ports/pagmo2/doxygen.patch
@@ -2,14 +2,12 @@ diff --git a/CMakeLists.txt b/CMakeLists.txt
index 34bad69..dfb942d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
-@@ -398,8 +398,8 @@ endif()
+@@ -398,8 +398,6 @@ endif()
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/config.hpp.in" "${CMAKE_CURRENT_BINARY_DIR}/include/pagmo/config.hpp" @ONLY)
# Configure the doc files.
-configure_file("${CMAKE_CURRENT_SOURCE_DIR}/doc/doxygen/Doxyfile.in" "${CMAKE_CURRENT_SOURCE_DIR}/doc/doxygen/Doxyfile" @ONLY)
-configure_file("${CMAKE_CURRENT_SOURCE_DIR}/doc/sphinx/conf.py.in" "${CMAKE_CURRENT_SOURCE_DIR}/doc/sphinx/conf.py" @ONLY)
-+# configure_file("${CMAKE_CURRENT_SOURCE_DIR}/doc/doxygen/Doxyfile.in" "${CMAKE_CURRENT_SOURCE_DIR}/doc/doxygen/Doxyfile" @ONLY)
-+# configure_file("${CMAKE_CURRENT_SOURCE_DIR}/doc/sphinx/conf.py.in" "${CMAKE_CURRENT_SOURCE_DIR}/doc/sphinx/conf.py" @ONLY)
# This is just a simple counter variable, internal use only.
set(_PAGMO_TEST_NUM "0") |
dan-shaw
added
requires:author-response
and removed
info:reviewed
BillyONeal
dismissed stale reviews from ras0219-msft, LilyWangLL, and github-actions[bot]
via
006c899
There was a problem hiding this comment.
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}The following files are affected:
ports/usd/portfile.cmake
You have modified or added at least one vcpkg.json where you should check the license field.
If you feel able to do so, please consider adding a "license" field to the following files:
ports/usd/vcpkg.json
Valid values for the license field can be found in the documentation
Merges / combines the following:
Credit to @ras0219 / @ras0219-msft , @vladimir-aubrecht , and @mathisloge for the initial work.