Skip to content

Allow a build system to use standardized paths while indexstore-db return realpaths #2033

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 5, 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
44 changes: 35 additions & 9 deletions Sources/BuildSystemIntegration/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1253,11 +1253,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
// it cached. We can just return it.
return mainFile
}
guard let mainFilesProvider else {
return uri
}

let mainFiles = await mainFilesProvider.mainFilesContainingFile(uri)
let mainFiles = await mainFiles(containing: uri)
if mainFiles.contains(uri) {
// If the main files contain the file itself, prefer to use that one
return uri
Expand All @@ -1270,6 +1267,37 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
}
}

/// Returns all main files that include the given document.
///
/// On Darwin platforms, this also performs the following normalization: indexstore-db by itself returns realpaths
/// but the build system might be using standardized Darwin paths (eg. realpath is `/private/tmp` but the standardized
/// path is `/tmp`). If the realpath that indexstore-db returns could not be found in the build system's source files
/// but the standardized path is part of the source files, return the standardized path instead.
package func mainFiles(containing uri: DocumentURI) async -> [DocumentURI] {
guard let mainFilesProvider else {
return [uri]
}
let mainFiles = Array(await mainFilesProvider.mainFiles(containing: uri, crossLanguage: false))
#if canImport(Darwin)
if let buildableSourceFiles = try? await self.buildableSourceFiles() {
return mainFiles.map { mainFile in
if buildableSourceFiles.contains(mainFile) {
return mainFile
}
guard let fileURL = mainFile.fileURL else {
return mainFile
}
let standardized = DocumentURI(fileURL.standardizedFileURL)
if buildableSourceFiles.contains(standardized) {
return standardized
}
return mainFile
}
}
#endif
return mainFiles
}

/// Returns the main file used for `uri`, if this is a registered file.
///
/// For testing purposes only.
Expand Down Expand Up @@ -1317,11 +1345,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
filesWithUpdatedDependencies.formUnion(sourceFiles.flatMap(\.sources).map(\.uri))
}

if let mainFilesProvider {
var mainFiles = await Set(events.asyncFlatMap { await mainFilesProvider.mainFilesContainingFile($0.uri) })
mainFiles.subtract(events.map(\.uri))
filesWithUpdatedDependencies.formUnion(mainFiles)
}
var mainFiles = await Set(events.asyncFlatMap { await self.mainFiles(containing: $0.uri) })
mainFiles.subtract(events.map(\.uri))
filesWithUpdatedDependencies.formUnion(mainFiles)

await self.filesDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies)
}
Expand Down
8 changes: 5 additions & 3 deletions Sources/BuildSystemIntegration/MainFilesProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ import LanguageServerProtocol

/// A type that can provide the set of main files that include a particular file.
package protocol MainFilesProvider: Sendable {
/// Returns the set of main files that contain the given file.
/// Returns all the files that (transitively) include the header file at the given path.
///
/// For example,
/// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
///
/// ### Examples
///
/// ```
/// mainFilesContainingFile("foo.cpp") == Set(["foo.cpp"])
/// mainFilesContainingFile("foo.h") == Set(["foo.cpp", "bar.cpp"])
/// ```
func mainFilesContainingFile(_: DocumentURI) async -> Set<DocumentURI>
func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) async -> Set<DocumentURI>
}
13 changes: 0 additions & 13 deletions Sources/SemanticIndex/CheckedIndex.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,6 @@ package final class CheckedIndex {
return index.unitTests(referencedByMainFiles: mainFilePaths).filter { checker.isUpToDate($0.location) }
}

/// Returns all the files that (transitively) include the header file at the given path.
///
/// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
package func mainFilesContainingFile(uri: DocumentURI, crossLanguage: Bool = false) -> [DocumentURI] {
return index.mainFilesContainingFile(path: uri.pseudoPath, crossLanguage: crossLanguage).compactMap {
let uri = DocumentURI(filePath: $0, isDirectory: false)
guard checker.indexHasUpToDateUnit(for: uri, mainFile: nil, index: self.index) else {
return nil
}
return uri
}
}

/// Returns all unit test symbols in the index.
package func unitTests() -> [SymbolOccurrence] {
return index.unitTests().filter { checker.isUpToDate($0.location) }
Expand Down
13 changes: 4 additions & 9 deletions Sources/SemanticIndex/SemanticIndexManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -430,19 +430,16 @@ package final actor SemanticIndexManager {
guard let sourceFiles else {
return []
}
let deletedFilesIndex = index.checked(for: .deletedFiles)
let modifiedFilesIndex = index.checked(for: .modifiedFiles)

let filesToReIndex =
await files
.asyncFilter {
.asyncCompactMap { uri -> (FileToIndex, Date?)? in
// First, check if we know that the file is up-to-date, in which case we don't need to hit the index or file
// system at all
if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate($0) {
return false
if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate(uri) {
return nil
}
return true
}.compactMap { uri -> (FileToIndex, Date?)? in
if sourceFiles.contains(uri) {
if !indexFilesWithUpToDateUnits, modifiedFilesIndex.hasUpToDateUnit(for: uri) {
return nil
Expand All @@ -455,9 +452,7 @@ package final actor SemanticIndexManager {
// Deterministically pick a main file. This ensures that we always pick the same main file for a header. This way,
// 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 =
deletedFilesIndex
.mainFilesContainingFile(uri: uri, crossLanguage: false)
let mainFile = await buildSystemManager.mainFiles(containing: uri)
.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
15 changes: 9 additions & 6 deletions Sources/SourceKitLSP/IndexStoreDB+MainFilesProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,16 @@ import SemanticIndex
import SwiftExtensions
#endif

extension UncheckedIndex {
package func mainFilesContainingFile(_ uri: DocumentURI) -> Set<DocumentURI> {
extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {
/// - Important: This may return realpaths when the build system might not be using realpaths. Use
/// `BuildSystemManager.mainFiles(containing:)` to work around that problem.
package func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
let mainFiles: Set<DocumentURI>
if let filePath = orLog("File path to get main files", { try uri.fileURL?.filePath }) {
let mainFilePaths = Set(self.underlyingIndexStoreDB.mainFilesContainingFile(path: filePath))
let mainFilePaths = self.underlyingIndexStoreDB.mainFilesContainingFile(
path: filePath,
crossLanguage: crossLanguage
)
mainFiles = Set(
mainFilePaths
.filter { FileManager.default.fileExists(atPath: $0) }
Expand All @@ -41,9 +46,7 @@ extension UncheckedIndex {
} else {
mainFiles = []
}
logger.info("mainFilesContainingFile(\(uri.forLogging)) -> \(mainFiles)")
logger.info("Main files for \(uri.forLogging): \(mainFiles)")
return mainFiles
}
}

extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {}
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ private final actor ManualMainFilesProvider: MainFilesProvider {
self.mainFiles[file] = mainFiles
}

func mainFilesContainingFile(_ file: DocumentURI) -> Set<DocumentURI> {
func mainFiles(containing file: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
if let result = mainFiles[file] {
return result
}
Expand Down
65 changes: 65 additions & 0 deletions Tests/SourceKitLSPTests/WorkspaceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,71 @@ final class WorkspaceTests: XCTestCase {
)
try XCTAssertEqual(XCTUnwrap(prepareUpToDateAgain).didPrepareTarget, false)
}

func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
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())")
try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)

defer {
if cleanScratchDirectories {
try? FileManager.default.removeItem(at: scratchDirectory)
}
}

_ = try MultiFileTestProject.writeFilesToDisk(
files: [
"test.h": "",
"test.c": """
#include "test.h"
""",
"compile_commands.json": """
[
{
"directory": "$TEST_DIR_BACKSLASH_ESCAPED",
"arguments": [
"clang",
"$TEST_DIR_BACKSLASH_ESCAPED/test.c",
"-DHAVE_SETTINGS",
"-index-store-path",
"$TEST_DIR_BACKSLASH_ESCAPED/index"
],
"file": "test.c",
"output": "$TEST_DIR_BACKSLASH_ESCAPED/build/test.o"
}
]
""",
],
scratchDirectory: scratchDirectory
)

let clang = try unwrap(await ToolchainRegistry.forTesting.default?.clang)
try await Process.checkNonZeroExit(
arguments: [
clang.filePath, "-index-store-path", scratchDirectory.appendingPathComponent("index").filePath,
scratchDirectory.appendingPathComponent("test.c").filePath,
"-fsyntax-only",
]
)

let testClient = try await TestSourceKitLSPClient(
options: .testDefault(experimentalFeatures: [.sourceKitOptionsRequest]),
workspaceFolders: [WorkspaceFolder(uri: DocumentURI(scratchDirectory), name: nil)]
)

// Check that we can infer build settings for the header from its main file. indexstore-db stores this main file
// path as `/private/tmp` while the build system only knows about it as `/tmp`.
let options = try await testClient.send(
SourceKitOptionsRequest(
textDocument: TextDocumentIdentifier(scratchDirectory.appendingPathComponent("test.h")),
prepareTarget: false,
allowFallbackSettings: false
)
)
XCTAssert(options.compilerArguments.contains("-DHAVE_SETTINGS"))
}
}

fileprivate let defaultSDKArgs: String = {
Expand Down