From 05a9237f2cd4f04afe8064c93161839cc121ae01 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 28 Apr 2022 10:28:41 +0200 Subject: [PATCH] Watch for changes to `compile_commands.json` and `compile_flags.txt` Similar to how we reload the package if Package.swift is changed, we also watch `compilation_database.json` and `compile_flags.txt` and notify the build system delegate that build settings changed if these files are modified. rdar://92388223 [#486] --- .../CompilationDatabaseBuildSystem.swift | 122 +++++++++++++++--- Sources/SourceKitLSP/SourceKitServer.swift | 2 + .../CompilationDatabaseTests.swift | 78 +++++++++++ 3 files changed, 183 insertions(+), 19 deletions(-) create mode 100644 Tests/SourceKitLSPTests/CompilationDatabaseTests.swift diff --git a/Sources/SKCore/CompilationDatabaseBuildSystem.swift b/Sources/SKCore/CompilationDatabaseBuildSystem.swift index da2cdde6..dcd7b8cf 100644 --- a/Sources/SKCore/CompilationDatabaseBuildSystem.swift +++ b/Sources/SKCore/CompilationDatabaseBuildSystem.swift @@ -24,30 +24,58 @@ import struct Foundation.URL /// one compilation database, located at the project root. public final class CompilationDatabaseBuildSystem { + /// Queue guarding the following properties: + /// - `compdb` + /// - `watchedFiles` + /// - `_indexStorePath` + let queue: DispatchQueue = .init(label: "CompilationDatabaseBuildSystem.queue", qos: .userInitiated) + /// The compilation database. - var compdb: CompilationDatabase? = nil + var compdb: CompilationDatabase? = nil { + didSet { + dispatchPrecondition(condition: .onQueue(queue)) + // Build settings have changed and thus the index store path might have changed. + // Recompute it on demand. + _indexStorePath = nil + } + } /// Delegate to handle any build system events. public weak var delegate: BuildSystemDelegate? = nil + let projectRoot: AbsolutePath? + let fileSystem: FileSystem - public lazy var indexStorePath: AbsolutePath? = { - if let allCommands = self.compdb?.allCommands { - for command in allCommands { - let args = command.commandLine - for i in args.indices.reversed() { - if args[i] == "-index-store-path" && i != args.endIndex - 1 { - return try? AbsolutePath(validating: args[i+1]) + /// The URIs for which the delegate has registered for change notifications, + /// mapped to the language the delegate specified when registering for change notifications. + var watchedFiles: [DocumentURI: Language] = [:] + + private var _indexStorePath: AbsolutePath? + public var indexStorePath: AbsolutePath? { + return queue.sync { + if let indexStorePath = _indexStorePath { + return indexStorePath + } + + if let allCommands = self.compdb?.allCommands { + for command in allCommands { + let args = command.commandLine + for i in args.indices.reversed() { + if args[i] == "-index-store-path" && i != args.endIndex - 1 { + _indexStorePath = try? AbsolutePath(validating: args[i+1]) + return _indexStorePath + } } } } + return nil } - return nil - }() + } public init(projectRoot: AbsolutePath? = nil, fileSystem: FileSystem = localFileSystem) { self.fileSystem = fileSystem + self.projectRoot = projectRoot if let path = projectRoot { self.compdb = tryLoadCompilationDatabase(directory: path, fileSystem) } @@ -61,16 +89,22 @@ extension CompilationDatabaseBuildSystem: BuildSystem { } public func registerForChangeNotifications(for uri: DocumentURI, language: Language) { - guard let delegate = self.delegate else { return } + queue.async { + self.watchedFiles[uri] = language - let settings = self._settings(for: uri) - DispatchQueue.global().async { + guard let delegate = self.delegate else { return } + + let settings = self.settings(for: uri) delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)]) } } /// We don't support change watching. - public func unregisterForChangeNotifications(for: DocumentURI) {} + public func unregisterForChangeNotifications(for uri: DocumentURI) { + queue.async { + self.watchedFiles[uri] = nil + } + } public func buildTargets(reply: @escaping (LSPResult<[BuildTarget]>) -> Void) { reply(.failure(buildTargetsNotSupported)) @@ -84,14 +118,18 @@ extension CompilationDatabaseBuildSystem: BuildSystem { reply(.failure(buildTargetsNotSupported)) } - func database(for url: URL) -> CompilationDatabase? { + /// Must be invoked on `queue`. + private func database(for url: URL) -> CompilationDatabase? { + dispatchPrecondition(condition: .onQueue(queue)) if let path = try? AbsolutePath(validating: url.path) { return database(for: path) } return compdb } - func database(for path: AbsolutePath) -> CompilationDatabase? { + /// Must be invoked on `queue`. + private func database(for path: AbsolutePath) -> CompilationDatabase? { + dispatchPrecondition(condition: .onQueue(queue)) if compdb == nil { var dir = path while !dir.isRoot { @@ -110,12 +148,51 @@ extension CompilationDatabaseBuildSystem: BuildSystem { return compdb } - public func filesDidChange(_ events: [FileEvent]) {} + private func fileEventShouldTriggerCompilationDatabaseReload(event: FileEvent) -> Bool { + switch event.uri.fileURL?.lastPathComponent { + case "compile_commands.json", "compile_flags.txt": + return true + default: + return false + } + } + + /// The compilation database has been changed on disk. + /// Reload it and notify the delegate about build setting changes. + /// Must be called on `queue`. + private func reloadCompilationDatabase() { + dispatchPrecondition(condition: .onQueue(queue)) + + guard let projectRoot = self.projectRoot else { return } + + self.compdb = tryLoadCompilationDatabase(directory: projectRoot, self.fileSystem) + + if let delegate = self.delegate { + var changedFiles: [DocumentURI: FileBuildSettingsChange] = [:] + for (uri, _) in self.watchedFiles { + if let settings = self.settings(for: uri) { + changedFiles[uri] = FileBuildSettingsChange(settings) + } else { + changedFiles[uri] = .removedOrUnavailable + } + } + delegate.fileBuildSettingsChanged(changedFiles) + } + } + + public func filesDidChange(_ events: [FileEvent]) { + queue.async { + if events.contains(where: { self.fileEventShouldTriggerCompilationDatabaseReload(event: $0) }) { + self.reloadCompilationDatabase() + } + } + } } extension CompilationDatabaseBuildSystem { - /// Exposed for *testing*. - public func _settings(for uri: DocumentURI) -> FileBuildSettings? { + /// Must be invoked on `queue`. + private func settings(for uri: DocumentURI) -> FileBuildSettings? { + dispatchPrecondition(condition: .onQueue(queue)) guard let url = uri.fileURL else { // We can't determine build settings for non-file URIs. return nil @@ -126,4 +203,11 @@ extension CompilationDatabaseBuildSystem { compilerArguments: Array(cmd.commandLine.dropFirst()), workingDirectory: cmd.directory) } + + /// Exposed for *testing*. + public func _settings(for uri: DocumentURI) -> FileBuildSettings? { + return queue.sync { + return self.settings(for: uri) + } + } } diff --git a/Sources/SourceKitLSP/SourceKitServer.swift b/Sources/SourceKitLSP/SourceKitServer.swift index a480898f..6afbe510 100644 --- a/Sources/SourceKitLSP/SourceKitServer.swift +++ b/Sources/SourceKitLSP/SourceKitServer.swift @@ -589,6 +589,8 @@ extension SourceKitServer { return FileSystemWatcher(globPattern: "**/*.\(fileExtension)", kind: [.create, .delete]) } watchers.append(FileSystemWatcher(globPattern: "**/Package.swift", kind: [.change])) + watchers.append(FileSystemWatcher(globPattern: "**/compile_commands.json", kind: [.create, .change, .delete])) + watchers.append(FileSystemWatcher(globPattern: "**/compile_flags.txt", kind: [.create, .change, .delete])) registry.registerDidChangeWatchedFiles(watchers: watchers) { self.dynamicallyRegisterCapability($0, registry) } diff --git a/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift b/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift new file mode 100644 index 00000000..0165ec15 --- /dev/null +++ b/Tests/SourceKitLSPTests/CompilationDatabaseTests.swift @@ -0,0 +1,78 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import LanguageServerProtocol +import XCTest +import SKCore +import TSCBasic + +final class CompilationDatabaseTests: XCTestCase { + func testModifyCompilationDatabase() throws { + let ws = try! mutableSourceKitTibsTestWorkspace(name: "ClangCrashRecoveryBuildSettings")! + let loc = ws.testLoc("loc") + + try! ws.openDocument(loc.url, language: .cpp) + + // Do a sanity check and verify that we get the expected result from a hover response before modifing the compile commands. + + let highlightRequest = DocumentHighlightRequest(textDocument: loc.docIdentifier, position: Position(line: 9, utf16index: 3)) + let preChangeHighlightResponse = try! ws.sk.sendSync(highlightRequest) + XCTAssertEqual(preChangeHighlightResponse, [ + DocumentHighlight(range: Position(line: 3, utf16index: 5).. CompilationDatabaseCompileCommand in + var command = command + command.commandLine.removeAll(where: { $0 == "-DFOO" }) + return command + } + let newCompilationDatabase = JSONCompilationDatabase(newCommands) + let newCompilationDatabaseData = try JSONEncoder().encode(newCompilationDatabase) + let newCompilationDatabaseStr = String(data: newCompilationDatabaseData, encoding: .utf8)! + builder.write(newCompilationDatabaseStr, to: compilationDatabaseUrl) + }) + + ws.sk.send(DidChangeWatchedFilesNotification(changes: [ + FileEvent(uri: DocumentURI(compilationDatabaseUrl), type: .changed) + ])) + + // DocumentHighlight should now point to the definition in the `#else` block. + + let expectedPostEditHighlight = [ + DocumentHighlight(range: Position(line: 5, utf16index: 5)..