Skip to content

Commit a8bb4da

Browse files
compnerdcompnerd
committed
PackageModel: correct and improve XCTest location on Windows
This corrects a long standing issue in locating XCTest on Windows. We erroneously assumed that we could use `DEVELOPER_DIR` to construct a path to XCTest. However, this is not correct for two reasons: 1. We may be targeting something other than Windows 2. We may have passed `-sdk` explicitly, which would move the location of the SDK to something that no longer is relative to `DEVELOPER_DIR`. Instead, we now construct the path relative to `SDKROOT` OR the explicit SDK specified. This reduces the dependency on the environment, and makes `SDKROOT` behave as intended as well as completely drops the need for `DEVELOPER_DIR` within SPM. This will also restore the ability for us to have relocatable SDK installations on Windows which eases some local testing for parallel SDK installations.
1 parent a78a11d commit a8bb4da

File tree

1 file changed

+108
-96
lines changed

1 file changed

+108
-96
lines changed

‎Sources/PackageModel/UserToolchain.swift

Lines changed: 108 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -226,12 +226,12 @@ public final class UserToolchain: Toolchain {
226226
// Windows uses a variable named SDKROOT to determine the root of
227227
// the SDK. This is not the same value as the SDKROOT parameter
228228
// in Xcode, however, the value represents a similar concept.
229-
if let SDKROOT = environment["SDKROOT"], let root = try? AbsolutePath(validating: SDKROOT) {
229+
if let SDKROOT = environment["SDKROOT"], let sdkroot = try? AbsolutePath(validating: SDKROOT) {
230230
var runtime: [String] = []
231231
var xctest: [String] = []
232232
var extraSwiftCFlags: [String] = []
233233

234-
if let settings = WindowsSDKSettings(reading: root.appending(component: "SDKSettings.plist"),
234+
if let settings = WindowsSDKSettings(reading: sdkroot.appending(component: "SDKSettings.plist"),
235235
diagnostics: nil, filesystem: localFileSystem) {
236236
switch settings.defaults.runtime {
237237
case .multithreadedDebugDLL:
@@ -245,43 +245,44 @@ public final class UserToolchain: Toolchain {
245245
}
246246
}
247247

248-
if let DEVELOPER_DIR = environment["DEVELOPER_DIR"],
249-
let root = try? AbsolutePath(validating: DEVELOPER_DIR)
250-
.appending(component: "Platforms")
251-
.appending(component: "Windows.platform") {
252-
if let info = WindowsPlatformInfo(reading: root.appending(component: "Info.plist"),
253-
diagnostics: nil, filesystem: localFileSystem) {
254-
let path: AbsolutePath =
255-
root.appending(component: "Developer")
256-
.appending(component: "Library")
257-
.appending(component: "XCTest-\(info.defaults.xctestVersion)")
258-
259-
xctest = [
260-
"-I", AbsolutePath("usr/lib/swift/windows/\(triple.arch)", relativeTo: path).pathString,
261-
"-L", AbsolutePath("usr/lib/swift/windows/\(triple.arch)", relativeTo: path).pathString,
262-
]
263-
264-
// Migration Path
265-
//
266-
// In order to support multiple parallel
267-
// installations of an SDK, we need to ensure that
268-
// we can have all the architecture variant
269-
// libraries available. Prior to this getting
270-
// enabled (~5.7), we always had a singular
271-
// installed SDK. Prefer the new variant which has
272-
// an architecture subdirectory in `bin` if
273-
// available.
274-
let implib: AbsolutePath =
275-
AbsolutePath("usr/lib/swift/windows/XCTest.lib", relativeTo: path)
276-
if localFileSystem.exists(implib) {
277-
xctest.append(contentsOf: ["-L", implib.parentDirectory.pathString])
278-
}
279-
280-
extraSwiftCFlags = info.defaults.extraSwiftCFlags ?? []
248+
// The layout of the SDK is as follows:
249+
//
250+
// Library/Developer/Platforms/[PLATFORM].platform/Developer/Library/XCTest-[VERSION]/...
251+
// Library/Developer/Platforms/[PLATFORM].platform/Developer/SDKs/[PLATFORM].sdk/...
252+
//
253+
// SDKROOT points to [PLATFORM].sdk
254+
let platform = sdkroot.parentDirectory.parentDirectory.parentDirectory
255+
256+
if let info = WindowsPlatformInfo(reading: platform.appending(component: "Info.plist"),
257+
diagnostics: nil, filesystem: localFileSystem) {
258+
let installation: AbsolutePath =
259+
platform.appending(component: "Developer")
260+
.appending(component: "Library")
261+
.appending(component: "XCTest-\(info.defaults.xctestVersion)")
262+
263+
xctest = [
264+
"-I", AbsolutePath("usr/lib/swift/windows/\(triple.arch)", relativeTo: installation).pathString,
265+
"-L", AbsolutePath("usr/lib/swift/windows/\(triple.arch)", relativeTo: installation).pathString,
266+
]
267+
268+
// Migration Path
269+
//
270+
// In order to support multiple parallel installations
271+
// of an SDK, we need to ensure that we can have all the
272+
// architecture variant libraries available. Prior to
273+
// this getting enabled (~5.7), we always had a singular
274+
// installed SDK. Prefer the new variant which has an
275+
// architecture subdirectory in `bin` if available.
276+
let implib: AbsolutePath =
277+
AbsolutePath("usr/lib/swift/windows/XCTest.lib", relativeTo: installation)
278+
if localFileSystem.exists(implib) {
279+
xctest.append(contentsOf: ["-L", implib.parentDirectory.pathString])
281280
}
281+
282+
extraSwiftCFlags = info.defaults.extraSwiftCFlags ?? []
282283
}
283284

284-
return [ "-sdk", root.pathString, ] + runtime + xctest + extraSwiftCFlags
285+
return [ "-sdk", sdkroot.pathString, ] + runtime + xctest + extraSwiftCFlags
285286
}
286287
}
287288

@@ -385,7 +386,7 @@ public final class UserToolchain: Toolchain {
385386
if case let .custom(_, useXcrun) = searchStrategy, !useXcrun {
386387
xctestPath = nil
387388
} else {
388-
xctestPath = try Self.deriveXCTestPath(triple: triple, environment: environment)
389+
xctestPath = try Self.deriveXCTestPath(destination: self.destination, triple: triple, environment: environment)
389390
}
390391

391392
self.configuration = .init(
@@ -462,79 +463,90 @@ public final class UserToolchain: Toolchain {
462463
}
463464

464465
// TODO: We should have some general utility to find tools.
465-
private static func deriveXCTestPath(triple: Triple, environment: EnvironmentVariables) throws -> AbsolutePath? {
466+
private static func deriveXCTestPath(destination: Destination, triple: Triple, environment: EnvironmentVariables) throws -> AbsolutePath? {
466467
if triple.isDarwin() {
467468
// XCTest is optional on macOS, for example when Xcode is not installed
468469
let xctestFindArgs = ["/usr/bin/xcrun", "--sdk", "macosx", "--find", "xctest"]
469470
if let path = try? Process.checkNonZeroExit(arguments: xctestFindArgs, environment: environment).spm_chomp() {
470471
return try AbsolutePath(validating: path)
471472
}
472473
} else if triple.isWindows() {
473-
if let DEVELOPER_DIR = environment["DEVELOPER_DIR"],
474-
let root = try? AbsolutePath(validating: DEVELOPER_DIR)
475-
.appending(component: "Platforms")
476-
.appending(component: "Windows.platform") {
477-
if let info = WindowsPlatformInfo(reading: root.appending(component: "Info.plist"),
478-
diagnostics: nil,
479-
filesystem: localFileSystem) {
480-
481-
let installation: AbsolutePath =
482-
root.appending(component: "Developer")
483-
.appending(component: "Library")
484-
.appending(component: "XCTest-\(info.defaults.xctestVersion)")
485-
486-
// Migration Path
487-
//
488-
// In order to support multiple parallel installations of an
489-
// SDK, we need to ensure that we can have all the
490-
// architecture variant libraries available. Prior to this
491-
// getting enabled (~5.7), we always had a singular
492-
// installed SDK. Prefer the new variant which has an
493-
// architecture subdirectory in `bin` if available.
494-
switch triple.arch {
495-
case .x86_64, .x86_64h:
496-
let path: AbsolutePath =
497-
installation.appending(component: "usr")
498-
.appending(component: "bin64")
499-
if localFileSystem.exists(path) {
500-
return path
501-
}
474+
let sdkroot: AbsolutePath
475+
476+
if let sdk = destination.sdk {
477+
sdkroot = sdk
478+
} else if let SDKROOT = environment["SDKROOT"], let sdk = try? AbsolutePath(validating: SDKROOT) {
479+
sdkroot = sdk
480+
} else {
481+
return .none
482+
}
502483

503-
case .i686:
504-
let path: AbsolutePath =
505-
installation.appending(component: "usr")
506-
.appending(component: "bin32")
507-
if localFileSystem.exists(path) {
508-
return path
509-
}
484+
// The layout of the SDK is as follows:
485+
//
486+
// Library/Developer/Platforms/[PLATFORM].platform/Developer/Library/XCTest-[VERSION]/...
487+
// Library/Developer/Platforms/[PLATFORM].platform/Developer/SDKs/[PLATFORM].sdk/...
488+
//
489+
// SDKROOT points to [PLATFORM].sdk
490+
let platform = sdkroot.parentDirectory.parentDirectory.parentDirectory
491+
492+
if let info = WindowsPlatformInfo(reading: platform.appending(component: "Info.plist"),
493+
diagnostics: nil, filesystem: localFileSystem) {
494+
let xctest: AbsolutePath =
495+
platform.appending(component: "Developer")
496+
.appending(component: "Library")
497+
.appending(component: "XCTest-\(info.defaults.xctestVersion)")
510498

511-
case .armv7:
512-
let path: AbsolutePath =
513-
installation.appending(component: "usr")
514-
.appending(component: "bin32a")
515-
if localFileSystem.exists(path) {
516-
return path
517-
}
499+
// Migration Path
500+
//
501+
// In order to support multiple parallel installations of an
502+
// SDK, we need to ensure that we can have all the architecture
503+
// variant libraries available. Prior to this getting enabled
504+
// (~5.7), we always had a singular installed SDK. Prefer the
505+
// new variant which has an architecture subdirectory in `bin`
506+
// if available.
507+
switch triple.arch {
508+
case .x86_64, .x86_64h:
509+
let path: AbsolutePath =
510+
xctest.appending(component: "usr")
511+
.appending(component: "bin64")
512+
if localFileSystem.exists(path) {
513+
return path
514+
}
518515

519-
case .arm64:
520-
let path: AbsolutePath =
521-
installation.appending(component: "usr")
522-
.appending(component: "bin64a")
523-
if localFileSystem.exists(path) {
524-
return path
525-
}
516+
case .i686:
517+
let path: AbsolutePath =
518+
xctest.appending(component: "usr")
519+
.appending(component: "bin32")
520+
if localFileSystem.exists(path) {
521+
return path
522+
}
523+
524+
case .armv7:
525+
let path: AbsolutePath =
526+
xctest.appending(component: "usr")
527+
.appending(component: "bin32a")
528+
if localFileSystem.exists(path) {
529+
return path
530+
}
526531

527-
default:
528-
// Fallback to the old-style layout. We should really
529-
// report an error in this case - this architecture is
530-
// unavailable.
531-
break
532+
case .arm64:
533+
let path: AbsolutePath =
534+
xctest.appending(component: "usr")
535+
.appending(component: "bin64a")
536+
if localFileSystem.exists(path) {
537+
return path
532538
}
533539

534-
// Assume that we are in the old-style layout.
535-
return installation.appending(component: "usr")
536-
.appending(component: "bin")
540+
default:
541+
// Fallback to the old-style layout. We should really
542+
// report an error in this case - this architecture is
543+
// unavailable.
544+
break
537545
}
546+
547+
// Assume that we are in the old-style layout.
548+
return xctest.appending(component: "usr")
549+
.appending(component: "bin")
538550
}
539551
}
540552
return .none

0 commit comments

Comments
 (0)