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

[opencascade] Fix usage issue #33252

Merged
merged 2 commits into from
Aug 25, 2023
Merged

Conversation

Cheney-W
Copy link
Contributor

Fixes #33138

  1. Add additional header file paths.
  2. Add conditions to control whether to search for eigen3. Perhaps eigen3 can be treated as a feature of opencascade.
  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@Cheney-W Cheney-W added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. labels Aug 18, 2023
@Cheney-W Cheney-W marked this pull request as ready for review August 21, 2023 01:56
LilyWangLL
LilyWangLL previously approved these changes Aug 21, 2023
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Aug 21, 2023
@dan-shaw
Copy link
Contributor

I am not exactly sure what this PR fixes. Isn't eigen3 not a dependency of opencascade since the default option is OFF in the CMakeLists.txt?

@dan-shaw dan-shaw marked this pull request as draft August 21, 2023 21:42
@dan-shaw dan-shaw removed the info:reviewed Pull Request changes follow basic guidelines label Aug 21, 2023
@Cheney-W
Copy link
Contributor Author

Yes, as can be seen from the cmakelists.txt, eigen3 is not a dependency of opencascade.
The issue arises because the fix-depend-freetype.patch patch added the find_dependency(Eigen3 REQUIRED) command for OpenCASCADEConfig.cmake.in file, leading to the OpenCASCADEConfig.cmak file always executing find_dependency(Eigen3 REQUIRED), which results in the following error:

Could not find a package configuration file provided by "Eigen3" with any of the following names:
Eigen3Config.cmake
eigen3-config.cmake

To fix this, I didn't remove find_dependency(Eigen3 REQUIRED) from the patch. Instead, I added a condition if(@USE_EIGEN@) to check whether eigen3 is enabled. This way, if eigen3 is added as a feature for opencascade in the future, we wouldn't need to add the related find_dependency() again.

If you think this approach is not appropriate, I can remove find_dependency(Eigen3 REQUIRED) from the patch.

@LilyWangLL LilyWangLL marked this pull request as ready for review August 25, 2023 02:12
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Aug 25, 2023
@dan-shaw dan-shaw merged commit 250ea54 into microsoft:master Aug 25, 2023
15 checks passed
@Cheney-W Cheney-W deleted the Dev/Cheney/33138 branch August 28, 2023 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenCASCADE]dependen on eigen3 and miss "OpenGl_khrplatform.h".
4 participants