Changes based on review feedback.

* Rename LD_MULTIARCH to _LD_MULTIARCH.  This build setting should not
  be used by external clients, so hide it using the underscore notation.
* Add in mappings for future supported architectures i386 and thumbv7.
* Drop arm64->arm64 mapping, handled by aarch64.
* Fix logic parsing for architecture prefix mappings, could have crashed
  accessing out of bound element.
* Rebase changes ontop of SDK platform registration refactor.

Author: kcieplak

Sources/SWBCore/PlatformRegistry.swift

@@ -349,7 +349,7 @@ public final class PlatformRegistry {
             var fs: any FSProxy
         }
 
-        for platformExtension in platformInfoExtensions() {
+        for platformExtension in await platformInfoExtensions() {
             do {
                 for (path, data) in try platformExtension.additionalPlatforms(context: Context(hostOperatingSystem: hostOperatingSystem, developerPath: delegate.developerPath, fs: fs)) {
                     await registerPlatform(path, .plDict(data), fs)
@@ -632,8 +632,8 @@ public final class PlatformRegistry {
             path.join("Developer").join("SDKs")
         ]
 
-        for platformExtension in platformInfoExtensions() {
-            await executableSearchPaths.append(contentsOf: platformExtension.additionalPlatformExecutableSearchPaths(platformName: name, platformPath: path))
+        for platformExtension in await platformInfoExtensions() {
+            await executableSearchPaths.append(contentsOf: platformExtension.additionalPlatformExecutableSearchPaths(platformName: name, platformPath: path, fs: localFS))
 
             platformExtension.adjustPlatformSDKSearchPaths(platformName: name, platformPath: path, sdkSearchPaths: &sdkSearchPaths)
 

Sources/SWBCore/Settings/BuiltinMacros.swift

@@ -803,8 +803,8 @@ public final class BuiltinMacros {
     public static let LD_RUNPATH_SEARCH_PATHS = BuiltinMacros.declareStringListMacro("LD_RUNPATH_SEARCH_PATHS")
     public static let LD_SDK_IMPORTS_FILE = BuiltinMacros.declarePathMacro("LD_SDK_IMPORTS_FILE")
     public static let LD_WARN_UNUSED_DYLIBS = BuiltinMacros.declareBooleanMacro("LD_WARN_UNUSED_DYLIBS")
-    public static let LD_MULTIARCH = BuiltinMacros.declareBooleanMacro("LD_MULTIARCH")
-    public static let LD_MULTIARCH_PREFIX_MAP = BuiltinMacros.declareStringListMacro("LD_MULTIARCH_PREFIX_MAP")
+    public static let _LD_MULTIARCH = BuiltinMacros.declareBooleanMacro("_LD_MULTIARCH")
+    public static let _LD_MULTIARCH_PREFIX_MAP = BuiltinMacros.declareStringListMacro("_LD_MULTIARCH_PREFIX_MAP")
     public static let LEX = BuiltinMacros.declarePathMacro("LEX")
     public static let LEXFLAGS = BuiltinMacros.declareStringListMacro("LEXFLAGS")
     public static let LIBRARIAN = BuiltinMacros.declareStringMacro("LIBRARIAN")
@@ -1860,8 +1860,8 @@ public final class BuiltinMacros {
         LD_RUNPATH_SEARCH_PATHS,
         LD_SDK_IMPORTS_FILE,
         LD_WARN_UNUSED_DYLIBS,
-        LD_MULTIARCH,
-        LD_MULTIARCH_PREFIX_MAP,
+        _LD_MULTIARCH,
+        _LD_MULTIARCH_PREFIX_MAP,
         LEGACY_DEVELOPER_DIR,
         LEX,
         LEXFLAGS,

Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift

@@ -1309,7 +1309,7 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
         // Note: On Linux you cannot invoke the llvm linker by the direct name for determining the version,
         // you need to use ld.<ALTERNATE_LINKER>
         let alternateLinker = scope.evaluate(BuiltinMacros.ALTERNATE_LINKER)
-        let isLinkerMultiarch = scope.evaluate(BuiltinMacros.LD_MULTIARCH)
+        let isLinkerMultiarch = scope.evaluate(BuiltinMacros._LD_MULTIARCH)
 
         var linkerPath = producer.hostOperatingSystem == .windows ? Path("ld.lld") : Path("ld")
         if alternateLinker != "" && alternateLinker != "ld" && alternateLinker != "link" {
@@ -1320,29 +1320,28 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
         // If the linker does not support multiple architectures update the path to include a subfolder based on the prefix map
         // to find the architecture specific executable.
         if !isLinkerMultiarch {
-            let archMap = scope.evaluate(BuiltinMacros.LD_MULTIARCH_PREFIX_MAP)
+            let archMap = scope.evaluate(BuiltinMacros._LD_MULTIARCH_PREFIX_MAP)
             let archMappings = archMap.reduce(into: [String: String]()) { mappings, map in
-                let split = map.components(separatedBy: ":")
-                if !split.isEmpty {
-                    return mappings[split[0]] = split[1]
+                let (arch, prefixDir) = map.split(":")
+                if !arch.isEmpty && !prefixDir.isEmpty {
+                    return mappings[arch] = prefixDir
                 }
             }
             if archMappings.isEmpty {
-                delegate.error("LD_MULTIARCH is 'false', but no prefix mappings are present in LD_MULTIARCH_PREFIX_MAP")
+                delegate.error("_LD_MULTIARCH is 'false', but no prefix mappings are present in _LD_MULTIARCH_PREFIX_MAP")
                 return nil
             }
             // Linkers that don't support multiple architectures cannot support universal binaries, so ARCHS will
             // contain the target architecture and can only be a single value.
-            let arch = scope.evaluate(BuiltinMacros.ARCHS)
-            if arch.count > 1 {
-                delegate.error("LD_MULTIARCH is 'false', but multiple ARCHS have been given, this is invalid")
+            guard let arch = scope.evaluate(BuiltinMacros.ARCHS).only else {
+                delegate.error("_LD_MULTIARCH is 'false', but multiple ARCHS have been given, this is invalid")
                 return nil
             }
-            if let prefix = archMappings[arch[0]] {
+            if let prefix = archMappings[arch] {
                 // Add in the target architecture prefix directory to path for search.
                 linkerPath = Path(prefix).join(linkerPath)
             } else {
-                delegate.error("Could not find prefix mapping for \(arch[0]) in LD_MULTIARCH_PREFIX_MAP")
+                delegate.error("Could not find prefix mapping for \(arch) in _LD_MULTIARCH_PREFIX_MAP")
                 return nil
             }
         }

Sources/SWBCore/Specs/CoreBuildSystem.xcspec

@@ -1149,7 +1149,7 @@ When `GENERATE_INFOPLIST_FILE` is enabled, sets the value of the [CFBundleIdenti
                 Category = "Linking - Warnings";
             },
             {
-                Name = "LD_MULTIARCH";
+                Name = "_LD_MULTIARCH";
                 Type = Boolean;
                 DefaultValue = YES;
             },

Sources/SWBCore/Specs/en.lproj/CoreBuildSystem.strings

@@ -423,13 +423,6 @@ You cannot create a PIE from `.o` files compiled with `-mdynamic-no-pic`. Using
 "[LD_WARN_DUPLICATE_LIBRARIES]-name" = "Duplicate Libraries";
 "[LD_WARN_DUPLICATE_LIBRARIES]-description" = "Warn for linking the same library multiple times.";
 
-"[LD_MULTIARCH]-name" = "Multiple Architecture Supporte Linker";
-"[LD_MULTIARCH]-description" = "Linker supports linking multiple target architectures.";
-
-"[LD_MULTIARCH_PREFIX_MAP]-name" = "Linker subfolder architecture map";
-"[LD_MULTIARCH_PREFIX_MAP]-description" = "Mapping of architecture to subfolder. <arch>:<subfolder>";
-
-
 // Localization Settings
 
 "[Localization]-category" = "Localization";

Sources/SWBTestSupport/CoreBasedTests.swift

@@ -302,7 +302,7 @@ extension CoreBasedTests {
     }
     package func linkPath(_ targetArchitecture: String) async throws -> Path? {
         let (core, defaultToolchain) = try await self.coreAndToolchain()
-        let prefixMapping = [ "x86_64": "x64", "aarch64": "arm64", "arm64": "arm64" ]
+        let prefixMapping = [ "x86_64": "x64", "aarch64": "arm64", "i386": "x86", "thumbv7" : "arm" ]
 
         guard let prefix = prefixMapping[targetArchitecture] else {
             return nil

Sources/SWBWindowsPlatform/Plugin.swift

@@ -18,19 +18,37 @@ import Foundation
     let plugin = WindowsPlugin()
     manager.register(WindowsPlatformSpecsExtension(), type: SpecificationsExtensionPoint.self)
     manager.register(WindowsEnvironmentExtension(plugin: plugin), type: EnvironmentExtensionPoint.self)
-    manager.register(WindowsPlatformExtension(), type: PlatformInfoExtensionPoint.self)
+    manager.register(WindowsPlatformExtension(plugin: plugin), type: PlatformInfoExtensionPoint.self)
     manager.register(WindowsSDKRegistryExtension(), type: SDKRegistryExtensionPoint.self)
 }
 
 public final class WindowsPlugin: Sendable {
     private let vsInstallations = AsyncSingleValueCache<[VSInstallation], any Error>()
+    private let latestVsInstalltionDirectory = AsyncSingleValueCache<Path?, any Error>()
 
     public func cachedVSInstallations() async throws -> [VSInstallation] {
         try await vsInstallations.value {
             // Always pass localFS because this will be cached, and executes a process on the host system so there's no reason to pass in any proxy.
             try await VSInstallation.findInstallations(fs: localFS)
         }
     }
+
+    func cachedLatestVSInstallDirectory(fs: any FSProxy) async throws -> Path? {
+       try await latestVsInstalltionDirectory.value {
+            let installations = try await cachedVSInstallations()
+            .sorted(by: { $0.installationVersion > $1.installationVersion })
+            if let latest = installations.first {
+                let msvcDir = latest.installationPath.join("VC").join("Tools").join("MSVC")
+                if fs.exists(msvcDir) {
+                    let versions = try fs.listdir(msvcDir).map { try Version($0) }.sorted { $0 > $1 }
+                    if let latestVersion = versions.first {
+                        return msvcDir.join(latestVersion.description)
+                    }
+                }
+            }
+            return nil
+        }
+    }
 }
 
 struct WindowsPlatformSpecsExtension: SpecificationsExtension {
@@ -49,10 +67,12 @@ struct WindowsPlatformSpecsExtension: SpecificationsExtension {
                 return [:]
             }
             return [vcToolsInstallDir: dir.str]
+        } else {
+           return [:]
         }
     }
 
-     @_spi(Testing)public func findLatestInstallDirectory(fs: any FSProxy) async throws -> Path? {
+    @_spi(Testing)public func findLatestInstallDirectory(fs: any FSProxy) async throws -> Path? {
     let plugin: WindowsPlugin
     let installations = try await plugin.cachedVSInstallations()
         .sorted(by: { $0.installationVersion > $1.installationVersion })
@@ -67,9 +87,9 @@ struct WindowsPlatformSpecsExtension: SpecificationsExtension {
     }
     return nil
 }
-}
 
 struct WindowsPlatformExtension: PlatformInfoExtension {
+    let plugin: WindowsPlugin
     func additionalPlatforms(context: any PlatformInfoExtensionAdditionalPlatformsContext) throws -> [(path: Path, data: [String: PropertyListItem])] {
         let operatingSystem = context.hostOperatingSystem
         guard operatingSystem == .windows else {
@@ -115,13 +135,19 @@ struct WindowsPlatformExtension: PlatformInfoExtension {
     }
 
     public func additionalPlatformExecutableSearchPaths(platformName: String, platformPath: Path, fs: any FSProxy) async -> [Path] {
-        guard let dir = try? await findLatestInstallDirectory(fs: fs) else {
+        guard let dir = try? await plugin.cachedLatestVSInstallDirectory(fs: fs) else {
             return []
         }
-        if Architecture.hostStringValue == "aarch64" {
-            return [dir.join("bin/Hostarm64/arm64")]
-        } else {
-            return [dir.join("bin/Hostx64/x64")]
+
+        // Note: Do not add in the target directories under the host as these will end up in the global search paths, i.e. PATH
+        // Let the commandlinetool discovery add in the target subdirectory based on the targeted architecture.
+        switch Architecture.hostStringValue {
+        case "aarch64":
+            return [dir.join("bin/Hostarm64")]
+        case "x86_64":
+            return [dir.join("bin/Hostx64")]
+        default:
+            return []
         }
     }
 }

Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec

@@ -53,15 +53,15 @@
                 IsInputDependency = Yes;
             },
             {
-                Name = LD_MULTIARCH;
+                Name = _LD_MULTIARCH;
                 Type = Boolean;
                 DefaultValue = NO;
                 Condition = "$(ALTERNATE_LINKER) == link";
             },
             {
-                Name = LD_MULTIARCH_PREFIX_MAP;
+                Name = _LD_MULTIARCH_PREFIX_MAP;
                 Type = StringList;
-                DefaultValue = "x86_64:x64 aarch64:arm64 arm64:arm64";
+                DefaultValue = "x86_64:x64 aarch64:arm64 i386:x86 thumbv7:arm";
                 Condition = "$(ALTERNATE_LINKER) == link";
             },
             {

Tests/SWBCoreTests/CommandLineToolSpecDiscoveredInfoTests.swift

@@ -159,7 +159,7 @@ import SWBMacro
     @Test
     func discoveredLdLinkerSpecInfo() async throws {
         var table = try await ldMacroTable()
-        table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
+        table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
         // Default Linker, just check we have one.
         try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
             #expect(!info.toolPath.isEmpty)
@@ -173,7 +173,7 @@ import SWBMacro
     @Test(.requireSDKs(.macOS))
     func discoveredLdLinkerSpecInfo_macOS() async throws {
         var table = try await ldMacroTable()
-        table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
+        table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
         // Default Linker
         try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
             #expect(!info.toolPath.isEmpty)
@@ -191,7 +191,7 @@ import SWBMacro
     @Test(.requireSDKs(.linux))
     func discoveredLdLinkerSpecInfo_Linux() async throws {
         var table = try await ldMacroTable()
-        table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
+        table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
         // Default Linker
         try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
             #expect(!info.toolPath.isEmpty)
@@ -237,7 +237,7 @@ import SWBMacro
     @Test(.requireSDKs(.windows))
     func discoveredLdLinkerSpecInfo_Windows() async throws {
         var table = try await ldMacroTable()
-        table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
+        table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
         try await withSpec(LdLinkerSpec.self, .deferred, platform: "windows", additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
             #expect(!info.toolPath.isEmpty)
             #expect(info.toolVersion != nil)
@@ -254,8 +254,8 @@ import SWBMacro
 
         // link.exe cannot be used for multipler architectures and requires a distinct link.exe for each target architecture
         table.push(BuiltinMacros.ALTERNATE_LINKER, literal: "link")
-        table.push(BuiltinMacros.LD_MULTIARCH, literal: false)
-        table.push(BuiltinMacros.LD_MULTIARCH_PREFIX_MAP, literal: ["x86_64:x64", "aarch64:arm64", "arm64:arm64"])
+        table.push(BuiltinMacros._LD_MULTIARCH, literal: false)
+        table.push(BuiltinMacros._LD_MULTIARCH_PREFIX_MAP, literal: ["x86_64:x64", "aarch64:arm64", "i386:x86", "thumbv7:arm"])
 
         // x86_64
         table.push(BuiltinMacros.ARCHS, literal: ["x86_64"])
@@ -268,6 +268,7 @@ import SWBMacro
             #expect(info.linker == .linkExe)
         }
 
+        // FIXME: Fails in swift-ci missing arm64 toolchain on x86_64 host.
         // // link aarch64
         // table.push(BuiltinMacros.ARCHS, literal: ["aarch64"])
         // try await withSpec(LdLinkerSpec.self, .deferred, platform: "windows", additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in