Skip to content

Commit e7062cf

Browse files
ahoppenahoppen
authored
Merge pull request #1508 from ahoppen/dont-reload-on-random-file-add
Add test case that we don't reload the package if a `.swift` file get added in a folder that doesn't affect compilation
2 parents 6aba969 + 89bd9b0 commit e7062cf

File tree

7 files changed

+95
-28
lines changed

7 files changed

+95
-28
lines changed

‎Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,19 @@ fileprivate extension ConfiguredTarget {
8989

9090
fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)
9191

92+
public struct SwiftPMTestHooks: Sendable {
93+
public var reloadPackageDidStart: (@Sendable () async -> Void)?
94+
public var reloadPackageDidFinish: (@Sendable () async -> Void)?
95+
96+
public init(
97+
reloadPackageDidStart: (@Sendable () async -> Void)? = nil,
98+
reloadPackageDidFinish: (@Sendable () async -> Void)? = nil
99+
) {
100+
self.reloadPackageDidStart = reloadPackageDidStart
101+
self.reloadPackageDidFinish = reloadPackageDidFinish
102+
}
103+
}
104+
92105
/// Swift Package Manager build system and workspace support.
93106
///
94107
/// This class implements the `BuildSystem` interface to provide the build settings for a Swift
@@ -124,6 +137,7 @@ public actor SwiftPMBuildSystem {
124137
@_spi(Testing)
125138
public var projectRoot: TSCAbsolutePath
126139

140+
private var buildDescription: SourceKitLSPAPI.BuildDescription?
127141
private var modulesGraph: ModulesGraph
128142
private let workspace: Workspace
129143
@_spi(Testing) public let toolsBuildParameters: BuildParameters
@@ -168,6 +182,8 @@ public actor SwiftPMBuildSystem {
168182
/// user's build.
169183
private var isForIndexBuild: Bool { experimentalFeatures.contains(.backgroundIndexing) }
170184

185+
private let testHooks: SwiftPMTestHooks
186+
171187
/// Creates a build system using the Swift Package Manager, if this workspace is a package.
172188
///
173189
/// - Parameters:
@@ -182,7 +198,8 @@ public actor SwiftPMBuildSystem {
182198
fileSystem: FileSystem = localFileSystem,
183199
buildSetup: BuildSetup,
184200
experimentalFeatures: Set<ExperimentalFeature>,
185-
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in }
201+
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in },
202+
testHooks: SwiftPMTestHooks
186203
) async throws {
187204
self.workspacePath = workspacePath
188205
self.buildSetup = buildSetup
@@ -193,6 +210,7 @@ public actor SwiftPMBuildSystem {
193210

194211
self.toolchain = toolchain
195212
self.experimentalFeatures = experimentalFeatures
213+
self.testHooks = testHooks
196214

197215
guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
198216
throw Error.noManifest(workspacePath: workspacePath)
@@ -287,7 +305,8 @@ public actor SwiftPMBuildSystem {
287305
toolchainRegistry: ToolchainRegistry,
288306
buildSetup: BuildSetup,
289307
experimentalFeatures: Set<ExperimentalFeature>,
290-
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void
308+
reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void,
309+
testHooks: SwiftPMTestHooks
291310
) async {
292311
guard let fileURL = uri.fileURL else {
293312
return nil
@@ -299,7 +318,8 @@ public actor SwiftPMBuildSystem {
299318
fileSystem: localFileSystem,
300319
buildSetup: buildSetup,
301320
experimentalFeatures: experimentalFeatures,
302-
reloadPackageStatusCallback: reloadPackageStatusCallback
321+
reloadPackageStatusCallback: reloadPackageStatusCallback,
322+
testHooks: testHooks
303323
)
304324
} catch Error.noManifest {
305325
return nil
@@ -315,8 +335,10 @@ extension SwiftPMBuildSystem {
315335
/// dependencies.
316336
public func reloadPackage(forceResolvedVersions: Bool) async throws {
317337
await reloadPackageStatusCallback(.start)
338+
await testHooks.reloadPackageDidStart?()
318339
defer {
319340
Task {
341+
await testHooks.reloadPackageDidFinish?()
320342
await reloadPackageStatusCallback(.end)
321343
}
322344
}
@@ -335,6 +357,7 @@ extension SwiftPMBuildSystem {
335357
observabilityScope: observabilitySystem.topScope
336358
)
337359
let buildDescription = BuildDescription(buildPlan: plan)
360+
self.buildDescription = buildDescription
338361

339362
/// Make sure to execute any throwing statements before setting any
340363
/// properties because otherwise we might end up in an inconsistent state
@@ -682,14 +705,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
682705
}
683706
switch event.type {
684707
case .created, .deleted:
685-
guard let path = try? AbsolutePath(validating: fileURL.path) else {
708+
guard let buildDescription else {
686709
return false
687710
}
688711

689-
return self.workspace.fileAffectsSwiftOrClangBuildSettings(
690-
filePath: path,
691-
packageGraph: self.modulesGraph
692-
)
712+
return buildDescription.fileAffectsSwiftOrClangBuildSettings(fileURL)
693713
case .changed:
694714
return fileURL.lastPathComponent == "Package.swift" || fileURL.lastPathComponent == "Package.resolved"
695715
default: // Unknown file change type

‎Sources/SemanticIndex/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ add_library(SemanticIndex STATIC
33
CheckedIndex.swift
44
CompilerCommandLineOption.swift
55
IndexTaskDescription.swift
6+
IndexTestHooks.swift
67
PreparationTaskDescription.swift
78
SemanticIndexManager.swift
8-
TestHooks.swift
99
UpdateIndexStoreTaskDescription.swift
1010
UpToDateTracker.swift
1111
)

‎Sources/SemanticIndex/TestHooks.swift‎ renamed to ‎Sources/SemanticIndex/IndexTestHooks.swift‎

File renamed without changes.

‎Sources/SourceKitLSP/CreateBuildSystem.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ func createBuildSystem(
3939
toolchainRegistry: toolchainRegistry,
4040
buildSetup: options.buildSetup,
4141
experimentalFeatures: options.experimentalFeatures,
42-
reloadPackageStatusCallback: reloadPackageStatusCallback
42+
reloadPackageStatusCallback: reloadPackageStatusCallback,
43+
testHooks: options.swiftpmTestHooks
4344
)
4445
}
4546

‎Sources/SourceKitLSP/SourceKitLSPServer+Options.swift‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import Foundation
1414
import LanguageServerProtocol
1515
import SKCore
1616
import SKSupport
17+
import SKSwiftPMWorkspace
1718
import SemanticIndex
1819

1920
import struct TSCBasic.AbsolutePath
@@ -70,6 +71,8 @@ extension SourceKitLSPServer {
7071

7172
public var indexTestHooks: IndexTestHooks
7273

74+
public var swiftpmTestHooks: SwiftPMTestHooks
75+
7376
public init(
7477
buildSetup: BuildSetup = .default,
7578
clangdOptions: [String] = [],
@@ -80,7 +83,8 @@ extension SourceKitLSPServer {
8083
swiftPublishDiagnosticsDebounceDuration: TimeInterval = 2, /* 2s */
8184
workDoneProgressDebounceDuration: Duration = .seconds(0),
8285
experimentalFeatures: Set<ExperimentalFeature> = [],
83-
indexTestHooks: IndexTestHooks = IndexTestHooks()
86+
indexTestHooks: IndexTestHooks = IndexTestHooks(),
87+
swiftpmTestHooks: SwiftPMTestHooks = SwiftPMTestHooks()
8488
) {
8589
self.buildSetup = buildSetup
8690
self.clangdOptions = clangdOptions
@@ -92,6 +96,7 @@ extension SourceKitLSPServer {
9296
self.experimentalFeatures = experimentalFeatures
9397
self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration
9498
self.indexTestHooks = indexTestHooks
99+
self.swiftpmTestHooks = swiftpmTestHooks
95100
}
96101
}
97102
}

‎Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
5454
toolchainRegistry: tr,
5555
fileSystem: fs,
5656
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
57-
experimentalFeatures: []
57+
experimentalFeatures: [],
58+
testHooks: SwiftPMTestHooks()
5859
)
5960
)
6061
}
@@ -81,7 +82,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
8182
toolchainRegistry: tr,
8283
fileSystem: fs,
8384
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
84-
experimentalFeatures: []
85+
experimentalFeatures: [],
86+
testHooks: SwiftPMTestHooks()
8587
)
8688
await assertThrowsError(try await buildSystem.generateBuildGraph(allowFileSystemWrites: false))
8789
}
@@ -111,7 +113,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
111113
toolchainRegistry: ToolchainRegistry(toolchains: []),
112114
fileSystem: fs,
113115
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
114-
experimentalFeatures: []
116+
experimentalFeatures: [],
117+
testHooks: SwiftPMTestHooks()
115118
)
116119
)
117120
}
@@ -142,7 +145,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
142145
toolchainRegistry: tr,
143146
fileSystem: fs,
144147
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
145-
experimentalFeatures: []
148+
experimentalFeatures: [],
149+
testHooks: SwiftPMTestHooks()
146150
)
147151
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
148152

