From cc11fe1b02ad60ea2fd4a5c177ea438bef4d2728 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Sat, 24 Aug 2024 18:11:52 -0700 Subject: [PATCH] Implement `topologicalSort` inside `BuildSystemManager` using dependency information --- .../BuildServerBuildSystem.swift | 4 -- .../BuildSystemManager.swift | 46 ++++++++++++++- .../BuiltInBuildSystem.swift | 8 --- .../CompilationDatabaseBuildSystem.swift | 4 -- .../SwiftPMBuildSystem.swift | 56 +++++-------------- 5 files changed, 59 insertions(+), 59 deletions(-) diff --git a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift b/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift index d253358c..14d61141 100644 --- a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift +++ b/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift @@ -336,10 +336,6 @@ extension BuildServerBuildSystem: BuiltInBuildSystem { package func waitForUpToDateBuildGraph() async {} - package func topologicalSort(of targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]? { - return nil - } - package func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) { // BuildServerBuildSystem does not support syntactic test discovery or background indexing. // (https://github.com/swiftlang/sourcekit-lsp/issues/1173). diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index 73fe526e..e9aa11ea 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/Sources/BuildSystemIntegration/BuildSystemManager.swift @@ -118,6 +118,8 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate { private var cachedTargetSources = RequestCache() + private var cachedTargetDepths: (buildTargets: [BuildTarget], depths: [BuildTargetIdentifier: Int])? = nil + /// The root of the project that this build system manages. For example, for SwiftPM packages, this is the folder /// containing Package.swift. For compilation databases it is the root folder based on which the compilation database /// was found. @@ -485,8 +487,50 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate { await self.buildSystem?.underlyingBuildSystem.waitForUpToDateBuildGraph() } + /// The root targets of the project have depth of 0 and all target dependencies have a greater depth than the target + // itself. + private func targetDepths(for buildTargets: [BuildTarget]) -> [BuildTargetIdentifier: Int] { + if let cachedTargetDepths, cachedTargetDepths.buildTargets == buildTargets { + return cachedTargetDepths.depths + } + var nonRoots: Set = [] + for buildTarget in buildTargets { + nonRoots.formUnion(buildTarget.dependencies) + } + let targetsById = Dictionary(elements: buildTargets, keyedBy: \.id) + var depths: [BuildTargetIdentifier: Int] = [:] + let rootTargets = buildTargets.filter { !nonRoots.contains($0.id) } + var worksList: [(target: BuildTargetIdentifier, depth: Int)] = rootTargets.map { ($0.id, 0) } + while let (target, depth) = worksList.popLast() { + depths[target] = max(depths[target, default: 0], depth) + for dependency in targetsById[target]?.dependencies ?? [] { + // Check if we have already recorded this target with a greater depth, in which case visiting it again will + // not increase its depth or any of its children. + if depths[target, default: 0] < depth + 1 { + worksList.append((dependency, depth + 1)) + } + } + } + cachedTargetDepths = (buildTargets, depths) + return depths + } + + /// 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. package func topologicalSort(of targets: [BuildTargetIdentifier]) async throws -> [BuildTargetIdentifier]? { - return await buildSystem?.underlyingBuildSystem.topologicalSort(of: targets) + guard let workspaceTargets = await orLog("Getting build targets for topological sort", { try await buildTargets() }) + else { + return nil + } + + let depths = targetDepths(for: workspaceTargets) + return targets.sorted { (lhs: BuildTargetIdentifier, rhs: BuildTargetIdentifier) -> Bool in + return depths[lhs, default: 0] > depths[rhs, default: 0] + } } /// Returns the list of targets that might depend on the given target and that need to be re-prepared when a file in diff --git a/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift b/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift index 489103cc..e60e146f 100644 --- a/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift +++ b/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift @@ -105,14 +105,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable { /// Wait until the build graph has been loaded. func waitForUpToDateBuildGraph() async - /// 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: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]? - /// The toolchain that should be used to open the given document. /// /// If `nil` is returned, then the default toolchain for the given language is used. diff --git a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift b/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift index b9d5c28b..d7b2af30 100644 --- a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift +++ b/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift @@ -159,10 +159,6 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem { package func waitForUpToDateBuildGraph() async {} - package func topologicalSort(of targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? { - return nil - } - private func database(for uri: DocumentURI) -> CompilationDatabase? { if let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) { return database(for: path) diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 84de720d..0f61b80c 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -210,10 +210,8 @@ package actor SwiftPMBuildSystem { /// Maps source and header files to the target that include them. private var fileToTargets: [DocumentURI: Set] = [:] - /// Maps target ids to their SwiftPM build target as well as the depth at which they occur in the build - /// graph. Top level targets on which no other target depends have a depth of `1`. Targets with dependencies have a - /// greater depth. - private var targets: [BuildTargetIdentifier: (buildTarget: SwiftBuildTarget, depth: Int)] = [:] + /// Maps target ids to their SwiftPM build target. + private var swiftPMTargets: [BuildTargetIdentifier: SwiftBuildTarget] = [:] private var targetDependencies: [BuildTargetIdentifier: Set] = [:] @@ -442,7 +440,7 @@ extension SwiftPMBuildSystem { /// properties because otherwise we might end up in an inconsistent state /// with only some properties modified. - self.targets = [:] + self.swiftPMTargets = [:] self.fileToTargets = [:] self.targetDependencies = [:] @@ -451,10 +449,7 @@ extension SwiftPMBuildSystem { guard let targetIdentifier else { return } - var depth = depth - if let existingDepth = targets[targetIdentifier]?.depth { - depth = max(existingDepth, depth) - } else { + if swiftPMTargets[targetIdentifier] == nil { for source in buildTarget.sources + buildTarget.headers { fileToTargets[DocumentURI(source), default: []].insert(targetIdentifier) } @@ -464,7 +459,7 @@ extension SwiftPMBuildSystem { { self.targetDependencies[parentIdentifier, default: []].insert(targetIdentifier) } - targets[targetIdentifier] = (buildTarget, depth) + swiftPMTargets[targetIdentifier] = buildTarget } await messageHandler?.sendNotificationToSourceKitLSP(DidChangeBuildTargetNotification(changes: nil)) @@ -523,9 +518,9 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { } package func buildTargets(request: BuildTargetsRequest) async throws -> BuildTargetsResponse { - let targets = self.targets.map { (targetId, target) in + let targets = self.swiftPMTargets.map { (targetId, target) in var tags: [BuildTargetTag] = [.test] - if target.depth != 1 { + if !target.isPartOfRootPackage { tags.append(.dependency) } return BuildTarget( @@ -547,10 +542,10 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { // TODO: Query The SwiftPM build system for the document's language and add it to SourceItem.data // (https://github.com/swiftlang/sourcekit-lsp/issues/1267) for target in request.targets { - guard let swiftPMTarget = self.targets[target] else { + guard let swiftPMTarget = self.swiftPMTargets[target] else { continue } - let sources = swiftPMTarget.buildTarget.sources.map { + let sources = swiftPMTarget.sources.map { SourceItem(uri: DocumentURI($0), kind: .file, generated: false) } result.append(SourcesItem(target: target, sources: sources)) @@ -568,13 +563,13 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { return try settings(forPackageManifest: path) } - guard let buildTarget = self.targets[request.target]?.buildTarget else { + guard let swiftPMTarget = self.swiftPMTargets[request.target] else { logger.fault("Did not find target \(request.target.forLogging)") return nil } - if !buildTarget.sources.lazy.map(DocumentURI.init).contains(request.textDocument.uri), - let substituteFile = buildTarget.sources.sorted(by: { $0.path < $1.path }).first + if !swiftPMTarget.sources.lazy.map(DocumentURI.init).contains(request.textDocument.uri), + let substituteFile = swiftPMTarget.sources.sorted(by: { $0.path < $1.path }).first { logger.info("Getting compiler arguments for \(url) using substitute file \(substituteFile)") // If `url` is not part of the target's source, it's most likely a header file. Fake compiler arguments for it @@ -585,7 +580,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { // getting its compiler arguments and then patching up the compiler arguments by replacing the substitute file // with the `.cpp` file. let buildSettings = FileBuildSettings( - compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: buildTarget), + compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: swiftPMTarget), workingDirectory: projectRoot.pathString ).patching(newFile: try resolveSymlinks(path).pathString, originalFile: substituteFile.absoluteString) return SourceKitOptionsResponse( @@ -595,7 +590,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { } return SourceKitOptionsResponse( - compilerArguments: try await compilerArguments(for: request.textDocument.uri, in: buildTarget), + compilerArguments: try await compilerArguments(for: request.textDocument.uri, in: swiftPMTarget), workingDirectory: projectRoot.pathString ) } @@ -635,14 +630,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { await self.packageLoadingQueue.async {}.valuePropagatingCancellation } - package func topologicalSort(of targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? { - return targets.sorted { (lhs: BuildTargetIdentifier, rhs: BuildTargetIdentifier) -> Bool in - let lhsDepth = self.targets[lhs]?.depth ?? 0 - let rhsDepth = self.targets[rhs]?.depth ?? 0 - return lhsDepth > rhsDepth - } - } - package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse { // TODO: Support preparation of multiple targets at once. (https://github.com/swiftlang/sourcekit-lsp/issues/1262) for target in request.targets { @@ -804,21 +791,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { } } - package func sourceFiles() -> [SourceFileInfo] { - var sourceFiles: [DocumentURI: SourceFileInfo] = [:] - for (buildTarget, depth) in self.targets.values { - for sourceFile in buildTarget.sources { - let uri = DocumentURI(sourceFile) - sourceFiles[uri] = SourceFileInfo( - uri: uri, - isPartOfRootProject: depth == 1 || (sourceFiles[uri]?.isPartOfRootProject ?? false), - mayContainTests: true - ) - } - } - return sourceFiles.values.sorted { $0.uri.pseudoPath < $1.uri.pseudoPath } - } - package func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async { testFilesDidChangeCallbacks.append(callback) }