Skip to content

Support background preparation of targets #1273

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 3 commits into from
May 13, 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
10 changes: 10 additions & 0 deletions Sources/SKCore/BuildServerBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,16 @@ extension BuildServerBuildSystem: BuildSystem {
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
}

public func generateBuildGraph() {}

public func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
return nil
}

public func prepare(targets: [ConfiguredTarget]) async throws {
throw PrepareNotSupportedError()
}

public func registerForChangeNotifications(for uri: DocumentURI) {
let request = RegisterForChanges(uri: uri, action: .register)
_ = self.buildServer?.send(request) { result in
Expand Down
32 changes: 29 additions & 3 deletions Sources/SKCore/BuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,19 @@ public struct SourceFileInfo: Sendable {
/// The URI of the source file.
public let uri: DocumentURI

/// `true` if this file belongs to the root project that the user is working on. It is false, if the file belongs
/// to a dependency of the project.
public let isPartOfRootProject: Bool

/// Whether the file might contain test cases. This property is an over-approximation. It might be true for files
/// from non-test targets or files that don't actually contain any tests. Keeping this list of files with
/// `mayContainTets` minimal as possible helps reduce the amount of work that the syntactic test indexer needs to
/// perform.
public let mayContainTests: Bool

public init(uri: DocumentURI, mayContainTests: Bool) {
public init(uri: DocumentURI, isPartOfRootProject: Bool, mayContainTests: Bool) {
self.uri = uri
self.isPartOfRootProject = isPartOfRootProject
self.mayContainTests = mayContainTests
}
}
Expand All @@ -64,6 +69,13 @@ public struct ConfiguredTarget: Hashable, Sendable {
}
}

/// An error build systems can throw from `prepare` if they don't support preparation of targets.
public struct PrepareNotSupportedError: Error, CustomStringConvertible {
public init() {}

public var description: String { "Preparation not supported" }
}

/// Provider of FileBuildSettings and other build-related information.
///
/// The primary role of the build system is to answer queries for
Expand Down Expand Up @@ -114,6 +126,22 @@ public protocol BuildSystem: AnyObject, Sendable {
/// Return the list of targets and run destinations that the given document can be built for.
func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]

/// Re-generate the build graph including all the tasks that are necessary for building the entire build graph, like
/// resolving package versions.
func generateBuildGraph() async throws

/// Sort the targets so that low-level targets occur before high-level targets.
///
/// This sorting is best effort but allows the indexer to prepare and index low-level targets first, which allows
/// index data to be available earlier.
///
/// `nil` if the build system doesn't support topological sorting of targets.
func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]?

/// Prepare the given targets for indexing and semantic functionality. This should build all swift modules of target
/// dependencies.
func prepare(targets: [ConfiguredTarget]) async throws

/// If the build system has knowledge about the language that this document should be compiled in, return it.
///
/// This is used to determine the language in which a source file should be background indexed.
Expand Down Expand Up @@ -146,5 +174,3 @@ public protocol BuildSystem: AnyObject, Sendable {
/// The callback might also be called without an actual change to `sourceFiles`.
func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async
}

public let buildTargetsNotSupported = ResponseError.methodNotFound(BuildTargets.method)
14 changes: 13 additions & 1 deletion Sources/SKCore/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,18 @@ extension BuildSystemManager {
return settings
}

public func generateBuildGraph() async throws {
try await self.buildSystem?.generateBuildGraph()
}

public func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {
return await buildSystem?.topologicalSort(of: targets)
}

public func prepare(targets: [ConfiguredTarget]) async throws {
try await buildSystem?.prepare(targets: targets)
}

public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
logger.debug("registerForChangeNotifications(\(uri.forLogging))")
let mainFile = await mainFile(for: uri, language: language)
Expand Down Expand Up @@ -247,7 +259,7 @@ extension BuildSystemManager {

public func testFiles() async -> [DocumentURI] {
return await sourceFiles().compactMap { (info: SourceFileInfo) -> DocumentURI? in
guard info.mayContainTests else {
guard info.isPartOfRootProject, info.mayContainTests else {
return nil
}
return info.uri
Expand Down
12 changes: 11 additions & 1 deletion Sources/SKCore/CompilationDatabaseBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
}

public func prepare(targets: [ConfiguredTarget]) async throws {
throw PrepareNotSupportedError()
}

public func generateBuildGraph() {}

public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
return nil
}

public func registerForChangeNotifications(for uri: DocumentURI) async {
self.watchedFiles.insert(uri)
}
Expand Down Expand Up @@ -208,7 +218,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
return []
}
return compdb.allCommands.map {
SourceFileInfo(uri: DocumentURI($0.url), mayContainTests: true)
SourceFileInfo(uri: DocumentURI($0.url), isPartOfRootProject: true, mayContainTests: true)
}
}

Expand Down
16 changes: 15 additions & 1 deletion Sources/SKSupport/Collection+PartitionIntoBatches.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//

public extension Collection {
public extension Collection where Index == Int {
/// Partition the elements of the collection into `numberOfBatches` roughly equally sized batches.
///
/// Elements are assigned to the batches round-robin. This ensures that elements that are close to each other in the
Expand All @@ -32,4 +32,18 @@ public extension Collection {
}
return batches.filter { !$0.isEmpty }
}

/// Partition the collection into batches that have a maximum size of `batchSize`.
///
/// The last batch will contain the remainder elements.
func partition(intoBatchesOfSize batchSize: Int) -> [[Element]] {
var batches: [[Element]] = []
batches.reserveCapacity(self.count / batchSize)
var lastIndex = self.startIndex
for index in stride(from: self.startIndex, to: self.endIndex, by: batchSize).dropFirst() + [self.endIndex] {
batches.append(Array(self[lastIndex..<index]))
lastIndex = index
}
return batches
}
}
131 changes: 110 additions & 21 deletions Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ public actor SwiftPMBuildSystem {

var fileToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
var sourceDirToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
var targets: [SwiftBuildTarget] = []

/// Maps target ids (aka. `ConfiguredTarget.targetID`) to their SwiftPM build target as well as an index in their
/// topological sorting. Targets with lower index are more low level, ie. targets with higher indices depend on
/// targets with lower indices.
var targets: [String: (index: Int, buildTarget: SwiftBuildTarget)] = [:]

/// The URIs for which the delegate has registered for change notifications,
/// mapped to the language the delegate specified when registering for change notifications.
Expand All @@ -119,6 +123,18 @@ public actor SwiftPMBuildSystem {
/// Force-unwrapped optional because initializing it requires access to `self`.
var fileDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil

/// A `ObservabilitySystem` from `SwiftPM` that logs.
private let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)")
})