@@ -207,7 +211,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
207211
toolchainRegistry: tr,
208212
fileSystem: localFileSystem,
209213
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
210-
experimentalFeatures: []
214+
experimentalFeatures: [],
215+
testHooks: SwiftPMTestHooks()
211216
)
212217
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
213218

@@ -270,7 +275,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
270275
toolchainRegistry: tr,
271276
fileSystem: fs,
272277
buildSetup: config,
273-
experimentalFeatures: []
278+
experimentalFeatures: [],
279+
testHooks: SwiftPMTestHooks()
274280
)
275281
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
276282

@@ -312,7 +318,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
312318
toolchainRegistry: tr,
313319
fileSystem: fs,
314320
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
315-
experimentalFeatures: []
321+
experimentalFeatures: [],
322+
testHooks: SwiftPMTestHooks()
316323
)
317324
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
318325

@@ -349,7 +356,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
349356
toolchainRegistry: tr,
350357
fileSystem: fs,
351358
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
352-
experimentalFeatures: []
359+
experimentalFeatures: [],
360+
testHooks: SwiftPMTestHooks()
353361
)
354362
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
355363

@@ -398,7 +406,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
398406
toolchainRegistry: tr,
399407
fileSystem: fs,
400408
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
401-
experimentalFeatures: []
409+
experimentalFeatures: [],
410+
testHooks: SwiftPMTestHooks()
402411
)
403412
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
404413

