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

[opencv4] Update to 4.8.0. #33048

Merged
merged 3 commits into from
Aug 9, 2023
Merged

[opencv4] Update to 4.8.0. #33048

merged 3 commits into from
Aug 9, 2023

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Aug 8, 2023 •
edited by Cheney-W
Loading

Fixes #32463

  • 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.

vicroms
vicroms reviewed Aug 8, 2023
View reviewed changes
ports/opencv4/portfile.cmake
@@ -1,14 +1,10 @@
file(READ "${CMAKE_CURRENT_LIST_DIR}/vcpkg.json" _contents)
string(JSON OPENCV_VERSION GET "${_contents}" version)

set(USE_QT_VERSION "6")
Copy link
Member

Choose a reason for hiding this comment

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

Should this variable be documented for people wanting to override it with an overlay port?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not familiar enough with what this does to document it. I just bumped version number.

Copy link
Contributor

Choose a reason for hiding this comment

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

The usage of ${VERSION} can completely replace it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so; ${VERSION} is that of OpenCV, not of QT.

vicroms
vicroms previously approved these changes Aug 8, 2023
View reviewed changes
Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

LGTM

@naari3 naari3 mentioned this pull request Aug 8, 2023
Closed
@Cheney-W
Copy link
Contributor

Cheney-W commented Aug 9, 2023

Cenit said that he will complete this update before mid-August. #32492 (comment)

@FrankXie05 FrankXie05 added info:internal This PR or Issue was filed by the vcpkg team. category:port-update The issue is with a library, which is requesting update new revision labels Aug 9, 2023
@BillyONeal BillyONeal closed this Aug 9, 2023
@BillyONeal BillyONeal reopened this Aug 9, 2023
@BillyONeal
Copy link
Member Author

@cenit Do you want to use this as a starting point or if it's green do you just want to use it?

@Cheney-W Sorry I missed your existing PR; I could have swore I searched for it but maybe I did so in the wrong place :/

@Cheney-W Cheney-W mentioned this pull request Aug 9, 2023
Closed
7 tasks
@FrankXie05 FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Aug 9, 2023
@JavierMatosD JavierMatosD merged commit a94dafa into microsoft:master Aug 9, 2023
15 checks passed
@BillyONeal BillyONeal deleted the opencv4-8 branch August 9, 2023 17:07
@cenit
Copy link
Contributor

cenit commented Aug 9, 2023

i have a good share of updates for the port, to make it more robust and portable. It will be for a port-update since this has been merged so quickly

@JavierMatosD
Copy link
Contributor

i have a good share of updates for the port, to make it more robust and portable. It will be for a port-update since this has been merged so quickly

My bad @cenit, I didn't notice @BillyONeal's comment until after I merged :(

@LilyWangLL
Copy link
Contributor

Now the new opencv4 conflicts with flatbuffers provided by vcpkg, the related CI run: https://dev.azure.com/vcpkg/public/_build/results?buildId=92692&view=results.

The reason for the conflict is a version mismatch, opencv4 uses flatbuffers version 23.5.9 of the third-party, while vcpkg's flatbuffers is version 23.5.26.

The related codes in [opencv4_ROOT]/modules/dnn/misc/tflite/schema_generated.h:

static_assert(FLATBUFFERS_VERSION_MAJOR == 23 &&
              FLATBUFFERS_VERSION_MINOR == 5 &&
              FLATBUFFERS_VERSION_REVISION == 9,
             "Non-compatible flatbuffers version included");

The failure log:

D:\buildtrees\opencv4\src\4.8.0-50001c828e.clean\modules\dnn\misc\tflite\schema_generated.h(12): error C2338: static_assert failed: 'Non-compatible flatbuffers version included'

@LilyWangLL
Copy link
Contributor

I submitted a PR #33141 to fix it.

@moritz-h moritz-h mentioned this pull request Aug 16, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrankXie05 FrankXie05 FrankXie05 approved these changes

@vicroms vicroms vicroms left review comments

Assignees

@FrankXie05 FrankXie05

Labels
category:port-update The issue is with a library, which is requesting update new revision info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[opencv4] update to 4.8.0
7 participants
@BillyONeal @Cheney-W @cenit @JavierMatosD @LilyWangLL @vicroms @FrankXie05