[vcpkg script] Ios support for meson and autotools based ports by m-kuhn · Pull Request #25670 · microsoft/vcpkg

m-kuhn · 2022-07-10T14:53:51Z

Describe the pull request

What does your PR fix?
- Improves ios support for autotools and meson based ports
- Supersedes [vcpkg] Add -isysroot args to CPPFLAGS #25201 by taking the same approach as for meson by picking up VCPKG_DETECTED_CMAKE_OSX_SYSROOT
- ~~Depends on [jemalloc|vcpkg_configure_make] update and some script fixes #25009 to be fully functional~~
Which triplets are supported/not supported? Have you updated the CI baseline?

all
Does your PR follow the maintainer guide?

Yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

m-kuhn · 2022-07-12T16:11:39Z

TODO: https://github.com/microsoft/vcpkg/pull/25670/files#diff-b92b07d5c0c681687d71463c6a368fae98e9ffedb241e2aadbde3aac688899a9L362-R379 add a condition on OR VCPKG_TARGET_IS_IOS, which indicates cross compiling even if architectures match.

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

Neumann-A · 2022-08-20T19:10:28Z

scripts/cmake/vcpkg_configure_make.cmake

i think arch flags and include like options need to be passed to CPP for all targets not just OSX

This copies the behavior of meson which depends on CMAKE_OSX_SYSROOT and VCPKG_OSX_ARCHITECTURES which are only available on osx.

Could you compare the build output of a simple cmake build and tell me if cmake automatically adds those flags. It might be required to export them from get_cmake_vars then.

There could be some magic along the lines of
#25201 alternatively. This approach is a bit less intrusive and builds on existing practices.

This approach is a bit less intrusive and builds on existing practices.

existing practice is wrong. The flags should come from cmake_get_vars and as far as I can tell they do. So the script itself is outdated.

So #25201 is preferable?

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

m-kuhn · 2022-08-21T07:18:42Z

This no longer depends on a different pr
@LilyWangLL @JackBoosY

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

m-kuhn · 2022-08-28T07:44:50Z

Windows builds fail for kglobalaccel which seems to be an unrelated issue. Depending on the presence of x11 related packages it tries to find qt5-x11extras introducing issues based on the order of dependency installation.

If it finds https://github.com/KDE/kglobalaccel/blob/v5.89.0/CMakeLists.txt#L51-L66

Proposal: adjust rule to include qt5-x11extras in kf5globalaccel/vcpkg.json with upstream rule, i.e. include on every platform but apple.

m-kuhn · 2022-08-29T11:35:27Z

@JackBoosY this is not dependent on a different PR, #26564 fixes an unrelated issue that surfaced here for reasons beyond my understanding.
On the other hand, #26520 conflicts with this PR and the two approaches should be compared in order to proceed to approval and merge.

JackBoosY · 2022-08-29T11:52:43Z

#26564 fixes an unrelated issue that surfaced here for reasons beyond my understanding.

It means we need to wait for #26564 merge first.

m-kuhn · 2022-08-29T12:42:39Z

#26520 (and possibly any world-rebuild) is in the exactly same situation and received an "approval" instead of "depends:different-pr" today.

m-kuhn · 2022-08-29T12:46:50Z

Furthermore, there are other osx build issues on both pull requests, which would ideally only be solved by one, not two persons in parallel, hence some coordination between the two approaches would be good and feedback from maintainers would help to identify which has the better potential to be continued.

JackBoosY · 2022-08-30T12:12:04Z

@BillyONeal Do you have any ideas that which PR should we pick between this PR and #26520?

BillyONeal · 2022-08-30T16:56:22Z

@BillyONeal Do you have any ideas that which PR should we pick between this PR and #26520?

Unfortunately I know very little about autotools or meson or iOS so I don't really feel qualified to add much here.

m-kuhn · 2022-08-31T13:46:44Z

I'll try to explain the changes, I think most of them are straightforward

Most changes switch conditions on "Darwin" (macOS) to also include "iOS" (either directly or by using the generic APPLE condition), so it enables existing code additionally for iOS
There is a portion which is a plain copy of meson, so I think we can trust this code to be in good shape. ~~That's a difference between this and [gettext] fix ios compile, add ios support to vcpkg_configure_make #26520 which instead takes a novel, different approach~~
The ios triplets have been extended with architecture conditions, this is analogue to what the macos triplets already have, so also a harmonization of things. That's another difference between this and [gettext] fix ios compile, add ios support to vcpkg_configure_make #26520 where for ios a different approach is used than for macos (that might be indeed a way to consider but in this case I'd propose to also switch macos handling and drop the architecture bits there)

…make_meson

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

JackBoosY · 2022-09-06T02:28:04Z

Waiting for merge #26646 and #26687

m-kuhn · 2022-09-06T09:41:31Z

Potentially core parts of this are superseded by #26617

…make_meson

github-actions

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

Details

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

