[vcpkg] Fix the case of current_path() before use on Windows. #13537

BillyONeal · 2020-09-15T07:40:12Z

This is a better solution than that I tried in #13144 -- rather than trying to cannoicalize the path completely, which would destroy symlinks etc; this form calls FindFirstFile on each path element to get the real case from the filesystem.

Fixes #13105.

(Note the wrong case in the CWD in the prompt, but the correct case in all the output:)

PS C:\DeV\vcPKG> .\vcpkg.exe install curl
Computing installation plan...
The following packages will be built and installed:
    curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows
  * zlib[core]:x86-windows
Additional packages (*) will be modified to complete this operation.
Detecting compiler hash for triplet x86-windows...
Starting package 1/2: zlib:x86-windows
Building package zlib[core]:x86-windows...
Using cached binary package: C:\Users\billy\AppData\Local\vcpkg/archives\c6\c61dd1bcc23348934c55f4ced77f1e4b0f353394.zipBuilding package zlib[core]:x86-windows... done
Installing package zlib[core]:x86-windows...
Installing package zlib[core]:x86-windows... done
Elapsed time for package zlib:x86-windows: 62.26 ms
Starting package 2/2: curl:x86-windows
Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows...
Could not locate cached archive: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip
-- Downloading https://github.com/curl/curl/archive/9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz...
-- Extracting source C:/Dev/vcpkg/downloads/curl-curl-9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz
-- Applying patch 0002_fix_uwp.patch
-- Applying patch 0004_nghttp2_staticlib.patch
-- Applying patch 0005_remove_imp_suffix.patch
-- Applying patch 0006_fix_tool_depends.patch
-- Applying patch 0007_disable_tool_export_curl_target.patch
-- Applying patch 0009_fix_openssl_config.patch
-- Applying patch 0010_fix_othertests_cmake.patch
-- Applying patch 0011_fix_static_build.patch
-- Using source at C:/Dev/vcpkg/buildtrees/curl/src/767f0f2a9e-91d24adee1.clean
-- Configuring x86-windows
-- Building x86-windows-dbg
-- Building x86-windows-rel
-- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/curl-config
-- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/vcpkg-cmake-wrapper.cmake
-- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/copyright
-- Performing post-build validation
-- Performing post-build validation done
Stored binary cache: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip
Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done
Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows...
Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done
Elapsed time for package curl:x86-windows: 1.299 min

Total elapsed time: 1.33 min

The package curl:x86-windows provides CMake targets:

    find_package(CURL CONFIG REQUIRED)
    target_link_libraries(main PRIVATE CURL::libcurl)

PS C:\DeV\vcPKG>

This is a better solution than that I tried in microsoft#13144 -- rather than trying to cannoicalize the path completely, which would destroy symlinks etc; this form calls FindFirstFile on each path element to get the real case from the filesystem. Fixes microsoft#13105. (Note the wrong case in the CWD in the prompt, but the correct case in all the output:) PS C:\DeV\vcPKG> .\vcpkg.exe install curl Computing installation plan... The following packages will be built and installed: curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows * zlib[core]:x86-windows Additional packages (*) will be modified to complete this operation. Detecting compiler hash for triplet x86-windows... Starting package 1/2: zlib:x86-windows Building package zlib[core]:x86-windows... Using cached binary package: C:\Users\billy\AppData\Local\vcpkg/archives\c6\c61dd1bcc23348934c55f4ced77f1e4b0f353394.zipBuilding package zlib[core]:x86-windows... done Installing package zlib[core]:x86-windows... Installing package zlib[core]:x86-windows... done Elapsed time for package zlib:x86-windows: 62.26 ms Starting package 2/2: curl:x86-windows Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... Could not locate cached archive: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip -- Downloading https://github.com/curl/curl/archive/9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz... -- Extracting source C:/Dev/vcpkg/downloads/curl-curl-9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz -- Applying patch 0002_fix_uwp.patch -- Applying patch 0004_nghttp2_staticlib.patch -- Applying patch 0005_remove_imp_suffix.patch -- Applying patch 0006_fix_tool_depends.patch -- Applying patch 0007_disable_tool_export_curl_target.patch -- Applying patch 0009_fix_openssl_config.patch -- Applying patch 0010_fix_othertests_cmake.patch -- Applying patch 0011_fix_static_build.patch -- Using source at C:/Dev/vcpkg/buildtrees/curl/src/767f0f2a9e-91d24adee1.clean -- Configuring x86-windows -- Building x86-windows-dbg -- Building x86-windows-rel -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/curl-config -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/vcpkg-cmake-wrapper.cmake -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/copyright -- Performing post-build validation -- Performing post-build validation done Stored binary cache: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done Elapsed time for package curl:x86-windows: 1.299 min Total elapsed time: 1.33 min The package curl:x86-windows provides CMake targets: find_package(CURL CONFIG REQUIRED) target_link_libraries(main PRIVATE CURL::libcurl) PS C:\DeV\vcPKG>

ras0219 · 2020-09-15T21:38:55Z

toolsrc/src/vcpkg/base/files.cpp

+        return true;
+    }
+
+    struct FindFirstOp


Might be worth making this non-movable and non-copiable to ensure older compilers don't miscompile this.

ras0219 · 2020-09-15T21:40:50Z

toolsrc/src/vcpkg/base/files.cpp

+                return GetLastError();
+            }
+
+            return 0;


Suggested change

return 0;

return ERROR_SUCCESS;

toolsrc/src/vcpkg/base/files.cpp

ras0219 · 2020-09-15T23:33:59Z

toolsrc/src/vcpkg/base/files.cpp

@@ -1197,4 +1267,105 @@ namespace vcpkg::Files
 #endif // ^^^ windows
 #endif // ^^^ std::experimental::filesystem
    }
+
+#ifdef _WIN32
+    fs::path win32_fix_path_case(const fs::path& source)


Consider banning relative paths: they're almost certainly a bug given the coding style of vcpkg

Relative paths are possible if the user supplies one on the command line. It's not difficult to support them so this component should, even though most of the time the input will be absolute.

In those cases, we would combine with the "relative-to" path (original_cwd) before washing it through this function.

Alternatively: this function could take an assumed-already-normalized root to fix relative to and return an absolute path. This could also save overall fs operations when resolving multiple subdirectories from a single root.

ras0219 · 2020-09-15T23:36:42Z

toolsrc/src/vcpkg/base/files.cpp

+            in_progress.append(first, next_slash);
+            FindFirstOp this_find;
+            unsigned long last_error = this_find.find_first(in_progress.c_str());
+            if (last_error == 0)


Suggested change

if (last_error == 0)

if (last_error == ERROR_SUCCESS)

ras0219 · 2020-09-15T23:38:24Z

toolsrc/src/vcpkg/base/files.cpp

+                VCPKG_LINE_INFO, "Attempt to fix case of a path containing wildcards: %s", fs::u8string(source));
+        }
+
+        std::wstring in_progress;


Suggested change

std::wstring in_progress;

std::wstring in_progress;

in_progress.reserve(native.size());

ras0219 · 2020-09-15T23:43:34Z

toolsrc/src/vcpkg-test/files.cpp

+    CHECK(win32_fix_path_case(L"\\??\\c:\\WiNdOws") == L"\\??\\c:\\WiNdOws");
+    CHECK(win32_fix_path_case(L"\\\\?\\c:\\WiNdOws") == L"\\\\?\\c:\\WiNdOws");
+    CHECK(win32_fix_path_case(L"\\\\.\\c:\\WiNdOws") == L"\\\\.\\c:\\WiNdOws");
+    CHECK(win32_fix_path_case(L"c:\\/\\/Nonexistent\\/path/here") == L"C:\\Nonexistent\\path\\here");


Suggestion: add a test case for ///server/name to indicate expected behavior.

ras0219 · 2020-09-15T23:59:48Z

toolsrc/src/vcpkg/base/files.cpp

@@ -1197,4 +1267,105 @@ namespace vcpkg::Files
 #endif // ^^^ windows
 #endif // ^^^ std::experimental::filesystem
    }
+
+#ifdef _WIN32
+    fs::path win32_fix_path_case(const fs::path& source)


Anywhere we load a path from an environment variable, we need to call this function. I know there are a few cases of this in binarycaching.

Can you push a commit fixing up those cases into this branch?

toolsrc/src/vcpkg/base/files.cpp

Co-authored-by: ras0219 <[email protected]>

[vcpkg] Fix the case of current_path() before use on Windows. (microsoft#13537)

…oft#13537) * Fix the case of current_path() before use on Windows. This is a better solution than that I tried in microsoft#13144 -- rather than trying to cannoicalize the path completely, which would destroy symlinks etc; this form calls FindFirstFile on each path element to get the real case from the filesystem. Fixes microsoft#13105. (Note the wrong case in the CWD in the prompt, but the correct case in all the output:) PS C:\DeV\vcPKG> .\vcpkg.exe install curl Computing installation plan... The following packages will be built and installed: curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows * zlib[core]:x86-windows Additional packages (*) will be modified to complete this operation. Detecting compiler hash for triplet x86-windows... Starting package 1/2: zlib:x86-windows Building package zlib[core]:x86-windows... Using cached binary package: C:\Users\billy\AppData\Local\vcpkg/archives\c6\c61dd1bcc23348934c55f4ced77f1e4b0f353394.zipBuilding package zlib[core]:x86-windows... done Installing package zlib[core]:x86-windows... Installing package zlib[core]:x86-windows... done Elapsed time for package zlib:x86-windows: 62.26 ms Starting package 2/2: curl:x86-windows Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... Could not locate cached archive: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip -- Downloading https://github.com/curl/curl/archive/9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz... -- Extracting source C:/Dev/vcpkg/downloads/curl-curl-9d954e49bce3706a9a2efb119ecd05767f0f2a9e.tar.gz -- Applying patch 0002_fix_uwp.patch -- Applying patch 0004_nghttp2_staticlib.patch -- Applying patch 0005_remove_imp_suffix.patch -- Applying patch 0006_fix_tool_depends.patch -- Applying patch 0007_disable_tool_export_curl_target.patch -- Applying patch 0009_fix_openssl_config.patch -- Applying patch 0010_fix_othertests_cmake.patch -- Applying patch 0011_fix_static_build.patch -- Using source at C:/Dev/vcpkg/buildtrees/curl/src/767f0f2a9e-91d24adee1.clean -- Configuring x86-windows -- Building x86-windows-dbg -- Building x86-windows-rel -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/curl-config -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/vcpkg-cmake-wrapper.cmake -- Installing: C:/Dev/vcpkg/packages/curl_x86-windows/share/curl/copyright -- Performing post-build validation -- Performing post-build validation done Stored binary cache: C:\Users\billy\AppData\Local\vcpkg/archives\9f\9fa16d473c9539e9ea7ab3922eff46618f3a4c4b.zip Building package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... Installing package curl[core,non-http,schannel,ssl,sspi,winssl]:x86-windows... done Elapsed time for package curl:x86-windows: 1.299 min Total elapsed time: 1.33 min The package curl:x86-windows provides CMake targets: find_package(CURL CONFIG REQUIRED) target_link_libraries(main PRIVATE CURL::libcurl) PS C:\DeV\vcPKG> * Fix *nix builds. * PR feedback. * Update toolsrc/src/vcpkg/base/files.cpp Co-authored-by: ras0219 <[email protected]>

BillyONeal added 2 commits September 15, 2020 00:39

Fix *nix builds.

a9f4d6c

ras0219 approved these changes Sep 16, 2020

View reviewed changes

JackBoosY assigned JackBoosY and PhoebeHui Sep 16, 2020

JackBoosY mentioned this pull request Sep 17, 2020

qt5:x64-windows build failure #13438

Closed

PhoebeHui added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. labels Sep 17, 2020

BillyONeal added 2 commits September 21, 2020 16:31

Merge remote-tracking branch 'origin/master' into normalize_path_case

d7a117c

PR feedback.

d9b457e

ras0219 approved these changes Sep 22, 2020

View reviewed changes

toolsrc/src/vcpkg/base/files.cpp Outdated Show resolved Hide resolved

Update toolsrc/src/vcpkg/base/files.cpp

5ce3def

Co-authored-by: ras0219 <[email protected]>

PhoebeHui mentioned this pull request Sep 22, 2020

vcpkg_fixup_pkgconfig failed to replace the string when root path contains Upper charactor on windows #12709

Closed

PhoebeHui changed the title ~~Fix the case of current_path() before use on Windows.~~ [vcpkg] Fix the case of current_path() before use on Windows. Sep 22, 2020

Merge remote-tracking branch 'origin/master' into normalize_path_case

7717b9a

BillyONeal merged commit 5d33bd3 into microsoft:master Sep 23, 2020

BillyONeal deleted the normalize_path_case branch September 23, 2020 03:16

Jimmy-Hu added a commit to Jimmy-Hu/vcpkg that referenced this pull request Sep 23, 2020

Merge pull request #333 from microsoft/master

3f285b4

[vcpkg] Fix the case of current_path() before use on Windows. (microsoft#13537)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg] Fix the case of current_path() before use on Windows. #13537

[vcpkg] Fix the case of current_path() before use on Windows. #13537

BillyONeal commented Sep 15, 2020 •

edited

Loading

ras0219 Sep 15, 2020

ras0219 Sep 15, 2020

ras0219 Sep 15, 2020

BillyONeal Sep 16, 2020

ras0219-msft Sep 17, 2020 •

edited

Loading

ras0219 Sep 15, 2020

ras0219 Sep 15, 2020

ras0219 Sep 15, 2020

ras0219 Sep 15, 2020

BillyONeal Sep 16, 2020

	std::wstring in_progress;
	std::wstring in_progress;
	in_progress.reserve(native.size());

[vcpkg] Fix the case of current_path() before use on Windows. #13537

[vcpkg] Fix the case of current_path() before use on Windows. #13537

Conversation

BillyONeal commented Sep 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ras0219-msft Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillyONeal commented Sep 15, 2020 •

edited

Loading

ras0219-msft Sep 17, 2020 •

edited

Loading