Skip to content

Commit 28e3f99

Browse files
committed
Allow a build system to use standardized paths while indexstore-db return realpaths
On Darwin platforms, this fixes the following problem: 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`). Because of this, when inferring the main file for a file, we might get a URI that the build system doesn’t know about. To fix this, 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, use the standardized path instead.
1 parent bafd8ba commit 28e3f99

File tree

7 files changed

+119
-41
lines changed

7 files changed

+119
-41
lines changed

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,11 +1253,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
12531253
// it cached. We can just return it.
12541254
return mainFile
12551255
}
1256-
guard let mainFilesProvider else {
1257-
return uri
1258-
}
12591256

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

1270+
/// Returns all main files that include the given document.
1271+
///
1272+
/// On Darwin platforms, this also performs the following normalization: indexstore-db by itself returns realpaths
1273+
/// but the build system might be using standardized Darwin paths (eg. realpath is `/private/tmp` but the standardized
1274+
/// path is `/tmp`). If the realpath that indexstore-db returns could not be found in the build system's source files
1275+
/// but the standardized path is part of the source files, return the standardized path instead.
1276+
package func mainFiles(containing uri: DocumentURI) async -> [DocumentURI] {
1277+
guard let mainFilesProvider else {
1278+
return [uri]
1279+
}
1280+
let mainFiles = Array(await mainFilesProvider.mainFiles(containing: uri, crossLanguage: false))
1281+
#if canImport(Darwin)
1282+
if let buildableSourceFiles = try? await self.buildableSourceFiles() {
1283+
return mainFiles.map { mainFile in
1284+
if buildableSourceFiles.contains(mainFile) {
1285+
return mainFile
1286+
}
1287+
guard let fileURL = mainFile.fileURL else {
1288+
return mainFile
1289+
}
1290+
let standardized = DocumentURI(fileURL.standardizedFileURL)
1291+
if buildableSourceFiles.contains(standardized) {
1292+
return standardized
1293+
}
1294+
return mainFile
1295+
}
1296+
}
1297+
#endif
1298+
return mainFiles
1299+
}
1300+
12731301
/// Returns the main file used for `uri`, if this is a registered file.
12741302
///
12751303
/// For testing purposes only.
@@ -1317,11 +1345,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
13171345
filesWithUpdatedDependencies.formUnion(sourceFiles.flatMap(\.sources).map(\.uri))
13181346
}
13191347

1320-
if let mainFilesProvider {
1321-
var mainFiles = await Set(events.asyncFlatMap { await mainFilesProvider.mainFilesContainingFile($0.uri) })
1322-
mainFiles.subtract(events.map(\.uri))
1323-
filesWithUpdatedDependencies.formUnion(mainFiles)
1324-
}
1348+
var mainFiles = await Set(events.asyncFlatMap { await self.mainFiles(containing: $0.uri) })
1349+
mainFiles.subtract(events.map(\.uri))
1350+
filesWithUpdatedDependencies.formUnion(mainFiles)
13251351

13261352
await self.filesDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies)
13271353
}

Sources/BuildSystemIntegration/MainFilesProvider.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@ import LanguageServerProtocol
1818

1919
/// A type that can provide the set of main files that include a particular file.
2020
package protocol MainFilesProvider: Sendable {
21-
/// Returns the set of main files that contain the given file.
21+
/// Returns all the files that (transitively) include the header file at the given path.
2222
///
23-
/// For example,
23+
/// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
24+
///
25+
/// ### Examples
2426
///
2527
/// ```
2628
/// mainFilesContainingFile("foo.cpp") == Set(["foo.cpp"])
2729
/// mainFilesContainingFile("foo.h") == Set(["foo.cpp", "bar.cpp"])
2830
/// ```
29-
func mainFilesContainingFile(_: DocumentURI) async -> Set<DocumentURI>
31+
func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) async -> Set<DocumentURI>
3032
}

Sources/SemanticIndex/CheckedIndex.swift

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,6 @@ package final class CheckedIndex {
151151
return index.unitTests(referencedByMainFiles: mainFilePaths).filter { checker.isUpToDate($0.location) }
152152
}
153153