ports/freexl/vcpkg.json
ports/libtasn1/vcpkg.json

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

LilyWangLL · 2022-10-08T09:03:01Z

Ping @m-kuhn for response. Is work still being done for this PR? Could you please resolve the conflicts?

m-kuhn · 2022-10-08T09:27:02Z

This has been superseded by #26617 and others

github-actions bot previously approved these changes Jul 10, 2022

View reviewed changes

m-kuhn mentioned this pull request Jul 10, 2022

[vcpkg] Add -isysroot args to CPPFLAGS #25201

Closed

m-kuhn changed the title ~~Synchronize scripts/get_cmake_vars and ports/vcpkg-cmakge-get-vars~~ Ios support for meson and autotools based ports Jul 10, 2022

JackBoosY assigned LilyWangLL Jul 11, 2022

LilyWangLL changed the title ~~Ios support for meson and autotools based ports~~ [vcpkg script] Ios support for meson and autotools based ports Jul 11, 2022

LilyWangLL added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed depends:different-pr This PR or Issue depends on a PR which has been filed labels Jul 11, 2022

m-kuhn dismissed github-actions[bot]’s stale review via f4b1530 August 20, 2022 05:06

m-kuhn force-pushed the ios_make_meson branch from a25da99 to f4b1530 Compare August 20, 2022 05:06

github-actions bot previously approved these changes Aug 20, 2022

View reviewed changes

Neumann-A reviewed Aug 20, 2022

View reviewed changes

m-kuhn dismissed github-actions[bot]’s stale review via a4d3bc8 August 21, 2022 07:14

github-actions bot approved these changes Aug 21, 2022

View reviewed changes

JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Aug 22, 2022

m-kuhn mentioned this pull request Aug 26, 2022

[gettext] fix ios compile, add ios support to vcpkg_configure_make #26520

Closed

m-kuhn closed this Aug 26, 2022

m-kuhn reopened this Aug 26, 2022

github-actions bot previously approved these changes Aug 26, 2022

View reviewed changes

m-kuhn added 5 commits August 27, 2022 14:45

Synchronize scripts/get_cmake_vars and ports/vcpkg-cmakge-get-vars

9666d33

Add VCPKG_OSX_ARCHITECTURE to ios triplets

fb17c90

meson/make improve ios support

af1e7a9

[manyports] ios support

a145a87

fixup make/meson

ff551a8

m-kuhn dismissed github-actions[bot]’s stale review via ff551a8 August 27, 2022 12:45

m-kuhn force-pushed the ios_make_meson branch from a4d3bc8 to ff551a8 Compare August 27, 2022 12:45

github-actions bot approved these changes Aug 27, 2022

View reviewed changes

m-kuhn mentioned this pull request Aug 28, 2022

[vcpkg baseline][kf5globalaccel] Fix build on windows when x11 is around #26564

Merged

JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Aug 29, 2022

JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Sep 1, 2022

Merge branch 'master' of https://github.com/microsoft/vcpkg into ios_…

76d9a90

…make_meson

github-actions bot approved these changes Sep 1, 2022

View reviewed changes

JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Sep 6, 2022

JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Sep 15, 2022

Merge branch 'master' of https://github.com/microsoft/vcpkg into ios_…

e65b4be

…make_meson

github-actions bot approved these changes Sep 15, 2022

View reviewed changes

m-kuhn marked this pull request as draft September 15, 2022 04:15

LilyWangLL added the requires:author-response label Oct 8, 2022

m-kuhn closed this Oct 8, 2022

Conversation

m-kuhn commented Jul 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

m-kuhn commented Jul 12, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Neumann-A Aug 20, 2022

Choose a reason for hiding this comment

Uh oh!

m-kuhn Aug 21, 2022

Choose a reason for hiding this comment

Uh oh!

Neumann-A Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

m-kuhn Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neumann-A Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

m-kuhn Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

m-kuhn commented Aug 21, 2022

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

m-kuhn commented Aug 28, 2022

Uh oh!

m-kuhn commented Aug 29, 2022

Uh oh!

JackBoosY commented Aug 29, 2022

Uh oh!

m-kuhn commented Aug 29, 2022

Uh oh!

m-kuhn commented Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackBoosY commented Aug 30, 2022

Uh oh!

BillyONeal commented Aug 30, 2022

Uh oh!

m-kuhn commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

JackBoosY commented Sep 6, 2022

Uh oh!

m-kuhn commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

LilyWangLL commented Oct 8, 2022

Uh oh!

m-kuhn commented Oct 8, 2022

Uh oh!

Reviewers

m-kuhn commented Jul 10, 2022 •

edited

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

m-kuhn Aug 31, 2022 •

edited

Loading

m-kuhn commented Aug 29, 2022 •

edited

Loading

m-kuhn commented Aug 31, 2022 •

edited

Loading

m-kuhn commented Sep 6, 2022 •

edited

Loading