Skip to content

[pcl] Update to 1.12.0 [rtabmap] Update to 0.20.13#18855

Merged
BillyONeal merged 24 commits intomicrosoft:masterfrom
raahilsha-z:master
Nov 5, 2021
Merged

[pcl] Update to 1.12.0 [rtabmap] Update to 0.20.13#18855
BillyONeal merged 24 commits intomicrosoft:masterfrom
raahilsha-z:master

Conversation

@raahilsha-z
Copy link
Contributor

@raahilsha-z raahilsha-z commented Jul 7, 2021

Describe the pull request

  • What does your PR fix?

Updates PCL to the latest 1.12.0

  • Which triplets are supported/not supported? Have you updated the CI baseline?

All currently supported triplets should still be supported, no changes needed to CI baseline

Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes updated

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@ghost
Copy link

ghost commented Jul 7, 2021

CLA assistant check
All CLA requirements met.

@raahilsha-z raahilsha-z marked this pull request as ready for review July 7, 2021 16:46
@raahilsha-z raahilsha-z changed the title [pcl] Update to 1.12.0.99 [pcl] Update to 1.12.0.99 [rtabmap] Update to 0.20.13 Jul 7, 2021
@raahilsha-z raahilsha-z changed the title [pcl] Update to 1.12.0.99 [rtabmap] Update to 0.20.13 [pcl] Update to 1.12.0 [rtabmap] Update to 0.20.13 Jul 7, 2021
@raahilsha-z
Copy link
Contributor Author

I'm unsure why the Linux and OSX checks are failing here, when they worked a couple commits ago: https://github.com/microsoft/vcpkg/pull/18855/checks?check_run_id=3012856022

The OSX one is failing to find a library called "m", and the Linux one is failing to find "GL"

@JonLiu1993 JonLiu1993 added the category:port-update The issue is with a library, which is requesting update new revision label Jul 8, 2021
@JackBoosY JackBoosY linked an issue Jul 8, 2021 that may be closed by this pull request
@JonLiu1993
Copy link
Contributor

@raahilsha-z ,Could you please take a look:
config-x64-linux-dbg-err.log
config-x64-linux-dbg-out.log

@raahilsha-z
Copy link
Contributor Author

raahilsha-z commented Jul 8, 2021

I just tried vcpkg install rtabmap:x64-linux on a fresh WSL2 Ubuntu 20.04 environment, and had no issues building pcl[vtk] in the process. I did have to run apt-get install libxmu-dev libxi-dev libgl-dev in order to get glew to compile, as per glew's portfile.cmake, and I see that glew and vtk are both succeeding inside of the CI environment, so I'm unsure why the Library not found: GL error is happening.

Update: I've tried this again on a fresh Ubuntu 18.04 LTS machine, set up by running vcpkg/scripts/azure-pipelines/linux/provision-image.sh. I then ran vcpkg install pcl[core,vtk]:x64-linux and get an OpenGL-related failure. However, the same issue does not occur on Ubuntu 20.04 LTS, and I am able to build pcl[core,vtk] fine on it.

Further, installing vtk[opengl] on Ubuntu 18.04 doesn't solve this issue either. I've attached a log below that seems to be more detailed than the one provided by the CI machine.

config-x64-linux-dbg-err.log

I've also noticed that rtabmap's manifest has "supports": "windows & !static" in it - does that imply that it shouldn't be built on Linux?

Another update: I also see Could NOT find OpenGL (missing: OPENGL_opengl_LIBRARY OPENGL_glx_LIBRARY) on Ubuntu 18.04, but not on 20.04

@JackBoosY
Copy link
Contributor

I've also noticed that rtabmap's manifest has "supports": "windows & !static" in it - does that imply that it shouldn't be built on Linux?

Yes, rtabmap only support Windows dynamic in vcpkg currently.

@JackBoosY
Copy link
Contributor

CMake Error at /mnt/vcpkg-ci/installed/x64-linux/share/cmake/Qt5Gui/Qt5GuiConfig.cmake:95 (message):
  Library not found: GL
