Fix symlink creation on older Windows versions

This patch ensures that the `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE` flag is not passed to `CreateSymbolicLinkW` when the developer mode registry key is not present and enabled on a supported version of Windows. This allows symlinks to be created when Bazel is run with elevated privileges, regardless of Windows version or developer mode. I also removed the dummy file creation check while refactoring that code for reuse. It seemed overly complicated vs. simply checking the registry and failing during runfiles creation if we're not admin. Please let me know if there's some subtle reason it needed to be done that way. Fixes bazelbuild#13169 - tested on Windows Server 2016. Closes bazelbuild#13488. PiperOrigin-RevId: 375035407
databricks · May 21, 2021 · 2f0927a · 2f0927a
1 parent 1544777
commit 2f0927a
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 85 deletions.
diff --git a/site/docs/windows.md b/site/docs/windows.md
@@ -32,8 +32,11 @@ 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 enables the following features:
 
 * [\-\-windows_enable_symlinks](command-line-reference.html#flag--windows_enable_symlinks)
 * [\-\-enable_runfiles](command-line-reference.html#flag--enable_runfiles)

diff --git 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
@@ -120,7 +120,7 @@ cat > "${VSTEMP}/windows_jni.bat" <<EOF
 @$pwd_drive
 @cd "$abs_pwd"
 @set TMP=$(cygpath -a -w "${VSTEMP}")
-@CL /O2 /EHsc /LD /Fe:"$(cygpath -a -w ${DLL})" /I "%TMP%" /I . ${WINDOWS_SOURCES[*]}
+@CL /O2 /EHsc /LD /Fe:"$(cygpath -a -w ${DLL})" /I "%TMP%" /I . ${WINDOWS_SOURCES[*]} /link /DEFAULTLIB:advapi32.lib
 EOF
 
 # Invoke the file and hopefully generate the .DLL .

diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
@@ -20,6 +20,7 @@
 
 #include <WinIoCtl.h>
 #include <stdint.h>  // uint8_t
+#include <versionhelpers.h>
 #include <windows.h>
 
 #include <memory>
@@ -39,6 +40,25 @@ namespace windows {
 using std::unique_ptr;
 using std::wstring;
 
+DWORD DetermineSymlinkPrivilegeFlag() {
+  DWORD val = 0;
+  DWORD valSize = sizeof(val);
+  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 ||
+      val == 0) {
+    return 0;
+  }
+  return SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
+}
+
 wstring AddUncPrefixMaybe(const wstring& path) {
   return path.empty() || IsDevNull(path.c_str()) || HasUncPrefix(path.c_str())
              ? path
@@ -446,13 +466,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
@@ -18,12 +18,12 @@
 #define WIN32_LEAN_AND_MEAN
 #endif
 
+#include <windows.h>
+
 #ifndef SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
 #define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x2
 #endif
 
-#include <windows.h>
-
 #include <memory>
 #include <string>
 
@@ -33,6 +33,16 @@ namespace windows {
 using std::unique_ptr;
 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 = DetermineSymlinkPrivilegeFlag();
+
 template <typename char_type>
 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
@@ -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
@@ -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);
@@ -394,10 +333,22 @@ class RunfilesCreator {
         if (blaze_util::IsDirectoryW(it.second.c_str())) {
           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());
+        if (!CreateSymbolicLinkW(
+                it.first.c_str(), it.second.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());
+          }
         }
       }
     }

diff --git 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",
 )