Correctly detect case insensitive conflicts #1768
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was first pointed out by @dg0yt in microsoft#1483 (comment) but somehow we missed that. It was discovered again by @ras0219-msft trying to write a post-build lint using the CI file lists of recent builds for new ports. `vcpkglib.h`: Remove SortedVector as the sort order we get here is not actually useful `commands.install.cpp`: * Change file_pack to InstalledFile with named values instead of first and second. (file_pack sounded like more than one file was involved) * Apply `std::move` a few places it was OK to do that. This also caused `extract_files_in_triplet` to get inlined into `build_list_of_installed_files`, its only caller. Note the call to `string::erase` and a move rather than making a copy of the suffix the `(const string&, size_t)` constructor. * Extract `check_for_install_conflicts` from `install_package` to make it clearer that `installed_files`, `package_files`, and `intersection` are dead after this check is complete (and thus those can be moved-from) * `stable_sort` to regroup by package name so that the sorted order within each group is preserved. Also added a comment for this. (I deleted that sort call entirely in a previous attempt because I initially read it as an attempt to sort the results of `set_intersection`, which are always already sorted. Missing that distinction is the whole reason I introduced `InstalledFile`) I want to fix that we are doing redundant `get_files_recursive` here but we don't actually have a `get_files_recursive_lexically_proximate`, only `_directories_` and `_regular_files_` versions, so will do that as a subsequent PR. Example with the unit test repro before and after: ```console PS C:\Dev\vcpkg> .\vcpkg.exe install --overlay-ports "C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports" a-conflict b-conflict --binarysource clear Computing installation plan... The following packages will be built and installed: a-conflict:x64-windows@0 -- C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\a-conflict b-conflict:x64-windows@0 -- C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\b-conflict Detecting compiler hash for triplet x64-windows... Compiler found: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe Installing 1/2 a-conflict:x64-windows@0... Building a-conflict:x64-windows@0... C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\a-conflict: info: installing overlay port from here -- Performing post-build validation Elapsed time to handle a-conflict:x64-windows: 111 ms a-conflict:x64-windows package ABI: 7d4b5f05d8fc195e77b84d5fb7722b9e23385392dd9f115632193da544222b9b Installing 2/2 b-conflict:x64-windows@0... Building b-conflict:x64-windows@0... C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\b-conflict: info: installing overlay port from here -- Performing post-build validation warning: File C:\Dev\vcpkg\installed\x64-windows\include/conflict-a-HEADER-only-MIXED.h was already present and will be overwritten warning: File C:\Dev\vcpkg\installed\x64-windows\include/CONFLICT-a-header-ONLY-mixed2.h was already present and will be overwritten Elapsed time to handle b-conflict:x64-windows: 112 ms b-conflict:x64-windows package ABI: 8010665bf1025c1dabed383a54f504013f467efc9ebbf1513380670586dbfd49 Total install time: 227 ms Installed contents are licensed to you by owners. Microsoft is not responsible for, nor does it grant any licenses to, third-party packages. Some packages did not declare an SPDX license. Check the `copyright` file for each package for more information about their licensing. a-conflict is header-only and can be used from CMake via: find_path(A_CONFLICT_INCLUDE_DIRS "CONFLICT-A-HEADER-ONLY-CAPS.h") target_include_directories(main PRIVATE ${A_CONFLICT_INCLUDE_DIRS}) b-conflict is header-only and can be used from CMake via: find_path(B_CONFLICT_INCLUDE_DIRS "CONFLICT-B-HEADER-ONLY-CAPS.h") target_include_directories(main PRIVATE ${B_CONFLICT_INCLUDE_DIRS}) All requested installations completed successfully in: 227 ms ```` Broken. ``` PS C:\Dev\vcpkg> .\vcpkg.exe x-ci-clean Clearing contents of C:\Dev\vcpkg\buildtrees Clearing contents of C:\Dev\vcpkg\installed Clearing contents of C:\Dev\vcpkg\packages PS C:\Dev\vcpkg> C:\Dev\vcpkg-tool\out\build\Win-x64-Debug-WithArtifacts\vcpkg.exe install --overlay-ports "C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports" a-conflict b-conflict --binarysource clear Computing installation plan... The following packages will be built and installed: a-conflict:x64-windows@0 -- C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\a-conflict b-conflict:x64-windows@0 -- C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\b-conflict Detecting compiler hash for triplet x64-windows... Compiler found: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe Installing 1/2 a-conflict:x64-windows@0... Building a-conflict:x64-windows@0... C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\a-conflict: info: installing overlay port from here -- Performing post-build validation Elapsed time to handle a-conflict:x64-windows: 114 ms a-conflict:x64-windows package ABI: 96c3b79bd6770ff5944d733c4d3272d5d4d1042a1a7cba65d2677aae5847226f Installing 2/2 b-conflict:x64-windows@0... Building b-conflict:x64-windows@0... C:\Dev\vcpkg-tool\azure-pipelines\e2e-ports\b-conflict: info: installing overlay port from here -- Performing post-build validation error: The following files are already installed in C:/Dev/vcpkg/installed/x64-windows and are in conflict with b-conflict:x64-windows Installed by a-conflict:x64-windows: include/CONFLICT-a-header-ONLY-mixed.h include/CONFLICT-a-header-ONLY-mixed2.h Elapsed time to handle b-conflict:x64-windows: 102 ms Please ensure you're using the latest port files with `git pull` and `vcpkg update`. Then check for known issues at: https://github.com/microsoft/vcpkg/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+b-conflict You can submit a new issue at: https://github.com/microsoft/vcpkg/issues/new?template=report-package-build-failure.md&title=%5Bb-conflict%5D+Build+error+on+x64-windows Include '[b-conflict] Build error' in your bug report title, the following version information in your bug description, and attach any relevant failure logs from above. vcpkg-tool version: 2999-12-31-unknownhash-debug vcpkg-scripts version: 6ecbbbdf31 2025-08-20 (5 days ago) ``` Also drive-by fixes the floating list being generated improperly and that more than "error" was colored red, and removes a few SortedVector uses that were immediately thrown away in test code.
|
Also the formatting bug looked like this: Installed by a-conflict:x64-windows
include/CONFLICT-HEADER-ONLY-CAPS.h
include/CONFLICT-header-ONLY-mixed.h
include/conflict-header-only-lowercase.hI didn't include more than 1 conflict in the test to show off the 'floating list' behavior because more than one conflict didn't repro the bug on shipping vcpkg.exe as easily. |
BillyONeal
commented
Aug 26, 2025
| std::vector<std::string> package_files = build_list_of_package_files(fs, package_dir); | ||
| std::vector<InstalledFile> intersection; | ||
|
|
||
| Util::sort(installed_files, InstalledFilePathCompare{}); |
There was a problem hiding this comment.
To clarify, I got rid of SortedVector here because I think showing the sort calls immediately before set_intersection is a Good Thing.
ras0219-msft
previously approved these changes
Aug 28, 2025
ras0219-msft
approved these changes
Aug 28, 2025
This was referenced Sep 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was first pointed out by @dg0yt in #1483 (comment) but somehow we missed that. It was discovered again by @ras0219-msft trying to write a post-build lint using the CI file lists of recent builds for new ports.
vcpkglib.h: Remove SortedVector as the sort order we get here is not actually usefulcommands.install.cpp:std::movea few places it was OK to do that. This also causedextract_files_in_tripletto get inlined intobuild_list_of_installed_files, its only caller. Note the call tostring::eraseand a move rather than making a copy of the suffix with the(const string&, size_t)constructor.check_for_install_conflictsfrominstall_packageto make it clearer thatinstalled_files,package_files, andintersectionare dead after this check is complete (and thus those can be moved-from)stable_sortto regroup by package name so that the sorted order within each group is preserved. Also added a comment for this. (I deleted that sort call entirely in a previous attempt because I initially read it as an attempt to sort the results ofset_intersection, which are always already sorted. Missing that distinction is the whole reason I introducedInstalledFile)I want to fix that we are doing redundant
get_files_recursivehere but we don't actually have aget_files_recursive_lexically_proximate, only_directories_and_regular_files_versions, so I did that as a subsequent PR #1770Example with the unit test repro before and after:
Broken.
Also drive-by fixes the floating list being generated improperly and that more than "error" was colored red, and removes a few SortedVector uses that were immediately thrown away in test code.