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

Move toolchain decision making entirely into the tool #350

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

ras0219-msft
Copy link
Contributor

This PR is a needed tool change to enable microsoft/vcpkg#22831 and comes with the added benefit of removing duplicate toolchain selection logic across the many helper scripts. With this, ports and helpers may always assume that VCPKG_CHAINLOAD_TOOLCHAIN_FILE is defined to the correct toolchain to pull in.

+@Neumann-A for review

@Neumann-A
Copy link
Contributor

as said in microsoft/vcpkg#22831. Do you really want to move it into the tool? I mean the triplet would be the correct location to simply set VCPKG_CHAINLOAD_TOOLCHAIN_FILE. This requires no selection variable at all and would remove any selection code. Maybe also move the toolchains into the triplet dir (maybe in a subfolder called toolchains). This would make it more visible and obvious for users how vcpkg uses toolchains (scripts/toolchains is kind of an implementation detail for me instead of being user facing which toolchains should be).
I also think that setting VCPKG_CHAINLOAD_TOOLCHAIN_FILE scales better since you don't need to add cases somewhere in the future.

I mean yeah it would be a breaking change for some custom triplets but it would be quite simple to inform users that they need to set VCPKG_CHAINLOAD_TOOLCHAIN_FILE in their custom triplets.

@ras0219-msft
Copy link
Contributor Author

I don't think we can make that breaking change any time soon -- though we could consider making it deprecated. In that light, I think this change is a pure win. Even with this change, we could add VCPKG_CHAINLOAD_TOOLCHAIN_FILE to all of the in-box triplets and everything will work out just fine. It also helps incrementally move us in that direction by allowing all other components in the system to act as though we were already there.

src/vcpkg/build.cpp Outdated Show resolved Hide resolved
Co-authored-by: Billy O'Neal <bion@microsoft.com>
@BillyONeal BillyONeal merged commit f3841c4 into microsoft:main Feb 17, 2022
@BillyONeal
Copy link
Member

I'm pretty sure this change is what broke a lot of the UWP ports in

Should we revert this for now?

@BillyONeal
Copy link
Member

Indeed:

PS C:\Dev\vcpkg> git -C ..\vcpkg-tool rev-parse HEAD
f562635b3b82e70f252844e94767d0907e2f48e9
PS C:\Dev\vcpkg> .\vcpkg.exe x-ci-clean
Starting vcpkg CI clean
Clearing contents of C:\Dev\vcpkg\buildtrees
Clearing contents of C:\Dev\vcpkg\installed
Clearing contents of C:\Dev\vcpkg\packages
Completed vcpkg CI clean
PS C:\Dev\vcpkg> ..\vcpkg-tool\out\build\x64-Debug\vcpkg.exe install expat:x64-uwp --no-binarycaching
Computing installation plan...
The following packages will be built and installed:
    expat[core]:x64-uwp -> 2.4.1
Detecting compiler hash for triplet x64-uwp...
Starting package 1/1: expat:x64-uwp
Building package expat[core]:x64-uwp...
-- Using cached libexpat-libexpat-a28238bdeebc087071777001245df1876a11f5ee.tar.gz.
-- Extracting source C:/Dev/vcpkg-downloads/libexpat-libexpat-a28238bdeebc087071777001245df1876a11f5ee.tar.gz
-- Applying patch pkgconfig.patch
-- Using source at C:/Dev/vcpkg/buildtrees/expat/src/876a11f5ee-539179dcdb.clean
-- Configuring x64-uwp
-- Building x64-uwp-dbg
-- Building x64-uwp-rel
-- Fixing pkgconfig file: C:/Dev/vcpkg/packages/expat_x64-uwp/lib/pkgconfig/expat.pc
-- Using cached msys-mingw-w64-i686-pkg-config-0.29.2-3-any.pkg.tar.zst.
-- Using cached msys-mingw-w64-i686-libwinpthread-git-9.0.0.6373.5be8fcd83-1-any.pkg.tar.zst.
-- Using msys root at C:/Dev/vcpkg-downloads/tools/msys2/9a1ec3f33446b195
-- Fixing pkgconfig file: C:/Dev/vcpkg/packages/expat_x64-uwp/debug/lib/pkgconfig/expat.pc
-- Installing: C:/Dev/vcpkg/packages/expat_x64-uwp/share/expat/copyright
-- Performing post-build validation
-- Performing post-build validation done
Installing package expat[core]:x64-uwp...
Elapsed time for package expat:x64-uwp: 28.44 s

Total elapsed time: 33.96 s

The package expat provides CMake targets:

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

PS C:\Dev\vcpkg> git -C ..\vcpkg-tool rev-parse HEAD
f3841c4f3e04885a73c4ab9db7b396b1e32f9cb6
PS C:\Dev\vcpkg> .\vcpkg.exe x-ci-clean
Starting vcpkg CI clean
Clearing contents of C:\Dev\vcpkg\buildtrees
Clearing contents of C:\Dev\vcpkg\installed
Clearing contents of C:\Dev\vcpkg\packages
Completed vcpkg CI clean
PS C:\Dev\vcpkg> ..\vcpkg-tool\out\build\x64-Debug\vcpkg.exe install expat:x64-uwp --no-binarycaching
Computing installation plan...
The following packages will be built and installed:
    expat[core]:x64-uwp -> 2.4.1
Detecting compiler hash for triplet x64-uwp...
Starting package 1/1: expat:x64-uwp
Building package expat[core]:x64-uwp...
-- Using cached libexpat-libexpat-a28238bdeebc087071777001245df1876a11f5ee.tar.gz.
-- Extracting source C:/Dev/vcpkg-downloads/libexpat-libexpat-a28238bdeebc087071777001245df1876a11f5ee.tar.gz
-- Applying patch pkgconfig.patch
-- Using source at C:/Dev/vcpkg/buildtrees/expat/src/876a11f5ee-539179dcdb.clean
-- Configuring x64-uwp
-- Building x64-uwp-dbg
-- Building x64-uwp-rel
-- Fixing pkgconfig file: C:/Dev/vcpkg/packages/expat_x64-uwp/lib/pkgconfig/expat.pc
-- Using cached msys-mingw-w64-i686-pkg-config-0.29.2-3-any.pkg.tar.zst.
-- Using cached msys-mingw-w64-i686-libwinpthread-git-9.0.0.6373.5be8fcd83-1-any.pkg.tar.zst.
-- Using msys root at C:/Dev/vcpkg-downloads/tools/msys2/9a1ec3f33446b195
-- Fixing pkgconfig file: C:/Dev/vcpkg/packages/expat_x64-uwp/debug/lib/pkgconfig/expat.pc
-- Installing: C:/Dev/vcpkg/packages/expat_x64-uwp/share/expat/copyright
-- Performing post-build validation
The following DLLs do not have the App Container bit set:

    C:/Dev/vcpkg/packages/expat_x64-uwp/debug/bin/libexpatd.dll
    C:/Dev/vcpkg/packages/expat_x64-uwp/bin/libexpat.dll

This bit is required for Windows Store apps.
Found 1 post-build check problem(s). To submit these ports to curated catalogs, please first correct the portfile:
    C:\Dev\vcpkg\ports\expat\portfile.cmake
-- Performing post-build validation done
Installing package expat[core]:x64-uwp...
Elapsed time for package expat:x64-uwp: 5.599 s

Total elapsed time: 11.23 s

The package expat provides CMake targets:

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

PS C:\Dev\vcpkg>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants