GPT 5.6 suggestions for applocal.ps1 parity - #3
Conversation
The native implementation had drifted from applocal.ps1 and the deployment hooks installed by several ports. Bring each translated operation back in line with the PowerShell behavior: * Keep the parent of installed-bin-dir as the active configuration root and detect debug from that directory's leaf name. applocal.ps1 likewise sets g_install_root directly from installedDir's parent: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/scripts/buildsystems/msbuild/applocal.ps1#L7-L8 Consequently, probe OpenNI2, Azure Kinect, Magnum, and Qt hooks relative to that active root, and select bin/magnum-d for debug without adding a second debug component. These are the paths passed by the original hook dispatch: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/scripts/buildsystems/msbuild/applocal.ps1#L117-L127 * Normalize imported names before inserting them into m_searched, and use case-insensitive comparisons for hook DLL names, plugin extensions, and Qt wildcard prefixes. This matches PowerShell's case-insensitive hashtable, -like, and -match behavior: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/scripts/buildsystems/msbuild/applocal.ps1#L104-L111 https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/qt5-base/qtdeploy.ps1#L27-L110 * Filter OpenNI2 drivers to DLL and INI files rather than copying every file in the directory: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/openni2/openni2deploy.ps1#L9-L14 * Filter Magnum plugins to DLL, CONF, and PDB files. Continue copying all three kinds, but recurse into dependencies only for DLLs because the native PE reader rejects CONF and PDB files while dumpbin simply yields no imports: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/magnum/magnumdeploy.ps1#L12-L20 Match Magnum module names case-insensitively as PowerShell does: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/magnum/magnumdeploy.ps1#L26-L39 * Resolve Qt plugin dependencies from the deployed plugin path rather than the installed source path, matching the argument passed to resolve by qtdeploy.ps1: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/qt5-base/qtdeploy.ps1#L12-L21 * Copy the Qt QML tree into the destination qml directory instead of flattening its contents into the executable directory: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/qt5-base/qtdeploy.ps1#L66-L74 * Keep Azure Kinect deployment rooted in the selected configuration, matching its hook: https://github.com/microsoft/vcpkg/blob/40a9bd4ccdf5dc14ff76d4ed47d46a226ce84a83/ports/azure-kinect-depth-engine/k4adeploy.ps1#L3-L9 Extend the applocal fixture with simultaneous release and debug trees and verify that each invocation selects the corresponding depth engine. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BillyONeal
left a comment
There was a problem hiding this comment.
I turned GPT's comments about changes it made into review comments.
| } | ||
|
|
||
| return BinaryPathDecodedInfo{maybe_installed_root, is_debug}; | ||
| Path installed_root = canonical_bin_dir.parent_path(); |
There was a problem hiding this comment.
Preserve the active configuration root instead of stripping debug, matching applocal.ps1 lines 7–8
Also:
Lines 77–79 and 194–201: Deploy Azure Kinect from the selected configuration root, matching k4adeploy.ps1 lines 3–9.
| m_fs.exists(m_installed / "bin/magnum-d/magnumdeploy.ps1", VCPKG_LINE_INFO)) | ||
| , m_qt_installed(m_fs.exists( | ||
| m_installed / (m_is_debug ? "debug/plugins/qtdeploy.ps1" : "plugins/qtdeploy.ps1"), VCPKG_LINE_INFO)) | ||
| , m_qt_installed(m_fs.exists(m_installed / "plugins/qtdeploy.ps1", VCPKG_LINE_INFO)) |
There was a problem hiding this comment.
Resolve hooks and plugins relative to that active root. This matches the paths passed by applocal.ps1 lines 117–127.
(Also lines 165-171)
| for (auto&& imported_name : imported_names) | ||
| { | ||
| if (m_searched.find(imported_name) != m_searched.end()) | ||
| const auto normalized_imported_name = Strings::ascii_to_lowercase(imported_name); |
There was a problem hiding this comment.
Normalize imported DLL names before tracking them because PowerShell hashtable keys are case-insensitive. See applocal.ps1 lines 104–111.
| const std::string& target_binary_name) | ||
| { | ||
| if (target_binary_name == "k4a.dll") | ||
| if (Strings::case_insensitive_ascii_equals(target_binary_name, "k4a.dll")) |
There was a problem hiding this comment.
- Lines 194, 209, 277–296, 321, 338–516: Use case-insensitive comparisons to match PowerShell’s
-like/-matchbehavior. Seeqtdeploy.ps1lines 27–110.
| for (auto&& c : children) | ||
| { | ||
| deploy_binary(drivers, installed_dir / "bin/OpenNI2/Drivers", c.filename().to_string()); | ||
| const auto filename = c.filename(); |
There was a problem hiding this comment.
Copy only .dll and .ini OpenNI2 drivers, matching openni2deploy.ps1 lines 9–14.
| { | ||
| deploy_binary(new_dir, magnum_plugins_dir / plugins_subdir_name, c.filename().to_string()); | ||
| resolve(c); | ||
| const auto filename = c.filename(); |
There was a problem hiding this comment.
Copy only .dll, .conf, and .pdb Magnum files, and parse dependencies only for DLLs. See magnumdeploy.ps1 lines 12–20.
| Debug::println("Deploying magnum plugins"); | ||
|
|
||
| if (target_binary_name == "MagnumAudio.dll" || target_binary_name == "MagnumAudio-d.dll") | ||
| if (Strings::case_insensitive_ascii_equals(target_binary_name, "MagnumAudio.dll") || |
There was a problem hiding this comment.
Match Magnum module names case-insensitively, consistent with magnumdeploy.ps1 lines 26–39.
| { | ||
| deploy_binary(new_dir, qt_plugins_dir / plugins_subdir_name, c_filename); | ||
| resolve(c); | ||
| resolve(new_dir / c_filename); |
There was a problem hiding this comment.
Resolve Qt dependencies from the deployed plugin path, matching qtdeploy.ps1 lines 12–21.
| if (m_fs.exists(bin_dir / "../qml", ec)) | ||
| { | ||
| m_fs.copy_regular_recursive(bin_dir / "../qml", target_binary_dir, VCPKG_LINE_INFO); | ||
| m_fs.copy_regular_recursive(bin_dir / "../qml", target_binary_dir / "qml", VCPKG_LINE_INFO); |
There was a problem hiding this comment.
Preserve the destination qml directory rather than flattening its contents, matching qtdeploy.ps1 lines 66–74.
e1cb53d
into
vicroms:fix/52458-z-applocal-debug-root
This change was written by GPT 5.6, see session: https://github.com/microsoft/vcpkg-tool/tasks/106bf187-ef2b-430e-a4cf-089e13ff057a
I was going to try to write additional tests for this but @vicroms I could have sworn you said you had some local work to validate against the original .ps1 and were working on testing for this stuff, so I figured I'd submit it to you ASAP.