Skip to content

Commit e198b4b

Browse files
committed
Do not apply file path normalization in mainFiles(containing:) to the file itself
Do not apply the standardized file normalization to the source file itself. Otherwise we would get the following behavior: - We have a build system that uses standardized file paths and index a file as /tmp/test.c - We are asking for the main files of /private/tmp/test.c - Since indexstore-db uses realpath for everything, we find the unit for /tmp/test.c as a unit containing /private/tmp/test.c, which has /private/tmp/test.c as the main file. - If we applied the path normalization, we would normalize /private/tmp/test.c to /tmp/test.c, thus reporting that /tmp/test.c is a main file containing /private/tmp/test.c, But that doesn't make sense (it would, in fact cause us to treat /private/tmp/test.c as a header file that we should index using /tmp/test.c as a main file.
1 parent f9fc58d commit e198b4b

File tree

6 files changed

+100
-10
lines changed

6 files changed

+100
-10
lines changed

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,19 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
13121312
#if canImport(Darwin)
13131313
if let buildableSourceFiles = try? await self.buildableSourceFiles() {
13141314
return mainFiles.map { mainFile in
1315+
if mainFile == uri {
1316+
// Do not apply the standardized file normalization to the source file itself. Otherwise we would get the
1317+
// following behavior:
1318+
// - We have a build system that uses standardized file paths and index a file as /tmp/test.c
1319+
// - We are asking for the main files of /private/tmp/test.c
1320+
// - Since indexstore-db uses realpath for everything, we find the unit for /tmp/test.c as a unit containg
1321+
// /private/tmp/test.c, which has /private/tmp/test.c as the main file.
1322+
// - If we applied the path normalization, we would normalize /private/tmp/test.c to /tmp/test.c, thus
1323+
// reporting that /tmp/test.c is a main file containing /private/tmp/test.c,
1324+
// But that doesn't make sense (it would, in fact cause us to treat /private/tmp/test.c as a header file that
1325+
// we should index using /tmp/test.c as a main file.
1326+
return mainFile
1327+
}
13151328
if buildableSourceFiles.contains(mainFile) {
13161329
return mainFile
13171330
}

Sources/SKTestSupport/CustomBuildServerTestProject.swift

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ package import LanguageServerProtocol
1717
import LanguageServerProtocolExtensions
1818
import SKLogging
1919
package import SKOptions
20-
import SourceKitLSP
20+
package import SourceKitLSP
2121
import SwiftExtensions
2222
import ToolchainRegistry
2323
import XCTest
@@ -245,21 +245,24 @@ package final class CustomBuildServerTestProject<BuildServer: CustomBuildServer>
245245
files: [RelativeFileLocation: String],
246246
buildServer buildServerType: BuildServer.Type,
247247
options: SourceKitLSPOptions? = nil,
248+
hooks: Hooks = Hooks(),
248249
enableBackgroundIndexing: Bool = false,
250+
testScratchDir: URL? = nil,
249251
testName: String = #function
250252
) async throws {
251-
let hooks: Hooks = Hooks(
252-
buildSystemHooks: BuildSystemHooks(injectBuildServer: { [buildServerBox] projectRoot, connectionToSourceKitLSP in
253-
let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
254-
buildServerBox.value = buildServer
255-
return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
256-
})
257-
)
253+
var hooks = hooks
254+
XCTAssertNil(hooks.buildSystemHooks.injectBuildServer)
255+
hooks.buildSystemHooks.injectBuildServer = { [buildServerBox] projectRoot, connectionToSourceKitLSP in
256+
let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
257+
buildServerBox.value = buildServer
258+
return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
259+
}
258260
try await super.init(
259261
files: files,
260262
options: options,
261263
hooks: hooks,
262264
enableBackgroundIndexing: enableBackgroundIndexing,
265+
testScratchDir: testScratchDir,
263266
testName: testName
264267
)
265268
}

Sources/SKTestSupport/MultiFileTestProject.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,11 @@ package class MultiFileTestProject {
139139
enableBackgroundIndexing: Bool = false,
140140
usePullDiagnostics: Bool = true,
141141
preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
142+
testScratchDir overrideTestScratchDir: URL? = nil,
142143
cleanUp: (@Sendable () -> Void)? = nil,
143144
testName: String = #function
144145
) async throws {
145-
scratchDirectory = try testScratchDir(testName: testName)
146+
scratchDirectory = try overrideTestScratchDir ?? testScratchDir(testName: testName)
146147
self.fileData = try Self.writeFilesToDisk(files: files, scratchDirectory: scratchDirectory)
147148

148149
self.testClient = try await TestSourceKitLSPClient(

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ package final actor SemanticIndexManager {
481481
// if we request the same header to be indexed twice, we'll pick the same unit file the second time around,
482482
// realize that its timestamp is later than the modification date of the header and we don't need to re-index.
483483
let mainFile = await buildSystemManager.mainFiles(containing: uri)
484+
.filter { sourceFiles.contains($0) }
484485
.sorted(by: { $0.stringValue < $1.stringValue }).first
485486
guard let mainFile else {
486487
logger.log("Not indexing \(uri) because its main file could not be inferred")

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,6 +2201,77 @@ final class BackgroundIndexingTests: XCTestCase {
22012201
}
22022202
)
22032203
}
2204+
2205+
func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
2206+
try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")
2207+
2208+
final class BuildSystem: CustomBuildServer {
2209+
let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker()
2210+
private let projectRoot: URL
2211+
private var testFileURL: URL { projectRoot.appendingPathComponent("test.c").standardized }
2212+
2213+
required init(projectRoot: URL, connectionToSourceKitLSP: any LanguageServerProtocol.Connection) {
2214+
self.projectRoot = projectRoot
2215+
}
2216+
2217+
func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse {
2218+
return initializationResponse(
2219+
initializeData: SourceKitInitializeBuildResponseData(
2220+
indexDatabasePath: try projectRoot.appendingPathComponent("index-db").filePath,
2221+
indexStorePath: try projectRoot.appendingPathComponent("index-store").filePath,
2222+
prepareProvider: true,
2223+
sourceKitOptionsProvider: true
2224+
)
2225+
)
2226+
}
2227+
2228+
func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse {
2229+
return BuildTargetSourcesResponse(items: [
2230+
SourcesItem(target: .dummy, sources: [SourceItem(uri: URI(testFileURL), kind: .file, generated: false)])
2231+
])
2232+
}
2233+
2234+
func textDocumentSourceKitOptionsRequest(
2235+
_ request: TextDocumentSourceKitOptionsRequest
2236+
) async throws -> TextDocumentSourceKitOptionsResponse? {
2237+
return TextDocumentSourceKitOptionsResponse(compilerArguments: [request.textDocument.uri.pseudoPath])
2238+
}
2239+
}
2240+
2241+
let scratchDirectory = URL(fileURLWithPath: "/tmp")
2242+
.appendingPathComponent("sourcekitlsp-test")
2243+
.appendingPathComponent(testScratchName())
2244+
let indexedFiles = ThreadSafeBox<[DocumentURI]>(initialValue: [])
2245+
let project = try await CustomBuildServerTestProject(
2246+
files: [
2247+
"test.c": "void x() {}"
2248+
],
2249+
buildServer: BuildSystem.self,
2250+
hooks: Hooks(
2251+
indexHooks: IndexHooks(
2252+
updateIndexStoreTaskDidStart: { task in
2253+
indexedFiles.withLock { indexedFiles in
2254+
indexedFiles += task.filesToIndex.map(\.file.sourceFile)
2255+
}
2256+
}
2257+
)
2258+
),
2259+
enableBackgroundIndexing: true,
2260+
testScratchDir: scratchDirectory
2261+
)
2262+
try await project.testClient.send(PollIndexRequest())
2263+
2264+
// Ensure that changing `/private/tmp/.../test.c` only causes `/tmp/.../test.c` to be indexed, not
2265+
// `/private/tmp/.../test.c`.
2266+
indexedFiles.value = []
2267+
let testFileURL = try XCTUnwrap(project.uri(for: "test.c").fileURL?.realpath)
2268+
try await "void y() {}".writeWithRetry(to: testFileURL)
2269+
project.testClient.send(
2270+
DidChangeWatchedFilesNotification(changes: [FileEvent(uri: DocumentURI(testFileURL), type: .changed)])
2271+
)
2272+
try await project.testClient.send(PollIndexRequest())
2273+
XCTAssertEqual(indexedFiles.value, [try project.uri(for: "test.c")])
2274+
}
22042275
}
22052276

22062277
extension HoverResponseContents {

Tests/SourceKitLSPTests/WorkspaceTests.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,8 @@ final class WorkspaceTests: XCTestCase {
13091309
try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")
13101310

13111311
// Explicitly create a directory at /tmp (which is a standardized path but whose realpath is /private/tmp)
1312-
let scratchDirectory = URL(fileURLWithPath: "/tmp").appendingPathComponent("sourcekitlsp-test-\(UUID())")
1312+
let scratchDirectory = URL(fileURLWithPath: "/tmp")
1313+
.appendingPathComponent(testScratchName())
13131314
try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)
13141315

13151316
defer {

0 commit comments

Comments
 (0)