Skip to content

Commit 26b8e0b

Browse files
committed
Implement fileAffectsSwiftOrClangBuildSettings with the logic from LLBuildManifestBuilder
Instead of inspecting file extensions to decide whether a file might affect build settings, check which files are in directories that are affecting compilation. This is the same logic that `LLBuildManifestBuilder` uses and should be more stable. In particular, this stops us from reloading the package manifest in SourceKit-LSP when a header file is written to a `.build` directory. rdar://128573306
1 parent b86d22e commit 26b8e0b

File tree

4 files changed

+64
-32
lines changed

4 files changed

+64
-32
lines changed

Sources/Build/BuildManifest/LLBuildManifestBuilder.swift

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -173,28 +173,11 @@ public class LLBuildManifestBuilder {
173173

174174
extension LLBuildManifestBuilder {
175175
private func addPackageStructureCommand() {
176-
let inputs = self.plan.graph.rootPackages.flatMap { package -> [Node] in
177-
var inputs = package.modules
178-
.map(\.sources.root)
179-
.sorted()
180-
.map { Node.directoryStructure($0) }
181-
182-
// Add the output paths of any prebuilds that were run, so that we redo the plan if they change.
183-
var derivedSourceDirPaths: [AbsolutePath] = []
184-
for result in self.plan.prebuildCommandResults.values.flatMap({ $0 }) {
185-
derivedSourceDirPaths.append(contentsOf: result.outputDirectories)
176+
let inputs = self.plan.inputs.map {
177+
switch $0 {
178+
case .directoryStructure(let path): return Node.directoryStructure(path)
179+
case .file(let path): return Node.file(path)
186180
}
187-
inputs.append(contentsOf: derivedSourceDirPaths.sorted().map { Node.directoryStructure($0) })
188-
189-
// FIXME: Need to handle version-specific manifests.
190-
inputs.append(file: package.manifest.path)
191-
192-
// FIXME: This won't be the location of Package.resolved for multiroot packages.
193-
inputs.append(file: package.path.appending("Package.resolved"))
194-
195-
// FIXME: Add config file as an input
196-
197-
return inputs
198181
}
199182

200183
let name = "PackageStructure"
@@ -214,7 +197,7 @@ extension LLBuildManifestBuilder {
214197
extension LLBuildManifestBuilder {
215198
// Creates commands for copying all binary artifacts depended on in the plan.
216199
private func addBinaryDependencyCommands() {
217-
// Make sure we don't have multiple copy commands for each destination by mapping each destination to
200+
// Make sure we don't have multiple copy commands for each destination by mapping each destination to
218201
// its source binary.
219202
var destinations = [AbsolutePath: AbsolutePath]()
220203
for target in self.plan.targetMap.values {

Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ extension BuildParameters {
163163

164164
/// A build plan for a package graph.
165165
public class BuildPlan: SPMBuildCore.BuildPlan {
166+
/// Return value of `inputs()`
167+
package enum Input {
168+
/// Any file in this directory affects the build plan
169+
case directoryStructure(AbsolutePath)
170+
/// The file at the given path affects the build plan
171+
case file(AbsolutePath)
172+
}
173+
166174
public enum Error: Swift.Error, CustomStringConvertible, Equatable {
167175
/// There is no buildable target in the graph.
168176
case noBuildableTarget
@@ -171,7 +179,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
171179
switch self {
172180
case .noBuildableTarget:
173181
return """
174-
The package does not contain a buildable target.
182+
The package does not contain a buildable target.
175183
Add at least one `.target` or `.executableTarget` to your `Package.swift`.
176184
"""
177185
}
@@ -654,6 +662,34 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
654662
}
655663
return try description.symbolGraphExtractArguments()
656664
}
665+
666+
/// Returns the files and directories that affect the build process of this build plan.
667+
package var inputs: [Input] {
668+
var inputs: [Input] = []
669+
for package in self.graph.rootPackages {
670+
inputs += package.modules
671+
.map(\.sources.root)
672+
.sorted()
673+
.map { .directoryStructure($0) }
674+
675+
// Add the output paths of any prebuilds that were run, so that we redo the plan if they change.
676+
var derivedSourceDirPaths: [AbsolutePath] = []
677+
for result in self.prebuildCommandResults.values.flatMap({ $0 }) {
678+
derivedSourceDirPaths.append(contentsOf: result.outputDirectories)
679+
}
680+
inputs.append(contentsOf: derivedSourceDirPaths.sorted().map { .directoryStructure($0) })
681+
682+
// FIXME: Need to handle version-specific manifests.
683+
inputs.append(.file(package.manifest.path))
684+
685+
// FIXME: This won't be the location of Package.resolved for multiroot packages.
686+
inputs.append(.file(package.path.appending("Package.resolved")))
687+
688+
// FIXME: Add config file as an input
689+
690+
}
691+
return inputs
692+
}
657693
}
658694

659695
extension Basics.Diagnostic {

Sources/SourceKitLSPAPI/BuildDescription.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,26 @@ public struct BuildDescription {
142142
getBuildTarget(for: $0, in: modulesGraph)
143143
}
144144
}
145+
146+
/// Returns `true` if the file at the given path might influence build settings for a `swiftc` or `clang` invocation
147+
/// generated by SwiftPM.
148+
public func fileAffectsSwiftOrClangBuildSettings(_ url: URL) -> Bool {
149+
guard let filePath = try? AbsolutePath(validating: url.path) else {
150+
return false
151+
}
152+
153+
for input in buildPlan.inputs {
154+
switch input {
155+
case .directoryStructure(let path):
156+
if path.isAncestor(of: filePath) {
157+
return true
158+
}
159+
case .file(let path):
160+
if filePath == path {
161+
return true
162+
}
163+
}
164+
}
165+
return false
166+
}
145167
}

Sources/Workspace/Workspace.swift

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,15 +1240,6 @@ extension Workspace {
12401240
}
12411241
}
12421242

1243-
/// Returns `true` if the file at the given path might influence build settings for a `swiftc` or `clang` invocation
1244-
/// generated by SwiftPM.
1245-
public func fileAffectsSwiftOrClangBuildSettings(filePath: AbsolutePath, packageGraph: ModulesGraph) -> Bool {
1246-
// TODO: Implement a more sophisticated check that also verifies if the file is in the sources directories of the passed in `packageGraph`.
1247-
FileRuleDescription.builtinRules.contains { fileRuleDescription in
1248-
fileRuleDescription.match(path: filePath, toolsVersion: self.currentToolsVersion)
1249-
}
1250-
}
1251-
12521243
public func acceptIdentityChange(
12531244
package: PackageIdentity,
12541245
version: Version,

0 commit comments

Comments
 (0)