Call Stack (most recent call first):
  /mnt/vcpkg-ci/installed/x64-linux/share/cmake/Qt5Gui/Qt5GuiConfig.cmake:277 (_qt5_Gui_process_prl_file)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:792 (_find_package)
  /mnt/vcpkg-ci/installed/x64-linux/share/cmake/Qt5Widgets/Qt5WidgetsConfig.cmake:231 (find_package)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:792 (_find_package)
  /mnt/vcpkg-ci/installed/x64-linux/share/cmake/Qt5/Qt5Config.cmake:28 (find_package)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:792 (_find_package)
  /mnt/vcpkg-ci/installed/x64-linux/share/vtk/VTK-vtk-module-find-packages.cmake:115 (find_package)
  /mnt/vcpkg-ci/installed/x64-linux/share/vtk/vtk-config.cmake:129 (include)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:792 (_find_package)
  cmake/pcl_find_vtk.cmake:25 (find_package)
  CMakeLists.txt:380 (include)

Related to qt5-gui.
cc @Neumann-A

@larshg
Copy link
Contributor

larshg commented Jul 15, 2021

I don't think the PNG patch is required anymore, with the use of VTK 9 - the PNG are properly prefixed with optimized / debug and get linked accordingly.

@raahilsha-z
Copy link
Contributor Author

@JackBoosY - I just tried out your fix-cmake_find_library_suffixes patch, and it worked fine for me both on WSL and in an Ubuntu VM! Thanks for debugging this :)

@JackBoosY
Copy link
Contributor

Ping @dan-shaw for review this PR first, it was blocked for a long time.

@JackBoosY JackBoosY removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Nov 1, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one portfile where deprecated functions are used.

Details

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/pcl/portfile.cmake
  • ports/rtabmap/portfile.cmake

@JackBoosY
Copy link
Contributor

Add DISABLE_PARALLEL_CONFIGURE to rtabmap to avoid configure error:

CMake Error at CMakeLists.txt:960 (CONFIGURE_FILE):
  CONFIGURE_FILE attempted to configure a file:
  D:/buildtrees/rtabmap/src/fde13e9126-d5e18ef027.clean/corelib/include/rtabmap/core/Version.h
  into a source directory.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Nov 1, 2021
@WangZP10969
Copy link

WangZP10969 commented Nov 2, 2021

First of all, thank you very much for your efforts. I tried this version and it compiles normally under vs2019 (vs2022pre will have errors during pcl compilation
install-x64-windows-rel-out.log
, But I think this should be caused by 2022pre)

I encountered a problem in #17662. I am not sure if there is a problem with my configuration method, but it seems that the performance of PCL1.12 and Qhull is similar to 1.11. HAVE_QHULL is not defined and part of the lib configuration is missing after manual definition.

I'm not sure if this is the cause of upstream or other problems, but just as a small hint.

@JJJohan
Copy link

JJJohan commented Nov 4, 2021

First of all, thank you very much for your efforts. I tried this version and it compiles normally under vs2019 (vs2022pre will have errors during pcl compilation install-x64-windows-rel-out.log , But I think this should be caused by 2022pre)

I encountered a problem in #17662. I am not sure if there is a problem with my configuration method, but it seems that the performance of PCL1.12 and Qhull is similar to 1.11. HAVE_QHULL is not defined and part of the lib configuration is missing after manual definition.

I'm not sure if this is the cause of upstream or other problems, but just as a small hint.

Hi WangZP, I ran into the same issue, as well as another one with cuda. I don't know what the proper fixes for these are, but I can provide some workarounds to get these building correctly (and linking as well 🙂)

These should work for both master as well as the raahilsha-z's branch.

For qlibhull:
In the vcpkg directory, open up the ports/pcl/portfile.cmake file and remove these two lines:

  1. fix-find-qhull.patch
  2. file(REMOVE ${SOURCE_PATH}/cmake/Modules/FindQhull.cmake)

That effectively reverts the PR changes mentioned in the #17662 issue and resolves the issue for me.

For CUDA:
This one's a bit of a dodgy workaround (yay!) as it involves editing the files after a build has been initiated (it worked for me, there's no doubt a more elegant way to do it. i.e. modifying the CMakeLists.txt files themselves and removing the cuda_common entry from the SUBSYS_DEPS variable)

  1. Start a build (i.e. vcpkg install pcl[cuda]:x64-windows)
  2. In the vcpkg directory, navigate to buildtrees/pcl/x64-windows-rel/cuda (as well as the x64-windows-dbg folder if required) and edit the .pc file found in the features, sample_consensus and segmentation folders, removing the mention of pcl_cuda_common-1.12.

It's a header-only module and doesn't produce the pkg-config files that it looks for after the build for patching / post-build related tasks which causes the whole thing to fail. It still seems to work fine afterwards for me.

@JackBoosY
Copy link
Contributor

Ping @dan-shaw for review and merge this.

@JackBoosY
Copy link
Contributor

First of all, thank you very much for your efforts. I tried this version and it compiles normally under vs2019 (vs2022pre will have errors during pcl compilation install-x64-windows-rel-out.log , But I think this should be caused by 2022pre)

I encountered a problem in #17662. I am not sure if there is a problem with my configuration method, but it seems that the performance of PCL1.12 and Qhull is similar to 1.11. HAVE_QHULL is not defined and part of the lib configuration is missing after manual definition.

I'm not sure if this is the cause of upstream or other problems, but just as a small hint.

We already known this issue and reported it to the Visual Studio team.

@BillyONeal BillyONeal merged commit 8acb35e into microsoft:master Nov 5, 2021
@BillyONeal
Copy link
Member

Thanks for the updates :)

@JackBoosY
Copy link
Contributor

Okay, the pcl virtualization feature will not be blocked.

@AlexLeungZ
Copy link

I ran into another issue when installing pcl with features vtk

vcpkg.json

{
    "name": "robocon-lidar-pcl",
    "version": "1.2",
    "description": "LiDar dev with pcl for robocon.",
    "dependencies": [
        {
            "name": "pcl",
            "features": [ "vtk" ],
            "platform": "(linux & x64)"
        },
        {
            "name": "cuda",
            "features": [ ],
            "platform": "(linux & x64)"
        }
    ]
}

The issue is as same as this one: #21195
The host environment, Steps to reproduce the behavior are as same as that one except for the vcpkg.json

Is there any way to use the pcl virtualization feature?

@AlexLeungZ
Copy link

AlexLeungZ commented Nov 5, 2021

For CUDA: This one's a bit of a dodgy workaround (yay!) as it involves editing the files after a build has been initiated (it worked for me, there's no doubt a more elegant way to do it. i.e. modifying the CMakeLists.txt files themselves and removing the cuda_common entry from the SUBSYS_DEPS variable)

  1. Start a build (i.e. vcpkg install pcl[cuda]:x64-windows)
  2. In the vcpkg directory, navigate to buildtrees/pcl/x64-windows-rel/cuda (as well as the x64-windows-dbg folder if required) and edit the .pc file found in the features, sample_consensus and segmentation folders, removing the mention of pcl_cuda_common-1.12.

It's a header-only module and doesn't produce the pkg-config files that it looks for after the build for patching / post-build related tasks which causes the whole thing to fail. It still seems to work fine afterwards for me.

Thanks for the idea of fixing pcl-cuda 🙂
I got a similar approach from yours which also successfully built for x64-linux and arm64-linux

Here are the steps

  1. create a new directory in your project directory (or wherever it is) with name pkgconfig
  2. inside that directory, create a file called pcl_cuda_common-1.12.pc
  3. pcl_cuda_common-1.12.pc
prefix=${pcfiledir}/../..

exec_prefix=${prefix}
libdir=${prefix}/lib
#includedir=${prefix}/include/pcl
includedir=${prefix}/include/pcl-1.12
Name: pcl_cuda_common-1.12
Description: 
Version: 1.12.0

Libs:  
Requires: 
Cflags: -I"${includedir}" 
  1. goto .../vcpkg/ports/pcl/portfile.cmake
  2. add this line vcpkg_host_path_list(PREPEND ENV{PKG_CONFIG_PATH} "your/path/to/that/pkgconfig") before vcpkg_fixup_pkgconfig()

I don't know what will be the consequence by doing that, but at least now the code will build.

Fix for vtk[core]: #21195
Fix for pcl[vtk]: #21241

@JackBoosY
Copy link
Contributor

All the failures will be fixed in #21276.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-update The issue is with a library, which is requesting update new revision info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.

Projects

None yet