/// Whether the SwiftPMBuildSystem may modify `Package.resolved` or not.
///
/// This is `false` if the `SwiftPMBuildSystem` is pointed at a `.index-build` directory that's independent of the
/// user's build. In this case `SwiftPMBuildSystem` is allowed to clone repositories even if no `Package.resolved`
/// exists.
private let forceResolvedVersions: Bool

/// Creates a build system using the Swift Package Manager, if this workspace is a package.
///
/// - Parameters:
Expand All @@ -132,11 +148,13 @@ public actor SwiftPMBuildSystem {
toolchainRegistry: ToolchainRegistry,
fileSystem: FileSystem = localFileSystem,
buildSetup: BuildSetup,
forceResolvedVersions: Bool,
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in }
) async throws {
self.workspacePath = workspacePath
self.fileSystem = fileSystem
self.toolchainRegistry = toolchainRegistry
self.forceResolvedVersions = forceResolvedVersions

guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
throw Error.noManifest(workspacePath: workspacePath)
Expand Down Expand Up @@ -204,7 +222,6 @@ public actor SwiftPMBuildSystem {
}
await delegate.filesDependenciesUpdated(filesWithUpdatedDependencies)
}

try await reloadPackage()
}

Expand All @@ -217,6 +234,7 @@ public actor SwiftPMBuildSystem {
url: URL,
toolchainRegistry: ToolchainRegistry,
buildSetup: BuildSetup,
forceResolvedVersions: Bool,
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void
) async {
do {
Expand All @@ -225,6 +243,7 @@ public actor SwiftPMBuildSystem {
toolchainRegistry: toolchainRegistry,
fileSystem: localFileSystem,
buildSetup: buildSetup,
forceResolvedVersions: forceResolvedVersions,
reloadPackageStatusCallback: reloadPackageStatusCallback
)
} catch Error.noManifest {
Expand All @@ -237,6 +256,9 @@ public actor SwiftPMBuildSystem {
}

extension SwiftPMBuildSystem {
public func generateBuildGraph() async throws {
try await self.reloadPackage()
}

/// (Re-)load the package settings by parsing the manifest and resolving all the targets and
/// dependencies.
Expand All @@ -248,13 +270,9 @@ extension SwiftPMBuildSystem {
}
}

let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)")
})

let modulesGraph = try self.workspace.loadPackageGraph(
rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]),
forceResolvedVersions: true,
forceResolvedVersions: forceResolvedVersions,
observabilityScope: observabilitySystem.topScope
)

Expand All @@ -272,7 +290,15 @@ extension SwiftPMBuildSystem {
/// with only some properties modified.
self.modulesGraph = modulesGraph

self.targets = try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph)
self.targets = Dictionary(
try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in
return (key: target.name, (index, target))
},
uniquingKeysWith: { first, second in
logger.fault("Found two targets with the same name \(first.buildTarget.name)")
return second
}
)

self.fileToTarget = [AbsolutePath: SwiftBuildTarget](
modulesGraph.allTargets.flatMap { target in
Expand Down Expand Up @@ -343,14 +369,8 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
return try settings(forPackageManifest: path)
}

let buildTargets = self.targets.filter({ $0.name == configuredTarget.targetID })
if buildTargets.count > 1 {
logger.error("Found multiple targets with name \(configuredTarget.targetID). Picking the first one")
}
guard let buildTarget = buildTargets.first else {
if buildTargets.isEmpty {
logger.error("Did not find target with name \(configuredTarget.targetID)")
}
guard let buildTarget = self.targets[configuredTarget.targetID]?.buildTarget else {
logger.error("Did not find target with name \(configuredTarget.targetID)")
return nil
}

Expand All @@ -368,7 +388,8 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
}

public func defaultLanguage(for document: DocumentURI) async -> Language? {
// TODO (indexing): Query The SwiftPM build system for the document's language
// TODO (indexing): Query The SwiftPM build system for the document's language.
// https://github.com/apple/sourcekit-lsp/issues/1267
return nil
}

Expand All @@ -395,6 +416,77 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
return []
}

public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in
let lhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
let rhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
return lhsIndex < rhsIndex
}
}

public func prepare(targets: [ConfiguredTarget]) async throws {
// TODO (indexing): Support preparation of multiple targets at once.
// https://github.com/apple/sourcekit-lsp/issues/1262
for target in targets {
try await prepare(singleTarget: target)
}
}

private func prepare(singleTarget target: ConfiguredTarget) async throws {
// TODO (indexing): Add a proper 'prepare' job in SwiftPM instead of building the target.
// https://github.com/apple/sourcekit-lsp/issues/1254
guard let toolchain = await toolchainRegistry.default else {
logger.error("Not preparing because not toolchain exists")
return
}
guard let swift = toolchain.swift else {
logger.error(
"Not preparing because toolchain at \(toolchain.identifier) does not contain a Swift compiler"
)
return
}
let arguments = [
swift.pathString, "build",
"--scratch-path", self.workspace.location.scratchDirectory.pathString,
"--disable-index-store",
"--target", target.targetID,
]
let process = Process(
arguments: arguments,
workingDirectory: workspacePath
)
try process.launch()
let result = try await process.waitUntilExitSendingSigIntOnTaskCancellation()
switch result.exitStatus.exhaustivelySwitchable {
case .terminated(code: 0):
break
case .terminated(code: let code):
// This most likely happens if there are compilation errors in the source file. This is nothing to worry about.
let stdout = (try? String(bytes: result.output.get(), encoding: .utf8)) ?? "<no stderr>"
let stderr = (try? String(bytes: result.stderrOutput.get(), encoding: .utf8)) ?? "<no stderr>"
logger.debug(
"""
Preparation of target \(target.targetID) terminated with non-zero exit code \(code)
Stderr:
\(stderr)
Stdout:
\(stdout)
"""
)
case .signalled(signal: let signal):
if !Task.isCancelled {
// The indexing job finished with a signal. Could be because the compiler crashed.
// Ignore signal exit codes if this task has been cancelled because the compiler exits with SIGINT if it gets
// interrupted.
logger.error("Preparation of target \(target.targetID) signaled \(signal)")
}
case .abnormal(exception: let exception):
if !Task.isCancelled {
logger.error("Preparation of target \(target.targetID) exited abnormally \(exception)")
}
}
}

public func registerForChangeNotifications(for uri: DocumentURI) async {
self.watchedFiles.insert(uri)
}
Expand Down Expand Up @@ -489,14 +581,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {

public func sourceFiles() -> [SourceFileInfo] {
return fileToTarget.compactMap { (path, target) -> SourceFileInfo? in
guard target.isPartOfRootPackage else {
// Don't consider files from package dependencies as possible test files.
return nil
}
// We should only set mayContainTests to `true` for files from test targets
// (https://github.com/apple/sourcekit-lsp/issues/1174).
return SourceFileInfo(
uri: DocumentURI(path.asURL),
isPartOfRootProject: target.isPartOfRootPackage,
mayContainTests: true
)
}
Expand Down
Loading