Skip to content

[6.0] Implement fileAffectsSwiftOrClangBuildSettings with the logic from LLBuildManifestBuilder #7709

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
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
27 changes: 5 additions & 22 deletions Sources/Build/BuildManifest/LLBuildManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,28 +174,11 @@ public class LLBuildManifestBuilder {

extension LLBuildManifestBuilder {
private func addPackageStructureCommand() {
let inputs = self.plan.graph.rootPackages.flatMap { package -> [Node] in
var inputs = package.targets
.map(\.sources.root)
.sorted()
.map { Node.directoryStructure($0) }

// Add the output paths of any prebuilds that were run, so that we redo the plan if they change.
var derivedSourceDirPaths: [AbsolutePath] = []
for result in self.plan.prebuildCommandResults.values.flatMap({ $0 }) {
derivedSourceDirPaths.append(contentsOf: result.outputDirectories)
let inputs = self.plan.inputs.map {
switch $0 {
case .directoryStructure(let path): return Node.directoryStructure(path)
case .file(let path): return Node.file(path)
}
inputs.append(contentsOf: derivedSourceDirPaths.sorted().map { Node.directoryStructure($0) })

// FIXME: Need to handle version-specific manifests.
inputs.append(file: package.manifest.path)

// FIXME: This won't be the location of Package.resolved for multiroot packages.
inputs.append(file: package.path.appending("Package.resolved"))

// FIXME: Add config file as an input

return inputs
}

let name = "PackageStructure"
Expand All @@ -215,7 +198,7 @@ extension LLBuildManifestBuilder {
extension LLBuildManifestBuilder {
// Creates commands for copying all binary artifacts depended on in the plan.
private func addBinaryDependencyCommands() {
// Make sure we don't have multiple copy commands for each destination by mapping each destination to
// Make sure we don't have multiple copy commands for each destination by mapping each destination to
// its source binary.
var destinations = [AbsolutePath: AbsolutePath]()
for target in self.plan.targetMap.values {
Expand Down
38 changes: 37 additions & 1 deletion Sources/Build/BuildPlan/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ extension BuildParameters {

/// A build plan for a package graph.
public class BuildPlan: SPMBuildCore.BuildPlan {
/// Return value of `inputs()`
package enum Input {
/// Any file in this directory affects the build plan
case directoryStructure(AbsolutePath)
/// The file at the given path affects the build plan
case file(AbsolutePath)
}

public enum Error: Swift.Error, CustomStringConvertible, Equatable {
/// There is no buildable target in the graph.
case noBuildableTarget
Expand All @@ -173,7 +181,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
switch self {
case .noBuildableTarget:
return """
The package does not contain a buildable target.
The package does not contain a buildable target.
Add at least one `.target` or `.executableTarget` to your `Package.swift`.
"""
}
Expand Down Expand Up @@ -656,6 +664,34 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
}
return try description.symbolGraphExtractArguments()
}

/// Returns the files and directories that affect the build process of this build plan.
package var inputs: [Input] {
var inputs: [Input] = []
for package in self.graph.rootPackages {
inputs += package.targets
.map(\.sources.root)
.sorted()
.map { .directoryStructure($0) }

// Add the output paths of any prebuilds that were run, so that we redo the plan if they change.
var derivedSourceDirPaths: [AbsolutePath] = []
for result in self.prebuildCommandResults.values.flatMap({ $0 }) {
derivedSourceDirPaths.append(contentsOf: result.outputDirectories)
}
inputs.append(contentsOf: derivedSourceDirPaths.sorted().map { .directoryStructure($0) })

// FIXME: Need to handle version-specific manifests.
inputs.append(.file(package.manifest.path))

// FIXME: This won't be the location of Package.resolved for multiroot packages.
inputs.append(.file(package.path.appending("Package.resolved")))

// FIXME: Add config file as an input

}
return inputs
}
}

extension Basics.Diagnostic {
Expand Down
27 changes: 27 additions & 0 deletions Sources/SourceKitLSPAPI/BuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,14 @@ private struct WrappedSwiftTargetBuildDescription: BuildTarget {
public struct BuildDescription {
private let buildPlan: Build.BuildPlan

/// The inputs of the build plan so we don't need to re-compute them on every call to
/// `fileAffectsSwiftOrClangBuildSettings`.
private let inputs: [BuildPlan.Input]

// FIXME: should not use `BuildPlan` in the public interface
public init(buildPlan: Build.BuildPlan) {
self.buildPlan = buildPlan
self.inputs = buildPlan.inputs
}

// FIXME: should not use `ResolvedTarget` in the public interface
Expand Down Expand Up @@ -143,4 +148,26 @@ public struct BuildDescription {
getBuildTarget(for: $0, in: modulesGraph)
}
}

/// Returns `true` if the file at the given path might influence build settings for a `swiftc` or `clang` invocation
/// generated by SwiftPM.
public func fileAffectsSwiftOrClangBuildSettings(_ url: URL) -> Bool {
guard let filePath = try? AbsolutePath(validating: url.path) else {
return false
}

for input in self.inputs {
switch input {
case .directoryStructure(let path):
if path.isAncestor(of: filePath) {
return true
}
case .file(let path):
if filePath == path {
return true
}
}
}
return false
}
}
9 changes: 0 additions & 9 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1204,15 +1204,6 @@ extension Workspace {
}
}

/// Returns `true` if the file at the given path might influence build settings for a `swiftc` or `clang` invocation
/// generated by SwiftPM.
public func fileAffectsSwiftOrClangBuildSettings(filePath: AbsolutePath, packageGraph: ModulesGraph) -> Bool {
// TODO: Implement a more sophisticated check that also verifies if the file is in the sources directories of the passed in `packageGraph`.
FileRuleDescription.builtinRules.contains { fileRuleDescription in
fileRuleDescription.match(path: filePath, toolsVersion: self.currentToolsVersion)
}
}

public func acceptIdentityChange(
package: PackageIdentity,
version: Version,
Expand Down