-
Notifications
You must be signed in to change notification settings - Fork 7.2k
[alicevision] Add new port #38034
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
[alicevision] Add new port #38034
Conversation
|
Waiting for liblzma issue to be fixed. |
@WangWeiLin-MV can you plz check whether |
|
Depends on #38071 |
|
@fabiencastan Can you please review this PR? |
WangWeiLin-MV
left a comment
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.
Indent needs to be unified for portfile.cmake.
|
Waiting for the upstream review this PR. |
|
Is there a problem to enable the SFM? It's at the core of alicevision, so it will limit a lot the usage. |
simogasp
left a comment
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.
just a few remarks
| SOURCE_PATH "${SOURCE_PATH}" | ||
| OPTIONS | ||
| -DALICEVISION_BUILD_SFM=OFF | ||
| -DALICEVISION_BUILD_MVS=ON |
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 on the other hand can be ON or OFF depending on the availability of CUDA (related to ALICEVISION_USE_CUDA) and GPU on the machine. Maybe we can leave it as a documentation note, the user might be able to build but not to execute without an NVidia card
|
@JackBoosY have you seen the review from @simogasp ? |
|
@fabiencastan vacation now, will continue this PR several days later. |
|
@fabiencastan Alicevision does not provide Edit: I saw a copy of FindCoinUtils.cmake in https://github.com/open-anatomy/SfM_gui_for_openMVG/blob/master/src/cmakeFindModules/FindCoinUtils.cmake, no idea whether we can use this with license. |
|
Currently we are using a fork: https://github.com/alicevision/CoinUtils/commits/av_develop/ There is also another PR on the official coinutils that has not been merged to add cmake. The best would be to get something merged and maybe in the meantime add the cmage config in the vcpkg patch of coinuils. |
Since such ports now use makefile, we may need to wait for the upstream PRs or add vcpkg-wrapper & Find.cmake. |
That's unfortunate because we moved to that fork cmake-based system (we used the CMakeLists from vcpkg at that time) hoping that someday the official repos would also move to cmake. Since we use vcpkg for the Windows CI it also made sense. How do you consume those libs built with make in vcpkg now in a project with cmake? Do you have to provide your own FindX.cmake? |
Yeah we currently may pick one of two ways:
|
|
|
|
I resolved the coinutils / clp /osi couldn't be found issue using pkgconfig, but got new issue about boost: AFAIK, |
|
The error seems to come from Boost::regex which is missing here: |
No, boost-regex was already added into dependencies and found in |
|
|
In the find_package, we list all the modules that we will need globally in the project, but then we need to list the boost modules that we are using in each element, like here: |
Add |
|
@fabiencastan @simogasp Any thought? |
|
Maybe we should replace |
|
I think |
This doesn't work since |
|
Could you please try using this PR : Briefly tested on msvc 2022. |
Will try later. |
|
Current: cannot find openmp on x64-windows, trying to fix this. |
|
@simogasp It's wired that openmp_cxx is not found in Windows but other ports which configured with openmp can find it, any idea on this? Edit: I tried to copy But the same code doesn't work in |
Tried with this PR changes but failed on link process: |
That's very strange indeed. It's true that we have a useless project nesting and we should change that, but it should not be an issue when finding dependencies. |
dg0yt
left a comment
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.
Edit: I tried to copy
find_package(OpenMP)to top ofroot/CMakeLists.txtand openmp_cxx could be found:
To the top? Before project(), languages and the corresponding cmake variables are not initialized. This doesn't deliver valid information.
CMakeFiles/CMakeConfigureLog.yaml should be a good report of cmake configure checks. And there is --trace-expand.
|
|
||
| vcpkg_cmake_install() | ||
|
|
||
| vcpkg_cmake_config_fixup(PACKAGE_NAME AliceVision CONFIG_PATH share/aliceVision/cmake) |
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.
| vcpkg_cmake_config_fixup(PACKAGE_NAME AliceVision CONFIG_PATH share/aliceVision/cmake) | |
| vcpkg_cmake_config_fixup(CONFIG_PATH share/aliceVision/cmake) |
Yeah, the thing is that we have 2 |
|
I don't complain about a second |
I mean add it after all |
|
I changed this to draft because the build is broken; please feel free to mark 'Ready for review' when it's working and you're happy with it |
|
@fabiencastan @simogasp Any help on this issue? |
|
Closing this due to inactivity. |
Continue PR #8829
Fixes #8819.
Related: alicevision/AliceVision#767