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, opencv3, opencv2] Add feature dc1394 #24595

Merged
merged 9 commits into from
Jun 10, 2022

Conversation

LilyWangLL
Copy link
Contributor

@LilyWangLL LilyWangLL commented May 7, 2022

Fixes #21781, Add feature dc1394 for opencv4 and opencv3 and opencv2. Feature dc1394 test pass on Linux.

@LilyWangLL LilyWangLL added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team. labels May 7, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

@JackBoosY
Copy link
Contributor

cc @cenit

@LilyWangLL LilyWangLL changed the title [opencv4, opencv3] Add feature dc1394 [opencv4, opencv3, opencv2] Add feature dc1394 May 7, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link
Contributor

@cenit cenit left a comment

Choose a reason for hiding this comment

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

you forgot to add the new feature to the opencv metaport

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/opencv/vcpkg.json
  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/opencv/vcpkg.json
  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

@LilyWangLL LilyWangLL marked this pull request as ready for review May 9, 2022 06:25
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label May 9, 2022
@cenit
Copy link
Contributor

cenit commented May 9, 2022

vcpkg-ci-opencv:x64-windows: cascade

this is NOT GOOD (it is not outside of this PR)
and it's a pity that this problem is still a green light in PR checks...

@LilyWangLL please verify where you added the dc1394 feature in vcpkg-ci-opencv, maybe you put it in a too-broad scope and made it impossible to build the port wherever the dependency is not supported

@cenit
Copy link
Contributor

cenit commented May 9, 2022

remove dc1394 from here

and put it here maybe

"features": [
"gtk"
],

(or even better, add a new category for !windows, since libdc1394 is supported there and not just on linux)

@cenit
Copy link
Contributor

cenit commented May 9, 2022

afterwards, please verify that vcpkg-ci-opencv is built and tested ON ALL TRIPLETS, none has to be a cascade failure!

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

afterwards, please verify that vcpkg-ci-opencv is built and tested ON ALL TRIPLETS, none has to be a cascade failure!

https://dev.azure.com/vcpkg/public/_build/results?buildId=71865&view=results

         vcpkg-ci-opencv:x86-windows:  cascade: 47a9c08c0fc66276067fbdd181fba8ae740af40fa18277b40a462f86a2021b9e
         vcpkg-ci-opencv:x64-windows:  cascade: 44957581fa12ed4fb478422e4a391e8cb85b66fd5f2047e6e3531034ff9777e3
         vcpkg-ci-opencv:x64-windows-static:  cascade: 0043ddbf763e6df8aa7e0fcedeb989372b190ade2df9320dbce8e98f68e68b4a
         vcpkg-ci-opencv:x64-windows-static-md:  cascade: 0169f30e50896ee3f1b142343b936be80adec51c6fb1992ecb57bcf4e2c68310
         vcpkg-ci-opencv:x64-uwp:  cascade: e3e8d4f64c4c83db260392b7410c2665fb85dcbdc233052ca8f979ff2c1333f1

@BillyONeal
Copy link
Member

BillyONeal commented May 9, 2022

(I didn't check all the triplets, I got through 6 and went "something is broken" and moved on.)

@BillyONeal BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels May 9, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/opencv/vcpkg.json
  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

@LilyWangLL
Copy link
Contributor Author

vcpkg-ci-opencv:x64-windows: cascade

this is NOT GOOD (it is not outside of this PR) and it's a pity that this problem is still a green light in PR checks...

@LilyWangLL please verify where you added the dc1394 feature in vcpkg-ci-opencv, maybe you put it in a too-broad scope and made it impossible to build the port wherever the dependency is not supported

remove dc1394 from here

and put it here maybe

"features": [
"gtk"
],

(or even better, add a new category for !windows, since libdc1394 is supported there and not just on linux)

This is my mistake, I have changed it. Thanks~

@cenit
Copy link
Contributor

cenit commented May 11, 2022

just by looking at the time spent by each pipeline, i saw that arm64-windows is still a cascade failure

what happened?
@BillyONeal

@cenit
Copy link
Contributor

cenit commented May 11, 2022

@LilyWangLL arm64-windows cascade failure is not your fault. It is another PR that broke it and went unnoticed...

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/opencv/vcpkg.json
  • scripts/test_ports/vcpkg-ci-opencv/vcpkg.json

Valid values for the license field can be found in the documentation

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jun 9, 2022
@ras0219-msft
Copy link
Contributor

Ok, manually validated that vcpkg-ci-opencv was successfully built for every triplet in 0de243c.

I've also opened microsoft/vcpkg-tool#579 which will (eventually) enable us to add =pass to the ci.baseline.txt to ensure "deep" ports like this don't suddenly become cascading failures when a port earlier in the tree adds a dependency.

Thanks for the PR!

@ras0219-msft ras0219-msft merged commit bde216b into microsoft:master Jun 10, 2022
@LilyWangLL LilyWangLL deleted the dev/LilyWang/PR22648 branch June 10, 2022 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist 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.

[opencv4:arm64-linux] build failure
6 participants