Skip to content

Commit 0b569a1

Browse files
authored
Don’t write output file map when getting compiler arguments for SourceKit-LSP (#8075)
We currently always write the output file map when calling `SwiftModuleBuildDescription.emitCommandLine`. This means that every time SourceKit-LSP queries build settings for a source file, we write the output file map. This is causing issues in SourceKit-LSP on Windows because we might be running a build (eg. for target preparation), which writes an output file map and at the same time try to get build settings for that file inside SourceKit-LSP, which calls into `emitCommandLine`, also trying to write an output file map, which fails because the output file map is exclusively opened by the build and thus cannot also write the output file map. While this might seem like a more theoretical race condition, this caused multiple tests to fail when running SourceKit-LSP tests locally on my machine. Work around this by adding a `writeOutputFileMap` argument to `emitCommandLine`, which we can set to `true` for calls from `SourceKitLSPAPI` to avoid writing out the output file map. Works around #8038. rdar://137963946
1 parent f78fc4e commit 0b569a1

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -719,9 +719,14 @@ public final class SwiftModuleBuildDescription {
719719
return []
720720
}
721721

722-
/// When `scanInvocation` argument is set to `true`, omit the side-effect producing arguments
723-
/// such as emitting a module or supplementary outputs.
724-
public func emitCommandLine(scanInvocation: Bool = false) throws -> [String] {
722+
/// - Parameters:
723+
/// - scanInvocation: When `true`, omit the side-effect producing arguments such as emitting a module or
724+
/// supplementary outputs.
725+
/// - writeOutputFileMap: When `false`, we assume that an output file map for this command line already exists at
726+
/// the expected location on disk. This is intended for SourceKit-LSP to get build settings for a file without
727+
/// writing out an output file map as a side effect. We expect that preparation of the module has already
728+
/// created the output file map.
729+
public func emitCommandLine(scanInvocation: Bool = false, writeOutputFileMap: Bool = true) throws -> [String] {
725730
var result: [String] = []
726731
result.append(self.buildParameters.toolchain.swiftCompilerPath.pathString)
727732

@@ -742,8 +747,12 @@ public final class SwiftModuleBuildDescription {
742747
result.append(self.moduleOutputPath.pathString)
743748

744749
result.append("-output-file-map")
745-
// FIXME: Eliminate side effect.
746-
result.append(try self.writeOutputFileMap().pathString)
750+
let outputFileMapPath = self.tempsPath.appending("output-file-map.json")
751+
if writeOutputFileMap {
752+
// FIXME: Eliminate side effect.
753+
try self.writeOutputFileMap(to: outputFileMapPath)
754+
}
755+
result.append(outputFileMapPath.pathString)
747756
}
748757

749758
if self.useWholeModuleOptimization {
@@ -769,8 +778,7 @@ public final class SwiftModuleBuildDescription {
769778
self.buildParameters.triple.isDarwin() && self.target.type == .library
770779
}
771780

772-
func writeOutputFileMap() throws -> AbsolutePath {
773-
let path = self.tempsPath.appending("output-file-map.json")
781+
func writeOutputFileMap(to path: AbsolutePath) throws {
774782
let masterDepsPath = self.tempsPath.appending("master.swiftdeps")
775783

776784
var content =
@@ -852,7 +860,6 @@ public final class SwiftModuleBuildDescription {
852860

853861
try fileSystem.createDirectory(path.parentDirectory, recursive: true)
854862
try self.fileSystem.writeFileContents(path, bytes: .init(encodingAsUTF8: content), atomically: true)
855-
return path
856863
}
857864

858865
/// Generates the module map for the Swift target and returns its path.

Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,9 @@ extension LLBuildManifestBuilder {
376376
let cmdName = target.getCommandName()
377377

378378
self.manifest.addWriteSourcesFileListCommand(sources: target.sources, sourcesFileListPath: target.sourcesFileListPath)
379+
let outputFileMapPath = target.tempsPath.appending("output-file-map.json")
380+
// FIXME: Eliminate side effect.
381+
try target.writeOutputFileMap(to: outputFileMapPath)
379382
self.manifest.addSwiftCmd(
380383
name: cmdName,
381384
inputs: inputs + [Node.file(target.sourcesFileListPath)],
@@ -392,7 +395,7 @@ extension LLBuildManifestBuilder {
392395
fileList: target.sourcesFileListPath,
393396
isLibrary: isLibrary,
394397
wholeModuleOptimization: target.useWholeModuleOptimization,
395-
outputFileMapPath: try target.writeOutputFileMap(), // FIXME: Eliminate side effect.
398+
outputFileMapPath: outputFileMapPath,
396399
prepareForIndexing: target.buildParameters.prepareForIndexing != .off
397400
)
398401
}

Sources/SourceKitLSPAPI/BuildDescription.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ private struct WrappedSwiftTargetBuildDescription: BuildTarget {
116116
func compileArguments(for fileURL: URL) throws -> [String] {
117117
// Note: we ignore the `fileURL` here as the expectation is that we get a command line for the entire target
118118
// in case of Swift.
119-
let commandLine = try description.emitCommandLine(scanInvocation: false)
119+
let commandLine = try description.emitCommandLine(scanInvocation: false, writeOutputFileMap: false)
120120
// First element on the command line is the compiler itself, not an argument.
121121
return Array(commandLine.dropFirst())
122122
}

0 commit comments

Comments
 (0)