Merge pull request #890 from daveinglis/building_tests_fixes_windows

Change how libraries are specified to the linker when using searched libs

Author: daveinglis

Sources/SWBCore/SpecImplementations/LinkerSpec.swift

@@ -89,18 +89,23 @@ open class LinkerSpec : CommandLineToolSpec, @unchecked Sendable {
         /// The path to the privacy file, if one exists.
         public let privacyFile: Path?
 
-        public init(kind: Kind, path: Path, mode: Mode, useSearchPaths: Bool, swiftModulePaths: [String: Path], swiftModuleAdditionalLinkerArgResponseFilePaths: [String: Path], explicitDependencies: [Path] = [], topLevelItemPath: Path? = nil, dsymPath: Path? = nil, xcframeworkSourcePath: Path? = nil, privacyFile: Path? = nil) {
+        public let libPrefix: String?
+
+        public init(kind: Kind, path: Path, mode: Mode, useSearchPaths: Bool, swiftModulePaths: [String: Path], swiftModuleAdditionalLinkerArgResponseFilePaths: [String: Path], prefix: String? = nil, explicitDependencies: [Path] = [], topLevelItemPath: Path? = nil, dsymPath: Path? = nil, xcframeworkSourcePath: Path? = nil, privacyFile: Path? = nil) {
             self.kind = kind
             self.path = path
             self.mode = mode
-            self.useSearchPaths = useSearchPaths
             self.swiftModulePaths = swiftModulePaths
             self.swiftModuleAdditionalLinkerArgResponseFilePaths = swiftModuleAdditionalLinkerArgResponseFilePaths
             self.explicitDependencies = explicitDependencies
             self.topLevelItemPath = topLevelItemPath
             self.dsymPath = dsymPath
             self.xcframeworkSourcePath = xcframeworkSourcePath
             self.privacyFile = privacyFile
+            self.libPrefix = prefix
+            // Only use search paths when no prefix is required or when the prefix matches
+            let hasValidPrefix = libPrefix.map { path.basename.hasPrefix($0) } ?? true
+            self.useSearchPaths = hasValidPrefix && useSearchPaths
         }
     }
 

Sources/SWBCore/SpecImplementations/Specs.swift

@@ -301,6 +301,9 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
     /// Returns `true` if the `isWrapperFolder` value is set in the XCSpec for the file spec.
     public let isWrapper: Bool
 
+    /// Returns any common prefix this file may have (currently used when specifying searched libraries to linker)
+    public let prefix: String?
+
     required init(_ parser: SpecParser, _ basedOnSpec: Spec?) {
         let basedOnFileTypeSpec = basedOnSpec as? FileTypeSpec ?? nil
 
@@ -318,8 +321,8 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
         self.isEmbeddableInProduct = parser.parseBool("IsEmbeddable") ?? false
         self.validateOnCopy = parser.parseBool("ValidateOnCopy") ?? false
         self.codeSignOnCopy = parser.parseBool("CodeSignOnCopy") ?? false
-
         self.isWrapper = parser.parseBool("IsWrapperFolder") ?? false
+        self.prefix = parser.parseString("Prefix")
 
         // Parse and ignore keys we have no use for.
         //
@@ -358,7 +361,6 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
         parser.parseStringList("MIMETypes")
         parser.parseString("Permissions")
         parser.parseString("PlistStructureDefinition")
-        parser.parseStringList("Prefix")
         parser.parseBool("RemoveHeadersOnCopy")
         parser.parseBool("RequiresHardTabs")
         parser.parseString("UTI")

Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift

@@ -1312,41 +1312,12 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
     private static func computeLibraryArgs(_ libraries: [LibrarySpecifier], scope: MacroEvaluationScope) -> (args: [String], inputs: [Path]) {
         // Construct the library arguments.
         return libraries.compactMap { specifier -> (args: [String], inputs: [Path]) in
-            let basename = specifier.path.basename
-
-            // FIXME: This isn't a good system, we need to redesign how we talk to the linker w.r.t. search paths and our notion of paths.
             switch specifier.kind {
-            case .static:
-                if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".a") {
-                    return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(".a")), [])
-                }
-                return (specifier.absolutePathFlagsForLd(), [specifier.path])
-            case .dynamic:
-                let suffix = ".\(scope.evaluate(BuiltinMacros.DYNAMIC_LIBRARY_EXTENSION))"
-                if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(suffix) {
-                    return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(suffix)), [])
-                }
-                return (specifier.absolutePathFlagsForLd(), [specifier.path])
-            case .textBased:
-                if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".tbd") {
-                    // .merge and .reexport are not supported for text-based libraries.
-                    return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(".tbd")), [])
-                }
-                return (specifier.absolutePathFlagsForLd(), [specifier.path])
-            case .framework:
-                let frameworkName = Path(basename).withoutSuffix
+            case .static, .dynamic, .textBased, .framework:
                 if specifier.useSearchPaths {
-                    return (specifier.searchPathFlagsForLd(frameworkName), [])
+                    return (specifier.searchPathFlagsForLd(), [])
                 }
