From b376ea5910f23bd4c4143192d0544528150b697c Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Tue, 18 May 2021 14:09:51 -0700 Subject: [PATCH 1/7] 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()); + } } } } From cea93b5c4776ffcf540e73eb167dffda29824104 Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Tue, 18 May 2021 15:13:11 -0700 Subject: [PATCH 2/7] Replace linkopts with pragma to fix bootstrapping --- src/main/native/windows/BUILD | 3 --- src/main/native/windows/file.h | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD index df15a45ee120bb..e8bc125c54c287 100644 --- a/src/main/native/windows/BUILD +++ b/src/main/native/windows/BUILD @@ -25,9 +25,6 @@ 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.h b/src/main/native/windows/file.h index 29a20672b94a35..fc080ad3199963 100644 --- a/src/main/native/windows/file.h +++ b/src/main/native/windows/file.h @@ -27,6 +27,8 @@ #include #include +#pragma comment(lib, "advapi32.dll") //RegGetValueW + namespace bazel { namespace windows { From 0a23bd3d294eebf26ab735a5202504bbba2d641f Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Tue, 18 May 2021 15:16:27 -0700 Subject: [PATCH 3/7] Fix typo --- src/main/native/windows/file.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h index fc080ad3199963..3b1ab62ab05122 100644 --- a/src/main/native/windows/file.h +++ b/src/main/native/windows/file.h @@ -27,7 +27,7 @@ #include #include -#pragma comment(lib, "advapi32.dll") //RegGetValueW +#pragma comment(lib, "advapi32.lib") //RegGetValueW namespace bazel { namespace windows { From dd8dd379c0e31b7f88bea8c507f29c269643c81b Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Wed, 19 May 2021 13:00:24 -0700 Subject: [PATCH 4/7] Explicitly check OS version and move linker options --- src/main/native/windows/BUILD | 3 +++ src/main/native/windows/build_windows_jni.sh | 2 +- src/main/native/windows/file.cc | 16 +++++++++++----- src/main/native/windows/file.h | 8 +++----- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD index e8bc125c54c287..3ec83a8e0e6c60 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/build_windows_jni.sh b/src/main/native/windows/build_windows_jni.sh index fafb82217c2357..28d86791ac6364 100644 --- a/src/main/native/windows/build_windows_jni.sh +++ b/src/main/native/windows/build_windows_jni.sh @@ -120,7 +120,7 @@ cat > "${VSTEMP}/windows_jni.bat" < #include // uint8_t #include +#include #include #include @@ -39,17 +40,22 @@ namespace windows { using std::unique_ptr; using std::wstring; -bool IsDeveloperModeEnabled() { +DWORD DetermineSymlinkPrivilegeFlag() { DWORD val = 0; DWORD valSize = sizeof(val); - if (RegGetValueW( + if (// The unprivileged create flag was introduced in Windows 10 build 14972: + // https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/ + !IsWindowsVersionOrGreater(10, 0, 14972) + // Check if developer mode is disabled: + || RegGetValueW( HKEY_LOCAL_MACHINE, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock", L"AllowDevelopmentWithoutDevLicense", RRF_RT_DWORD, nullptr, &val, - &valSize) != ERROR_SUCCESS) { - return false; + &valSize) != ERROR_SUCCESS + || val == 0) { + return 0; } - return val != 0; + return SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; } diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h index 3b1ab62ab05122..421d97112e586f 100644 --- a/src/main/native/windows/file.h +++ b/src/main/native/windows/file.h @@ -27,8 +27,6 @@ #include #include -#pragma comment(lib, "advapi32.lib") //RegGetValueW - namespace bazel { namespace windows { @@ -37,13 +35,13 @@ using std::wstring; bool IsDeveloperModeEnabled(); +DWORD DetermineSymlinkPrivilegeFlag(); + // 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; +const DWORD symlinkPrivilegeFlag = DetermineSymlinkPrivilegeFlag(); template bool HasUncPrefix(const char_type* path) { From 8eea8e55d248a22ea470982fcedc75545bd93340 Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Wed, 19 May 2021 13:16:08 -0700 Subject: [PATCH 5/7] Add linkopts to java tools --- tools/jdk/BUILD.java_tools | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/jdk/BUILD.java_tools b/tools/jdk/BUILD.java_tools index 3a7d841e13c08d..cabc2db4ed8b64 100644 --- a/tools/jdk/BUILD.java_tools +++ b/tools/jdk/BUILD.java_tools @@ -308,6 +308,9 @@ cc_library( "java_tools/src/main/native/windows/file.h", "java_tools/src/main/native/windows/util.h", ], + linkopts = [ + "-DEFAULTLIB:advapi32.lib", + ], strip_include_prefix = "java_tools", ) From 14243c053f83de965f25ba5eac95d8384dd614b6 Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Wed, 19 May 2021 13:25:39 -0700 Subject: [PATCH 6/7] Update Windows docs --- site/docs/windows.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/docs/windows.md b/site/docs/windows.md index e243bf9fbd15b9..4d18883b1819f1 100644 --- a/site/docs/windows.md +++ b/site/docs/windows.md @@ -32,8 +32,7 @@ fsutil 8dot3name set 0 ### Enable symlink support -Some features require Bazel to create file symlink on Windows, you can allow Bazel to do that by enabling [Developer Mode](https://docs.microsoft.com/en-us/windows/uwp/get-started/enable-your-device-for-development) on Windows (Only works for Windows 10, version 1703 or newer). -After enabling the Developer Mode, you should be able to use the following features: +Some features require Bazel to be able to create file symlinks on Windows, either by enabling [Developer Mode](https://docs.microsoft.com/en-us/windows/uwp/get-started/enable-your-device-for-development) (on Windows 10 version 1703 or newer), or by running Bazel as an administrator. This will enable the following features: * [\-\-windows_enable_symlinks](command-line-reference.html#flag--windows_enable_symlinks) * [\-\-enable_runfiles](command-line-reference.html#flag--enable_runfiles) From 8726e7db437a3596ecffc59b879fddb2e2651645 Mon Sep 17 00:00:00 2001 From: Kevin Hogeland Date: Wed, 19 May 2021 13:51:42 -0700 Subject: [PATCH 7/7] Fix command arg order --- src/main/native/windows/build_windows_jni.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/native/windows/build_windows_jni.sh b/src/main/native/windows/build_windows_jni.sh index 28d86791ac6364..cc440d6cca877c 100644 --- a/src/main/native/windows/build_windows_jni.sh +++ b/src/main/native/windows/build_windows_jni.sh @@ -120,7 +120,7 @@ cat > "${VSTEMP}/windows_jni.bat" <