-
Notifications
You must be signed in to change notification settings - Fork 287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make comparison of installed files case-insensitive #1483
Conversation
6f82d08
to
97098bd
Compare
97098bd
to
88a2370
Compare
src/vcpkg/commands.install.cpp
Outdated
@@ -234,11 +234,23 @@ namespace vcpkg | |||
// The VS2015 standard library requires comparison operators of T and U | |||
// to also support comparison of T and T, and of U and U, due to debug checks. | |||
#if _MSC_VER <= 1910 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't care about VS2015 so you can delete this if you want. Not worth resetting testing if that's the only change.
{ | ||
"name": "duplicate-file-a", | ||
"version": "0.0.1" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format-manifest
these.
This change leads to a CI regression in https://dev.azure.com/vcpkg/public/_build/results?buildId=107274&view=results:
|
The CI run for the tool update, https://dev.azure.com/vcpkg/public/_build/results?buildId=107231&view=results, had a warning only:
|
@dg0yt I don't believe this is a regression, I believe this was broken before and we are finally detecting that it is broken. |
Last vcpkg.ci run now, https://dev.azure.com/vcpkg/public/_build/results?buildId=107290 x64-android, case-sensitive filesystem: No error. x64-osx, case-insensitive filesystem: warning
But there is the error in that vcpkg.pr run. Only for two in three android triplets.!? |
Hm, this PR here claims to "Make comparison of installed files case-insensitive" but actually changes "case_insensitive_ascii_less", not "case_insensitive_ascii_equals". The particular case of interest has this input set (order undefined):
|
vcpkg.pr https://dev.azure.com/vcpkg/public/_build/results?buildId=107523 Errors:
All ports were built from source, due to the CMake update (?) and touching the baseline. vcpkg.ci https://dev.azure.com/vcpkg/public/_build/results?buildId=107290 Only warnings about overwriting. |
In vcpkg's registry, before microsoft/vcpkg-tool#1483 , we missed that `influxdb-cxx` conflicts with `libproxy`, because the former spells the header `include/Proxy.h` while the latter spells it `include/proxy.h`. This caused the ports to silently damage each other on case insensitive file systems, such as on Windows and macOS. Given that the names "Proxy.h", "Transport.h", and "Point.h" are all extremely likely to conflict with other users, and `influxdb-cxx` already seems to have a convention for header names to be prefixed with "InfluxDB", we, the vcpkg registry maintainers, believe it would be best if these headers were renamed to resolve the conflict.
Fixes the check that prevents two ports from installing a file with the same name by making the comparison case insensitive.