-
Notifications
You must be signed in to change notification settings - Fork 295
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
Fix regression on windows: Env var names are case insensitive #1501
Fix regression on windows: Env var names are case insensitive #1501
Conversation
src/vcpkg/base/system.process.cpp
Outdated
@@ -661,8 +661,9 @@ namespace vcpkg | |||
{ | |||
auto pos = env_var.find('='); | |||
auto key = env_var.substr(0, pos); | |||
if (Util::Sets::contains(env_strings, key) || | |||
Util::any_of(env_prefix_string, [&](auto&& group) { return Strings::starts_with(key, group); })) | |||
if (Util::Sets::contains(env_strings, key) || Util::any_of(env_prefix_string, [&](auto&& group) { |
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.
Is Util::Sets::contains
case-insensitive? SystemRoot
is in env_strings
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.
Good catch. It too late for me 😅 Will fix that :)
Since this blew up in an important way, can you add a test? |
Seems to fix microsoft/vcpkg#41235 |
Thanks for the superb diagnostics @tmp64 and the fix @autoantwort |
Fix rtools40 build. vcpkg-tool released a change which no longer treated environment variables as case-insensitive, then removed the variables -- breaking tools which depend on those variables. Explanation: microsoft/vcpkg#41199 (comment) x-ref: ``` - microsoft/vcpkg-tool#1501 - via cameleon-rs/cameleon#193 - via microsoft/vcpkg#41154 ``` --- TYPE: NO_HISTORY DESC: Fix rtools40 build.
Fixes: ``` error: https://github.com/google/brotli/archive/v1.1.0.tar.gz: WinHttpSendRequest failed with exit code 10106 ``` Apply workaround: microsoft/vcpkg#41199 (comment) Bug commit: microsoft/vcpkg-tool@d3fc357#diff-33cf7c311a76d4a838f91b078c2f8cbc984557379f7b345a268ec6deb665a91eR666-R675 Fix commit: microsoft/vcpkg-tool@3122da7 Fix PR: microsoft/vcpkg-tool#1501 Fixes #15128 Reported-by: Viktor Szakats Closes #15133
Fixes microsoft/vcpkg#41199
Thank you @tmp64 for identifying why the problem exists :)
The old code was also case insensitive, because
get_environment_variable
is case insensitive