[vcpkg] Remove start-of-string regex check for overlay triplet name matching.…

[gorlak]

This change allows for adding a prefix to android overlay ports, which many other systems support already. The android system relies on regex matching of the triplet name to deduce the ABI, so this change just allows for that string to be anywhere in the overlay triplet string.

• What does your PR fix?

  Fixes the android system behaving differently than other systems with regards to overlay tripets and naming. Android doesn't allow overlay triplets to have prefixes. At our site we like to prefix our overlay triplets with "overlay-" to completely differentiate them from built-in and community triplets. This is currently not possible with the android system since it's ABI deduction regex patterns match on start of the triplet string (name).

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

  This change should not impact triplet support, only what the name of overlay triplets are allowed to be.

• Does your PR follow the maintainer guide?

  I think so.

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

  I have not.

[JackBoosY]

I think... that's not correct. Let's see what's @talregev @christophe-calmejane @jherico said.

[gorlak]

Hmmm, I just tested it...

>cp triplets\community\arm64-android.cmake overlay-arm64-android.cmake
>vcpkg.exe install --x-install-root=./installed --overlay-triplets=. --triplet=overlay-arm64-android libpng
Computing installation plan...
The following packages will be built and installed:
    libpng[core]:overlay-arm64-android -> 1.6.37#15
  * vcpkg-cmake[core]:x64-windows -> 2021-07-30
  * vcpkg-cmake-config[core]:x64-windows -> 2021-05-22#1
  * zlib[core]:overlay-arm64-android -> 1.2.11#12
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x64-windows...
Detecting compiler hash for triplet overlay-arm64-android...
Error: while detecting compiler information:
The log content at C:\Users\geoff.evans\Projects\gorlak-forks-vcpkg\buildtrees\detect_compiler\stdout-overlay-arm64-android.log is:
-- Configuring overlay-arm64-android
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:127 (message):
    Command failed: ninja -v
    Working Directory: C:/Users/geoff.evans/Projects/gorlak-forks-vcpkg/buildtrees/detect_compiler/overlay-arm64-android-rel/vcpkg-parallel-configure
    Error code: 1
    See logs for more information:
      C:\Users\geoff.evans\Projects\gorlak-forks-vcpkg\buildtrees\detect_compiler\config-overlay-arm64-android-out.log

Call Stack (most recent call first):
  scripts/cmake/vcpkg_configure_cmake.cmake:356 (vcpkg_execute_required_process)
  scripts/detect_compiler/portfile.cmake:18 (vcpkg_configure_cmake)
  scripts/ports.cmake:140 (include)

Error: vcpkg was unable to detect the active compiler's information. See above for the CMake failure output.

Unknown ABI for target triplet overlay-arm64-android is the error in config-overlay-arm64-android-out.log

with my patch:

>cp triplets\community\arm64-android.cmake overlay-arm64-android.cmake
>vcpkg.exe install --x-install-root=./installed --overlay-triplets=. --triplet=overlay-arm64-android sqlite3
Computing installation plan...
The following packages will be built and installed:
    sqlite3[core]:overlay-arm64-android -> 3.35.5
Detecting compiler hash for triplet overlay-arm64-android...
Could not locate cached archive: C:\Users\geoff.evans\AppData\Local\vcpkg\archives\a6\a65b07f5b417d5faea56ef02cba1b136fcb00ad248ecf548085464e9c14a261c.zip
Starting package 1/1: sqlite3:overlay-arm64-android
Building package sqlite3[core]:overlay-arm64-android...
-- [OVERLAY] Loading triplet configuration from: C:\Users\geoff.evans\Projects\gorlak-forks-vcpkg\overlay-arm64-android.cmake
-- Downloading https://sqlite.org/2021/sqlite-amalgamation-3350500.zip -> sqlite-amalgamation-3350500.zip...
-- Extracting source C:/Users/geoff.evans/Projects/gorlak-forks-vcpkg/downloads/sqlite-amalgamation-3350500.zip
-- Applying patch fix-arm-uwp.patch
-- Using source at C:/Users/geoff.evans/Projects/gorlak-forks-vcpkg/buildtrees/sqlite3/src/3350500-adf155e1e1.clean
-- Configuring overlay-arm64-android
-- Building overlay-arm64-android-dbg
-- Building overlay-arm64-android-rel
-- Performing post-build validation
-- Performing post-build validation done
Stored binary cache: C:\Users\geoff.evans\AppData\Local\vcpkg\archives\a6\a65b07f5b417d5faea56ef02cba1b136fcb00ad248ecf548085464e9c14a261c.zip
Building package sqlite3[core]:overlay-arm64-android... done
Installing package sqlite3[core]:overlay-arm64-android...
Installing package sqlite3[core]:overlay-arm64-android... done
Elapsed time for package sqlite3:overlay-arm64-android: 8.016 s

