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

Enable pcl_simulation in osx #4325

Merged

Conversation

SergioRAgostinho
Copy link
Member

This PR enables building pcl_simulation + associated tools and Qt+VTK things on our CI.

-Werror was disabled to due OpenGl deprecation warnings.

.ci/azure-pipelines/build/macos.yaml Outdated Show resolved Hide resolved
simulation/tools/CMakeLists.txt Outdated Show resolved Hide resolved
simulation/CMakeLists.txt Outdated Show resolved Hide resolved
@SergioRAgostinho SergioRAgostinho marked this pull request as draft August 13, 2020 12:36
@SergioRAgostinho SergioRAgostinho marked this pull request as ready for review August 13, 2020 12:47
@SergioRAgostinho SergioRAgostinho marked this pull request as draft August 13, 2020 13:20
simulation/CMakeLists.txt Outdated Show resolved Hide resolved
find_package(GLEW)
if(APPLE)
# glew in homebrew installs glew-config.cmake
find_package(glew)
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
find_package(glew)
find_package(glew CONFIG)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively just use the the original? The FindGLEW.cmake, finds glew with config search internally.

simulation/CMakeLists.txt Outdated Show resolved Hide resolved
@kunaltyagi kunaltyagi added this to the pcl-1.11.1 milestone Aug 13, 2020
@@ -4,7 +4,7 @@ set(SUBSYS_DEPS common io surface kdtree features search octree visualization fi

set(build FALSE)
find_package(OpenGL)
find_package(GLEW)
find_package(GLEW CONFIG)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't GLEW be found, if you just use the original find_pacakge(GLEW) ?

Else you need to keep the:

if(APPLE)
  find_pacakge(glew CONFIG)
ELSE()
  find_pacakge(GLEW)
ENDIF()

Copy link
Member Author

Choose a reason for hiding this comment

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

It can, but it's incorrect. The package comes with links to non-existing locations in my setup.

@SergioRAgostinho SergioRAgostinho marked this pull request as ready for review August 13, 2020 15:59
@kunaltyagi kunaltyagi added the needs: author reply Specify why not closed/merged yet label Aug 13, 2020
@SergioRAgostinho
Copy link
Member Author

With the old dependencies we managed to build all this

-- The following subsystems will be built:
--   common
--   kdtree
--   octree
--   search
--   sample_consensus
--   filters
--   2d
--   geometry
--   io
--   features
--   ml
--   segmentation
--   visualization
--   surface
--   registration
--   keypoints
--   tracking
--   recognition
--   stereo
--   apps
       building: 
       |_ 3d_rec_framework
       |_ cloud_composer
       |_ in_hand_scanner
       |_ modeler
       |_ point_cloud_editor
--   outofcore
--   examples
--   people
--   simulation
--   global_tests
--   tests_2d
--   tests_common
--   tests_features
--   tests_filters
--   tests_geometry
--   tests_io
--   tests_kdtree
--   tests_keypoints
--   tests_people
--   tests_octree
--   tests_outofcore
--   tests_recognition
--   tests_registration
--   tests_search
--   tests_surface
--   tests_segmentation
--   tests_sample_consensus
--   tests_visualization
--   tools

leaving it here to see what happens if we remove pkg-config

@SergioRAgostinho
Copy link
Member Author

the macos jobs now take more than the gcc stages to build.

@kunaltyagi
Copy link
Member

Please go ahead with the merge. Also, please merge the release commit and start the release pipeline.

@kunaltyagi kunaltyagi removed the needs: author reply Specify why not closed/merged yet label Aug 14, 2020
@SergioRAgostinho
Copy link
Member Author

I've just cleaned up the commits and rebased. I will wait for build completion before merging just to be sure.

@SergioRAgostinho SergioRAgostinho merged commit 9b34891 into PointCloudLibrary:master Aug 14, 2020
@SergioRAgostinho SergioRAgostinho added the changelog: fix Meta-information for changelog generation label Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants