Skip to content

Commit 048ab95

Browse files
tomerdtomerd
authored
refactor RelativePath to allow late stage canonicalization in support of windows (#5910)
motivation: delay canonicalization of relative path to the construction of absolute path from it, to better fit how windows paths work changes: * move RelativePath and AbsolutePath conformance to ExpressibleByStringLiteral and ExpressibleByStringInterpolation to TSC * update call sites and tests to use throwing constructor of RelativePath
1 parent 96db9f6 commit 048ab95

File tree

58 files changed

+3299
-2230
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+3299
-2230
lines changed

‎Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ public final class ClangTargetBuildDescription {
325325
}
326326
"""
327327

328-
let implFileSubpath = RelativePath("resource_bundle_accessor.m")
328+
let implFileSubpath = try RelativePath(validating: "resource_bundle_accessor.m")
329329

330330
// Add the file to the derived sources.
331331
derivedSources.relativePaths.append(implFileSubpath)

‎Sources/Build/BuildDescription/ProductBuildDescription.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,12 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
137137
let librarian = self.buildParameters.toolchain.librarianPath.pathString
138138
let triple = self.buildParameters.triple
139139
if triple.isWindows(), librarian.hasSuffix("link") || librarian.hasSuffix("link.exe") {
140-
return [librarian, "/LIB", "/OUT:\(binaryPath.pathString)", "@\(self.linkFileListPath.pathString)"]
140+
return try [librarian, "/LIB", "/OUT:\(binaryPath.pathString)", "@\(self.linkFileListPath.pathString)"]
141141
}
142142
if triple.isDarwin(), librarian.hasSuffix("libtool") {
143-
return [librarian, "-static", "-o", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
143+
return try [librarian, "-static", "-o", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
144144
}
145-
return [librarian, "crs", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
145+
return try [librarian, "crs", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
146146
}
147147

148148
/// The arguments to link and create this product.
@@ -166,7 +166,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
166166
}
167167

168168
args += ["-L", self.buildParameters.buildPath.pathString]
169-
args += ["-o", binaryPath.pathString]
169+
args += try ["-o", binaryPath.pathString]
170170
args += ["-module-name", self.product.name.spm_mangledToC99ExtendedIdentifier()]
171171
args += self.dylibs.map { "-l" + $0.product.name }
172172

@@ -209,7 +209,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
209209
case .library(.dynamic):
210210
args += ["-emit-library"]
211211
if self.buildParameters.triple.isDarwin() {
212-
let relativePath = "@rpath/\(buildParameters.binaryRelativePath(for: self.product).pathString)"
212+
let relativePath = try "@rpath/\(buildParameters.binaryRelativePath(for: self.product).pathString)"
213213
args += ["-Xlinker", "-install_name", "-Xlinker", relativePath]
214214
}
215215
args += self.deadStripArguments

‎Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public final class SwiftTargetBuildDescription {
327327
}
328328
"""
329329

330-
let subpath = RelativePath("embedded_resources.swift")
330+
let subpath = try RelativePath(validating: "embedded_resources.swift")
331331
self.derivedSources.relativePaths.append(subpath)
332332
let path = self.derivedSources.root.appending(subpath)
333333
try self.fileSystem.writeIfChanged(path: path, bytes: stream.bytes)
@@ -374,7 +374,7 @@ public final class SwiftTargetBuildDescription {
374374
}
375375
"""
376376

377-
let subpath = RelativePath("resource_bundle_accessor.swift")
377+
let subpath = try RelativePath(validating: "resource_bundle_accessor.swift")
378378

379379
// Add the file to the derived sources.
380380
self.derivedSources.relativePaths.append(subpath)
@@ -410,7 +410,7 @@ public final class SwiftTargetBuildDescription {
410410
#else
411411
try self.requiredMacroProducts.forEach { macro in
412412
if let macroTarget = macro.targets.first {
413-
let executablePath = self.buildParameters.binaryPath(for: macro).pathString
413+
let executablePath = try self.buildParameters.binaryPath(for: macro).pathString
414414
args += ["-Xfrontend", "-load-plugin-executable", "-Xfrontend", "\(executablePath)#\(macroTarget.c99name)"]
415415
} else {
416416
throw InternalError("macro product \(macro.name) has no targets") // earlier validation should normally catch this

‎Sources/Build/BuildOperation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
422422
) { name, path in
423423
try buildOperationForPluginDependencies.build(subset: .product(name))
424424
if let builtTool = try buildOperationForPluginDependencies.buildPlan.buildProducts.first(where: { $0.product.name == name}) {
425-
return builtTool.binaryPath
425+
return try builtTool.binaryPath
426426
} else {
427427
return nil
428428
}

‎Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ public struct BuildDescription: Codable {
357357
)
358358
self.swiftTargetScanArgs = targetCommandLines
359359
self.generatedSourceTargetSet = Set(generatedSourceTargets)
360-
self.builtTestProducts = plan.buildProducts.filter { $0.product.type == .test }.map { desc in
361-
BuiltTestProduct(
360+
self.builtTestProducts = try plan.buildProducts.filter { $0.product.type == .test }.map { desc in
361+
try BuiltTestProduct(
362362
productName: desc.product.name,
363363
binaryPath: desc.binaryPath
364364
)

‎Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,18 +171,18 @@ extension LLBuildManifestBuilder {
171171
/// Returns the virtual node that will build the entire bundle.
172172
private func createResourcesBundle(
173173
for target: TargetBuildDescription
174-
) -> Node? {
174+
) throws -> Node? {
175175
guard let bundlePath = target.bundlePath else { return nil }
176176

177177
var outputs: [Node] = []
178178

179-
let infoPlistDestination = RelativePath("Info.plist")
179+
let infoPlistDestination = try RelativePath(validating: "Info.plist")
180180

181181
// Create a copy command for each resource file.
182182
for resource in target.resources {
183183
switch resource.rule {
184184
case .copy, .process:
185-
let destination = bundlePath.appending(resource.destination)
185+
let destination = try bundlePath.appending(resource.destination)
186186
let (_, output) = addCopyCommand(from: resource.path, to: destination)
187187
outputs.append(output)
188188
case .embedInCode:
@@ -604,7 +604,7 @@ extension LLBuildManifestBuilder {
604604
// don't need to block building of a module until its resources are assembled but
605605
// we don't currently have a good way to express that resources should be built
606606
// whenever a module is being built.
607-
if let resourcesNode = createResourcesBundle(for: .swift(target)) {
607+
if let resourcesNode = try createResourcesBundle(for: .swift(target)) {
608608
inputs.append(resourcesNode)
609609
}
610610

@@ -626,7 +626,7 @@ extension LLBuildManifestBuilder {
626626
guard let planProduct = plan.productMap[product] else {
627627
throw InternalError("unknown product \(product)")
628628
}
629-
inputs.append(file: planProduct.binaryPath)
629+
try inputs.append(file: planProduct.binaryPath)
630630
}
631631
return
632632
}
@@ -655,7 +655,7 @@ extension LLBuildManifestBuilder {
655655
throw InternalError("unknown product \(product)")
656656
}
657657
// Establish a dependency on binary of the product.
658-
inputs.append(file: planProduct.binaryPath)
658+
try inputs.append(file: planProduct.binaryPath)
659659

660660
// For automatic and static libraries, and plugins, add their targets as static input.
661661
case .library(.automatic), .library(.static), .plugin:
@@ -798,7 +798,7 @@ extension LLBuildManifestBuilder {
798798
// don't need to block building of a module until its resources are assembled but
799799
// we don't currently have a good way to express that resources should be built
800800
// whenever a module is being built.
801-
if let resourcesNode = createResourcesBundle(for: .clang(target)) {
801+
if let resourcesNode = try createResourcesBundle(for: .clang(target)) {
802802
inputs.append(resourcesNode)
803803
}
804804

@@ -820,7 +820,7 @@ extension LLBuildManifestBuilder {
820820
throw InternalError("unknown product \(product)")
821821
}
822822
// Establish a dependency on binary of the product.
823-
let binary = planProduct.binaryPath
823+
let binary = try planProduct.binaryPath
824824
inputs.append(file: binary)
825825

826826
case .library(.automatic), .library(.static), .plugin:
@@ -973,7 +973,7 @@ extension LLBuildManifestBuilder {
973973

974974
switch buildProduct.product.type {
975975
case .library(.static):
976-
self.manifest.addShellCmd(
976+
try self.manifest.addShellCmd(
977977
name: cmdName,
978978
description: "Archiving \(buildProduct.binaryPath.prettyPath())",
979979
inputs: buildProduct.objects.map(Node.file),
@@ -982,9 +982,9 @@ extension LLBuildManifestBuilder {
982982
)
983983

984984
default:
985-
let inputs = buildProduct.objects + buildProduct.dylibs.map(\.binaryPath)
985+
let inputs = try buildProduct.objects + buildProduct.dylibs.map{ try $0.binaryPath }
986986

987-
self.manifest.addShellCmd(
987+
try self.manifest.addShellCmd(
988988
name: cmdName,
989989
description: "Linking \(buildProduct.binaryPath.prettyPath())",
990990
inputs: inputs.map(Node.file),
@@ -998,7 +998,7 @@ extension LLBuildManifestBuilder {
998998
let output: Node = .virtual(targetName)
999999

10001000
self.manifest.addNode(output, toTarget: targetName)
1001-
self.manifest.addPhonyCmd(
1001+
try self.manifest.addPhonyCmd(
10021002
name: output.name,
10031003
inputs: [.file(buildProduct.binaryPath)],
10041004
outputs: [output]

‎Sources/Commands/PackageTools/PluginCommand.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ struct PluginCommand: SwiftCommand {
259259
// Build the product referenced by the tool, and add the executable to the tool map. Product dependencies are not supported within a package, so if the tool happens to be from the same package, we instead find the executable that corresponds to the product. There is always one, because of autogeneration of implicit executables with the same name as the target if there isn't an explicit one.
260260
try buildSystem.build(subset: .product(name))
261261
if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: { $0.product.name == name }) {
262-
return builtTool.binaryPath
262+
return try builtTool.binaryPath
263263
} else {
264264
return nil
265265
}

‎Sources/Commands/SwiftRunTool.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public struct SwiftRunTool: SwiftCommand {
175175

176176
case .run:
177177
// Detect deprecated uses of swift run to interpret scripts.
178-
if let executable = options.executable, isValidSwiftFilePath(fileSystem: swiftTool.fileSystem, path: executable) {
178+
if let executable = options.executable, try isValidSwiftFilePath(fileSystem: swiftTool.fileSystem, path: executable) {
179179
swiftTool.observabilityScope.emit(.runFileDeprecation)
180180
// Redirect execution to the toolchain's swift executable.
181181
let swiftInterpreterPath = try swiftTool.getDestinationToolchain().swiftInterpreterPath
@@ -262,7 +262,7 @@ public struct SwiftRunTool: SwiftCommand {
262262
}
263263

264264
/// Determines if a path points to a valid swift file.
265-
private func isValidSwiftFilePath(fileSystem: FileSystem, path: String) -> Bool {
265+
private func isValidSwiftFilePath(fileSystem: FileSystem, path: String) throws -> Bool {
266266
guard path.hasSuffix(".swift") else { return false }
267267
//FIXME: Return false when the path is not a valid path string.
268268
let absolutePath: AbsolutePath
@@ -276,7 +276,7 @@ public struct SwiftRunTool: SwiftCommand {
276276
guard let cwd = fileSystem.currentWorkingDirectory else {
277277
return false
278278
}
279-
absolutePath = AbsolutePath(cwd, path)
279+
absolutePath = try AbsolutePath(cwd, validating: path)
280280
}
281281
return fileSystem.isFile(absolutePath)
282282
}

‎Sources/Commands/Utilities/PluginDelegate.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,15 @@ final class PluginDelegate: PluginInvocationDelegate {
129129
return $0.product.name == name
130130
}
131131
}
132-
let builtArtifacts: [PluginInvocationBuildResult.BuiltArtifact] = builtProducts.compactMap {
132+
let builtArtifacts: [PluginInvocationBuildResult.BuiltArtifact] = try builtProducts.compactMap {
133133
switch $0.product.type {
134134
case .library(let kind):
135-
return .init(
135+
return try .init(
136136
path: $0.binaryPath.pathString,
137137
kind: (kind == .dynamic) ? .dynamicLibrary : .staticLibrary
138138
)
139139
case .executable:
140-
return .init(path: $0.binaryPath.pathString, kind: .executable)
140+
return try .init(path: $0.binaryPath.pathString, kind: .executable)
141141
default:
142142
return nil
143143
}

‎Sources/PackageLoading/TargetSourcesBuilder.swift

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,10 @@ public struct TargetSourcesBuilder {
174174
let ignored = pathToRule.filter { $0.value == .ignored }.map { $0.key }
175175
let others = pathToRule.filter { $0.value == .none }.map { $0.key }
176176

177-
diagnoseConflictingResources(in: resources)
177+
try diagnoseConflictingResources(in: resources)
178178
diagnoseCopyConflictsWithLocalizationDirectories(in: resources)
179179
diagnoseLocalizedAndUnlocalizedVariants(in: resources)
180-
diagnoseInfoPlistConflicts(in: resources)
180+
try diagnoseInfoPlistConflicts(in: resources)
181181
diagnoseInvalidResource(in: target.resources)
182182

183183
// It's an error to contain mixed language source files.
@@ -309,10 +309,10 @@ public struct TargetSourcesBuilder {
309309
return Self.resource(for: path, with: rule, defaultLocalization: defaultLocalization, targetName: target.name, targetPath: targetPath, observabilityScope: observabilityScope)
310310
}
311311

312-
private func diagnoseConflictingResources(in resources: [Resource]) {
313-
let duplicateResources = resources.spm_findDuplicateElements(by: \.destination)
312+
private func diagnoseConflictingResources(in resources: [Resource]) throws {
313+
let duplicateResources = resources.spm_findDuplicateElements(by: \.destinationForGrouping)
314314
for resources in duplicateResources {
315-
self.observabilityScope.emit(.conflictingResource(path: resources[0].destination, targetName: target.name))
315+
try self.observabilityScope.emit(.conflictingResource(path: resources[0].destination, targetName: target.name))
316316

317317
for resource in resources {
318318
let relativePath = resource.path.relative(to: targetPath)
@@ -346,9 +346,9 @@ public struct TargetSourcesBuilder {
346346
}
347347
}
348348

349-
private func diagnoseInfoPlistConflicts(in resources: [Resource]) {
349+
private func diagnoseInfoPlistConflicts(in resources: [Resource]) throws {
350350
for resource in resources {
351-
if resource.destination == RelativePath("Info.plist") {
351+
if try resource.destination == RelativePath(validating: "Info.plist") {
352352
self.observabilityScope.emit(.infoPlistResourceConflict(
353353
path: resource.path.relative(to: targetPath),
354354
targetName: target.name))
@@ -770,3 +770,13 @@ extension PackageReference.Kind {
770770
}
771771
}
772772
}
773+
774+
extension PackageModel.Resource {
775+
fileprivate var destinationForGrouping: RelativePath? {
776+
do {
777+
return try self.destination
778+
} catch {
779+
return .none
780+
}
781+
}
782+
}

‎Sources/PackageModel/Resource.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ public struct Resource: Codable, Equatable {
2424

2525
/// The relative location of the resource in the resource bundle.
2626
public var destination: RelativePath {
27-
switch self.rule {
28-
case .process(.some(let localization)):
29-
return RelativePath("\(localization).\(Self.localizationDirectoryExtension)/\(path.basename)")
30-
default:
31-
return RelativePath(path.basename)
27+
get throws {
28+
switch self.rule {
29+
case .process(.some(let localization)):
30+
return try RelativePath(validating: "\(localization).\(Self.localizationDirectoryExtension)/\(path.basename)")
31+
default:
32+
return try RelativePath(validating: path.basename)
33+
}
3234
}
3335
}
3436

‎Sources/PackageModel/ToolsVersion.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ public struct ToolsVersion: Equatable, Hashable, Codable, Sendable {
142142
/// The subpath to the PackageDescription runtime library.
143143
public var runtimeSubpath: RelativePath {
144144
if self < .v4_2 {
145-
return RelativePath("4")
145+
return try! RelativePath(validating: "4") // try! safe
146146
}
147-
return RelativePath("4_2")
147+
return try! RelativePath(validating: "4_2") // try! safe
148148
}
149149

150150
/// The swift language version based on this tools version.

‎Sources/PackageModel/Toolset.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ extension Toolset {
102102
toolPath = absolutePath
103103
} else {
104104
let rootPath = rootPaths.first ?? toolsetPath.parentDirectory
105-
toolPath = rootPath.appending(RelativePath(path))
105+
toolPath = rootPath.appending(path)
106106
}
107107
} else {
108108
toolPath = nil

‎Sources/PackageModel/UserToolchain.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ public final class UserToolchain: Toolchain {
6868
// bootstrap script.
6969
let swiftCompiler = try resolveSymlinks(self.swiftCompilerPath)
7070

71-
let runtime = swiftCompiler.appending(
72-
RelativePath("../../lib/swift/clang/lib/darwin/libclang_rt.\(sanitizer.shortName)_osx_dynamic.dylib")
71+
let runtime = try swiftCompiler.appending(
72+
RelativePath(validating: "../../lib/swift/clang/lib/darwin/libclang_rt.\(sanitizer.shortName)_osx_dynamic.dylib")
7373
)
7474

7575
// Ensure that the runtime is present.

‎Sources/PackageRegistry/RegistryDownloadsManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ extension PackageIdentity {
325325
guard let registryIdentity = self.registry else {
326326
throw StringError("invalid package identifier \(self), expected registry scope and name")
327327
}
328-
return RelativePath(registryIdentity.scope.description).appending(component: registryIdentity.name.description)
328+
return try RelativePath(validating: registryIdentity.scope.description).appending(component: registryIdentity.name.description)
329329
}
330330

331331
internal func downloadPath(version: Version) throws -> RelativePath {

0 commit comments

Comments
 (0)