154-
/// Returns all the files that (transitively) include the header file at the given path.
155-
///
156-
/// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
157-
package func mainFilesContainingFile(uri: DocumentURI, crossLanguage: Bool = false) -> [DocumentURI] {
158-
return index.mainFilesContainingFile(path: uri.pseudoPath, crossLanguage: crossLanguage).compactMap {
159-
let uri = DocumentURI(filePath: $0, isDirectory: false)
160-
guard checker.indexHasUpToDateUnit(for: uri, mainFile: nil, index: self.index) else {
161-
return nil
162-
}
163-
return uri
164-
}
165-
}
166-
167154
/// Returns all unit test symbols in the index.
168155
package func unitTests() -> [SymbolOccurrence] {
169156
return index.unitTests().filter { checker.isUpToDate($0.location) }

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -430,19 +430,16 @@ package final actor SemanticIndexManager {
430430
guard let sourceFiles else {
431431
return []
432432
}
433-
let deletedFilesIndex = index.checked(for: .deletedFiles)
434433
let modifiedFilesIndex = index.checked(for: .modifiedFiles)
435434

436435
let filesToReIndex =
437436
await files
438-
.asyncFilter {
437+
.asyncCompactMap { uri -> (FileToIndex, Date?)? in
439438
// 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
440439
// system at all
441-
if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate($0) {
442-
return false
440+
if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate(uri) {
441+
return nil
443442
}
444-
return true
445-
}.compactMap { uri -> (FileToIndex, Date?)? in
446443
if sourceFiles.contains(uri) {
447444
if !indexFilesWithUpToDateUnits, modifiedFilesIndex.hasUpToDateUnit(for: uri) {
448445
return nil
@@ -455,9 +452,7 @@ package final actor SemanticIndexManager {
455452
// Deterministically pick a main file. This ensures that we always pick the same main file for a header. This way,
456453
// if we request the same header to be indexed twice, we'll pick the same unit file the second time around,
457454
// realize that its timestamp is later than the modification date of the header and we don't need to re-index.
458-
let mainFile =
459-
deletedFilesIndex
460-
.mainFilesContainingFile(uri: uri, crossLanguage: false)
455+
let mainFile = await buildSystemManager.mainFiles(containing: uri)
461456
.sorted(by: { $0.stringValue < $1.stringValue }).first
462457
guard let mainFile else {
463458
logger.log("Not indexing \(uri) because its main file could not be inferred")

Sources/SourceKitLSP/IndexStoreDB+MainFilesProvider.swift

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,16 @@ import SemanticIndex
2828
import SwiftExtensions
2929
#endif
3030

31-
extension UncheckedIndex {
32-
package func mainFilesContainingFile(_ uri: DocumentURI) -> Set<DocumentURI> {
31+
extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {
32+
/// - Important: This may return realpaths when the build system might not be using realpaths. Use
33+
/// `BuildSystemManager.mainFiles(containing:)` to work around that problem.
34+
package func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
3335
let mainFiles: Set<DocumentURI>
3436
if let filePath = orLog("File path to get main files", { try uri.fileURL?.filePath }) {
35-
let mainFilePaths = Set(self.underlyingIndexStoreDB.mainFilesContainingFile(path: filePath))
37+
let mainFilePaths = self.underlyingIndexStoreDB.mainFilesContainingFile(
38+
path: filePath,
39+
crossLanguage: crossLanguage
40+
)
3641
mainFiles = Set(
3742
mainFilePaths
3843
.filter { FileManager.default.fileExists(atPath: $0) }
@@ -41,9 +46,7 @@ extension UncheckedIndex {
4146
} else {
4247
mainFiles = []
4348
}
44-
logger.info("mainFilesContainingFile(\(uri.forLogging)) -> \(mainFiles)")
49+
logger.info("Main files for \(uri.forLogging): \(mainFiles)")
4550
return mainFiles
4651
}
4752
}
48-
49-
extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {}

Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ private final actor ManualMainFilesProvider: MainFilesProvider {
327327
self.mainFiles[file] = mainFiles
328328
}
329329

330-
func mainFilesContainingFile(_ file: DocumentURI) -> Set<DocumentURI> {
330+
func mainFiles(containing file: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
331331
if let result = mainFiles[file] {
332332
return result
333333
}

Tests/SourceKitLSPTests/WorkspaceTests.swift

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,71 @@ final class WorkspaceTests: XCTestCase {
13151315
)
13161316
try XCTAssertEqual(XCTUnwrap(prepareUpToDateAgain).didPrepareTarget, false)
13171317
}
1318+
1319+
func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
1320+
try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")
1321+
1322+
// Explicitly create a directory at /tmp (which is a standardized path but whose realpath is /private/tmp)
1323+
let scratchDirectory = URL(fileURLWithPath: "/tmp").appendingPathComponent("sourcekitlsp-test-\(UUID())")
1324+
try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)
1325+
1326+
defer {
1327+
if cleanScratchDirectories {
1328+
try? FileManager.default.removeItem(at: scratchDirectory)
1329+
}
1330+
}
1331+
1332+
_ = try MultiFileTestProject.writeFilesToDisk(
1333+
files: [
1334+
"test.h": "",
1335+
"test.c": """
1336+
#include "test.h"
1337+
""",
1338+
"compile_commands.json": """
1339+
[
1340+
{
1341+
"directory": "$TEST_DIR_BACKSLASH_ESCAPED",
1342+
"arguments": [
1343+
"clang",
1344+
"$TEST_DIR_BACKSLASH_ESCAPED/test.c",
1345+
"-DHAVE_SETTINGS",
1346+
"-index-store-path",
1347+
"$TEST_DIR_BACKSLASH_ESCAPED/index"
1348+
],
1349+
"file": "test.c",
1350+
"output": "$TEST_DIR_BACKSLASH_ESCAPED/build/test.o"
1351+
}
1352+
]
1353+
""",
1354+
],
1355+
scratchDirectory: scratchDirectory
1356+
)
1357+
1358+
let clang = try unwrap(await ToolchainRegistry.forTesting.default?.clang)
1359+
try await Process.checkNonZeroExit(
1360+
arguments: [
1361+
clang.filePath, "-index-store-path", scratchDirectory.appendingPathComponent("index").filePath,
1362+
scratchDirectory.appendingPathComponent("test.c").filePath,
1363+
"-fsyntax-only",
1364+
]
1365+
)
1366+
1367+
let testClient = try await TestSourceKitLSPClient(
1368+
options: .testDefault(experimentalFeatures: [.sourceKitOptionsRequest]),
1369+
workspaceFolders: [WorkspaceFolder(uri: DocumentURI(scratchDirectory), name: nil)]
1370+
)
1371+
1372+
// Check that we can infer build settings for the header from its main file. indexstore-db stores this main file
1373+
// path as `/private/tmp` while the build system only knows about it as `/tmp`.
1374+
let options = try await testClient.send(
1375+
SourceKitOptionsRequest(
1376+
textDocument: TextDocumentIdentifier(scratchDirectory.appendingPathComponent("test.h")),
1377+
prepareTarget: false,
1378+
allowFallbackSettings: false
1379+
)
1380+
)
1381+
XCTAssert(options.compilerArguments.contains("-DHAVE_SETTINGS"))
1382+
}
13181383
}
13191384

13201385
fileprivate let defaultSDKArgs: String = {

0 commit comments

Comments
 (0)