Fix an issue that caused us to not get compiler arguments when opening a SwiftPM package at a symlink

When you had a package at `/pkg_real` and a symlink `/pkg` pointing to `/pkg_real`, then opened a workspace at `/pkg`, we wouldn’t get any build settings for any of the files. This was masked in tests because they still called `SwiftPMBuildSystem.targets(for:)`, which handled symlink resolution but wasn’t called in production.

Handle symlink resolution `BuildSystemManager`, remove `SwiftPMBuildSystem.targets(for:)` and its related members/methods, and migrate the tests to go through `BuildSystemManager`, which is what production code does. A nice side effect of this is that the tests will log the requests sent to the build system.

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -21,6 +21,7 @@ import SwiftExtensions
 import ToolchainRegistry
 
 import struct TSCBasic.AbsolutePath
+import func TSCBasic.resolveSymlinks
 
 fileprivate typealias RequestCache<Request: RequestType & Hashable> = Cache<Request, Request.Response>
 
@@ -78,6 +79,18 @@ fileprivate extension BuildTarget {
   }
 }
 
+fileprivate extension DocumentURI {
+  /// If this is a file URI pointing to a symlink, return the realpath of the URI, otherwise return `nil`.
+  var symlinksResolved: DocumentURI? {
+    guard let fileUrl = fileURL, let path = AbsolutePath(validatingOrNil: fileUrl.path),
+      let symlinksResolved = try? TSCBasic.resolveSymlinks(path), symlinksResolved != path
+    else {
+      return nil
+    }
+    return DocumentURI(symlinksResolved.asURL)
+  }
+}
+
 /// Entry point for all build system queries.
 package actor BuildSystemManager: QueueBasedMessageHandler {
   package static let signpostLoggingCategory: String = "build-system-manager-message-handling"
@@ -570,9 +583,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       return nil
     }
     if connectionToBuildSystem == nil {
-      // If there is no build system and we only have the fallback build system,
-      // we will never get real build settings. Consider the build settings
-      // non-fallback.
+      // If there is no build system and we only have the fallback build system, we will never get real build settings.
+      // Consider the build settings non-fallback.
       settings.isFallback = false
     }
     return settings
@@ -588,16 +600,37 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     for document: DocumentURI,
     language: Language
   ) async -> FileBuildSettings? {
-    let mainFile = await mainFile(for: document, language: language)
-    let target = await canonicalTarget(for: mainFile)
-    guard var settings = await buildSettings(for: mainFile, in: target, language: language) else {
+    func mainFileAndSettings(
+      basedOn document: DocumentURI
+    ) async -> (mainFile: DocumentURI, settings: FileBuildSettings)? {
+      let mainFile = await self.mainFile(for: document, language: language)
+      let target = await canonicalTarget(for: mainFile)
+      guard let settings = await buildSettings(for: mainFile, in: target, language: language) else {
+        return nil
+      }
+      return (mainFile, settings)
+    }
+
+    var settings: FileBuildSettings?
+    var mainFile: DocumentURI?
+    if let mainFileAndSettings = await mainFileAndSettings(basedOn: document) {
+      (mainFile, settings) = mainFileAndSettings
+    }
+    if settings?.isFallback ?? true, let symlinksResolved = document.symlinksResolved,
+      let mainFileAndSettings = await mainFileAndSettings(basedOn: symlinksResolved)
+    {
+      (mainFile, settings) = mainFileAndSettings
+    }
+    guard var settings, let mainFile else {
       return nil
     }
+
     if mainFile != document {
       // If the main file isn't the file itself, we need to patch the build settings
       // to reference `document` instead of `mainFile`.
       settings = settings.patching(newFile: document, originalFile: mainFile)
     }
+
     await BuildSettingsLogger.shared.log(settings: settings, for: document)
     return settings
   }

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -204,9 +204,6 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
   /// The entry point via with we can access the `SourceKitLSPAPI` provided by SwiftPM.
   private var buildDescription: SourceKitLSPAPI.BuildDescription?
 
-  /// Maps source and header files to the target that include them.
-  private var fileToTargets: [DocumentURI: Set<BuildTargetIdentifier>] = [:]
-
   /// Maps target ids to their SwiftPM build target.
   private var swiftPMTargets: [BuildTargetIdentifier: SwiftBuildTarget] = [:]
 
@@ -405,19 +402,13 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     /// with only some properties modified.
 
     self.swiftPMTargets = [:]
-    self.fileToTargets = [:]
     self.targetDependencies = [:]
 
     buildDescription.traverseModules { buildTarget, parent, depth in
       let targetIdentifier = orLog("Getting build target identifier") { try BuildTargetIdentifier(buildTarget) }
       guard let targetIdentifier else {
         return
       }
-      if swiftPMTargets[targetIdentifier] == nil {
-        for source in buildTarget.sources + buildTarget.headers {
-          fileToTargets[DocumentURI(source), default: []].insert(targetIdentifier)
-        }
-      }
       if let parent,
         let parentIdentifier = orLog("Getting parent build target identifier", { try BuildTargetIdentifier(parent) })
       {
@@ -576,27 +567,6 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     )
   }
 
-  package func targets(for uri: DocumentURI) -> [BuildTargetIdentifier] {
-    guard let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) else {
-      // We can't determine targets for non-file URIs.
-      return []
-    }
-
-    let targets = buildTargets(for: uri)
-    if !targets.isEmpty {
-      // Sort targets to get deterministic ordering. The actual order does not matter.
-      return targets.sorted { $0.uri.stringValue < $1.uri.stringValue }
-    }
-
-    if path.basename == "Package.swift"
-      && projectRoot == (try? TSCBasic.resolveSymlinks(TSCBasic.AbsolutePath(path.parentDirectory)))
-    {
-      return [BuildTargetIdentifier.forPackageManifest]
-    }
-
-    return []
-  }
-
   package func waitForBuildSystemUpdates(request: WorkspaceWaitForBuildSystemUpdatesRequest) async -> VoidResponse {
     await self.packageLoadingQueue.async {}.valuePropagatingCancellation
     return VoidResponse()
@@ -707,23 +677,6 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
     }
   }
 
-  /// Returns the resolved target descriptions for the given file, if one is known.
-  private func buildTargets(for file: DocumentURI) -> Set<BuildTargetIdentifier> {
-    if let targets = fileToTargets[file] {
-      return targets
-    }
-
-    if let fileURL = file.fileURL,
-      let realpath = try? resolveSymlinks(AbsolutePath(validating: fileURL.path)),
-      let targets = fileToTargets[DocumentURI(realpath.asURL)]
-    {
-      fileToTargets[file] = targets
-      return targets
-    }
-
-    return []
-  }
-
   /// An event is relevant if it modifies a file that matches one of the file rules used by the SwiftPM workspace.
   private func fileEventShouldTriggerPackageReload(event: FileEvent) -> Bool {
     guard let fileURL = event.uri.fileURL else {