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

Update vcpkg-tool to 2023-02-16. #29664

Merged
merged 22 commits into from
Feb 24, 2023

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Feb 15, 2023

@BillyONeal BillyONeal added info:internal This PR or Issue was filed by the vcpkg team. category:infrastructure Pertaining to the CI/Testing infrastrucutre labels Feb 15, 2023
github-actions[bot]
github-actions bot previously approved these changes Feb 15, 2023
@autoantwort
Copy link
Contributor

You probably also have to merge microsoft/vcpkg-tool#906 to get a green ci.

github-actions[bot]
github-actions bot previously approved these changes Feb 16, 2023
@BillyONeal BillyONeal changed the title Update vcpkg-tool to 2023-02-15. Update vcpkg-tool to 2023-02-16. Feb 16, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Feb 17, 2023

The new CI regressions are expected, due to unblocking of port builds by no longer pulling in unsupported features via unsupported features and ports. This is the work which started with #28619, lead to microsoft/vcpkg-tool#846, but was ignored in the course of merging microsoft/vcpkg-tool#856 which unblocked even more ports.

As a quick mitigation, I suggest to compare with the CI build before this PR, and then add expected failures to the CI baseline.

@walbourn
Copy link
Member

Verified this update resolves all the issues with the new xbox supports term.

…CPKG_POLICY_SKIP_DUMPBIN_CHECKS now that checking for msvc no longer requires dumpbin.
@BillyONeal
Copy link
Member Author

The new CI regressions are expected, due to unblocking of port builds by no longer pulling in unsupported features via unsupported features and ports. This is the work which started with #28619, lead to microsoft/vcpkg-tool#846, but was ignored in the course of merging microsoft/vcpkg-tool#856 which unblocked even more ports.

Yep! Also the dumpbin dependency removal resulted in post-build checks being more accurate in a few places.

As a quick mitigation, I suggest to compare with the CI build before this PR, and then add expected failures to the CI baseline.

I'm verifying for each case.

@BillyONeal
Copy link
Member Author

Saving this screenshot demonstrating why I changed vtk:

image

Upstream has:
```
// in buildtrees\harfbuzz\src\6.0.0-21fda9578f.clean\src\hb-ft.h
#if defined(WINAPI_FAMILY) && (WINAPI_FAMILY != WINAPI_FAMILY_DESKTOP_APP)
#define generic GenericFromFreeTypeLibrary
#endif
```
resulting in
```
../src/6.0.0-21fda9578f.clean/src/hb-ft.cc(985): error C2039: 'generic': is not a member of 'FT_FaceRec_'
C:\Dev\vcpkg\installed\x64-uwp\include\freetype/freetype.h(1077): note: see declaration of 'FT_FaceRec_'
../src/6.0.0-21fda9578f.clean/src/hb-ft.cc(1010): error C2039: 'generic': is not a member of 'FT_FaceRec_'
C:\Dev\vcpkg\installed\x64-uwp\include\freetype/freetype.h(1077): note: see declaration of 'FT_FaceRec_'
```
so this problem is an upstream issue with not supporting non-desktop Windows.
@BillyONeal
Copy link
Member Author

Similarly, what changed for nghttp2:

image

```
warning: The App Container bit must be set for Windows Store apps. The following DLLs do not have the App Container bit set:

    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libgcc_s_seh-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libgfortran-5.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libquadmath-0.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libwinpthread-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libgcc_s_seh-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libgfortran-5.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libquadmath-0.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libwinpthread-1.dll
```

This previously succeeded in CI but the resulting program could not be distributed.
@BillyONeal
Copy link
Member Author

[vcpkg-gfortran] Also not supported on UWP.

warning: The App Container bit must be set for Windows Store apps. The following DLLs do not have the App Container bit set:

    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libgcc_s_seh-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libgfortran-5.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libquadmath-0.dll
    D:\packages\vcpkg-gfortran_x64-uwp\debug\bin\libwinpthread-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libgcc_s_seh-1.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libgfortran-5.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libquadmath-0.dll
    D:\packages\vcpkg-gfortran_x64-uwp\bin\libwinpthread-1.dll

This previously succeeded in CI but the resulting program could not be distributed because it would fail the WACK. (And we could not detect the problem before because it passed skip-dumpbin-checks)

```
D:\installed\x64-uwp\include\asio/detail/impl/win_iocp_io_context.ipp(535): error C2039: 'VerSetConditionMask': is not a member of '`global namespace''
D:\installed\x64-uwp\include\asio/detail/impl/win_iocp_io_context.ipp(535): error C3861: 'VerSetConditionMask': identifier not found
D:\installed\x64-uwp\include\asio/detail/impl/win_iocp_io_context.ipp(538): error C2039: 'VerifyVersionInfo': is not a member of '`global namespace''
D:\installed\x64-uwp\include\asio/detail/impl/win_iocp_io_context.ipp(538): error C3861: 'VerifyVersionInfo': identifier not found
```
@Neumann-A
Copy link
Contributor

[vcpkg-gfortran] Also not supported on UWP.

Be aware that we had the discussion somewhere already.

@BillyONeal
Copy link
Member Author

Be aware that we had the discussion somewhere already.

I don't necessarily want to relitigate something that has been litigated before but I see little reason in claiming to people that something is available only for it to blow up in their face when they try to ship. Do you have a link to the previous discussion which may have landed on mitigations?

@Neumann-A
Copy link
Contributor

I don't necessarily want to relitigate something that has been litigated before but I see little reason in claiming to people that something is available only for it to blow up in their face when they try to ship. Do you have a link to the previous discussion which may have landed on mitigations?

#19413 (review)

seems like the result was that uwp is already broken but it was never marked as broken.

@BillyONeal
Copy link
Member Author

seems like the result was that uwp is already broken but it was never marked as broken.

Yeah there I said

Given that UWP is already broken for gfortran et al. here, don't worry about UWP interaction in whatever the solution is.

so it seems like this is just documenting that it's broken rather than actually breaking anything.

@Neumann-A
Copy link
Contributor

so it seems like this is just documenting that it's broken rather than actually breaking anything.

That is because the lapack dependency on uwp is fulfilled via clapack.

find_package(blend2d CONFIG REQUIRED)
target_link_libraries(main PRIVATE blend2d::blend2d)

Also, define BL_STATIC before any blend2d includes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be burned in in some way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is bad. But this PR isn't making that worse.

ports/monkeys-audio/vcpkg.json Outdated Show resolved Hide resolved
@BillyONeal
Copy link
Member Author

[lapack-reference]:
warning: Detected outdated dynamic CRT in the following files:
D:\packages\lapack-reference_x86-windows\debug\bin\liblapack.dll:msvcrt.dll
D:\packages\lapack-reference_x86-windows\bin\liblapack.dll:msvcrt.dll
To inspect the dll files, use:
dumpbin.exe /dependents mylibfile.dll

…2-15

# Conflicts:
#	versions/m-/monkeys-audio.json
@autoantwort
Copy link
Contributor

Hm I don't understand why it is necessary to bump the port version of every boost port.

@BillyONeal
Copy link
Member Author

Hm I don't understand why it is necessary to bump the port version of every boost port.

#29540 and #29481 touch boost-build that the entire Boost word depends on, but forgot to rerun generate-ports.ps1. Really, only those ports which transitively depend on boost-random should need to be affected here.

@BillyONeal BillyONeal dismissed github-actions[bot]’s stale review February 21, 2023 09:36

Actions can no longer approve itself.

…2-15

# Conflicts:
#	ports/openimageio/portfile.cmake
#	ports/vtk/pegtl.patch
#	versions/m-/monkeys-audio.json
#	versions/o-/openimageio.json
#	versions/v-/vtk.json
@BillyONeal
Copy link
Member Author

All VTK changes removed due to 8c0fe4a landing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants