Skip to content

Commit bdcdb21

Browse files
kcieplakkcieplak
committed
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.
1 parent 74ed654 commit bdcdb21

File tree

9 files changed

+62
-59
lines changed

9 files changed

+62
-59
lines changed

‎Sources/SWBCore/PlatformRegistry.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ public final class PlatformRegistry {
349349
var fs: any FSProxy
350350
}
351351

352-
for platformExtension in platformInfoExtensions() {
352+
for platformExtension in await platformInfoExtensions() {
353353
do {
354354
for (path, data) in try platformExtension.additionalPlatforms(context: Context(hostOperatingSystem: hostOperatingSystem, developerPath: delegate.developerPath, fs: fs)) {
355355
await registerPlatform(path, .plDict(data), fs)
@@ -632,8 +632,8 @@ public final class PlatformRegistry {
632632
path.join("Developer").join("SDKs")
633633
]
634634

635-
for platformExtension in platformInfoExtensions() {
636-
await executableSearchPaths.append(contentsOf: platformExtension.additionalPlatformExecutableSearchPaths(platformName: name, platformPath: path))
635+
for platformExtension in await platformInfoExtensions() {
636+
await executableSearchPaths.append(contentsOf: platformExtension.additionalPlatformExecutableSearchPaths(platformName: name, platformPath: path, fs: localFS))
637637

638638
platformExtension.adjustPlatformSDKSearchPaths(platformName: name, platformPath: path, sdkSearchPaths: &sdkSearchPaths)
639639

‎Sources/SWBCore/Settings/BuiltinMacros.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -803,8 +803,8 @@ public final class BuiltinMacros {
803803
public static let LD_RUNPATH_SEARCH_PATHS = BuiltinMacros.declareStringListMacro("LD_RUNPATH_SEARCH_PATHS")
804804
public static let LD_SDK_IMPORTS_FILE = BuiltinMacros.declarePathMacro("LD_SDK_IMPORTS_FILE")
805805
public static let LD_WARN_UNUSED_DYLIBS = BuiltinMacros.declareBooleanMacro("LD_WARN_UNUSED_DYLIBS")
806-
public static let LD_MULTIARCH = BuiltinMacros.declareBooleanMacro("LD_MULTIARCH")
807-
public static let LD_MULTIARCH_PREFIX_MAP = BuiltinMacros.declareStringListMacro("LD_MULTIARCH_PREFIX_MAP")
806+
public static let _LD_MULTIARCH = BuiltinMacros.declareBooleanMacro("_LD_MULTIARCH")
807+
public static let _LD_MULTIARCH_PREFIX_MAP = BuiltinMacros.declareStringListMacro("_LD_MULTIARCH_PREFIX_MAP")
808808
public static let LEX = BuiltinMacros.declarePathMacro("LEX")
809809
public static let LEXFLAGS = BuiltinMacros.declareStringListMacro("LEXFLAGS")
810810
public static let LIBRARIAN = BuiltinMacros.declareStringMacro("LIBRARIAN")
@@ -1860,8 +1860,8 @@ public final class BuiltinMacros {
18601860
LD_RUNPATH_SEARCH_PATHS,
18611861
LD_SDK_IMPORTS_FILE,
18621862
LD_WARN_UNUSED_DYLIBS,
1863-
LD_MULTIARCH,
1864-
LD_MULTIARCH_PREFIX_MAP,
1863+
_LD_MULTIARCH,
1864+
_LD_MULTIARCH_PREFIX_MAP,
18651865
LEGACY_DEVELOPER_DIR,
18661866
LEX,
18671867
LEXFLAGS,

‎Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
13091309
// Note: On Linux you cannot invoke the llvm linker by the direct name for determining the version,
13101310
// you need to use ld.<ALTERNATE_LINKER>
13111311
let alternateLinker = scope.evaluate(BuiltinMacros.ALTERNATE_LINKER)
1312-
let isLinkerMultiarch = scope.evaluate(BuiltinMacros.LD_MULTIARCH)
1312+
let isLinkerMultiarch = scope.evaluate(BuiltinMacros._LD_MULTIARCH)
13131313

13141314
var linkerPath = producer.hostOperatingSystem == .windows ? Path("ld.lld") : Path("ld")
13151315
if alternateLinker != "" && alternateLinker != "ld" && alternateLinker != "link" {
@@ -1320,29 +1320,28 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
13201320
// If the linker does not support multiple architectures update the path to include a subfolder based on the prefix map
13211321
// to find the architecture specific executable.
13221322
if !isLinkerMultiarch {
1323-
let archMap = scope.evaluate(BuiltinMacros.LD_MULTIARCH_PREFIX_MAP)
1323+
let archMap = scope.evaluate(BuiltinMacros._LD_MULTIARCH_PREFIX_MAP)
13241324
let archMappings = archMap.reduce(into: [String: String]()) { mappings, map in
1325-
let split = map.components(separatedBy: ":")
1326-
if !split.isEmpty {
1327-
return mappings[split[0]] = split[1]
1325+
let (arch, prefixDir) = map.split(":")
1326+
if !arch.isEmpty && !prefixDir.isEmpty {
1327+
return mappings[arch] = prefixDir
13281328
}
13291329
}
13301330
if archMappings.isEmpty {
1331-
delegate.error("LD_MULTIARCH is 'false', but no prefix mappings are present in LD_MULTIARCH_PREFIX_MAP")
1331+
delegate.error("_LD_MULTIARCH is 'false', but no prefix mappings are present in _LD_MULTIARCH_PREFIX_MAP")
13321332
return nil
13331333
}
13341334
// Linkers that don't support multiple architectures cannot support universal binaries, so ARCHS will
13351335
// contain the target architecture and can only be a single value.
1336-
let arch = scope.evaluate(BuiltinMacros.ARCHS)
1337-
if arch.count > 1 {
1338-
delegate.error("LD_MULTIARCH is 'false', but multiple ARCHS have been given, this is invalid")
1336+
guard let arch = scope.evaluate(BuiltinMacros.ARCHS).only else {
1337+
delegate.error("_LD_MULTIARCH is 'false', but multiple ARCHS have been given, this is invalid")
13391338
return nil
13401339
}
1341-
if let prefix = archMappings[arch[0]] {
1340+
if let prefix = archMappings[arch] {
13421341
// Add in the target architecture prefix directory to path for search.
13431342
linkerPath = Path(prefix).join(linkerPath)
13441343
} else {
1345-
delegate.error("Could not find prefix mapping for \(arch[0]) in LD_MULTIARCH_PREFIX_MAP")
1344+
delegate.error("Could not find prefix mapping for \(arch) in _LD_MULTIARCH_PREFIX_MAP")
13461345
return nil
13471346
}
13481347
}

‎Sources/SWBCore/Specs/CoreBuildSystem.xcspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,7 @@ When `GENERATE_INFOPLIST_FILE` is enabled, sets the value of the [CFBundleIdenti
11491149
Category = "Linking - Warnings";
11501150
},
11511151
{
1152-
Name = "LD_MULTIARCH";
1152+
Name = "_LD_MULTIARCH";
11531153
Type = Boolean;
11541154
DefaultValue = YES;
11551155
},

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -423,13 +423,6 @@ You cannot create a PIE from `.o` files compiled with `-mdynamic-no-pic`. Using
423423
"[LD_WARN_DUPLICATE_LIBRARIES]-name" = "Duplicate Libraries";
424424
"[LD_WARN_DUPLICATE_LIBRARIES]-description" = "Warn for linking the same library multiple times.";
425425

426-
"[LD_MULTIARCH]-name" = "Multiple Architecture Supporte Linker";
427-
"[LD_MULTIARCH]-description" = "Linker supports linking multiple target architectures.";
428-
429-
"[LD_MULTIARCH_PREFIX_MAP]-name" = "Linker subfolder architecture map";
430-
"[LD_MULTIARCH_PREFIX_MAP]-description" = "Mapping of architecture to subfolder. <arch>:<subfolder>";
431-
432-
433426
// Localization Settings
434427

435428
"[Localization]-category" = "Localization";

‎Sources/SWBTestSupport/CoreBasedTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ extension CoreBasedTests {
302302
}
303303
package func linkPath(_ targetArchitecture: String) async throws -> Path? {
304304
let (core, defaultToolchain) = try await self.coreAndToolchain()
305-
let prefixMapping = [ "x86_64": "x64", "aarch64": "arm64", "arm64": "arm64" ]
305+
let prefixMapping = [ "x86_64": "x64", "aarch64": "arm64", "i386": "x86", "thumbv7" : "arm" ]
306306

307307
guard let prefix = prefixMapping[targetArchitecture] else {
308308
return nil

‎Sources/SWBWindowsPlatform/Plugin.swift

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,37 @@ import Foundation
1818
let plugin = WindowsPlugin()
1919
manager.register(WindowsPlatformSpecsExtension(), type: SpecificationsExtensionPoint.self)
2020
manager.register(WindowsEnvironmentExtension(plugin: plugin), type: EnvironmentExtensionPoint.self)
21-
manager.register(WindowsPlatformExtension(), type: PlatformInfoExtensionPoint.self)
21+
manager.register(WindowsPlatformExtension(plugin: plugin), type: PlatformInfoExtensionPoint.self)
2222
manager.register(WindowsSDKRegistryExtension(), type: SDKRegistryExtensionPoint.self)
2323
}
2424

2525
final class WindowsPlugin: Sendable {
2626
private let vsInstallations = AsyncSingleValueCache<[VSInstallation], any Error>()
27+
private let latestVsInstalltionDirectory = AsyncSingleValueCache<Path?, any Error>()
2728

2829
func cachedVSInstallations() async throws -> [VSInstallation] {
2930
try await vsInstallations.value {
3031
// 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.
3132
try await VSInstallation.findInstallations(fs: localFS)
3233
}
3334
}
35+
36+
func cachedLatestVSInstallDirectory(fs: any FSProxy) async throws -> Path? {
37+
try await latestVsInstalltionDirectory.value {
38+
let installations = try await cachedVSInstallations()
39+
.sorted(by: { $0.installationVersion > $1.installationVersion })
40+
if let latest = installations.first {
41+
let msvcDir = latest.installationPath.join("VC").join("Tools").join("MSVC")
42+
if fs.exists(msvcDir) {
43+
let versions = try fs.listdir(msvcDir).map { try Version($0) }.sorted { $0 > $1 }
44+
if let latestVersion = versions.first {
45+
return msvcDir.join(latestVersion.description)
46+
}
47+
}
48+
}
49+
return nil
50+
}
51+
}
3452
}
3553

3654
struct WindowsPlatformSpecsExtension: SpecificationsExtension {
@@ -39,37 +57,23 @@ struct WindowsPlatformSpecsExtension: SpecificationsExtension {
3957
}
4058
}
4159

42-
private func findLatestInstallDirectory(fs: any FSProxy) async throws -> Path? {
43-
let plugin: WindowsPlugin
44-
let installations = try await plugin.cachedVSInstallations()
45-
.sorted(by: { $0.installationVersion > $1.installationVersion })
46-
if let latest = installations.first {
47-
let msvcDir = latest.installationPath.join("VC").join("Tools").join("MSVC")
48-
if fs.exists(msvcDir) {
49-
let versions = try fs.listdir(msvcDir).map { try Version($0) }.sorted { $0 > $1 }
50-
if let latestVersion = versions.first {
51-
return msvcDir.join(latestVersion.description)
52-
}
53-
}
54-
}
55-
return nil
56-
}
57-
5860
struct WindowsEnvironmentExtension: EnvironmentExtension {
5961
let plugin: WindowsPlugin
6062

6163
func additionalEnvironmentVariables(context: any EnvironmentExtensionAdditionalEnvironmentVariablesContext) async throws -> [String: String] {
6264
if context.hostOperatingSystem == .windows {
6365
let vcToolsInstallDir = "VCToolsInstallDir"
64-
guard let dir = try? await findLatestInstallDirectory(fs: context.fs) else {
66+
guard let dir = try? await plugin.cachedLatestVSInstallDirectory(fs: context.fs) else {
6567
return [:]
6668
}
6769
return [vcToolsInstallDir: dir.str]
6870
}
71+
return [:]
6972
}
7073
}
7174

7275
struct WindowsPlatformExtension: PlatformInfoExtension {
76+
let plugin: WindowsPlugin
7377
func additionalPlatforms(context: any PlatformInfoExtensionAdditionalPlatformsContext) throws -> [(path: Path, data: [String: PropertyListItem])] {
7478
let operatingSystem = context.hostOperatingSystem
7579
guard operatingSystem == .windows else {
@@ -115,13 +119,19 @@ struct WindowsPlatformExtension: PlatformInfoExtension {
115119
}
116120

117121
public func additionalPlatformExecutableSearchPaths(platformName: String, platformPath: Path, fs: any FSProxy) async -> [Path] {
118-
guard let dir = try? await findLatestInstallDirectory(fs: fs) else {
122+
guard let dir = try? await plugin.cachedLatestVSInstallDirectory(fs: fs) else {
119123
return []
120124
}
121-
if Architecture.hostStringValue == "aarch64" {
122-
return [dir.join("bin/Hostarm64/arm64")]
123-
} else {
124-
return [dir.join("bin/Hostx64/x64")]
125+
126+
// Note: Do not add in the target directories under the host as these will end up in the global search paths, i.e. PATH
127+
// Let the commandlinetool discovery add in the target subdirectory based on the targeted architecture.
128+
switch Architecture.hostStringValue {
129+
case "aarch64":
130+
return [dir.join("bin/Hostarm64")]
131+
case "x86_64":
132+
return [dir.join("bin/Hostx64")]
133+
default:
134+
return []
125135
}
126136
}
127137
}

‎Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@
5353
IsInputDependency = Yes;
5454
},
5555
{
56-
Name = LD_MULTIARCH;
56+
Name = _LD_MULTIARCH;
5757
Type = Boolean;
5858
DefaultValue = NO;
5959
Condition = "$(ALTERNATE_LINKER) == link";
6060
},
6161
{
62-
Name = LD_MULTIARCH_PREFIX_MAP;
62+
Name = _LD_MULTIARCH_PREFIX_MAP;
6363
Type = StringList;
64-
DefaultValue = "x86_64:x64 aarch64:arm64 arm64:arm64";
64+
DefaultValue = "x86_64:x64 aarch64:arm64 i386:x86 thumbv7:arm";
6565
Condition = "$(ALTERNATE_LINKER) == link";
6666
},
6767
{

‎Tests/SWBCoreTests/CommandLineToolSpecDiscoveredInfoTests.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ import SWBMacro
159159
@Test
160160
func discoveredLdLinkerSpecInfo() async throws {
161161
var table = try await ldMacroTable()
162-
table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
162+
table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
163163
// Default Linker, just check we have one.
164164
try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
165165
#expect(!info.toolPath.isEmpty)
@@ -173,7 +173,7 @@ import SWBMacro
173173
@Test(.requireSDKs(.macOS))
174174
func discoveredLdLinkerSpecInfo_macOS() async throws {
175175
var table = try await ldMacroTable()
176-
table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
176+
table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
177177
// Default Linker
178178
try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
179179
#expect(!info.toolPath.isEmpty)
@@ -191,7 +191,7 @@ import SWBMacro
191191
@Test(.requireSDKs(.linux))
192192
func discoveredLdLinkerSpecInfo_Linux() async throws {
193193
var table = try await ldMacroTable()
194-
table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
194+
table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
195195
// Default Linker
196196
try await withSpec(LdLinkerSpec.self, .deferred, additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
197197
#expect(!info.toolPath.isEmpty)
@@ -237,7 +237,7 @@ import SWBMacro
237237
@Test(.requireSDKs(.windows))
238238
func discoveredLdLinkerSpecInfo_Windows() async throws {
239239
var table = try await ldMacroTable()
240-
table.push(BuiltinMacros.LD_MULTIARCH, literal: true)
240+
table.push(BuiltinMacros._LD_MULTIARCH, literal: true)
241241
try await withSpec(LdLinkerSpec.self, .deferred, platform: "windows", additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in
242242
#expect(!info.toolPath.isEmpty)
243243
#expect(info.toolVersion != nil)
@@ -254,8 +254,8 @@ import SWBMacro
254254

255255
// link.exe cannot be used for multipler architectures and requires a distinct link.exe for each target architecture
256256
table.push(BuiltinMacros.ALTERNATE_LINKER, literal: "link")
257-
table.push(BuiltinMacros.LD_MULTIARCH, literal: false)
258-
table.push(BuiltinMacros.LD_MULTIARCH_PREFIX_MAP, literal: ["x86_64:x64", "aarch64:arm64", "arm64:arm64"])
257+
table.push(BuiltinMacros._LD_MULTIARCH, literal: false)
258+
table.push(BuiltinMacros._LD_MULTIARCH_PREFIX_MAP, literal: ["x86_64:x64", "aarch64:arm64", "i386:x86", "thumbv7:arm"])
259259

260260
// x86_64
261261
table.push(BuiltinMacros.ARCHS, literal: ["x86_64"])
@@ -268,6 +268,7 @@ import SWBMacro
268268
#expect(info.linker == .linkExe)
269269
}
270270

271+
// FIXME: Fails in swift-ci missing arm64 toolchain on x86_64 host.
271272
// // link aarch64
272273
// table.push(BuiltinMacros.ARCHS, literal: ["aarch64"])
273274
// try await withSpec(LdLinkerSpec.self, .deferred, platform: "windows", additionalTable: table) { (info: DiscoveredLdLinkerToolSpecInfo) in

0 commit comments

Comments
 (0)