Skip to content

Do not apply file path normalization in mainFiles(containing:) to the file itself #2061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Sources/BuildSystemIntegration/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,19 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
#if canImport(Darwin)
if let buildableSourceFiles = try? await self.buildableSourceFiles() {
return mainFiles.map { mainFile in
if mainFile == uri {
// 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 containg
// /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.
return mainFile
}
if buildableSourceFiles.contains(mainFile) {
return mainFile
}
Expand Down
19 changes: 11 additions & 8 deletions Sources/SKTestSupport/CustomBuildServerTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package import LanguageServerProtocol
import LanguageServerProtocolExtensions
import SKLogging
package import SKOptions
import SourceKitLSP
package import SourceKitLSP
import SwiftExtensions
import ToolchainRegistry
import XCTest
Expand Down Expand Up @@ -245,21 +245,24 @@ package final class CustomBuildServerTestProject<BuildServer: CustomBuildServer>
files: [RelativeFileLocation: String],
buildServer buildServerType: BuildServer.Type,
options: SourceKitLSPOptions? = nil,
hooks: Hooks = Hooks(),
enableBackgroundIndexing: Bool = false,
testScratchDir: URL? = nil,
testName: String = #function
) async throws {
let hooks: Hooks = Hooks(
buildSystemHooks: BuildSystemHooks(injectBuildServer: { [buildServerBox] projectRoot, connectionToSourceKitLSP in
let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
buildServerBox.value = buildServer
return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
})
)
var hooks = hooks
XCTAssertNil(hooks.buildSystemHooks.injectBuildServer)
hooks.buildSystemHooks.injectBuildServer = { [buildServerBox] projectRoot, connectionToSourceKitLSP in
let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
buildServerBox.value = buildServer
return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
}
try await super.init(
files: files,
options: options,
hooks: hooks,
enableBackgroundIndexing: enableBackgroundIndexing,
testScratchDir: testScratchDir,
testName: testName
)
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SKTestSupport/MultiFileTestProject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ package class MultiFileTestProject {
enableBackgroundIndexing: Bool = false,
usePullDiagnostics: Bool = true,
preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
testScratchDir overrideTestScratchDir: URL? = nil,
cleanUp: (@Sendable () -> Void)? = nil,
testName: String = #function
) async throws {
scratchDirectory = try testScratchDir(testName: testName)
scratchDirectory = try overrideTestScratchDir ?? testScratchDir(testName: testName)
self.fileData = try Self.writeFilesToDisk(files: files, scratchDirectory: scratchDirectory)

self.testClient = try await TestSourceKitLSPClient(
Expand Down
1 change: 1 addition & 0 deletions Sources/SemanticIndex/SemanticIndexManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ package final actor SemanticIndexManager {
// if we request the same header to be indexed twice, we'll pick the same unit file the second time around,
// realize that its timestamp is later than the modification date of the header and we don't need to re-index.
let mainFile = await buildSystemManager.mainFiles(containing: uri)
.filter { sourceFiles.contains($0) }
.sorted(by: { $0.stringValue < $1.stringValue }).first
guard let mainFile else {
logger.log("Not indexing \(uri) because its main file could not be inferred")
Expand Down
71 changes: 71 additions & 0 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,77 @@ final class BackgroundIndexingTests: XCTestCase {
}
)
}

func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")

final class BuildSystem: CustomBuildServer {
let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker()
private let projectRoot: URL
private var testFileURL: URL { projectRoot.appendingPathComponent("test.c").standardized }

required init(projectRoot: URL, connectionToSourceKitLSP: any LanguageServerProtocol.Connection) {
self.projectRoot = projectRoot
}

func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse {
return initializationResponse(
initializeData: SourceKitInitializeBuildResponseData(
indexDatabasePath: try projectRoot.appendingPathComponent("index-db").filePath,
indexStorePath: try projectRoot.appendingPathComponent("index-store").filePath,
prepareProvider: true,
sourceKitOptionsProvider: true
)
)
}

func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse {
return BuildTargetSourcesResponse(items: [
SourcesItem(target: .dummy, sources: [SourceItem(uri: URI(testFileURL), kind: .file, generated: false)])
])
}

func textDocumentSourceKitOptionsRequest(
_ request: TextDocumentSourceKitOptionsRequest
) async throws -> TextDocumentSourceKitOptionsResponse? {
return TextDocumentSourceKitOptionsResponse(compilerArguments: [request.textDocument.uri.pseudoPath])
}
}

let scratchDirectory = URL(fileURLWithPath: "/tmp")
.appendingPathComponent("sourcekitlsp-test")
.appendingPathComponent(testScratchName())
let indexedFiles = ThreadSafeBox<[DocumentURI]>(initialValue: [])
let project = try await CustomBuildServerTestProject(
files: [
"test.c": "void x() {}"
],
buildServer: BuildSystem.self,
hooks: Hooks(
indexHooks: IndexHooks(
updateIndexStoreTaskDidStart: { task in
indexedFiles.withLock { indexedFiles in
indexedFiles += task.filesToIndex.map(\.file.sourceFile)
}
}
)
),
enableBackgroundIndexing: true,
testScratchDir: scratchDirectory
)
try await project.testClient.send(PollIndexRequest())

// Ensure that changing `/private/tmp/.../test.c` only causes `/tmp/.../test.c` to be indexed, not
// `/private/tmp/.../test.c`.
indexedFiles.value = []
let testFileURL = try XCTUnwrap(project.uri(for: "test.c").fileURL?.realpath)
try await "void y() {}".writeWithRetry(to: testFileURL)
project.testClient.send(
DidChangeWatchedFilesNotification(changes: [FileEvent(uri: DocumentURI(testFileURL), type: .changed)])
)
try await project.testClient.send(PollIndexRequest())
XCTAssertEqual(indexedFiles.value, [try project.uri(for: "test.c")])
}
}

extension HoverResponseContents {
Expand Down
3 changes: 2 additions & 1 deletion Tests/SourceKitLSPTests/WorkspaceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,8 @@ final class WorkspaceTests: XCTestCase {
try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")

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

defer {
Expand Down