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.

Sign up for GitHub

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

Jump to bottom

[vcpkg baseline][openvino] Don't pick OpenCV; cleanup #42017

Merged
merged 2 commits into from
Nov 11, 2024
Merged

[vcpkg baseline][openvino] Don't pick OpenCV; cleanup #42017

merged 2 commits into from
Nov 11, 2024

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 7, 2024

Fixes installation order bug.

This was referenced Nov 7, 2024
Open
Open
@jimwang118 jimwang118 added the category:code-cleanup This PR cleans up code, without fixing any existing bugs nor adding any features. label Nov 7, 2024
@dg0yt dg0yt mentioned this pull request Nov 7, 2024
Merged
7 tasks
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 7, 2024

Port bug, not just cleanup. And not an insignificant bug: blocking other PRs now.

@dg0yt dg0yt changed the title [openvino] Don't pick OpenCV; cleanup [vcpkg baseline][openvino] Don't pick OpenCV; cleanup Nov 7, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 7, 2024

vcpkg.ci failing, too: https://dev.azure.com/vcpkg/public/_build/results?buildId=109079&view=logs&j=f1a99d27-903c-5ce5-ab6b-a2a85be7b723&t=3d4837a4-bf80-5754-7acb-3d75317102c8

@jimwang118 jimwang118 added category:port-bug The issue is with a library, which is something the port should already support and removed category:code-cleanup This PR cleans up code, without fixing any existing bugs nor adding any features. labels Nov 8, 2024
@jimwang118
Copy link
Contributor

All feature test passed with x64-windows triplet.

@jimwang118 jimwang118 added info:reviewed Pull Request changes follow basic guidelines category:code-cleanup This PR cleans up code, without fixing any existing bugs nor adding any features. category:port-bug The issue is with a library, which is something the port should already support and removed category:port-bug The issue is with a library, which is something the port should already support labels Nov 8, 2024
sandye51
sandye51 reviewed Nov 11, 2024
View reviewed changes
ports/openvino/portfile.cmake
"-DENABLE_PYTHON=OFF"
"-DCPACK_GENERATOR=VCPKG"
"-DENABLE_JS=OFF"
"-DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}"
Copy link

Choose a reason for hiding this comment

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

I think we can also disable https://cmake.org/cmake/help/latest/module/FindWget.html (used here https://github.com/openvinotoolkit/openvino/blob/releases/2024/5/cmake/developer_package/download/download_and_check.cmake#L5)

Copy link
Contributor Author

@dg0yt dg0yt Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This PR is a minimal fix for current regression.
EDIT: Okay, it also includes a cleanup. But I won't delay the merge by new changes.

(wget in CMake, seriously? 🤦)

sandye51
sandye51 reviewed Nov 11, 2024
View reviewed changes
ports/openvino/portfile.cmake
@@ -118,21 +119,25 @@ if(CMAKE_HOST_WIN32)
list(APPEND FEATURE_OPTIONS "-DENABLE_API_VALIDATOR=OFF")
endif()

vcpkg_find_acquire_program(PKGCONFIG)
Copy link

Choose a reason for hiding this comment

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

is it possible to disable some pkg-config modules search like it's done for cmake ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No.

@vicroms vicroms merged commit d17cb8a into microsoft:master Nov 11, 2024
16 checks passed
@dg0yt dg0yt deleted the openvino branch November 11, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandye51 sandye51 sandye51 approved these changes

Assignees

@jimwang118 jimwang118

Labels
category:code-cleanup This PR cleans up code, without fixing any existing bugs nor adding any features. category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dg0yt @jimwang118 @sandye51 @vicroms