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

Switched QHull ImGUI and GoogleTest to use FetchContent #6645

Merged
merged 5 commits into from
Apr 15, 2024

Conversation

dbs4261
Copy link
Contributor

@dbs4261 dbs4261 commented Feb 9, 2024

Changed the CMake scripts for 3rdparty dependencies to not mark files in the Open3D git repo as generated. This is because the clean target would delete them and break the build.

Motivation and Context

When running the clean target on some generators, source files not fetched by ExternalProject would be deleted. This would prevent rebuilding without using git to reset those files. See issue #6644 for details.

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

Switched GoogleTest and QHull dependencies to use FetchContent instead of ExternalProject. Removed GENERATED property from source files passed to open3d_build_3rdparty_library.

…iles at configure time. This means that the open3d_build_3rdparty_library function doesn't need to mark source files as generated which was causing them to be removed when running the clean target.
…iles at configure time. This means that the open3d_build_3rdparty_library function doesn't need to mark source files as generated which was causing them to be removed when running the clean target.
Copy link

update-docs bot commented Feb 9, 2024

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@ssheorey ssheorey self-requested a review February 13, 2024 15:32
@ssheorey
Copy link
Member

ssheorey commented Mar 7, 2024

Hi @dbs4261 please check the CI errors. Removing GENERATED property causes CMake to fail with some file not found errors for 3rd party C++ sources.

@dbs4261
Copy link
Contributor Author

dbs4261 commented Mar 19, 2024

@ssheorey How can I run the CI pipeline locally to verify my fix?

@ssheorey
Copy link
Member

This PR doesn't depend on the platform. It should be sufficient if Open3D builds for you on your machine. Try building the library (make libopen3d), python package (make pip-package) and the tests (make tests) and running the tests on your machine.

Make sure the build succeeds from an empty build folder.

dbs4261 added 2 commits March 22, 2024 12:51
…es are provided at configure time instead of build time.
…urces. Sources are now downloaded at configure time.
@dbs4261
Copy link
Contributor Author

dbs4261 commented Mar 22, 2024

Hmmm, Looks like the ubuntu-wheel build is failing because c-cache is out of space? Not sure what to do about that.

@dbs4261 dbs4261 changed the title Switched QHull and GoogleTest to use FetchContent Switched QHull ImGUI and GoogleTest to use FetchContent Mar 22, 2024
Copy link
Member

@ssheorey ssheorey left a comment

Choose a reason for hiding this comment

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

Thanks @dbs4261 for simplifying the build process!

@ssheorey
Copy link
Member

Hmmm, Looks like the ubuntu-wheel build is failing because c-cache is out of space? Not sure what to do about that.

Fixing that in a separate PR. You can ignore that for this PR.

@ssheorey ssheorey merged commit 172367e into isl-org:main Apr 15, 2024
28 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake with Ninja generator delets some 3rdparty source files
2 participants