-                let absPathArgs = specifier.absolutePathFlagsForLd()
-                let returnPath: Path
-                if let pathArg = absPathArgs.last, Path(pathArg).basename == frameworkName {
-                    returnPath = Path(pathArg)
-                }
-                else {
-                    returnPath = specifier.path
-                }
-                return (absPathArgs, [returnPath])
+                return (specifier.absolutePathFlagsForLd(), [specifier.path])
             case .object:
                 // Object files are added to linker inputs in the sources task producer.
                 return ([], [])
@@ -1582,35 +1553,47 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
 
 /// Extensions to `LinkerSpec.LibrarySpecifier` specific to the dynamic linker.
 fileprivate extension LinkerSpec.LibrarySpecifier {
-    func searchPathFlagsForLd(_ name: String) -> [String] {
+
+    func searchPathFlagsForLd() -> [String] {
+        precondition(useSearchPaths)
+        // Extract basename once to avoid redundant operations
+        let basename = path.basename
+        let basenameWithoutSuffix = Path(basename).withoutSuffix
+        // Strip the prefix if one exists and is present in the basename
+        let strippedName: String
+        if let prefix = libPrefix, basename.hasPrefix(prefix) {
+            strippedName = basenameWithoutSuffix.withoutPrefix(prefix)
+        } else {
+            strippedName = basenameWithoutSuffix
+        }
         switch (kind, mode) {
         case (.dynamic, .normal):
-            return ["-l" + name]
+            return ["-l" + strippedName]
         case (.dynamic, .reexport):
-            return ["-Xlinker", "-reexport-l" + name]
+            return ["-Xlinker", "-reexport-l" + strippedName]
         case (.dynamic, .merge):
-            return ["-Xlinker", "-merge-l" + name]
+            return ["-Xlinker", "-merge-l" + strippedName]
         case (.dynamic, .reexport_merge):
-            return ["-Xlinker", "-no_merge-l" + name]
+            return ["-Xlinker", "-no_merge-l" + strippedName]
         case (.dynamic, .weak):
-            return ["-weak-l" + name]
+            return ["-weak-l" + strippedName]
         case (.static, .weak),
              (.textBased, .weak):
-            return ["-weak-l" + name]
+            return ["-weak-l" + strippedName]
         case (.static, _),
              (.textBased, _):
             // Other modes are not supported for these kinds.
-            return ["-l" + name]
+            return ["-l" + strippedName]
         case (.framework, .normal):
-            return ["-framework", name]
+            return ["-framework", strippedName]
         case (.framework, .reexport):
-            return ["-Xlinker", "-reexport_framework", "-Xlinker", name]
+            return ["-Xlinker", "-reexport_framework", "-Xlinker", strippedName]
         case (.framework, .merge):
-            return ["-Xlinker", "-merge_framework", "-Xlinker", name]
+            return ["-Xlinker", "-merge_framework", "-Xlinker", strippedName]
         case (.framework, .reexport_merge):
-            return ["-Xlinker", "-no_merge_framework", "-Xlinker", name]
+            return ["-Xlinker", "-no_merge_framework", "-Xlinker", strippedName]
         case (.framework, .weak):
-            return ["-weak_framework", name]
+            return ["-weak_framework", strippedName]
         case (.object, _):
             // Object files are added to linker inputs in the sources task producer.
             return []
@@ -1752,15 +1735,17 @@ public final class LibtoolLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @u
                     delegate.warning("Product \(cbc.output.basename) cannot weak-link \(specifier.kind) \(basename)")
                 }
 
-                if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".a") {
+                if specifier.useSearchPaths {
                     // Locate using search paths: Add a -l option and *don't* add the path to the library as an input to the task.
-                    return ["-l" + basename.withoutPrefix("lib").withoutSuffix(".a")]
-                }
-                else {
-                    // Locate using an absolute path: Add the path as an option and as an input to the task.
-                    inputPaths.append(specifier.path)
-                    return [specifier.path.str]
+                    let basename = specifier.path.basename
+                    let expectedPrefix = specifier.libPrefix ?? "lib"
+                    if basename.hasPrefix(expectedPrefix) {
+                        return ["-l" + Path(basename).withoutSuffix.withoutPrefix(expectedPrefix)]
+                    }
                 }
+                // Locate using an absolute path: Add the path as an option and as an input to the task.
+                inputPaths.append(specifier.path)
+                return [specifier.path.str]
 
             case .object:
                 // Object files are added to linker inputs in the sources task producer and so end up in the link-file-list.

Sources/SWBGenericUnixPlatform/Specs/Unix.xcspec

@@ -91,4 +91,14 @@
         IconNamePrefix = "TargetPlugin";
         DefaultTargetName = "Object File";
     },
+    {
+        Domain = generic-unix;
+        Type = FileType;
+        Identifier = compiled.mach-o.dylib;
+        BasedOn = compiled.mach-o;
+        Prefix = lib;
+        Extensions = (so);
+        IsLibrary = YES;
+        IsDynamicLibrary = YES;
+    }
 )

Sources/SWBTaskConstruction/TaskProducers/BuildPhaseTaskProducers/SourcesTaskProducer.swift

@@ -507,16 +507,25 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     useSearchPaths: useSearchPaths,
                     swiftModulePaths: swiftModulePaths,
                     swiftModuleAdditionalLinkerArgResponseFilePaths: swiftModuleAdditionalLinkerArgResponseFilePaths,
+                    prefix: fileType.prefix,
                     privacyFile: privacyFile
                 )
             } else if fileType.conformsTo(context.lookupFileType(identifier: "compiled.mach-o.dylib")!) {
+                let adjustedAbsolutePath: Path
+                // On Windows, ensure import libraries (.lib) are used instead of DLLs.
+                if context.sdkVariant?.llvmTargetTripleSys == "windows" && absolutePath.fileSuffix.lowercased() == ".dll" {
+                    adjustedAbsolutePath = Path(absolutePath.withoutSuffix + ".lib")
+                } else {
+                    adjustedAbsolutePath = absolutePath
+                }
                 return LinkerSpec.LibrarySpecifier(
                     kind: .dynamic,
-                    path: absolutePath,
+                    path: adjustedAbsolutePath,
                     mode: linkageModeForDylib(),
                     useSearchPaths: useSearchPaths,
                     swiftModulePaths: [:],
                     swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
+                    prefix: fileType.prefix,
                     privacyFile: privacyFile
                 )
             } else if fileType.conformsTo(context.lookupFileType(identifier: "sourcecode.text-based-dylib-definition")!) {
@@ -527,17 +536,18 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     useSearchPaths: useSearchPaths,
                     swiftModulePaths: [:],
                     swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
+                    prefix: fileType.prefix,
                     privacyFile: privacyFile
                 )
             } else if fileType.conformsTo(context.lookupFileType(identifier: "wrapper.framework")!) {
-                func kindFromSettings(_ settings: Settings) -> LinkerSpec.LibrarySpecifier.Kind? {
+                func kindFromSettings(_ settings: Settings) -> (kind: LinkerSpec.LibrarySpecifier.Kind, prefix: String?)? {
                     switch settings.globalScope.evaluate(BuiltinMacros.MACH_O_TYPE) {
                     case "staticlib":
-                        return .static
+                        return (.static, context.lookupFileType(identifier: "archive.ar")?.prefix)
                     case "mh_dylib":
-                        return .dynamic
+                        return (.dynamic, context.lookupFileType(identifier: "compiled.mach-o.dylib")?.prefix)
                     case "mh_object":
-                        return .object
+                        return (.object, nil)
                     default:
                         return nil
                     }
@@ -547,9 +557,11 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                 let path: Path
                 let dsymPath: Path?
                 let topLevelItemPath: Path?
+                let prefix: String?
                 if let settingsForRef, let presumedKind = kindFromSettings(settingsForRef), !useSearchPaths {
                     // If we have a Settings from a cross-project reference, use the _actual_ library path. This prevents downstream code from reconstituting the framework path by joining the framework path with the basename of the framework, which won't be correct for deep frameworks which also need the Versions/A path component.
-                    kind = presumedKind
+                    kind = presumedKind.kind
+                    prefix = presumedKind.prefix
                     path = settingsForRef.globalScope.evaluate(BuiltinMacros.TARGET_BUILD_DIR).join(settingsForRef.globalScope.evaluate(BuiltinMacros.EXECUTABLE_PATH)).normalize()
                     topLevelItemPath = absolutePath
                     if shouldGenerateDSYM(settingsForRef.globalScope) {
@@ -563,6 +575,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     path = absolutePath
                     topLevelItemPath = nil
                     dsymPath = nil
+                    prefix = nil
                 }
 
                 return LinkerSpec.LibrarySpecifier(
@@ -572,6 +585,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     useSearchPaths: useSearchPaths,
                     swiftModulePaths: [:],
                     swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
+                    prefix: prefix,
                     topLevelItemPath: topLevelItemPath,
                     dsymPath: dsymPath,
                     privacyFile: privacyFile
@@ -581,7 +595,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     kind: .object,
                     path: absolutePath,
                     mode: buildFile.shouldLinkWeakly ? .weak : .normal,
-                    useSearchPaths: useSearchPaths,
+                    useSearchPaths: false,
                     swiftModulePaths: swiftModulePaths,
                     swiftModuleAdditionalLinkerArgResponseFilePaths: swiftModuleAdditionalLinkerArgResponseFilePaths,
                     privacyFile: privacyFile
@@ -621,10 +635,16 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                 }
 
                 let libraryKind: LinkerSpec.LibrarySpecifier.Kind
+                let prefix: String?
                 switch library.libraryType {
-                case .framework: libraryKind = .framework; break
-                case .dynamicLibrary: libraryKind = .dynamic; break
-                case .staticLibrary: libraryKind = .static; break
+                case .framework: libraryKind = .framework; prefix = nil
+                case .dynamicLibrary:
+                    libraryKind = .dynamic;
+                    prefix = context.lookupFileType(identifier: "compiled.mach-o.dylib")?.prefix
+                case .staticLibrary:
+                    libraryKind = .static
+                    prefix = context.lookupFileType(identifier: "archive.ar")?.prefix
+                break
                 case let .unknown(fileExtension):
                     // An error of type this type should have already been manifested.
                     assertionFailure("unknown xcframework type: \(fileExtension)")
@@ -651,6 +671,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
                     useSearchPaths: useSearchPaths,
                     swiftModulePaths: [:],
                     swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
+                    prefix: prefix,
                     explicitDependencies: outputFilePaths,
                     xcframeworkSourcePath: xcframeworkPath,
                     privacyFile: nil

Sources/SWBTestSupport/RunDestinationTestSupport.swift

@@ -319,7 +319,7 @@ extension RunDestinationInfo {
     /// An `Environment` object with `PATH` or `LD_LIBRARY_PATH` set appropriately pointing into the toolchain to be able to run a built Swift binary in tests.
     ///
     /// - note: On macOS, the OS provided Swift runtime is used, so `DYLD_LIBRARY_PATH` is never set for Mach-O destinations.
-    package func hostRuntimeEnvironment(_ core: Core, initialEnvironment: Environment = Environment()) -> Environment {
+    package func hostRuntimeEnvironment(_ core: Core, initialEnvironment: Environment = Environment()) throws -> Environment {
         var environment = initialEnvironment
         guard let toolchain = core.toolchainRegistry.defaultToolchain else {
             return environment
@@ -328,6 +328,21 @@ extension RunDestinationInfo {
         case .elf:
             environment.prependPath(key: "LD_LIBRARY_PATH", value: toolchain.path.join("usr/lib/swift/\(platform)").str)
         case .pe:
+            if let path = core.platformRegistry.lookup(name: platform)?.platform?.path.join("Developer/Library") {
+                func matchesArch(_ path: Path) -> Bool {
+                    switch Architecture.hostStringValue {
+                    case "x86_64":
+                        return path.basename == "bin64"
+                    case "aarch64":
+                        return path.basename == "bin64a"
+                    default:
+                        return false
+                    }
+                }
+                for dir in try localFS.traverse(path, { $0 }).sorted() where localFS.isDirectory(dir) && matchesArch(dir) {
+                    environment.prependPath(key: .path, value: dir.str)
+                }
+            }
             environment.prependPath(key: .path, value: core.developerPath.path.join("Runtimes").join(toolchain.version.description).join("usr/bin").str)
         case .macho:
             // Fall back to the OS provided Swift runtime