Skip to content

Commit a8d9e01

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 c57c44d commit a8d9e01

File tree

9 files changed

+58
-64
lines changed

9 files changed

+58
-64
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: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,49 +26,34 @@ struct WindowsPlatformSpecsExtension: SpecificationsExtension {
2626
}
2727
}
2828

29-
private func findLatestInstallDirectory(context: any EnvironmentExtensionAdditionalEnvironmentVariablesContext) async throws -> Path? {
30-
if context.hostOperatingSystem == .windows {
31-
let vcToolsInstallDir = "VCToolsInstallDir"
32-
let installations = try await VSInstallation.findInstallations(fs: context.fs)
33-
.sorted(by: { $0.installationVersion > $1.installationVersion })
34-
if let latest = installations.first {
35-
let msvcDir = latest.installationPath.join("VC").join("Tools").join("MSVC")
36-
let versions = try context.fs.listdir(msvcDir).map { try Version($0) }.sorted { $0 > $1 }
37-
if let latestVersion = versions.first {
38-
let dir = msvcDir.join(latestVersion.description).str
39-
return Path(dir)
40-
}
29+
private func findLatestInstallDirectory(fs: any FSProxy) async throws -> Path? {
30+
let installations = try await VSInstallation.findInstallations(fs: fs)
31+
.sorted(by: { $0.installationVersion > $1.installationVersion })
32+
if let latest = installations.first {
33+
let msvcDir = latest.installationPath.join("VC").join("Tools").join("MSVC")
34+
let versions = try fs.listdir(msvcDir).map { try Version($0) }.sorted { $0 > $1 }
35+
if let latestVersion = versions.first {
36+
let dir = msvcDir.join(latestVersion.description).str
37+
return Path(dir)
4138
}
4239
}
4340
return nil
4441
}
4542

4643
struct WindowsEnvironmentExtension: EnvironmentExtension {
4744
func additionalEnvironmentVariables(context: any EnvironmentExtensionAdditionalEnvironmentVariablesContext) async throws -> [String: String] {
45+
guard context.hostOperatingSystem == .windows else {
46+
return [:]
47+
}
4848
// Add the environment variable for the MSVC toolset for Swift and Clang to find it
4949
let vcToolsInstallDir = "VCToolsInstallDir"
50-
guard let dir = try? await findLatestInstallDirectory(context: context) else {
50+
guard let dir = try? await findLatestInstallDirectory(fs: context.fs) else {
5151
return [:]
5252
}
5353
return [vcToolsInstallDir: dir.str]
5454
}
5555
}
5656

57-
struct WindowsPlatformExtension: PlatformInfoExtension {
58-
public func additionalPlatformExecutableSearchPaths(platformName: String, platformPath: Path, fs: any FSProxy) async -> [Path] {
59-
guard let dir = try? await findLatestInstallDirectory(fs: fs) else {
60-
return []
61-
}
62-
// Note: Do not add in the target directories under the host as these will end up in the global search paths, i.e. PATH
63-
// Let the commandlinetool discovery add in the target subdirectory based on the targeted architecture.
64-
if Architecture.hostStringValue == "aarch64" {
65-
return [dir.join("bin/Hostarm64")]
66-
} else {
67-
return [dir.join("bin/Hostx64")]
68-
}
69-
}
70-
}
71-
7257
struct WindowsPlatformExtension: PlatformInfoExtension {
7358
func additionalPlatforms(context: any PlatformInfoExtensionAdditionalPlatformsContext) throws -> [(path: Path, data: [String: PropertyListItem])] {
7459
let operatingSystem = context.hostOperatingSystem
@@ -113,6 +98,22 @@ struct WindowsPlatformExtension: PlatformInfoExtension {
11398
sdkSearchPaths = []
11499
}
115100
}
101+
102+
public func additionalPlatformExecutableSearchPaths(platformName: String, platformPath: Path, fs: any FSProxy) async -> [Path] {
103+
guard let dir = try? await findLatestInstallDirectory(fs: fs) else {
104+
return []
105+
}
106+
// Note: Do not add in the target directories under the host as these will end up in the global search paths, i.e. PATH
107+
// Let the commandlinetool discovery add in the target subdirectory based on the targeted architecture.
108+
switch Architecture.hostStringValue {
109+
case "aarch64":
110+
return [dir.join("bin/Hostarm64")]
111+
case "x86_64":
112+
return [dir.join("bin/Hostx64")]
113+
default:
114+
return []
115+
}
116+
}
116117
}
117118

118119
struct WindowsSDKRegistryExtension: SDKRegistryExtension {

‎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)