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

[libjpeg-turbo] Enabled jpeg7 and jpeg8 APIs #14483

Merged
merged 7 commits into from
Dec 9, 2020

Conversation

ziriax
Copy link
Contributor

@ziriax ziriax commented Nov 9, 2020

Describe the pull request

@ziriax ziriax changed the title Enabled jpeg7 and jpeg8 APIs in libjpeg-turbo [libjpeg-turbo] Enabled jpeg7 and jpeg8 APIs Nov 9, 2020
@ziriax ziriax marked this pull request as ready for review November 9, 2020 19:39
@NancyLi1013 NancyLi1013 self-assigned this Nov 10, 2020
@NancyLi1013 NancyLi1013 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Nov 10, 2020
Copy link
Contributor

@NancyLi1013 NancyLi1013 left a comment

Choose a reason for hiding this comment

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

You should also add Feature field in CONTROL file and use vcpkg_check_features() to handle these two features in portfile.cmake file.

Please refer to these docs

https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/control-files.md
https://github.com/microsoft/vcpkg/blob/master/docs/users/manifests.md
https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/vcpkg_check_features.md

@ziriax
Copy link
Contributor Author

ziriax commented Nov 10, 2020

You should also add Feature field in CONTROL file and use vcpkg_check_features() to handle these two features in portfile.cmake file.

Please refer to these docs

https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/control-files.md
https://github.com/microsoft/vcpkg/blob/master/docs/users/manifests.md
https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/vcpkg_check_features.md

Thanks for the feedback.

I'll do my best, I have zero experience with vcpkg.

I pushed a commit that hopefully improves this.

@NancyLi1013
Copy link
Contributor

The codes that handle features look good.

Have you tested these two features? Can they work fine?

@ziriax
Copy link
Contributor Author

ziriax commented Nov 12, 2020

It seems the CI failed because some resources couldn't be downloaded:

2020-11-12T12:47:52.7611064Z -- Downloading https://gitlab.kitware.com/cmake/cmake/-/raw/ab8bd48352df060c4ead210fe30dc4736646206b/Modules/FindPkgConfig.cmake?inline=false... Failed. Status: 7;"Couldn't connect to server"
2020-11-12T12:47:52.7618928Z CMake Error at scripts/cmake/vcpkg_download_distfile.cmake:182 (message):

2020-11-12T12:57:57.9194247Z -- Downloading https://gitlab.kitware.com//paraview/visitbridge/-/archive/c2605b5c3115bc4869c76a0d8bfdd8939b59f283/visitbridge-c2605b5c3115bc4869c76a0d8bfdd8939b59f283.tar.gz... Failed. Status: 7;"Couldn't connect to server"
2020-11-12T12:57:57.9194940Z CMake Error at scripts/cmake/vcpkg_download_distfile.cmake:182 (message):

Does the CI restart automatically?

@NancyLi1013
Copy link
Contributor

Thanks for your kindly reminder. I will rerun the failed triplets and let's see the new test result.

@NancyLi1013
Copy link
Contributor

All CI checks have passed now.

Sorry for the long delay.

Have you tested these two features? Can they work fine on x86-windows and x64-windows-static?

@ziriax
Copy link
Contributor Author

ziriax commented Dec 4, 2020

All CI checks have passed now.

Sorry for the long delay.

Have you tested these two features? Can they work fine on x86-windows and x64-windows-static?

Thanks for the feedback.

No I haven't tested them yet in depth.

Can I commit tests somewhere in this repos, or should I create another repo for testing these?

@NancyLi1013
Copy link
Contributor

Can I commit tests somewhere in this repos, or should I create another repo for testing these?

You can test them manually on your machine, then update the status here. If you don't have enough time to doing this test, I'll test on my local.

@ziriax
Copy link
Contributor Author

ziriax commented Dec 7, 2020

You can test them manually on your machine, then update the status here. If you don't have enough time to doing this test, I'll test on my local.

I indeed have a lot of other work to do, so I would greatly appreciate it if you would do the tests.

@NancyLi1013 NancyLi1013 added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed requires:author-response labels Dec 7, 2020
@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Dec 8, 2020

All features have passed with the following triplets:

  • x86-windows
  • x64-windows-static

LGTM now, thanks for your PR @ziriax.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function labels Dec 8, 2020
@BillyONeal BillyONeal merged commit 7692569 into microsoft:master Dec 9, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

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:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libjpeg-turbo] Please enable jpeg7 and jpeg8 APIs
3 participants