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

[vcpkg baseline][opencv4] Control intel-mkl dependency #30754

Closed
wants to merge 1 commit into from

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Apr 10, 2023

Assumed to fix the openimagio:x64-linux CI baseline regression, given the <opencv4 libs> <intel-mkl libs> argument sequence of the failing link step.

(This PR is just to resolve the baseline regression without completely disabling intel-mkl for opencv4, not to ensure that opencv4[mkl] correctly exports link libraries for downstream usage as in openimagio.)

Related: #30570.

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

@dg0yt dg0yt changed the title [vcpkg baseline][lapack] Control intel-mkl dependency [vcpkg baseline][opencv4] Control intel-mkl dependency Apr 10, 2023
@JonLiu1993 JonLiu1993 added the category:port-bug The issue is with a library, which is something the port should already support label Apr 11, 2023
@BillyONeal
Copy link
Member

Sadly I think this is an alternative :(

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Apr 11, 2023
@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 12, 2023
@BillyONeal
Copy link
Member

I don't think this fix is correct. opencv should be listening to what the blas and lapack metaports say in order to determine which blas/lapack to use, and shouldn't really be controlling something that is an alternative like this with the feature mechanism. I'm looking at a fix now.

@BillyONeal
Copy link
Member

BillyONeal commented Apr 14, 2023

I think the right fix is to always pass OPENCV_LAPACK_FIND_PACKAGE_ONLY. If someone wants to use mkl, they should overlay the lapack port to do that. What do you think?

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 14, 2023

I cannot check the details soon, but I tend to agree that the mkl feature shouldn't be there.

@BillyONeal
Copy link
Member

I cannot check the details soon, but I tend to agree that the mkl feature shouldn't be there.

OK, I validated that that change actually fixes the problem and have submitted a competing proposal here: #30883

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 17, 2023

Replaced by #30883.

@dg0yt dg0yt closed this Apr 17, 2023
@dg0yt dg0yt deleted the opencv4 branch April 17, 2023 05:05
@JonLiu1993 JonLiu1993 removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 17, 2023
BillyONeal added a commit that referenced this pull request Apr 17, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants