diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index f57ad28d..fe9a062c 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -182,15 +182,13 @@ package actor SwiftPMBuildSystem { /// The entry point via with we can access the `SourceKitLSPAPI` provided by SwiftPM. private var buildDescription: SourceKitLSPAPI.BuildDescription? - private var modulesGraph: ModulesGraph + /// Maps source and header files to the target that include them. + private var fileToTargets: [DocumentURI: Set] = [:] - private var fileToTargets: [DocumentURI: [SwiftBuildTarget]] = [:] - private var sourceDirToTargets: [DocumentURI: [SwiftBuildTarget]] = [:] - - /// Maps configured targets ids 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. - private var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:] + /// Maps configured targets 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: [ConfiguredTarget: (buildTarget: SwiftBuildTarget, depth: Int)] = [:] /// Creates a build system using the Swift Package Manager, if this workspace is a package. /// @@ -316,12 +314,6 @@ package actor SwiftPMBuildSystem { flags: buildFlags ) - self.modulesGraph = try ModulesGraph( - rootPackages: [], - packages: IdentifiableSet(), - dependencies: [], - binaryArtifacts: [:] - ) self.reloadPackageStatusCallback = reloadPackageStatusCallback // The debounce duration of 500ms was chosen arbitrarily without scientific research. @@ -404,45 +396,26 @@ extension SwiftPMBuildSystem { /// Make sure to execute any throwing statements before setting any /// properties because otherwise we might end up in an inconsistent state /// with only some properties modified. - self.modulesGraph = modulesGraph - self.targets = Dictionary( - try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in - return (key: ConfiguredTarget(target), value: (index, target)) - }, - uniquingKeysWith: { first, second in - logger.fault("Found two targets with the same name \(first.buildTarget.name)") - return second + self.targets = [:] + self.fileToTargets = [:] + buildDescription.traverseModules { buildTarget, parent, depth in + let configuredTarget = ConfiguredTarget(buildTarget) + var depth = depth + if let existingDepth = targets[configuredTarget]?.depth { + depth = max(existingDepth, depth) + } else { + for source in buildTarget.sources + buildTarget.headers { + fileToTargets[DocumentURI(source), default: []].insert(configuredTarget) + } } - ) - - self.fileToTargets = [DocumentURI: [SwiftBuildTarget]]( - modulesGraph.allModules.flatMap { target in - return target.sources.paths.compactMap { (filePath) -> (key: DocumentURI, value: [SwiftBuildTarget])? in - guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else { - return nil - } - return (key: DocumentURI(filePath.asURL), value: [buildTarget]) - } - }, - uniquingKeysWith: { $0 + $1 } - ) - - self.sourceDirToTargets = [DocumentURI: [SwiftBuildTarget]]( - modulesGraph.allModules.compactMap { (target) -> (DocumentURI, [SwiftBuildTarget])? in - guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else { - return nil - } - return (key: DocumentURI(target.sources.root.asURL), value: [buildTarget]) - }, - uniquingKeysWith: { $0 + $1 } - ) - - guard let delegate = self.delegate else { - return + targets[configuredTarget] = (buildTarget, depth) + } + + if let delegate = self.delegate { + await delegate.fileBuildSettingsChanged(self.watchedFiles) + await delegate.fileHandlingCapabilityChanged() } - await delegate.fileBuildSettingsChanged(self.watchedFiles) - await delegate.fileHandlingCapabilityChanged() for testFilesDidChangeCallback in testFilesDidChangeCallbacks { await testFilesDidChangeCallback() } @@ -556,7 +529,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { let targets = buildTargets(for: uri) if !targets.isEmpty { - return targets.map(ConfiguredTarget.init) + return targets.sorted { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) } } if path.basename == "Package.swift" @@ -567,10 +540,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { return [ConfiguredTarget.forPackageManifest] } - if let targets = try? inferredTargets(for: path) { - return targets - } - return [] } @@ -580,27 +549,27 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in - let lhsIndex = self.targets[lhs]?.index ?? self.targets.count - let rhsIndex = self.targets[rhs]?.index ?? self.targets.count - return lhsIndex < rhsIndex + let lhsDepth = self.targets[lhs]?.depth ?? 0 + let rhsDepth = self.targets[rhs]?.depth ?? 0 + return lhsDepth > rhsDepth } } package func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? { - let targetIndices = targets.compactMap { self.targets[$0]?.index } - let minimumTargetIndex: Int? - if targetIndices.count == targets.count { - minimumTargetIndex = targetIndices.min() + let targetDepths = targets.compactMap { self.targets[$0]?.depth } + let minimumTargetDepth: Int? + if targetDepths.count == targets.count { + minimumTargetDepth = targetDepths.max() } else { // One of the targets didn't have an entry in self.targets. We don't know what might depend on it. - minimumTargetIndex = nil + minimumTargetDepth = nil } // Files that occur before the target in the topological sorting don't depend on it. // Ideally, we should consult the dependency graph here for more accurate dependency analysis instead of relying on // a flattened list (https://github.com/swiftlang/sourcekit-lsp/issues/1312). return self.targets.compactMap { (configuredTarget, value) -> ConfiguredTarget? in - if let minimumTargetIndex, value.index <= minimumTargetIndex { + if let minimumTargetDepth, value.depth >= minimumTargetDepth { return nil } return configuredTarget @@ -725,7 +694,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { } /// Returns the resolved target descriptions for the given file, if one is known. - private func buildTargets(for file: DocumentURI) -> [SwiftBuildTarget] { + private func buildTargets(for file: DocumentURI) -> Set { if let targets = fileToTargets[file] { return targets } @@ -775,7 +744,9 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { guard event.uri.fileURL?.pathExtension == "swift", let targets = fileToTargets[event.uri] else { continue } - filesWithUpdatedDependencies.formUnion(targets.flatMap(\.sources).map(DocumentURI.init)) + for target in targets { + filesWithUpdatedDependencies.formUnion(self.targets[target]?.buildTarget.sources.map(DocumentURI.init) ?? []) + } } // If a `.swiftmodule` file is updated, this means that we have performed a build / are @@ -801,66 +772,28 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem { } package func sourceFiles() -> [SourceFileInfo] { - return fileToTargets.compactMap { (uri, targets) -> SourceFileInfo? in - // We should only set mayContainTests to `true` for files from test targets - // (https://github.com/swiftlang/sourcekit-lsp/issues/1174). - return SourceFileInfo( - uri: uri, - isPartOfRootProject: targets.contains(where: \.isPartOfRootPackage), - mayContainTests: true - ) + 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) } -} - -extension SwiftPMBuildSystem { - - // MARK: Implementation details /// Retrieve settings for a package manifest (Package.swift). private func settings(forPackageManifest path: AbsolutePath) throws -> FileBuildSettings? { - func impl(_ path: AbsolutePath) -> FileBuildSettings? { - for package in modulesGraph.packages where path == package.manifest.path { - let compilerArgs = workspace.interpreterFlags(for: package.path) + [path.pathString] - return FileBuildSettings(compilerArguments: compilerArgs) - } - return nil - } - - if let result = impl(path) { - return result - } - - let canonicalPath = try resolveSymlinks(path) - return canonicalPath == path ? nil : impl(canonicalPath) - } - - /// This finds the target a file belongs to based on its location in the file system. - /// - /// This is primarily intended to find the target a header belongs to. - private func inferredTargets(for path: AbsolutePath) throws -> [ConfiguredTarget] { - func impl(_ path: AbsolutePath) throws -> [ConfiguredTarget] { - var dir = path.parentDirectory - while !dir.isRoot { - if let buildTargets = sourceDirToTargets[DocumentURI(dir.asURL)] { - return buildTargets.map(ConfiguredTarget.init) - } - dir = dir.parentDirectory - } - return [] - } - - let result = try impl(path) - if !result.isEmpty { - return result - } - - let canonicalPath = try resolveSymlinks(path) - return try canonicalPath == path ? [] : impl(canonicalPath) + let compilerArgs = workspace.interpreterFlags(for: path.parentDirectory) + [path.pathString] + return FileBuildSettings(compilerArguments: compilerArgs) } } diff --git a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift index d7495412..633857ac 100644 --- a/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift +++ b/Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift @@ -656,8 +656,8 @@ final class SwiftPMBuildSystemTests: XCTestCase { .compilerArguments XCTAssertNotNil(argsManifest) - assertArgumentsDoNotContain(manifest.pathString, arguments: argsManifest ?? []) - assertArgumentsContain(try resolveSymlinks(manifest).pathString, arguments: argsManifest ?? []) + assertArgumentsContain(manifest.pathString, arguments: argsManifest ?? []) + assertArgumentsDoNotContain(try resolveSymlinks(manifest).pathString, arguments: argsManifest ?? []) } }