Skip to content

fixes for using system packages #7177

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

Merged
merged 3 commits into from
May 27, 2025
Merged

Conversation

christian-rauch
Copy link
Contributor

@christian-rauch christian-rauch commented Feb 22, 2025

I was building Open3D with most system packages enabled and some features deactivated. This uncovered a couple of compilation and linking issues, such as:

  1. usingExtractZIP when WITH_MINIZIP is off
  2. using GLFW APIs that are only available from version 3.4 onwards
  3. linking OpenSSL when it has not been found before when USE_SYSTEM_CURL and USE_SYSTEM_OPENSSL are set

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Fixing the build

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

Copy link

update-docs bot commented Feb 22, 2025

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 March 8, 2025 01:00
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 @christian-rauch for these fixes! Please see one comment below.

@christian-rauch
Copy link
Contributor Author

Btw, python util/check_style.py --apply requires an old clang-format version. Also with the oldest possible setting required_clang_format_major = 14 on Ubuntu 24.04, this failed in the regex. Ubuntu returns something like Ubuntu clang-format version 18.1.3 (1ubuntu1), i.e. it prepends Ubuntu to the string.

@christian-rauch
Copy link
Contributor Author

The CI is showing errors seemingly unrelated to this PR, such as

2025-03-12T22:53:54.4354260Z �[1;33m[Open3D WARNING] Read LOG failed: unable to open file: /Users/runner/open3d_data/extract/SampleRedwoodRGBDImages/trajectory.log�[0;m
2025-03-12T22:53:54.4381880Z ./util/ci_utils.sh: line 302: 99802 Segmentation fault: 11  ./bin/tests "$unitTestFlags"

and the main branch also does not pass the CI.

@christian-rauch
Copy link
Contributor Author

@ssheorey Can you have a look at this again?

@ssheorey ssheorey self-requested a review May 23, 2025 19:25
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 @christian-rauch looks good.

I'll update with main to check if we can pass CI.

@christian-rauch
Copy link
Contributor Author

@ssheorey I remove the "WITH_MINIZIP" commit to see if this reduces test failures. I am still getting test failures without this commit, but those are the same as on the main branch, so I guess this is ok?

@ssheorey
Copy link
Member

These CI failures are OK - they are unrelated (some are github issues - out of disk space on runners).
Do you want to try again with the minizip change?

@christian-rauch
Copy link
Contributor Author

Do you want to try again with the minizip change?

No, let's merge this first. I am quite sure that the MINIZIP commit caused some additional test failures. I will send a separate PR for this to better disentangle the test failure causes.

@ssheorey ssheorey merged commit babbd7a into isl-org:main May 27, 2025
33 of 39 checks passed
@christian-rauch christian-rauch deleted the fixes branch May 27, 2025 20:51
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.

3 participants