Move from Hunter to VCPKG #1170
Conversation
There was a problem hiding this comment.
Thanks a lot!
Looking forward to get this in and say goodbye to Hunter :)
I left mostly nitpicks and questions, let's discuss on case by case basis what makes sense to address before merge and what we can split for later (for example moving as many packages as possible to stock)
| @@ -44,7 +52,7 @@ jobs: | |||
| run: | | |||
| sudo apt update | |||
| python -m pip install --upgrade pip | |||
| sudo apt install libusb-1.0-0-dev libopencv-dev libpcl-dev | |||
| sudo apt install libusb-1.0-0-dev pkg-config bison autoconf libtool libxi-dev libxtst-dev libxrandr-dev libx11-dev libxft-dev libxext-dev nasm flex libudev-dev | |||
There was a problem hiding this comment.
What VCPKG package needs these?
There was a problem hiding this comment.
Most of these are needed for various dependencies of gtk which is required by opencv to use imshow etc. Nasm is required by ffmpeg. libudev is required by libusb
.github/workflows/python-main.yml
Outdated
| @@ -194,7 +204,10 @@ jobs: | |||
| python-architecture: [x64] # TODO(Morato) - re-enable x86 - it complains that OpenCV even though it's 32 bit is not compatible | |||
| fail-fast: false | |||
| env: | |||
| DEPTHAI_BUILD_BASALT: ON | |||
| # DEPTHAI_BUILD_BASALT: ON | |||
There was a problem hiding this comment.
Must've commented out to speed up build, will reenable
.github/workflows/python-main.yml
Outdated
| @@ -388,30 +400,34 @@ jobs: | |||
| needs: build-docstrings | |||
| runs-on: ubuntu-latest | |||
| container: | |||
| image: mmorato/depthai-manylinux2014:0.4 # TODO(mmorato) temporary location, push to luxonis namespace | |||
| image: quay.io/pypa/manylinux_2_28_x86_64 # TODO(mmorato) temporary location, push to luxonis namespace | |||
There was a problem hiding this comment.
| image: quay.io/pypa/manylinux_2_28_x86_64 # TODO(mmorato) temporary location, push to luxonis namespace | |
| image: quay.io/pypa/manylinux_2_28_x86_64 |
|
|
||
| - name: Build and install depthai-core | ||
| run: | | ||
| cmake -S . -B build_core -D CMAKE_BUILD_TYPE=Release -D CMAKE_TOOLCHAIN_FILE=$PWD/cmake/toolchain/pic.cmake | ||
| cmake --build build_core --target install --parallel 4 | ||
| echo "DEPTHAI_INSTALLATION_DIR=$PWD/build_core/install/" >> $GITHUB_ENV | ||
|
|
||
| - name: Append build hash if not a tagged commit |
There was a problem hiding this comment.
Not sure if it was needed? Cache is now separate for each job
cmake/ports/rtabmap/portfile.cmake
Outdated
There was a problem hiding this comment.
Can we use stock?
https://vcpkg.io/en/package/rtabmap
There was a problem hiding this comment.
Current stock version has some issues, might get updated in the future
cmake/ports/spdlog/portfile.cmake
Outdated
There was a problem hiding this comment.
Can we use stock?
https://vcpkg.io/en/package/spdlog
cmake/ports/tbb/portfile.cmake
Outdated
There was a problem hiding this comment.
Can we use stock?
https://vcpkg.io/en/package/tbb
There was a problem hiding this comment.
This specific version is required by basalt so until that codebase is update unfortunately not
cmake/triplets/x64-linux.cmake
Outdated
| endif() | ||
|
|
||
| if(PORT MATCHES "opencv") | ||
| set(VCPKG_LIBRARY_LINKAGE dynamic) |
There was a problem hiding this comment.
How come we go with dynamic linking here?
There was a problem hiding this comment.
We should likely also link libusb dynamically here
This PR introduces following changes: