From 9816cf0f8d122cc2e0f252920348e7f5ad4cae79 Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Tue, 18 May 2021 13:46:03 -0700 Subject: [PATCH] Fix symlink creation on older Windows versions --- src/main/native/windows/BUILD | 3 + src/main/native/windows/file.cc | 29 +++++++-- src/main/native/windows/file.h | 14 +++- src/main/tools/BUILD | 6 -- src/main/tools/build-runfiles-windows.cc | 82 +++++------------------- 5 files changed, 53 insertions(+), 81 deletions(-) diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD index e8bc125c54c287..df15a45ee120bb 100644 --- a/src/main/native/windows/BUILD +++ b/src/main/native/windows/BUILD @@ -25,6 +25,9 @@ cc_library( "file.h", "util.h", ], + linkopts = [ + "-DEFAULTLIB:advapi32.lib", # RegGetValueW + ], visibility = [ "//src/main/cpp:__subpackages__", "//src/main/tools:__pkg__", diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc index 3e42f54c8ad477..532ce52d77b123 100644 --- a/src/main/native/windows/file.cc +++ b/src/main/native/windows/file.cc @@ -39,6 +39,20 @@ namespace windows { using std::unique_ptr; using std::wstring; +bool IsDeveloperModeEnabled() { + DWORD val = 0; + DWORD valSize = sizeof(val); + if (RegGetValueW( + HKEY_LOCAL_MACHINE, + L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock", + L"AllowDevelopmentWithoutDevLicense", RRF_RT_DWORD, nullptr, &val, + &valSize) != ERROR_SUCCESS) { + return false; + } + return val != 0; +} + + wstring AddUncPrefixMaybe(const wstring& path) { return path.empty() || IsDevNull(path.c_str()) || HasUncPrefix(path.c_str()) ? path @@ -446,13 +460,14 @@ int CreateSymlink(const wstring& symlink_name, const wstring& symlink_target, } if (!CreateSymbolicLinkW(name.c_str(), target.c_str(), - SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE)) { - // The flag SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE requires - // developer mode enabled, which we expect if using symbolic linking. - *error = MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"CreateSymlink", symlink_target, - L"createSymbolicLinkW failed"); - return CreateSymlinkResult::kError; + symlinkPrivilegeFlag)) { + *error = MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"CreateSymlink", symlink_target, + GetLastError() == ERROR_PRIVILEGE_NOT_HELD + ? L"createSymbolicLinkW failed (permission denied). Either " + "Windows developer mode or admin privileges are required." + : L"createSymbolicLinkW failed"); + return CreateSymlinkResult::kError; } return CreateSymlinkResult::kSuccess; } diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h index 465d8b56b977ae..29a20672b94a35 100644 --- a/src/main/native/windows/file.h +++ b/src/main/native/windows/file.h @@ -18,12 +18,12 @@ #define WIN32_LEAN_AND_MEAN #endif +#include + #ifndef SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE #define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x2 #endif -#include - #include #include @@ -33,6 +33,16 @@ namespace windows { using std::unique_ptr; using std::wstring; +bool IsDeveloperModeEnabled(); + +// The flag SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE requires +// developer mode to be enabled. If it is not enabled, or the current +// version of Windows does not support it, do not use the flag. +// The process will need to be run with elevated privileges. +const DWORD symlinkPrivilegeFlag = IsDeveloperModeEnabled() + ? SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE + : 0; + template bool HasUncPrefix(const char_type* path) { // Return true iff `path` starts with "\\?\", "\\.\", or "\??\". diff --git a/src/main/tools/BUILD b/src/main/tools/BUILD index 7bb1a332c8aabf..d73f886321224d 100644 --- a/src/main/tools/BUILD +++ b/src/main/tools/BUILD @@ -60,12 +60,6 @@ cc_binary( "//src/conditions:windows": ["build-runfiles-windows.cc"], "//conditions:default": ["build-runfiles.cc"], }), - linkopts = select({ - "//src/conditions:windows": [ - "-DEFAULTLIB:advapi32.lib", # RegGetValueW - ], - "//conditions:default": [], - }), deps = ["//src/main/cpp/util:filesystem"] + select({ "//src/conditions:windows": ["//src/main/native/windows:lib-file"], "//conditions:default": [], diff --git a/src/main/tools/build-runfiles-windows.cc b/src/main/tools/build-runfiles-windows.cc index 0f219c3a6cfe70..4386ea7514d0b7 100644 --- a/src/main/tools/build-runfiles-windows.cc +++ b/src/main/tools/build-runfiles-windows.cc @@ -129,19 +129,6 @@ bool ReadSymlink(const wstring& abs_path, wstring* target, wstring* error) { return false; } -bool IsDeveloperModeEnabled() { - DWORD val = 0; - DWORD valSize = sizeof(val); - if (RegGetValueW( - HKEY_LOCAL_MACHINE, - L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock", - L"AllowDevelopmentWithoutDevLicense", RRF_RT_DWORD, nullptr, &val, - &valSize) != ERROR_SUCCESS) { - return false; - } - return val != 0; -} - } // namespace class RunfilesCreator { @@ -211,10 +198,8 @@ class RunfilesCreator { } void CreateRunfiles() { - bool symlink_needs_privilege = - DoesCreatingSymlinkNeedAdminPrivilege(runfiles_output_base_); ScanTreeAndPrune(runfiles_output_base_); - CreateFiles(symlink_needs_privilege); + CreateFiles(); CopyManifestFile(); } @@ -247,48 +232,6 @@ class RunfilesCreator { } } - bool DoesCreatingSymlinkNeedAdminPrivilege(const wstring& runfiles_base_dir) { - // Creating symlinks without admin privilege is enabled by Developer Mode, - // available since Windows Version 1703. - if (IsDeveloperModeEnabled()) { - return false; - } - wstring dummy_link = runfiles_base_dir + L"\\dummy_link"; - wstring dummy_target = runfiles_base_dir + L"\\dummy_target"; - - // Try creating symlink with admin privilege - bool created = - CreateSymbolicLinkW(dummy_link.c_str(), dummy_target.c_str(), 0); - - // on a rare occasion the dummy_link may exist from a previous run - // retry after deleting the existing link - if (!created && GetLastError() == ERROR_ALREADY_EXISTS) { - DeleteFileOrDie(dummy_link); - created = - CreateSymbolicLinkW(dummy_link.c_str(), dummy_target.c_str(), 0); - } - - // If we couldn't create symlink, print out an error message and exit. - if (!created) { - if (GetLastError() == ERROR_PRIVILEGE_NOT_HELD) { - die(L"CreateSymbolicLinkW failed:\n%hs\n", - "Bazel needs to create symlink for building runfiles tree.\n" - "Creating symlink on Windows requires either of the following:\n" - " 1. Program is running with elevated privileges (Admin " - "rights).\n" - " 2. The system version is Windows 10 Creators Update (1703) or " - "later and " - "developer mode is enabled.", - GetLastErrorString().c_str()); - } else { - die(L"CreateSymbolicLinkW failed: %hs", GetLastErrorString().c_str()); - } - } - - DeleteFileOrDie(dummy_link); - return true; - } - // This function scan the current directory, remove all // files/symlinks/directories that are not presented in manifest file. If a // symlink already exists and points to the correct target, this function @@ -360,11 +303,7 @@ class RunfilesCreator { ::FindClose(handle); } - void CreateFiles(bool creating_symlink_needs_admin_privilege) { - DWORD privilege_flag = creating_symlink_needs_admin_privilege - ? 0 - : SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; - + void CreateFiles() { for (const auto& it : manifest_file_map) { // Ensure the parent directory exists wstring parent_dir = GetParentDirFromPath(it.first); @@ -395,9 +334,20 @@ class RunfilesCreator { create_dir = SYMBOLIC_LINK_FLAG_DIRECTORY; } if (!CreateSymbolicLinkW(it.first.c_str(), it.second.c_str(), - privilege_flag | create_dir)) { - die(L"CreateSymbolicLinkW failed (%s -> %s): %hs", it.first.c_str(), - it.second.c_str(), GetLastErrorString().c_str()); + bazel::windows::symlinkPrivilegeFlag | create_dir)) { + + if (GetLastError() == ERROR_PRIVILEGE_NOT_HELD) { + die(L"CreateSymbolicLinkW failed:\n%hs\n", + "Bazel needs to create symlinks to build the runfiles tree.\n" + "Creating symlinks on Windows requires one of the following:\n" + " 1. Bazel is run with administrator privileges.\n" + " 2. The system version is Windows 10 Creators Update (1703) or " + "later and developer mode is enabled.", + GetLastErrorString().c_str()); + } else { + die(L"CreateSymbolicLinkW failed (%s -> %s): %hs", it.first.c_str(), + it.second.c_str(), GetLastErrorString().c_str()); + } } } }