Skip to content

Miscellaneous adjustments to make tests pass on Windows #1751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions Sources/BuildSystemIntegration/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>
Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 20 additions & 11 deletions Sources/BuildSystemIntegration/CompilationDatabase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 []
Expand All @@ -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)
Expand All @@ -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
}
}
4 changes: 3 additions & 1 deletion Sources/BuildSystemIntegration/DetermineBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
37 changes: 13 additions & 24 deletions Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Sources/Diagnose/ReproducerBundle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions Sources/SKSupport/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions Sources/SKSupport/DocumentURI+symlinkTarget.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 1 addition & 1 deletion Sources/SKTestSupport/BuildServerTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import XCTest
fileprivate let sdkArgs =
if let defaultSDKPath {
"""
"-sdk", "\(defaultSDKPath)",
"-sdk", "\(defaultSDKPath.replacing(#"\"#, with: #"\\"#))",
"""
} else {
""
Expand Down
8 changes: 7 additions & 1 deletion Sources/SKTestSupport/FindTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
14 changes: 14 additions & 0 deletions Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -103,6 +116,7 @@ package struct IndexedSingleSwiftFileTestProject {
"-I", usrLibDir.path,
"-F", frameworksDir.path,
]
#endif
}

let compilationDatabase = JSONCompilationDatabase(
Expand Down
5 changes: 4 additions & 1 deletion Sources/SKTestSupport/MultiFileTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
38 changes: 11 additions & 27 deletions Sources/SKTestSupport/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Clang/ClangLanguageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading