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

[opencv4,dv-processing] no absolute paths #26586

Merged

Conversation

autoantwort
Copy link
Contributor

For #20469

opencv4's pkgconfig support is deprecated and non functional.

@BillyONeal
Copy link
Member

I am OK with this change but would like to hear @cenit 's thoughts since they've done a lot of work on the opencv stuff.

@autoantwort
Copy link
Contributor Author

@BillyONeal The pkgconfig generation was enabled in #25621

@BillyONeal
Copy link
Member

Makes sense, thanks!

@BillyONeal BillyONeal merged commit 427f956 into microsoft:master Aug 29, 2022
@cenit
Copy link
Contributor

cenit commented Aug 29, 2022

not sure the PR is ok anyway

my comment was not saying pkgconfig generation is broken at all

also, pkgconfig generation now depends on architecture, so fixup is necessary on linux anyway. Not sure why there is this race on enabling/disabling things without proper review and discussion

@cenit
Copy link
Contributor

cenit commented Aug 29, 2022

I am OK with this change but would like to hear @cenit 's thoughts since they've done a lot of work on the opencv stuff.

me and myself (joking about they) would like to thank you for the @, otherwise i would have noticed the PR only after daily ci run 😄

@cenit
Copy link
Contributor

cenit commented Aug 29, 2022

also, as always, there is no consistency between opencv3 and 4, again due to rush and no discussion. There i suppose it's still always enabled. Is it a race to provide good user experience or a race to what?

@autoantwort autoantwort deleted the opencv4_dv-processing-no-absolute branch August 30, 2022 10:16
@BillyONeal
Copy link
Member

I am OK with this change but would like to hear @cenit 's thoughts since they've done a lot of work on the opencv stuff.

me and myself (joking about they) would like to thank you for the @, otherwise i would have noticed the PR only after daily ci run 😄

I guess I've just gotten into the habit of always saying 'they' unless I know someone fairly personally.

also, as always, there is no consistency between opencv3 and 4, again due to rush and no discussion. There i suppose it's still always enabled. Is it a race to provide good user experience or a race to what?

The status quo for vcpkg is that we care a lot less about things that are not turned on in CI. We added =pass specifically because opencv has a lot of dependencies that caused it to often be in the 'firing line' so to speak. For non-canonical packages (those that conflict with the one that is turned on) like opencv3, boringssl, and friends, the status is that (1) we aren't accepting any more of these now that registries exist, and (2) the ones we still have are maintained at best effort of folks who currently use them since they get at best spot checks from us.

As for 'rush' / consistency, given that the PR that turned this on only affected opencv4 it made sense to merge the backout-to-fix-absolute-paths change also affecting only opencv4.

We have kept @autoantwort's very useful validity check in a holding pattern for more than a year and we want folks to finally have the fruits of that labor turned on.

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