Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor RelativePath to allow late stage canonicalization in support of windows #5910

Merged
merged 4 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ public final class ClangTargetBuildDescription {
}
"""

let implFileSubpath = RelativePath("resource_bundle_accessor.m")
let implFileSubpath = try RelativePath(validating: "resource_bundle_accessor.m")

// Add the file to the derived sources.
derivedSources.relativePaths.append(implFileSubpath)
Expand Down
10 changes: 5 additions & 5 deletions Sources/Build/BuildDescription/ProductBuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,12 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
let librarian = self.buildParameters.toolchain.librarianPath.pathString
let triple = self.buildParameters.triple
if triple.isWindows(), librarian.hasSuffix("link") || librarian.hasSuffix("link.exe") {
return [librarian, "/LIB", "/OUT:\(binaryPath.pathString)", "@\(self.linkFileListPath.pathString)"]
return try [librarian, "/LIB", "/OUT:\(binaryPath.pathString)", "@\(self.linkFileListPath.pathString)"]
}
if triple.isDarwin(), librarian.hasSuffix("libtool") {
return [librarian, "-static", "-o", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
return try [librarian, "-static", "-o", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
}
return [librarian, "crs", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
return try [librarian, "crs", binaryPath.pathString, "@\(self.linkFileListPath.pathString)"]
}

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

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

Expand Down Expand Up @@ -209,7 +209,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
case .library(.dynamic):
args += ["-emit-library"]
if self.buildParameters.triple.isDarwin() {
let relativePath = "@rpath/\(buildParameters.binaryRelativePath(for: self.product).pathString)"
let relativePath = try "@rpath/\(buildParameters.binaryRelativePath(for: self.product).pathString)"
args += ["-Xlinker", "-install_name", "-Xlinker", relativePath]
}
args += self.deadStripArguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public final class SwiftTargetBuildDescription {
}
"""

let subpath = RelativePath("embedded_resources.swift")
let subpath = try RelativePath(validating: "embedded_resources.swift")
self.derivedSources.relativePaths.append(subpath)
let path = self.derivedSources.root.appending(subpath)
try self.fileSystem.writeIfChanged(path: path, bytes: stream.bytes)
Expand Down Expand Up @@ -374,7 +374,7 @@ public final class SwiftTargetBuildDescription {
}
"""

let subpath = RelativePath("resource_bundle_accessor.swift")
let subpath = try RelativePath(validating: "resource_bundle_accessor.swift")

// Add the file to the derived sources.
self.derivedSources.relativePaths.append(subpath)
Expand Down Expand Up @@ -410,7 +410,7 @@ public final class SwiftTargetBuildDescription {
#else
try self.requiredMacroProducts.forEach { macro in
if let macroTarget = macro.targets.first {
let executablePath = self.buildParameters.binaryPath(for: macro).pathString
let executablePath = try self.buildParameters.binaryPath(for: macro).pathString
args += ["-Xfrontend", "-load-plugin-executable", "-Xfrontend", "\(executablePath)#\(macroTarget.c99name)"]
} else {
throw InternalError("macro product \(macro.name) has no targets") // earlier validation should normally catch this
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
) { name, path in
try buildOperationForPluginDependencies.build(subset: .product(name))
if let builtTool = try buildOperationForPluginDependencies.buildPlan.buildProducts.first(where: { $0.product.name == name}) {
return builtTool.binaryPath
return try builtTool.binaryPath
} else {
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ public struct BuildDescription: Codable {
)
self.swiftTargetScanArgs = targetCommandLines
self.generatedSourceTargetSet = Set(generatedSourceTargets)
self.builtTestProducts = plan.buildProducts.filter { $0.product.type == .test }.map { desc in
BuiltTestProduct(
self.builtTestProducts = try plan.buildProducts.filter { $0.product.type == .test }.map { desc in
try BuiltTestProduct(
productName: desc.product.name,
binaryPath: desc.binaryPath
)
Expand Down
24 changes: 12 additions & 12 deletions Sources/Build/LLBuildManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,18 +171,18 @@ extension LLBuildManifestBuilder {
/// Returns the virtual node that will build the entire bundle.
private func createResourcesBundle(
for target: TargetBuildDescription
) -> Node? {
) throws -> Node? {
guard let bundlePath = target.bundlePath else { return nil }

var outputs: [Node] = []

let infoPlistDestination = RelativePath("Info.plist")
let infoPlistDestination = try RelativePath(validating: "Info.plist")

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

Expand All @@ -626,7 +626,7 @@ extension LLBuildManifestBuilder {
guard let planProduct = plan.productMap[product] else {
throw InternalError("unknown product \(product)")
}
inputs.append(file: planProduct.binaryPath)
try inputs.append(file: planProduct.binaryPath)
}
return
}
Expand Down Expand Up @@ -655,7 +655,7 @@ extension LLBuildManifestBuilder {
throw InternalError("unknown product \(product)")
}
// Establish a dependency on binary of the product.
inputs.append(file: planProduct.binaryPath)
try inputs.append(file: planProduct.binaryPath)

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

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

case .library(.automatic), .library(.static), .plugin:
Expand Down Expand Up @@ -973,7 +973,7 @@ extension LLBuildManifestBuilder {

switch buildProduct.product.type {
case .library(.static):
self.manifest.addShellCmd(
try self.manifest.addShellCmd(
name: cmdName,
description: "Archiving \(buildProduct.binaryPath.prettyPath())",
inputs: buildProduct.objects.map(Node.file),
Expand All @@ -982,9 +982,9 @@ extension LLBuildManifestBuilder {
)

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

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

self.manifest.addNode(output, toTarget: targetName)
self.manifest.addPhonyCmd(
try self.manifest.addPhonyCmd(
name: output.name,
inputs: [.file(buildProduct.binaryPath)],
outputs: [output]
Expand Down
2 changes: 1 addition & 1 deletion Sources/Commands/PackageTools/PluginCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ struct PluginCommand: SwiftCommand {
// 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.
try buildSystem.build(subset: .product(name))
if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: { $0.product.name == name }) {
return builtTool.binaryPath
return try builtTool.binaryPath
} else {
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/Commands/SwiftRunTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public struct SwiftRunTool: SwiftCommand {

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

/// Determines if a path points to a valid swift file.
private func isValidSwiftFilePath(fileSystem: FileSystem, path: String) -> Bool {
private func isValidSwiftFilePath(fileSystem: FileSystem, path: String) throws -> Bool {
guard path.hasSuffix(".swift") else { return false }
//FIXME: Return false when the path is not a valid path string.
let absolutePath: AbsolutePath
Expand All @@ -276,7 +276,7 @@ public struct SwiftRunTool: SwiftCommand {
guard let cwd = fileSystem.currentWorkingDirectory else {
return false
}
absolutePath = AbsolutePath(cwd, path)
absolutePath = try AbsolutePath(cwd, validating: path)
}
return fileSystem.isFile(absolutePath)
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/Commands/Utilities/PluginDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,15 @@ final class PluginDelegate: PluginInvocationDelegate {
return $0.product.name == name
}
}
let builtArtifacts: [PluginInvocationBuildResult.BuiltArtifact] = builtProducts.compactMap {
let builtArtifacts: [PluginInvocationBuildResult.BuiltArtifact] = try builtProducts.compactMap {
switch $0.product.type {
case .library(let kind):
return .init(
return try .init(
path: $0.binaryPath.pathString,
kind: (kind == .dynamic) ? .dynamicLibrary : .staticLibrary
)
case .executable:
return .init(path: $0.binaryPath.pathString, kind: .executable)
return try .init(path: $0.binaryPath.pathString, kind: .executable)
default:
return nil
}
Expand Down
24 changes: 17 additions & 7 deletions Sources/PackageLoading/TargetSourcesBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ public struct TargetSourcesBuilder {
let ignored = pathToRule.filter { $0.value == .ignored }.map { $0.key }
let others = pathToRule.filter { $0.value == .none }.map { $0.key }

diagnoseConflictingResources(in: resources)
try diagnoseConflictingResources(in: resources)
diagnoseCopyConflictsWithLocalizationDirectories(in: resources)
diagnoseLocalizedAndUnlocalizedVariants(in: resources)
diagnoseInfoPlistConflicts(in: resources)
try diagnoseInfoPlistConflicts(in: resources)
diagnoseInvalidResource(in: target.resources)

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

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

for resource in resources {
let relativePath = resource.path.relative(to: targetPath)
Expand Down Expand Up @@ -346,9 +346,9 @@ public struct TargetSourcesBuilder {
}
}

private func diagnoseInfoPlistConflicts(in resources: [Resource]) {
private func diagnoseInfoPlistConflicts(in resources: [Resource]) throws {
for resource in resources {
if resource.destination == RelativePath("Info.plist") {
if try resource.destination == RelativePath(validating: "Info.plist") {
self.observabilityScope.emit(.infoPlistResourceConflict(
path: resource.path.relative(to: targetPath),
targetName: target.name))
Expand Down Expand Up @@ -770,3 +770,13 @@ extension PackageReference.Kind {
}
}
}

extension PackageModel.Resource {
fileprivate var destinationForGrouping: RelativePath? {
do {
return try self.destination
} catch {
return .none
}
}
}
12 changes: 7 additions & 5 deletions Sources/PackageModel/Resource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ public struct Resource: Codable, Equatable {

/// The relative location of the resource in the resource bundle.
public var destination: RelativePath {
switch self.rule {
case .process(.some(let localization)):
return RelativePath("\(localization).\(Self.localizationDirectoryExtension)/\(path.basename)")
default:
return RelativePath(path.basename)
get throws {
switch self.rule {
case .process(.some(let localization)):
return try RelativePath(validating: "\(localization).\(Self.localizationDirectoryExtension)/\(path.basename)")
default:
return try RelativePath(validating: path.basename)
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageModel/ToolsVersion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ public struct ToolsVersion: Equatable, Hashable, Codable, Sendable {
/// The subpath to the PackageDescription runtime library.
public var runtimeSubpath: RelativePath {
if self < .v4_2 {
return RelativePath("4")
return try! RelativePath(validating: "4") // try! safe
}
return RelativePath("4_2")
return try! RelativePath(validating: "4_2") // try! safe
}

/// The swift language version based on this tools version.
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageModel/Toolset.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ extension Toolset {
toolPath = absolutePath
} else {
let rootPath = rootPaths.first ?? toolsetPath.parentDirectory
toolPath = rootPath.appending(RelativePath(path))
toolPath = rootPath.appending(path)
}
} else {
toolPath = nil
Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageModel/UserToolchain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public final class UserToolchain: Toolchain {
// bootstrap script.
let swiftCompiler = try resolveSymlinks(self.swiftCompilerPath)

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

// Ensure that the runtime is present.
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageRegistry/RegistryDownloadsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ extension PackageIdentity {
guard let registryIdentity = self.registry else {
throw StringError("invalid package identifier \(self), expected registry scope and name")
}
return RelativePath(registryIdentity.scope.description).appending(component: registryIdentity.name.description)
return try RelativePath(validating: registryIdentity.scope.description).appending(component: registryIdentity.name.description)
}

internal func downloadPath(version: Version) throws -> RelativePath {
Expand Down
Loading