Merge pull request swiftlang#1751 from ahoppen/windows-fixes

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -23,7 +23,6 @@ package import ToolchainRegistry
 
 package import struct TSCBasic.AbsolutePath
 package import struct TSCBasic.RelativePath
-package import func TSCBasic.resolveSymlinks
 #else
 import BuildServerProtocol
 import Dispatch
@@ -37,7 +36,6 @@ import ToolchainRegistry
 
 import struct TSCBasic.AbsolutePath
 import struct TSCBasic.RelativePath
-import func TSCBasic.resolveSymlinks
 #endif
 
 fileprivate typealias RequestCache<Request: RequestType & Hashable> = Cache<Request, Request.Response>
@@ -96,18 +94,6 @@ fileprivate extension BuildTarget {
   }
 }
 
-fileprivate extension DocumentURI {
-  /// If this is a file URI pointing to a symlink, return the realpath of the URI, otherwise return `nil`.
-  var symlinkTarget: DocumentURI? {
-    guard let fileUrl = fileURL, let path = AbsolutePath(validatingOrNil: fileUrl.path),
-      let symlinksResolved = try? TSCBasic.resolveSymlinks(path), symlinksResolved != path
-    else {
-      return nil
-    }
-    return DocumentURI(symlinksResolved.asURL)
-  }
-}
-
 fileprivate extension InitializeBuildResponse {
   var sourceKitData: SourceKitInitializeBuildResponseData? {
     guard dataKind == nil || dataKind == .sourceKit else {

Sources/BuildSystemIntegration/CompilationDatabase.swift

@@ -21,7 +21,6 @@ package import struct TSCBasic.AbsolutePath
 package import protocol TSCBasic.FileSystem
 package import struct TSCBasic.RelativePath
 package import var TSCBasic.localFileSystem
-package import func TSCBasic.resolveSymlinks
 #else
 import BuildServerProtocol
 import Foundation
@@ -33,7 +32,10 @@ import struct TSCBasic.AbsolutePath
 import protocol TSCBasic.FileSystem
 import struct TSCBasic.RelativePath
 import var TSCBasic.localFileSystem
-import func TSCBasic.resolveSymlinks
+#endif
+
+#if os(Windows)
+import WinSDK
 #endif
 
 /// A single compilation database command.
@@ -99,7 +101,7 @@ package struct CompilationDatabaseCompileCommand: Equatable, Codable {
   /// it falls back to `filename`, which is more likely to be the identifier
   /// that a caller will be looking for.
   package var uri: DocumentURI {
-    if filename.hasPrefix("/") || !directory.hasPrefix("/") {
+    if filename.isAbsolutePath || !directory.isAbsolutePath {
       return DocumentURI(filePath: filename, isDirectory: false)
     } else {
       return DocumentURI(URL(fileURLWithPath: directory).appendingPathComponent(filename, isDirectory: false))
@@ -262,7 +264,7 @@ package struct JSONCompilationDatabase: CompilationDatabase, Equatable, Codable
     if let indices = pathToCommands[uri] {
       return indices.map { commands[$0] }
     }
-    if let fileURL = uri.fileURL, let indices = pathToCommands[DocumentURI(fileURL.resolvingSymlinksInPath())] {
+    if let fileURL = uri.fileURL, let indices = pathToCommands[DocumentURI(fileURL.realpath)] {
       return indices.map { commands[$0] }
     }
     return []
@@ -278,13 +280,8 @@ package struct JSONCompilationDatabase: CompilationDatabase, Equatable, Codable
     let uri = command.uri
     pathToCommands[uri, default: []].append(commands.count)
 
-    if let fileURL = uri.fileURL,
-      let symlinksResolved = try? resolveSymlinks(AbsolutePath(validating: fileURL.path))
-    {
-      let canonical = DocumentURI(filePath: symlinksResolved.pathString, isDirectory: false)
-      if canonical != uri {
-        pathToCommands[canonical, default: []].append(commands.count)
-      }
+    if let symlinkTarget = uri.symlinkTarget {
+      pathToCommands[symlinkTarget, default: []].append(commands.count)
     }
 
     commands.append(command)
@@ -295,3 +292,15 @@ enum CompilationDatabaseDecodingError: Error {
   case missingCommandOrArguments
   case fixedDatabaseDecodingError
 }
+
+fileprivate extension String {
+  var isAbsolutePath: Bool {
+    #if os(Windows)
+    Array(self.utf16).withUnsafeBufferPointer { buffer in
+      return !PathIsRelativeW(buffer.baseAddress)
+    }
+    #else
+    return self.hasPrefix("/")
+    #endif
+  }
+}

Sources/BuildSystemIntegration/DetermineBuildSystem.swift

@@ -57,7 +57,9 @@ package func determineBuildSystem(
         return .compilationDatabase(projectRoot: projectRoot)
       }
     case .swiftPM:
-      if let projectRoot = SwiftPMBuildSystem.projectRoot(for: workspaceFolderPath, options: options) {
+      if let projectRootURL = SwiftPMBuildSystem.projectRoot(for: workspaceFolderUrl, options: options),
+        let projectRoot = try? AbsolutePath(validating: projectRootURL.path)
+      {
         return .swiftPM(projectRoot: projectRoot)
       }
     }

Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift

@@ -26,7 +26,6 @@ import struct TSCBasic.FileSystemError
 import func TSCBasic.getEnvSearchPaths
 import var TSCBasic.localFileSystem
 import func TSCBasic.lookupExecutablePath
-import func TSCBasic.resolveSymlinks
 
 #if compiler(>=6.3)
 #warning("We have had a one year transition period to the pull based build server. Consider removing this build server")

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -15,7 +15,7 @@ package import Basics
 @preconcurrency import Build
 package import BuildServerProtocol
 import Dispatch
-import Foundation
+package import Foundation
 package import LanguageServerProtocol
 @preconcurrency import PackageGraph
 import PackageLoading
@@ -33,12 +33,10 @@ package import ToolchainRegistry
 package import struct Basics.AbsolutePath
 package import struct Basics.IdentifiableSet
 package import struct Basics.TSCAbsolutePath
-import struct Foundation.URL
-package import struct TSCBasic.AbsolutePath
-package import protocol TSCBasic.FileSystem
-package import class TSCBasic.Process
-package import var TSCBasic.localFileSystem
-package import func TSCBasic.resolveSymlinks
+import struct TSCBasic.AbsolutePath
+import protocol TSCBasic.FileSystem
+import class TSCBasic.Process
+import var TSCBasic.localFileSystem
 package import class ToolchainRegistry.Toolchain
 #else
 import Basics
@@ -68,7 +66,6 @@ import struct TSCBasic.AbsolutePath
 import protocol TSCBasic.FileSystem
 import class TSCBasic.Process
 import var TSCBasic.localFileSystem
-import func TSCBasic.resolveSymlinks
 import class ToolchainRegistry.Toolchain
 #endif
 
@@ -240,26 +237,18 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
 
   private var targetDependencies: [BuildTargetIdentifier: Set<BuildTargetIdentifier>] = [:]
 
-  static package func projectRoot(
-    for path: TSCBasic.AbsolutePath,
-    options: SourceKitLSPOptions
-  ) -> TSCBasic.AbsolutePath? {
-    guard var path = try? resolveSymlinks(path) else {
-      return nil
-    }
+  static package func projectRoot(for path: URL, options: SourceKitLSPOptions) -> URL? {
+    var path = path.realpath
     while true {
       let packagePath = path.appending(component: "Package.swift")
-      if localFileSystem.isFile(packagePath) {
-        let contents = try? localFileSystem.readFileContents(packagePath)
-        if contents?.cString.contains("PackageDescription") == true {
-          return path
-        }
+      if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
+        return path
       }
 
-      if path.isRoot {
-        return nil
+      if (try? AbsolutePath(validating: path.path))?.isRoot ?? true {
+        break
       }
-      path = path.parentDirectory
+      path.deleteLastPathComponent()
     }
     return nil
   }
@@ -611,7 +600,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
       let buildSettings = FileBuildSettings(
         compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: swiftPMTarget),
         workingDirectory: projectRoot.pathString
-      ).patching(newFile: DocumentURI(try resolveSymlinks(path).asURL), originalFile: DocumentURI(substituteFile))
+      ).patching(newFile: DocumentURI(path.asURL.realpath), originalFile: DocumentURI(substituteFile))
       return TextDocumentSourceKitOptionsResponse(
         compilerArguments: buildSettings.compilerArguments,
         workingDirectory: buildSettings.workingDirectory

Sources/Diagnose/ReproducerBundle.swift

@@ -26,7 +26,7 @@ func makeReproducerBundle(for requestInfo: RequestInfo, toolchain: Toolchain, bu
     encoding: .utf8
   )
   if let toolchainPath = toolchain.path {
-    try toolchainPath.asURL.resolvingSymlinksInPath().path
+    try toolchainPath.asURL.realpath.path
       .write(
         to: bundlePath.appendingPathComponent("toolchain.txt"),
         atomically: true,

Sources/SKSupport/CMakeLists.txt

@@ -6,6 +6,7 @@ add_library(SKSupport STATIC
   Debouncer.swift
   Dictionary+InitWithElementsKeyedBy.swift
   DocumentURI+CustomLogStringConvertible.swift
+  DocumentURI+symlinkTarget.swift
   FileSystem.swift
   LineTable.swift
   LocalConnection.swift

Sources/SKSupport/DocumentURI+symlinkTarget.swift

@@ -0,0 +1,36 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#if compiler(>=6)
+import Foundation
+package import LanguageServerProtocol
+import struct TSCBasic.AbsolutePath
+#else
+import Foundation
+import LanguageServerProtocol
+
+import struct TSCBasic.RelativePath
+#endif
+
+extension DocumentURI {
+  /// If this is a file URI pointing to a symlink, return the realpath of the URI, otherwise return `nil`.
+  package var symlinkTarget: DocumentURI? {
+    guard let fileUrl = fileURL else {
+      return nil
+    }
+    let realpath = DocumentURI(fileUrl.realpath)
+    if realpath == self {
+      return nil
+    }
+    return realpath
+  }
+}

Sources/SKTestSupport/BuildServerTestProject.swift

@@ -17,7 +17,7 @@ import XCTest
 fileprivate let sdkArgs =
   if let defaultSDKPath {
     """
-    "-sdk", "\(defaultSDKPath)",
+    "-sdk", "\(defaultSDKPath.replacing(#"\"#, with: #"\\"#))",
     """
   } else {
     ""

Sources/SKTestSupport/FindTool.swift

@@ -48,8 +48,14 @@ package func findTool(name: String) async -> URL? {
     return nil
   }
   #if os(Windows)
-  path = String((path.split { $0.isNewline })[0])
+  // where.exe returns all files that match the name. We only care about the first one.
+  if let newlineIndex = path.firstIndex(where: \.isNewline) {
+    path = String(path[..<newlineIndex])
+  }
   #endif
   path = path.trimmingCharacters(in: .whitespacesAndNewlines)
+  if path.isEmpty {
+    return nil
+  }
   return URL(fileURLWithPath: path, isDirectory: false)
 }

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -87,6 +87,19 @@ package struct IndexedSingleSwiftFileTestProject {
 
       // The following are needed so we can import XCTest
       let sdkUrl = URL(fileURLWithPath: sdk)
+      #if os(Windows)
+      let xctestModuleDir =
+        sdkUrl
+        .deletingLastPathComponent()
+        .deletingLastPathComponent()
+        .appendingPathComponent("Library")
+        .appendingPathComponent("XCTest-development")
+        .appendingPathComponent("usr")
+        .appendingPathComponent("lib")
+        .appendingPathComponent("swift")
+        .appendingPathComponent("windows")
+      compilerArguments += ["-I", xctestModuleDir.path]
+      #else
       let usrLibDir =
         sdkUrl
         .deletingLastPathComponent()
@@ -103,6 +116,7 @@ package struct IndexedSingleSwiftFileTestProject {
         "-I", usrLibDir.path,
         "-F", frameworksDir.path,
       ]
+      #endif
     }
 
     let compilationDatabase = JSONCompilationDatabase(

Sources/SKTestSupport/MultiFileTestProject.swift

@@ -102,7 +102,10 @@ package class MultiFileTestProject {
 
     var fileData: [String: FileData] = [:]
     for (fileLocation, markedText) in files {
-      let markedText = markedText.replacingOccurrences(of: "$TEST_DIR", with: scratchDirectory.path)
+      let markedText =
+        markedText
+        .replacingOccurrences(of: "$TEST_DIR_URL", with: scratchDirectory.absoluteString)
+        .replacingOccurrences(of: "$TEST_DIR", with: scratchDirectory.path)
       let fileURL = fileLocation.url(relativeTo: scratchDirectory)
       try FileManager.default.createDirectory(
         at: fileURL.deletingLastPathComponent(),

Sources/SKTestSupport/Utils.swift

@@ -13,10 +13,12 @@
 #if compiler(>=6)
 package import Foundation
 package import LanguageServerProtocol
+import SwiftExtensions
 package import struct TSCBasic.AbsolutePath
 #else
 import Foundation
 import LanguageServerProtocol
+import SwiftExtensions
 import struct TSCBasic.AbsolutePath
 #endif
 
@@ -66,10 +68,17 @@ package func testScratchDir(testName: String = #function) throws -> URL {
   if let firstDash = uuid.firstIndex(of: "-") {
     uuid = uuid[..<firstDash]
   }
-  let url = FileManager.default.temporaryDirectory
-    .realpath
+  // Including the test name in the directory frequently makes path lengths of test files exceed the maximum path length
+  // on Windows. Choose shorter directory names on that platform to avoid that issue.
+  #if os(Windows)
+  let url = FileManager.default.temporaryDirectory.realpath
+    .appendingPathComponent("lsp-test")
+    .appendingPathComponent("\(uuid)")
+  #else
+  let url = FileManager.default.temporaryDirectory.realpath
     .appendingPathComponent("sourcekit-lsp-test-scratch")
     .appendingPathComponent("\(testBaseName)-\(uuid)")
+  #endif
   try? FileManager.default.removeItem(at: url)
   try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true)
   return url
@@ -94,31 +103,6 @@ package func withTestScratchDir<T>(
   return try await body(try AbsolutePath(validating: scratchDirectory.path))
 }
 
-fileprivate extension URL {
-  /// Assuming this is a file URL, resolves all symlinks in the path.
-  ///
-  /// - Note: We need this because `URL.resolvingSymlinksInPath()` not only resolves symlinks but also standardizes the
-  ///   path by stripping away `private` prefixes. Since sourcekitd is not performing this standardization, using
-  ///   `resolvingSymlinksInPath` can lead to slightly mismatched URLs between the sourcekit-lsp response and the test
-  ///   assertion.
-  var realpath: URL {
-    #if canImport(Darwin)
-    return self.path.withCString { path in
-      guard let realpath = Darwin.realpath(path, nil) else {
-        return self
-      }
-      let result = URL(fileURLWithPath: String(cString: realpath))
-      free(realpath)
-      return result
-    }
-    #else
-    // Non-Darwin platforms don't have the `/private` stripping issue, so we can just use `self.resolvingSymlinksInPath`
-    // here.
-    return self.resolvingSymlinksInPath()
-    #endif
-  }
-}
-
 let globalModuleCache: URL? = {
   if let customModuleCache = ProcessInfo.processInfo.environment["SOURCEKIT_LSP_TEST_MODULE_CACHE"] {
     if customModuleCache.isEmpty {

Sources/SourceKitLSP/Clang/ClangLanguageService.swift

@@ -326,7 +326,7 @@ actor ClangLanguageService: LanguageService, MessageHandler {
       // safer, can also indefinitely hang as `CreateRemoteThread` may not be serviced depending on the state of
       // the process. This just attempts to terminate the process, risking a deadlock and resource leaks, which is fine
       // since we only use `crash` from tests.
-      _ = TerminateProcess(self.hClangd, 0)
+      _ = TerminateProcess(self.hClangd, 255)
     }
     #else
     if let pid = self.clangdPid {