mirror of
https://github.com/aphrody-labs/vcpkg-tool.git
synced 2026-10-13 21:09:43 +00:00
Billy O'NealandCopilot
a4ef2437f9
Inserting vcpkg-tool 2026-09-26 into Visual Studio caused @sean-mcmanus 's GPT 6 Sol code review bot to re-review some previous changes. It said: > ✨Copilot (agent227): [Minor] The new x-update-baseline diff output in [vcpkg-tool PR 2057](https://github.com/microsoft/vcpkg-tool/pull/2057) builds its [version snapshot](https://github.com/microsoft/vcpkg-tool/blob/51bf87ca6e9bf3e622d84ff323bd202ab1ca0c0b/src/vcpkg/commands.update-baseline.cpp#L63-L81) with versions.emplace(action.spec.name(), ...), so a host and target action for the same port collapse to whichever entry is visited first. The versioned planner supports both specs in one action plan (see its [host-tool test](https://github.com/microsoft/vcpkg-tool/blob/51bf87ca6e9bf3e622d84ff323bd202ab1ca0c0b/src/vcpkg-test/dependencies.cpp#L2083-L2159) ) and resolves their version constraints separately. If only the discarded triplet's selected version changes on an update, the command can print There were no changes in the ports. despite a real dependency-version change. Could the snapshot key/report include the triplet when both variants are present? This is a reporting issue; the baseline update itself remains intact. Addressing this is the majority of this change: if host/target triplet diverge on selected version, we now print the whole spec rather than just the port version. I would normally describe the edge cases handled here, but I think the unit tests added in src/vcpkg-test/commands.update-baseline.cpp are a better place to start. > ✨Copilot (agent227): [Minor] The bundled release adds O_CLOEXEC to the POSIX lock handle, but [its new regression test](https://github.com/microsoft/vcpkg-tool/blob/51bf87ca6e9bf3e622d84ff323bd202ab1ca0c0b/src/vcpkg-test/files.cpp#L1141-L1177) calls lock.reset() before attempting the second lock. That destructor explicitly calls [flock(LOCK_UN)](https://github.com/microsoft/vcpkg-tool/blob/51bf87ca6e9bf3e622d84ff323bd202ab1ca0c0b/src/vcpkg/base/files.cpp#L4273-L4280) , releasing the shared open-file-description lock even when the exec'd child inherited the descriptor. I reproduced the second acquisition succeeding both with and without close-on-exec, so the test would pass after removing the production flag. Could the test let the lock-owning parent exit without explicitly unlocking while the exec'd child is still alive, then check acquisition from a separate process? That would actually protect the file-lock fix in this release. This test was a sticking point back in the review; as far as I can tell because the actual lock object is inherited by the child process there's no way to close our copy without also closing the child's copy. All suggestions to test this better go WAY out of contract trying to add stuff to cough up the `fd` and similar which I do not want to do, so I just deleted the meaningless test. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>