[vcpkg.cmake] Support VCPKG_BUILD_TYPE triplets with VCPKG_APPLOCAL_DEPS

[rpjohnst]

A triplet that sets VCPKG_BUILD_TYPE will (ports permitting) only have one set of installed files, rather than one for debug and one for release. The VCPKG_APPLOCAL_DEPS option copies installed libraries into the build directory, but uses a $<CONFIG:Debug> generator expression to decide which set of files to use, which leads to silent failures on these triplets.

(I realized this when GTest test discovery started failing after I switched from x64-windows to x64-windows-release, because it runs the test binary from its position in the build directory, but the gtest DLLs were no longer present. A similar problem, this one outside of vcpkg's control, is that DLL dependencies built in the same CMake build can easily wind up in different build subdirectories...)

Rather than assuming $<CONFIG:Debug> implies a debug/ installed directory, handle this like CMAKE_FIND_ROOT_PATH: Pass both variants of the installed bin directory to applocal.ps1, and let it check both of them in the appropriate order.

[rpjohnst]

Not sure what's going on with the CLA check- it neither finished nor commented here.

[dg0yt]

Shouldn't the argument remain quoted?

[dg0yt]

Quotes?

[dg0yt]

Escaped quotes?

[dg0yt]

IIUC this will add , to the list of characters which must not be used in file paths.

[rpjohnst]

I've reworked this to use separate arguments to applocal.ps1 rather than a PowerShell array. This should also resolve the quoting issues, given the use of VERBATIM.

The solution for the X_VCPKG_APPLOCAL_DEPS_INSTALL is a bit less robust, but I'm not really sure how to accomplish this perfectly given the use of install(CODE ...).

[rpjohnst]

I'm a bit stuck on this arm64 failure. The error that does exist in the logs that got uploaded makes it look like pkgconf.exe got installed incorrectly in some way, but there is no copy-tool-dependencies-* log file to see what happened, and Z_VCPKG_COPY_TOOL_DEPENDENCIES_COUNT is not included either. I don't have an ARM64 machine to run this locally.

Does anyone have any suggestions on better ways to debug this?

Edit: Hm, GitHub is showing me that arm64_windows actually passed on commit 0a97440, and the failure started with the merge from master. But I don't see any changes to the failing qca port there, or anything related to applocal other than the new XBox triplets. Maybe I missed something in there?

[JonLiu1993]

I'm a bit stuck on this arm64 failure. The error that does exist in the logs that got uploaded makes it look like pkgconf.exe got installed incorrectly in some way, but there is no copy-tool-dependencies-* log file to see what happened, and Z_VCPKG_COPY_TOOL_DEPENDENCIES_COUNT is not included either. I don't have an ARM64 machine to run this locally.

Does anyone have any suggestions on better ways to debug this?

Edit: Hm, GitHub is showing me that arm64_windows actually passed on commit 0a97440, and the failure started with the merge from master. But I don't see any changes to the failing qca port there, or anything related to applocal other than the new XBox triplets. Maybe I missed something in there?

@rpjohnst, I can help you test locally, could you please tell me how to test it?

[rpjohnst]

@JonLiu1993 Sure, thanks! At this point I'm mostly wondering if a local build of qca:arm64-windows (the one that failed in the last CI run) would have any more detailed log output than what is available from Azure Pipelines, and in particular whether and how applocal.ps1 might be related to the pkgconf.exe failure that is visible in the existing logs.

[JonLiu1993]

@JonLiu1993 Sure, thanks! At this point I'm mostly wondering if a local build of qca:arm64-windows (the one that failed in the last CI run) would have any more detailed log output than what is available from Azure Pipelines, and in particular whether and how applocal.ps1 might be related to the pkgconf.exe failure that is visible in the existing logs.

qca:arm64-windows is a CI error, not related to this PR, we will fix it as soon as possible.

[BillyONeal]

I'm not convinced that this is the correct behavior:

• a debug program choosing release dependencies almost always leads to silent ABI mismatch explosions at runtime or pragma detect_mismatch errors at link time, so ever silently falling back to release bits in debug builds or vice versa is bad news
• this still doesn't actually fix msbuild because you didn't fix how the import libs and such are found
• this is touching the powershell whose days are numbered entirely post replacing applocal.ps1 with C++ 😍 vcpkg-tool#814

I can see an argument for a "I am a release only triplet please only check release locations for me" but that wouldn't ever introduce 'try one and then the other' behavior.

Tagging vcpkg-team-review for other maintainers to look at...

[JonLiu1993]

I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags.

[BillyONeal]

@dan-shaw @ras0219-msft @JavierMatosD @vicroms and I discussed this today. We affirm that this feature makes sense, but that this implementation has problems. In particular, "try one then fallback to the other" is not the correct behavior, at least on Windows.

• this still doesn't actually fix msbuild because you didn't fix how the import libs and such are found

Don't forget to address this if you decide to pick this back up (the part that tries to add *.lib to the link line).

This might require the user explicitly telling us that they want no-debug behavior. (Which might already happen with:

<PropertyGroup>
  <VcpkgConfiguration>Release</VcpkgConfiguration>
</PropertyGroup>

)

[rpjohnst]

I probably won't have time to work on this in the near future, but some context on that issue in case anyone wants to pick it up (or for future me):

I went with the fallback approach based on how CMAKE_PREFIX_PATH, CMAKE_LIBRARY_PATH, and CMAKE_FIND_ROOT_PATH work(ed at the time?):

vcpkg/scripts/buildsystems/vcpkg.cmake

Lines 411 to 428 in dcef3d5

	function(z_vcpkg_add_vcpkg_to_cmake_path list suffix)
	set(vcpkg_paths
	"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}${suffix}"
	"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug${suffix}"
	)
	if(NOT DEFINED CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE MATCHES "^[Dd][Ee][Bb][Uu][Gg]$")
	list(REVERSE vcpkg_paths) # Debug build: Put Debug paths before Release paths.
	endif()
	if(VCPKG_PREFER_SYSTEM_LIBS)
	list(APPEND "${list}" "${vcpkg_paths}")
	else()
	list(INSERT "${list}" "0" "${vcpkg_paths}") # CMake 3.15 is required for list(PREPEND ...).
	endif()
	set("${list}" "${${list}}" PARENT_SCOPE)
	endfunction()
	z_vcpkg_add_vcpkg_to_cmake_path(CMAKE_PREFIX_PATH "")
	z_vcpkg_add_vcpkg_to_cmake_path(CMAKE_LIBRARY_PATH "/lib/manual-link")
	z_vcpkg_add_vcpkg_to_cmake_path(CMAKE_FIND_ROOT_PATH "")

. Specifically, this places the debug or release path first depending on the value of CMAKE_BUILD_TYPE, which supports single-build-type triplets via fallback built into CMake and toolchains.

In normal operation with both a build and release directory however, there actually is not supposed to be any really granular fallback going on, because both directories have the relevant files. So this may already be an issue if, for example, only one of the debug or release directories gets a generated file, and the other build type is supposed to search a different directory or not use it at all. (Probably not a common way to design a library, or this would have come up before.)

The reason I used this to replace the $<CONFIG:Debug> generator expressions is that generator expressions don't have access to VCPKG_BUILD_TYPE. And I seem to remember the further issue that z_vcpkg_add_vcpkg_to_cmake_path does not have access to VCPKG_BUILD_TYPE either, which is probably why it relies on the list ordering trick in the first place. So a "proper" fix for this is probably going to involve some plumbing to let vcpkg.cmake know about VCPKG_BUILD_TYPE somehow?

[JonLiu1993]

Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done.