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

[sail] Add new port #14925

Merged
merged 21 commits into from Dec 11, 2020
Merged

[sail] Add new port #14925

merged 21 commits into from Dec 11, 2020

Conversation

HappySeaFox
Copy link
Contributor

@HappySeaFox HappySeaFox commented Dec 3, 2020

Describe the pull request

Draft PR to test a new port: SAIL.

  • What does your PR fix? Fixes #
    N/A.

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    Only Windows non-static platform is supported. CI baseline is not updated.

  • Does your PR follow the maintainer guide?
    Yes.

@ghost
Copy link

ghost commented Dec 3, 2020

CLA assistant check
All CLA requirements met.

@HappySeaFox HappySeaFox changed the title [sail] Add port, do not merge [sail] Test new port Dec 3, 2020
@HappySeaFox HappySeaFox changed the title [sail] Test new port [sail] Add new port Dec 3, 2020
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 4, 2020
ports/sail/portfile.cmake Show resolved Hide resolved
ports/sail/portfile.cmake Outdated Show resolved Hide resolved
ports/sail/portfile.cmake Outdated Show resolved Hide resolved
ports/sail/vcpkg.json Outdated Show resolved Hide resolved
ports/sail/vcpkg.json Show resolved Hide resolved
Dmitry Baryshev and others added 2 commits December 4, 2020 09:57
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
ports/sail/portfile.cmake Show resolved Hide resolved
ports/sail/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@HappySeaFox
Copy link
Contributor Author

Updated REF to master. I think it's ready for a review.

@HappySeaFox HappySeaFox marked this pull request as ready for review December 7, 2020 11:47
@HappySeaFox
Copy link
Contributor Author

@NancyLi1013 Could you please review once more?

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Dec 10, 2020

@smoked-herring

Sorry for the delay.

I noticed that there were also some *.pc files in lib/pkgconfig. So it would be better to add vcpkg_fixup_pkgconfig().

I also found that there were some *.info and *.dll files listed in bin/sail/codes. Do you know what these files are used for?

For the usage of sail, have you tested it?

@HappySeaFox HappySeaFox marked this pull request as draft December 10, 2020 09:13
@HappySeaFox
Copy link
Contributor Author

@NancyLi1013

Thanks for the review!

I noticed that there were also some *.pc files in lib/pkgconfig. So it would be better to add vcpkg_fixup_pkgconfig().

Added vcpkg_fixup_pkgconfig().

I also found that there were some *.info and *.dll files listed in bin/sail/codes. Do you know what these files are used for?

These are dynamically loaded codecs (plugins). They provide actual image decoding capabilities. Static build is not supported for now, so codecs are distributed as standalone DLL plugins.

For the usage of sail, have you tested it?

Yes, there are demo apps. I tested all of them against the port, everything works fine. The only caveat is that a user needs to copy codecs (bin/sail/codecs) and their dependencies by hand into the application directory. I also added a usage file to inform the user about this peculiarity.

@HappySeaFox HappySeaFox marked this pull request as ready for review December 10, 2020 09:49
@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Dec 11, 2020
@BillyONeal BillyONeal merged commit c3e6d93 into microsoft:master Dec 11, 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:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants