Merge pull request #2000 from ahoppen/index-store-path-option

If a build server doesn’t specify `-index-store-path` in the SourceKit options, add it during background indexing

Author: ahoppen

Documentation/Configuration File.md

@@ -37,8 +37,6 @@ The structure of the file is currently not guaranteed to be stable. Options may
 - `buildSettingsTimeout: integer`: Number of milliseconds to wait for build settings from the build system before using fallback build settings.
 - `clangdOptions: string[]`: Extra command line arguments passed to `clangd` when launching it.
 - `index`: Options related to indexing.
-  - `indexStorePath: string`: Directory in which a separate compilation stores the index store. By default, inferred from the build system.
-  - `indexDatabasePath: string`: Directory in which the indexstore-db should be stored. By default, inferred from the build system.
   - `indexPrefixMap: [string: string]`: Path remappings for remapping index data for local use.
   - `updateIndexStoreTimeout: integer`: Number of seconds to wait for an update index store task to finish before killing it.
 - `logging`: Options related to logging, changing SourceKit-LSP’s logging behavior on non-Apple platforms. On Apple platforms, logging is done through the [system log](Diagnose%20Bundle.md#Enable%20Extended%20Logging). These options can only be set globally and not per workspace.

Sources/BuildSystemIntegration/FixedCompilationDatabaseBuildSystem.swift

@@ -24,10 +24,10 @@ import LanguageServerProtocol
 #endif
 
 func lastIndexStorePathArgument(in compilerArgs: [String]) -> String? {
-  for i in compilerArgs.indices.reversed() {
-    if compilerArgs[i] == "-index-store-path" && i + 1 < compilerArgs.count {
-      return compilerArgs[i + 1]
-    }
+  if let indexStorePathIndex = compilerArgs.lastIndex(of: "-index-store-path"),
+    indexStorePathIndex + 1 < compilerArgs.count
+  {
+    return compilerArgs[indexStorePathIndex + 1]
   }
   return nil
 }

Sources/SKOptions/SourceKitLSPOptions.swift

@@ -182,10 +182,6 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
   }
 
   public struct IndexOptions: Sendable, Codable, Equatable {
-    /// Directory in which a separate compilation stores the index store. By default, inferred from the build system.
-    public var indexStorePath: String?
-    /// Directory in which the indexstore-db should be stored. By default, inferred from the build system.
-    public var indexDatabasePath: String?
     /// Path remappings for remapping index data for local use.
     public var indexPrefixMap: [String: String]?
     /// A hint indicating how many cores background indexing should use at most (value between 0 and 1). Background indexing is not required to honor this setting.
@@ -208,23 +204,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
     }
 
     public init(
-      indexStorePath: String? = nil,
-      indexDatabasePath: String? = nil,
       indexPrefixMap: [String: String]? = nil,
       maxCoresPercentageToUseForBackgroundIndexing: Double? = nil,
       updateIndexStoreTimeout: Int? = nil
     ) {
-      self.indexStorePath = indexStorePath
-      self.indexDatabasePath = indexDatabasePath
       self.indexPrefixMap = indexPrefixMap
       self.maxCoresPercentageToUseForBackgroundIndexing = maxCoresPercentageToUseForBackgroundIndexing
       self.updateIndexStoreTimeout = updateIndexStoreTimeout
     }
 
     static func merging(base: IndexOptions, override: IndexOptions?) -> IndexOptions {
       return IndexOptions(
-        indexStorePath: override?.indexStorePath ?? base.indexStorePath,
-        indexDatabasePath: override?.indexDatabasePath ?? base.indexDatabasePath,
         indexPrefixMap: override?.indexPrefixMap ?? base.indexPrefixMap,
         maxCoresPercentageToUseForBackgroundIndexing: override?.maxCoresPercentageToUseForBackgroundIndexing
           ?? base.maxCoresPercentageToUseForBackgroundIndexing,

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -38,7 +38,6 @@ package struct IndexedSingleSwiftFileTestProject {
   package let testClient: TestSourceKitLSPClient
   package let fileURI: DocumentURI
   package let positions: DocumentPositions
-  package let indexDBURL: URL
 
   /// Writes a single file to a temporary directory on disk and compiles it to index it.
   ///
@@ -62,7 +61,6 @@ package struct IndexedSingleSwiftFileTestProject {
 
     let testFileURL = testWorkspaceDirectory.appendingPathComponent("test.swift")
     let indexURL = testWorkspaceDirectory.appendingPathComponent("index")
-    self.indexDBURL = testWorkspaceDirectory.appendingPathComponent("index-db")
     guard let swiftc = await ToolchainRegistry.forTesting.default?.swiftc else {
       throw Error.swiftcNotFound
     }
@@ -147,13 +145,8 @@ package struct IndexedSingleSwiftFileTestProject {
     }
 
     // Create the test client
-    var options = try await SourceKitLSPOptions.testDefault()
-    options.indexOrDefault = SourceKitLSPOptions.IndexOptions(
-      indexStorePath: try indexURL.filePath,
-      indexDatabasePath: try indexDBURL.filePath
-    )
     self.testClient = try await TestSourceKitLSPClient(
-      options: options,
+      options: try await SourceKitLSPOptions.testDefault(),
       capabilities: capabilities,
       workspaceFolders: [
         WorkspaceFolder(uri: DocumentURI(testWorkspaceDirectory))

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -299,6 +299,33 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
     await indexStoreUpToDateTracker.markUpToDate([file.sourceFile], updateOperationStartDate: startDate)
   }
 
+  /// If `args` does not contain an `-index-store-path` argument, add it, pointing to the build system's index store
+  /// path. If an `-index-store-path` already exists, validate that it matches the build system's index store path and
+  /// replace it by the build system's index store path if they don't match.
+  private func addOrReplaceIndexStorePath(in args: [String], for uri: DocumentURI) async throws -> [String] {
+    var args = args
+    guard let buildSystemIndexStorePath = await self.buildSystemManager.initializationData?.indexStorePath else {
+      struct NoIndexStorePathError: Error {}
+      throw NoIndexStorePathError()
+    }
+    if let indexStorePathIndex = args.lastIndex(of: "-index-store-path"), indexStorePathIndex + 1 < args.count {
+      let indexStorePath = args[indexStorePathIndex + 1]
+      if indexStorePath != buildSystemIndexStorePath {
+        logger.error(
+          """
+          Compiler arguments for \(uri) specify index store path \(indexStorePath) but build system specified an \
+          incompatible index store path \(buildSystemIndexStorePath). Overriding with the path specified by the build \
+          system.
+          """
+        )
+        args[indexStorePathIndex + 1] = buildSystemIndexStorePath
+      }
+    } else {
+      args += ["-index-store-path", buildSystemIndexStorePath]
+    }
+    return args
+  }
+
   private func updateIndexStore(
     forSwiftFile uri: DocumentURI,
     buildSettings: FileBuildSettings,
@@ -311,7 +338,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
       return
     }
 
-    let args =
+    var args =
       try [swiftc.filePath] + buildSettings.compilerArguments + [
         "-index-file",
         "-index-file-path", uri.pseudoPath,
@@ -320,6 +347,7 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
         // Fake an output path so that we get a different unit file for every Swift file we background index
         "-index-unit-output-path", uri.pseudoPath + ".o",
       ]
+    args = try await addOrReplaceIndexStorePath(in: args, for: uri)
 
     try await runIndexingProcess(
       indexFile: uri,
@@ -341,10 +369,13 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
       return
     }
 
+    var args = [try clang.filePath] + buildSettings.compilerArguments
+    args = try await addOrReplaceIndexStorePath(in: args, for: uri)
+
     try await runIndexingProcess(
       indexFile: uri,
       buildSettings: buildSettings,
-      processArguments: [clang.filePath] + buildSettings.compilerArguments,
+      processArguments: args,
       workingDirectory: buildSettings.workingDirectory.map(AbsolutePath.init(validating:))
     )
   }

Sources/SourceKitLSP/Workspace.swift

@@ -273,17 +273,13 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
 
     let indexOptions = options.indexOrDefault
     let indexStorePath: URL? =
-      if let indexStorePath = indexOptions.indexStorePath {
-        URL(fileURLWithPath: indexStorePath, relativeTo: rootUri?.fileURL)
-      } else if let indexStorePath = await buildSystemManager.initializationData?.indexStorePath {
+      if let indexStorePath = await buildSystemManager.initializationData?.indexStorePath {
         URL(fileURLWithPath: indexStorePath, relativeTo: rootUri?.fileURL)
       } else {
         nil
       }
     let indexDatabasePath: URL? =
-      if let indexDatabasePath = indexOptions.indexDatabasePath {
-        URL(fileURLWithPath: indexDatabasePath, relativeTo: rootUri?.fileURL)
-      } else if let indexDatabasePath = await buildSystemManager.initializationData?.indexDatabasePath {
+      if let indexDatabasePath = await buildSystemManager.initializationData?.indexDatabasePath {
         URL(fileURLWithPath: indexDatabasePath, relativeTo: rootUri?.fileURL)
       } else {
         nil

Sources/sourcekit-lsp/SourceKitLSP.swift

@@ -112,18 +112,6 @@ struct SourceKitLSP: AsyncParsableCommand {
   )
   var clangdOptions = [String]()
 
-  @Option(
-    name: .customLong("index-store-path", withSingleDash: true),
-    help: "Override index-store-path from the build system"
-  )
-  var indexStorePath: String?
-
-  @Option(
-    name: .customLong("index-db-path", withSingleDash: true),
-    help: "Override index-database-path from the build system"
-  )
-  var indexDatabasePath: String?
-
   @Option(
     name: .customLong("index-prefix-map", withSingleDash: true),
     parsing: .unconditionalSingleValue,
@@ -177,8 +165,6 @@ struct SourceKitLSP: AsyncParsableCommand {
       compilationDatabase: SourceKitLSPOptions.CompilationDatabaseOptions(searchPaths: compilationDatabaseSearchPaths),
       clangdOptions: clangdOptions,
       index: SourceKitLSPOptions.IndexOptions(
-        indexStorePath: indexStorePath,
-        indexDatabasePath: indexDatabasePath,
         indexPrefixMap: [String: String](
           indexPrefixMappings.map { ($0.original, $0.replacement) },
           uniquingKeysWith: { lhs, rhs in rhs }

Tests/SourceKitLSPTests/IndexTests.swift

@@ -128,7 +128,8 @@ final class IndexTests: XCTestCase {
       XCTAssertEqual(jump.first?.uri, project.fileURI)
       XCTAssertEqual(jump.first?.range.lowerBound, project.positions["1️⃣"])
 
-      let tmpContents = try listdir(project.indexDBURL)
+      let indexDBURL = project.fileURI.fileURL!.deletingLastPathComponent().appendingPathComponent("IndexDatabase")
+      let tmpContents = try listdir(indexDBURL)
       guard let versionedPath = tmpContents.filter({ $0.lastPathComponent.starts(with: "v") }).only else {
         XCTFail("expected one version path 'v[0-9]*', found \(tmpContents)")
         return nil

config.schema.json

@@ -126,11 +126,6 @@
       "description" : "Options related to indexing.",
       "markdownDescription" : "Options related to indexing.",
       "properties" : {
-        "indexDatabasePath" : {
-          "description" : "Directory in which the indexstore-db should be stored. By default, inferred from the build system.",
-          "markdownDescription" : "Directory in which the indexstore-db should be stored. By default, inferred from the build system.",
-          "type" : "string"
-        },
         "indexPrefixMap" : {
           "additionalProperties" : {
             "type" : "string"
@@ -139,11 +134,6 @@
           "markdownDescription" : "Path remappings for remapping index data for local use.",
           "type" : "object"
         },
-        "indexStorePath" : {
-          "description" : "Directory in which a separate compilation stores the index store. By default, inferred from the build system.",
-          "markdownDescription" : "Directory in which a separate compilation stores the index store. By default, inferred from the build system.",
-          "type" : "string"
-        },
         "updateIndexStoreTimeout" : {
           "description" : "Number of seconds to wait for an update index store task to finish before killing it.",
           "markdownDescription" : "Number of seconds to wait for an update index store task to finish before killing it.",