@@ -463,7 +472,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
463472
toolchainRegistry: tr,
464473
fileSystem: fs,
465474
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
466-
experimentalFeatures: []
475+
experimentalFeatures: [],
476+
testHooks: SwiftPMTestHooks()
467477
)
468478
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
469479

@@ -507,7 +517,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
507517
toolchainRegistry: tr,
508518
fileSystem: fs,
509519
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
510-
experimentalFeatures: []
520+
experimentalFeatures: [],
521+
testHooks: SwiftPMTestHooks()
511522
)
512523
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
513524

@@ -588,7 +599,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
588599
toolchainRegistry: ToolchainRegistry.forTesting,
589600
fileSystem: fs,
590601
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
591-
experimentalFeatures: []
602+
experimentalFeatures: [],
603+
testHooks: SwiftPMTestHooks()
592604
)
593605
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
594606

@@ -640,7 +652,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
640652
toolchainRegistry: tr,
641653
fileSystem: fs,
642654
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
643-
experimentalFeatures: []
655+
experimentalFeatures: [],
656+
testHooks: SwiftPMTestHooks()
644657
)
645658
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
646659

@@ -708,7 +721,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
708721
toolchainRegistry: ToolchainRegistry.forTesting,
709722
fileSystem: fs,
710723
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
711-
experimentalFeatures: []
724+
experimentalFeatures: [],
725+
testHooks: SwiftPMTestHooks()
712726
)
713727
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
714728

@@ -748,7 +762,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
748762
toolchainRegistry: tr,
749763
fileSystem: fs,
750764
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
751-
experimentalFeatures: []
765+
experimentalFeatures: [],
766+
testHooks: SwiftPMTestHooks()
752767
)
753768
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
754769

@@ -789,7 +804,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
789804
toolchainRegistry: tr,
790805
fileSystem: fs,
791806
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
792-
experimentalFeatures: []
807+
experimentalFeatures: [],
808+
testHooks: SwiftPMTestHooks()
793809
)
794810

795811
assertEqual(await swiftpmBuildSystem.projectRoot, try resolveSymlinks(tempDir.appending(component: "pkg")))
@@ -824,7 +840,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
824840
toolchainRegistry: tr,
825841
fileSystem: fs,
826842
buildSetup: SourceKitLSPServer.Options.testDefault.buildSetup,
827-
experimentalFeatures: []
843+
experimentalFeatures: [],
844+
testHooks: SwiftPMTestHooks()
828845
)
829846
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false)
830847

‎Tests/SourceKitLSPTests/BackgroundIndexingTests.swift‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import LSPTestSupport
1414
import LanguageServerProtocol
1515
@_spi(Testing) import SKCore
16+
import SKSupport
1617
import SKTestSupport
1718
import SemanticIndex
1819
import SourceKitLSP
@@ -1222,6 +1223,29 @@ final class BackgroundIndexingTests: XCTestCase {
12221223
return hoverAfterPackageUpdate?.contents.markupContent?.value.contains("Do something v1.1.0") ?? false
12231224
}
12241225
}
1226+
1227+
func testAddingRandomSwiftFileDoesNotTriggerPackageReload() async throws {
1228+
let packageInitialized = AtomicBool(initialValue: false)
1229+
1230+
var serverOptions = SourceKitLSPServer.Options.testDefault
1231+
serverOptions.swiftpmTestHooks.reloadPackageDidStart = {
1232+
if packageInitialized.value {
1233+
XCTFail("Build graph should not get reloaded when random file gets added")
1234+
}
1235+
}
1236+
let project = try await SwiftPMTestProject(
1237+
files: ["Test.swift": ""],
1238+
serverOptions: serverOptions,
1239+
enableBackgroundIndexing: true
1240+
)
1241+
packageInitialized.value = true
1242+
project.testClient.send(
1243+
DidChangeWatchedFilesNotification(changes: [
1244+
FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("random.swift")), type: .created)
1245+
])
1246+
)
1247+
_ = try await project.testClient.send(PollIndexRequest())
1248+
}
12251249
}
12261250

12271251
extension HoverResponseContents {

0 commit comments

Comments
 (0)