Skip to content

Commit c31b62e

Browse files
ahoppenahoppen
authored
Merge pull request #2259 from ahoppen/only-reload-current-package
Improve package reloading logic when manifest is changed
2 parents 0d507e5 + ac0eb22 commit c31b62e

File tree

3 files changed

+190
-12
lines changed

3 files changed

+190
-12
lines changed

‎Sources/BuildServerIntegration/SwiftPMBuildServer.swift‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,12 @@ package actor SwiftPMBuildServer: BuiltInBuildServer {
138138

139139
private var targetDependencies: [BuildTargetIdentifier: Set<BuildTargetIdentifier>] = [:]
140140

141-
static package func searchForConfig(in path: URL, options: SourceKitLSPOptions) -> BuildServerSpec? {
141+
/// Regular expression that matches version-specific package manifest file names such as [email protected]
142+
private static var versionSpecificPackageManifestNameRegex: Regex<(Substring, Substring, Substring?, Substring?)> {
143+
#/^Package@swift-(\d+)(?:\.(\d+))?(?:\.(\d+))?.swift$/#
144+
}
142145

146+
static package func searchForConfig(in path: URL, options: SourceKitLSPOptions) -> BuildServerSpec? {
143147
let packagePath = path.appendingPathComponent("Package.swift")
144148
if (try? String(contentsOf: packagePath, encoding: .utf8))?.contains("PackageDescription") ?? false {
145149
return BuildServerSpec(kind: .swiftPM, projectRoot: path, configPath: packagePath)
@@ -558,12 +562,11 @@ package actor SwiftPMBuildServer: BuiltInBuildServer {
558562
// (https://github.com/swiftlang/sourcekit-lsp/issues/1267)
559563
for target in request.targets {
560564
if target == .forPackageManifest {
561-
let packageManifestName = #/^Package@swift-(\d+)(?:\.(\d+))?(?:\.(\d+))?.swift$/#
562565
let versionSpecificManifests = try? FileManager.default.contentsOfDirectory(
563566
at: projectRoot,
564567
includingPropertiesForKeys: nil
565568
).compactMap { (url) -> SourceItem? in
566-
guard (try? packageManifestName.wholeMatch(in: url.lastPathComponent)) != nil else {
569+
guard (try? Self.versionSpecificPackageManifestNameRegex.wholeMatch(in: url.lastPathComponent)) != nil else {
567570
return nil
568571
}
569572
return SourceItem(
@@ -821,11 +824,28 @@ package actor SwiftPMBuildServer: BuiltInBuildServer {
821824
}
822825
}
823826

827+
private func isPackageManifestOrPackageResolved(_ url: URL) -> Bool {
828+
guard url.lastPathComponent.contains("Package") else {
829+
// Fast check to early exit for files that don't like a package manifest or Package.resolved
830+
return false
831+
}
832+
guard
833+
url.lastPathComponent == "Package.resolved" || url.lastPathComponent == "Package.swift"
834+
|| (try? Self.versionSpecificPackageManifestNameRegex.wholeMatch(in: url.lastPathComponent)) != nil
835+
else {
836+
return false
837+
}
838+
return url.deletingLastPathComponent() == self.projectRoot
839+
}
840+
824841
/// An event is relevant if it modifies a file that matches one of the file rules used by the SwiftPM workspace.
825842
private func fileEventShouldTriggerPackageReload(event: FileEvent) -> Bool {
826843
guard let fileURL = event.uri.fileURL else {
827844
return false
828845
}
846+
if isPackageManifestOrPackageResolved(fileURL) {
847+
return true
848+
}
829849
switch event.type {
830850
case .created, .deleted:
831851
guard let buildDescription else {
@@ -834,7 +854,8 @@ package actor SwiftPMBuildServer: BuiltInBuildServer {
834854

835855
return buildDescription.fileAffectsSwiftOrClangBuildSettings(fileURL)
836856
case .changed:
837-
return fileURL.lastPathComponent == "Package.swift" || fileURL.lastPathComponent == "Package.resolved"
857+
// Only modified package manifests should trigger a package reload and that's handled above.
858+
return false
838859
default: // Unknown file change type
839860
return false
840861
}

‎Tests/SourceKitLSPTests/BackgroundIndexingTests.swift‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,7 +1325,7 @@ final class BackgroundIndexingTests: XCTestCase {
13251325
// Updating Package.swift causes a package reload but should not cause dependencies to be updated.
13261326
project.testClient.send(
13271327
DidChangeWatchedFilesNotification(changes: [
1328-
FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("Package.resolved")), type: .changed)
1328+
FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("Package.swift")), type: .changed)
13291329
])
13301330
)
13311331
try await project.testClient.send(SynchronizeRequest(index: true))
@@ -1360,14 +1360,19 @@ final class BackgroundIndexingTests: XCTestCase {
13601360
])
13611361
)
13621362
try await project.testClient.send(SynchronizeRequest(index: true))
1363+
let dependencyCheckoutFile = try XCTUnwrap(
1364+
FileManager.default.findFiles(
1365+
named: "Dependency.swift",
1366+
in: project.scratchDirectory
1367+
.appendingPathComponent(".build")
1368+
.appendingPathComponent("index-build")
1369+
.appendingPathComponent("checkouts")
1370+
).only
1371+
)
1372+
// Check that modifying Package.resolved actually modified the dependency checkout inside the package
1373+
assertContains(try String(contentsOf: dependencyCheckoutFile, encoding: .utf8), "Do something v1.1.0")
13631374
project.testClient.send(
1364-
DidChangeWatchedFilesNotification(
1365-
changes: FileManager.default.findFiles(
1366-
named: "Dependency.swift",
1367-
in: project.scratchDirectory.appendingPathComponent(".build").appendingPathComponent("index-build")
1368-
.appendingPathComponent("checkouts")
1369-
).map { FileEvent(uri: DocumentURI($0), type: .changed) }
1370-
)
1375+
DidChangeWatchedFilesNotification(changes: [FileEvent(uri: DocumentURI(dependencyCheckoutFile), type: .changed)])
13711376
)
13721377

13731378
try await repeatUntilExpectedResult {

‎Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,4 +561,156 @@ final class SwiftPMIntegrationTests: XCTestCase {
561561
return location.range.lowerBound == Position(line: 3, utf16index: 4)
562562
}
563563
}
564+
565+
func testChangePackageManifestFile() async throws {
566+
let project = try await SwiftPMTestProject(
567+
files: [
568+
"Lib.swift": """
569+
#if MY_FLAG
570+
#error("MY_FLAG set")
571+
#else
572+
#error("MY_FLAG not set")
573+
#endif
574+
"""
575+
],
576+
manifest: """
577+
// swift-tools-version: 5.7
578+
import PackageDescription
579+
let package = Package(
580+
name: "MyLibrary",
581+
targets: [.target(name: "MyLibrary")]
582+
)
583+
"""
584+
)
585+
586+
let (uri, _) = try project.openDocument("Lib.swift")
587+
let initialDiagnostics = try await project.testClient.send(
588+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
589+
)
590+
XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG not set"])
591+
592+
try await project.changeFileOnDisk(
593+
"Package.swift",
594+
newMarkedContents: """
595+
// swift-tools-version: 5.7
596+
import PackageDescription
597+
let package = Package(
598+
name: "MyLibrary",
599+
targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])]
600+
)
601+
"""
602+
)
603+
try await repeatUntilExpectedResult {
604+
let diagnosticsAfterUpdate = try await project.testClient.send(
605+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
606+
)
607+
return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_FLAG set"]
608+
}
609+
}
610+
611+
func testChangeVersionSpecificPackageManifestFile() async throws {
612+
let project = try await SwiftPMTestProject(
613+
files: [
614+
"Lib.swift": """
615+
#if MY_FLAG
616+
#error("MY_FLAG set")
617+
#elseif MY_OTHER_FLAG
618+
#error("MY_OTHER_FLAG set")
619+
#else
620+
#error("no flag set")
621+
#endif
622+
""",
623+
"/[email protected]": """
624+
// swift-tools-version: 6.1
625+
import PackageDescription
626+
let package = Package(
627+
name: "MyLibrary",
628+
targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])]
629+
)
630+
""",
631+
],
632+
manifest: """
633+
// swift-tools-version: 5.7
634+
import PackageDescription
635+
let package = Package(
636+
name: "MyLibrary",
637+
targets: [.target(name: "MyLibrary")]
638+
)
639+
"""
640+
)
641+
642+
let (uri, _) = try project.openDocument("Lib.swift")
643+
let initialDiagnostics = try await project.testClient.send(
644+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
645+
)
646+
XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG set"])
647+
648+
try await project.changeFileOnDisk(
649+
"[email protected]",
650+
newMarkedContents: """
651+
// swift-tools-version: 6.1
652+
import PackageDescription
653+
let package = Package(
654+
name: "MyLibrary",
655+
targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_OTHER_FLAG")])]
656+
)
657+
"""
658+
)
659+
try await repeatUntilExpectedResult {
660+
let diagnosticsAfterUpdate = try await project.testClient.send(
661+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
662+
)
663+
return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_OTHER_FLAG set"]
664+
}
665+
}
666+
667+
func testAddVersionSpecificPackageManifestFile() async throws {
668+
let project = try await SwiftPMTestProject(
669+
files: [
670+
"Lib.swift": """
671+
#if MY_FLAG
672+
#error("MY_FLAG set")
673+
#else
674+
#error("MY_FLAG not set")
675+
#endif
676+
"""
677+
],
678+
manifest: """
679+
// swift-tools-version: 5.7
680+
import PackageDescription
681+
let package = Package(
682+
name: "MyLibrary",
683+
targets: [.target(name: "MyLibrary")]
684+
)
685+
"""
686+
)
687+
688+
let (uri, _) = try project.openDocument("Lib.swift")
689+
let initialDiagnostics = try await project.testClient.send(
690+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
691+
)
692+
XCTAssertEqual(initialDiagnostics.fullReport?.items.map(\.message), ["MY_FLAG not set"])
693+
694+
let versionSpecificManifestUrl = project.scratchDirectory.appending(component: "[email protected]")
695+
try await """
696+
// swift-tools-version: 6.1
697+
import PackageDescription
698+
let package = Package(
699+
name: "MyLibrary",
700+
targets: [.target(name: "MyLibrary", swiftSettings: [.define("MY_FLAG")])]
701+
)
702+
""".writeWithRetry(to: versionSpecificManifestUrl)
703+
704+
project.testClient.send(
705+
DidChangeWatchedFilesNotification(changes: [
706+
FileEvent(uri: DocumentURI(versionSpecificManifestUrl), type: .created)
707+
])
708+
)
709+
try await repeatUntilExpectedResult {
710+
let diagnosticsAfterUpdate = try await project.testClient.send(
711+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
712+
)
713+
return diagnosticsAfterUpdate.fullReport?.items.map(\.message) == ["MY_FLAG set"]
714+
}
715+
}
564716
}

0 commit comments

Comments
 (0)