Total elapsed time: 11.4 s

The package sqlite3:overlay-arm64-android provides CMake targets:

    find_package(unofficial-sqlite3 CONFIG REQUIRED)
    target_link_libraries(main PRIVATE unofficial::sqlite3::sqlite3)

[christophe-calmejane]

Well, I don't know if the policy changed, but when I added the regex match, I was specifically told to prevent matching something before arch-android: #12634 (review)

[gorlak]

Oh are the policies listed somewhere? I don't care to debate 😊... much less get into how things should or should not be named.

The spirit of this PR is to make the system cmakes work in a similar manner to each other (at least in terms of overlay naming). Maybe that need isn't best fulfilled by loosening the naming on the android side. Perhaps we make the naming more stringent on the non-android side? So, some enforcement in the other systems? Or generalized frontend enforcement sourcing/validating the overlay triplets?

Does anyone else see utility in being able to easily identify a triplet as being an overlay triplet (vs. vcpkg-controlled triplet), or the ability to side-by-side build and compare the artifacts of overlay triplets (vs. vcpkg-controlled triplets)? If nobody sees value here, perhaps we all just walk away :D

[talregev]

Please do not remove the start from names.
As different from other port, android port is bound to names. The names represent the architecture. It like to pass parameter, or set parameter inside the triplet.
So there for, like the other port can be any name they can, android should be able do some of the flexible.
People should have suffix that describe their need for their need. So remove the start symbol is actually differentiate more from the other ports.
So I am suggesting to leave it as it, until we find a parameter to pass to the port that tell which architecture we want to compile.

[dg0yt]

The names represent the architecture. It like to pass parameter, or set parameter inside the triplet.

Triplet names are really overused for configuring the build. I don't really understand why it is necessary to parse the target triplet name when it would be fairly safe to read the triplet file (e.g. in a function scope, only exporting required variables such as VCPKG_TARGET_ARCHITECTURE to the parent scope).

I also don't see how this change might break reasonable use cases. Do we really need to worry about some custom x64-windows-arm-android not targeting android before this PR?

[JackBoosY]

The toolchain should be set according to the settings in the triplet file instead of using the triplet file name.

[dg0yt]

The toolchain should be set according to the settings in the triplet file instead of using the triplet file name.

It is only done in script mode (ports.cmake). But there is no example of that in CMake project mode - in particular when using the vcpkg cmake toolchain with a user project.

it would be fairly safe to read the triplet file (e.g. in a function scope, only exporting required variables such as VCPKG_TARGET_ARCHITECTURE to the parent scope).

I have to admit that I didn't think of triplets setting environment variables. But OTOH I wonder if it isn't desirable to configure the user project with the same environment as vcpkg (in awareness of the fact that the changes to the environment variables will be lost after CMake configuration, i.e. for the actual build).

@ras0219 Any background information why reading triplets - or configuration exported from triplets - is avoided in the cmake toolchain (for user projects)?

[JackBoosY]

I hope my colleagues will put forward more points.

[ras0219-msft]

It is intended that the name of the triplet is 100% independent from the meaning. The user should be able to make a triplet named strawberry and access all of the functionality of vcpkg, including targeting Android. Renaming a triplet should not change behavior (unless the user explicitly inflicts it upon themselves).

That unfortunately is not the case for Android -- this issue was missed during the initial PR for Android and we don't block orthogonal improvements on fixing this original sin. However, we are strongly opposed to expanding this dependency.

Given the above general concerns, this PR looks fine to me. There is no concern that "x64-windows-arm64-android" will be misinterpreted because the only way this file (android.cmake) is pulled in is if their triplet already says set(VCPKG_CMAKE_SYSTEM_NAME Android).

However, the larger long-term fix is likely to be some combination of:

1. A general-purpose mechanism for the triplet to add arbitrary flags to CMake builds
2. A better framework for handling CPU variations that are otherwise treated similarly (neon, v7 vs v6, etc). Today's VCPKG_TARGET_ARCHITECTURE is intended to be entirely orthogonal and drives evaluation of supports expressions.

Any background information why reading triplets - or configuration exported from triplets - is avoided in the cmake toolchain (for user projects)?

The triplet is intended to be equivalent to a toolchain for portfiles, and the user owns both of them (via VCPKG_CHAINLOAD_TOOLCHAIN_FILE). The assumption is that the user can simply customize the toolchain file with their own settings just as easily as changing the triplet, so there's no need to communicate between them. However, in practice we've found extreme resistance from users to having two files. We've also made the problem worse by adding and documenting many ways to customize the built-in toolchains from the triplet such that many users may not even know that they "own" the toolchain.

[JackBoosY]

@dg0yt Any objections?

[dg0yt]

@dg0yt Any objections?

No, in the context presented by @ras0219-msft: It is okay for now, but vcpkg needs to improve this in the future.