Skip to content

Commit ef50e94

Browse files
committed
Reduce use of AbsolutePath with precondition failure
Since it comes up regularly as a perceived user facing crash, we should reduce our use of the plain `AbsolutePath` initializer. I actually don't think it makes sense to even provide a non-throwing initializer that takes anything but a `StaticString` and even that's debatable. This doesn't resolve all possible uses and specifically excludes uses in tests, but does fix several user facing issues, e.g. ``` ❯ export SWIFTPM_TESTS_MODULECACHE=blah ❯ swift test zsh: illegal hardware instruction swift test ```
1 parent e78cc4c commit ef50e94

21 files changed

+79
-67
lines changed

Sources/Basics/HTPClient+URLSession.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ private class DownloadTaskManager: NSObject, URLSessionDownloadDelegate {
179179
}
180180

181181
do {
182-
try task.fileSystem.move(from: AbsolutePath(location.path), to: task.destination)
182+
try task.fileSystem.move(from: AbsolutePath(validating: location.path), to: task.destination)
183183
} catch {
184184
task.moveFileError = error
185185
}

Sources/Basics/Sandbox.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ public enum Sandbox {
3131
strictness: Strictness = .default,
3232
writableDirectories: [AbsolutePath] = [],
3333
readOnlyDirectories: [AbsolutePath] = []
34-
) -> [String] {
34+
) throws -> [String] {
3535
#if os(macOS)
36-
let profile = macOSSandboxProfile(strictness: strictness, writableDirectories: writableDirectories, readOnlyDirectories: readOnlyDirectories)
36+
let profile = try macOSSandboxProfile(strictness: strictness, writableDirectories: writableDirectories, readOnlyDirectories: readOnlyDirectories)
3737
return ["/usr/bin/sandbox-exec", "-p", profile] + command
3838
#else
3939
// rdar://40235432, rdar://75636874 tracks implementing sandboxes for other platforms.
@@ -59,7 +59,7 @@ fileprivate func macOSSandboxProfile(
5959
strictness: Sandbox.Strictness,
6060
writableDirectories: [AbsolutePath],
6161
readOnlyDirectories: [AbsolutePath]
62-
) -> String {
62+
) throws -> String {
6363
var contents = "(version 1)\n"
6464

6565
// Deny everything by default.
@@ -94,7 +94,7 @@ fileprivate func macOSSandboxProfile(
9494
else if strictness == .writableTemporaryDirectory {
9595
// Add `subpath` expressions for the regular and the Foundation temporary directories.
9696
for tmpDir in ["/tmp", NSTemporaryDirectory()] {
97-
writableDirectoriesExpression += ["(subpath \(resolveSymlinks(AbsolutePath(tmpDir)).quotedAsSubpathForSandboxProfile))"]
97+
writableDirectoriesExpression += try ["(subpath \(resolveSymlinks(AbsolutePath(validating: tmpDir)).quotedAsSubpathForSandboxProfile))"]
9898
}
9999
}
100100

Sources/Build/BuildOperation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
543543
// TODO: We need to also use any working directory, but that support isn't yet available on all platforms at a lower level.
544544
var commandLine = [command.configuration.executable.pathString] + command.configuration.arguments
545545
if !self.disableSandboxForPluginCommands {
546-
commandLine = Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory])
546+
commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory])
547547
}
548548
let processResult = try TSCBasic.Process.popen(arguments: commandLine, environment: command.configuration.environment)
549549
let output = try processResult.utf8Output() + processResult.utf8stderrOutput()

Sources/Build/BuildOperationBuildSystemDelegateHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ final class TestDiscoveryCommand: CustomLLBuildCommand, TestBuildCommand {
112112
let store = try IndexStore.open(store: index, api: api)
113113

114114
// FIXME: We can speed this up by having one llbuild command per object file.
115-
let tests = try store.listTests(in: tool.inputs.map{ AbsolutePath($0.name) })
115+
let tests = try store.listTests(in: tool.inputs.map{ try AbsolutePath(validating: $0.name) })
116116

117117
let outputs = tool.outputs.compactMap{ try? AbsolutePath(validating: $0.name) }
118118
let testsByModule = Dictionary(grouping: tests, by: { $0.module.spm_mangledToC99ExtendedIdentifier() })

Sources/Build/BuildPlan.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ extension AbsolutePath {
4545

4646
extension BuildParameters {
4747
/// Returns the directory to be used for module cache.
48-
public var moduleCache: AbsolutePath {
48+
public func moduleCache() throws -> AbsolutePath {
4949
// FIXME: We use this hack to let swiftpm's functional test use shared
5050
// cache so it doesn't become painfully slow.
5151
if let path = ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"] {
52-
return AbsolutePath(path)
52+
return try AbsolutePath(validating: path)
5353
}
5454
return buildPath.appending(component: "ModuleCache")
5555
}
@@ -406,7 +406,7 @@ public final class ClangTargetBuildDescription {
406406
args += ["-I", clangTarget.includeDir.pathString]
407407
args += additionalFlags
408408
if enableModules {
409-
args += moduleCacheArgs
409+
args += try moduleCacheArgs()
410410
}
411411
args += buildParameters.sanitizers.compileCFlags()
412412

@@ -477,8 +477,8 @@ public final class ClangTargetBuildDescription {
477477
}
478478

479479
/// Module cache arguments.
480-
private var moduleCacheArgs: [String] {
481-
return ["-fmodules-cache-path=\(buildParameters.moduleCache.pathString)"]
480+
private func moduleCacheArgs() throws -> [String] {
481+
return try ["-fmodules-cache-path=\(buildParameters.moduleCache().pathString)"]
482482
}
483483

484484
/// Generate the resource bundle accessor, if appropriate.
@@ -804,7 +804,7 @@ public final class SwiftTargetBuildDescription {
804804
// `/\(resourceBundleName)/\(resourcePath)`, which allows us to pass this path to JS APIs like `fetch` directly, or to
805805
// `<img src=` HTML attributes. The resources are loaded from the server, and we can't hardcode the host part in the URL.
806806
// Making URLs relative by starting them with `/\(resourceBundleName)` makes it work in the browser.
807-
let mainPath = AbsolutePath(Bundle.main.bundlePath).appending(component: bundlePath.basename).pathString
807+
let mainPath = try AbsolutePath(validating: Bundle.main.bundlePath).appending(component: bundlePath.basename).pathString
808808
mainPathSubstitution = #""\#(mainPath.asSwiftStringLiteralConstant)""#
809809
} else {
810810
mainPathSubstitution = #"Bundle.main.bundleURL.appendingPathComponent("\#(bundlePath.basename.asSwiftStringLiteralConstant)").path"#
@@ -874,7 +874,7 @@ public final class SwiftTargetBuildDescription {
874874
args += ["-j\(buildParameters.jobs)"]
875875
args += activeCompilationConditions
876876
args += additionalFlags
877-
args += moduleCacheArgs
877+
args += try moduleCacheArgs()
878878
args += stdlibArguments
879879
args += buildParameters.sanitizers.compileSwiftFlags()
880880
args += ["-parseable-output"]
@@ -1028,7 +1028,7 @@ public final class SwiftTargetBuildDescription {
10281028
result += ["-j\(buildParameters.jobs)"]
10291029
result += activeCompilationConditions
10301030
result += additionalFlags
1031-
result += moduleCacheArgs
1031+
result += try moduleCacheArgs()
10321032
result += stdlibArguments
10331033
result += self.buildSettingsFlags()
10341034

@@ -1075,7 +1075,7 @@ public final class SwiftTargetBuildDescription {
10751075
result += ["-j\(buildParameters.jobs)"]
10761076
result += activeCompilationConditions
10771077
result += additionalFlags
1078-
result += moduleCacheArgs
1078+
result += try moduleCacheArgs()
10791079
result += stdlibArguments
10801080
result += buildParameters.sanitizers.compileSwiftFlags()
10811081
result += ["-parseable-output"]
@@ -1235,8 +1235,8 @@ public final class SwiftTargetBuildDescription {
12351235
}
12361236

12371237
/// Module cache arguments.
1238-
private var moduleCacheArgs: [String] {
1239-
return ["-module-cache-path", buildParameters.moduleCache.pathString]
1238+
private func moduleCacheArgs() throws -> [String] {
1239+
return try ["-module-cache-path", buildParameters.moduleCache().pathString]
12401240
}
12411241

12421242
private var stdlibArguments: [String] {

Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ extension LLBuildManifestBuilder {
621621
let displayName = command.configuration.displayName ?? execPath.basename
622622
var commandLine = [execPath.pathString] + command.configuration.arguments
623623
if !self.disableSandboxForPluginCommands {
624-
commandLine = Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [result.pluginOutputDirectory])
624+
commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [result.pluginOutputDirectory])
625625
}
626626
manifest.addShellCmd(
627627
name: displayName + "-" + ByteString(encodingAsUTF8: uniquedName).sha256Checksum,

Sources/Commands/SwiftPackageTool.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1654,7 +1654,7 @@ extension SwiftPackageTool.Config {
16541654
let workspace = try swiftTool.getActiveWorkspace()
16551655
return try .init(
16561656
fileSystem: swiftTool.fileSystem,
1657-
localMirrorsFile: workspace.location.localMirrorsConfigurationFile,
1657+
localMirrorsFile: try workspace.location.localMirrorsConfigurationFile(),
16581658
sharedMirrorsFile: workspace.location.sharedMirrorsConfigurationFile
16591659
)
16601660
}

Sources/Commands/Utilities/SymbolGraphExtract.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public struct SymbolGraphExtract {
5757
commandLine += ["-module-name", target.c99name]
5858
commandLine += try buildParameters.targetTripleArgs(for: target)
5959
commandLine += buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: true)
60-
commandLine += ["-module-cache-path", buildParameters.moduleCache.pathString]
60+
commandLine += try ["-module-cache-path", buildParameters.moduleCache().pathString]
6161
if verboseOutput {
6262
commandLine += ["-v"]
6363
}

Sources/Commands/Utilities/TestingSupport.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ enum TestingSupport {
8383
)
8484
// Add the sdk platform path if we have it. If this is not present, we
8585
// might always end up failing.
86-
if let sdkPlatformFrameworksPath = Destination.sdkPlatformFrameworkPaths() {
86+
if let sdkPlatformFrameworksPath = try Destination.sdkPlatformFrameworkPaths() {
8787
// appending since we prefer the user setting (if set) to the one we inject
8888
env.appendPath("DYLD_FRAMEWORK_PATH", value: sdkPlatformFrameworksPath.fwk.pathString)
8989
env.appendPath("DYLD_LIBRARY_PATH", value: sdkPlatformFrameworksPath.lib.pathString)

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,11 @@ public final class ManifestLoader: ManifestLoaderProtocol {
631631
if self.isManifestSandboxEnabled {
632632
let cacheDirectories = [self.databaseCacheDir, moduleCachePath].compactMap{ $0 }
633633
let strictness: Sandbox.Strictness = toolsVersion < .v5_3 ? .manifest_pre_53 : .default
634-
cmd = Sandbox.apply(command: cmd, strictness: strictness, writableDirectories: cacheDirectories)
634+
do {
635+
cmd = try Sandbox.apply(command: cmd, strictness: strictness, writableDirectories: cacheDirectories)
636+
} catch {
637+
return completion(.failure(error))
638+
}
635639
}
636640

637641
// Run the compiled manifest.
@@ -696,7 +700,9 @@ public final class ManifestLoader: ManifestLoaderProtocol {
696700
guard let sdkRoot = foundPath?.spm_chomp(), !sdkRoot.isEmpty else {
697701
return nil
698702
}
699-
let path = AbsolutePath(sdkRoot)
703+
guard let path = try? AbsolutePath(validating: sdkRoot) else {
704+
return nil
705+
}
700706
sdkRootPath = path
701707
self.sdkRootCache.put(path)
702708
#endif

Sources/PackageLoading/PkgConfig.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public struct PkgConfig {
7676
let pkgFileFinder = PCFileFinder(brewPrefix: brewPrefix)
7777
self.pcFile = try pkgFileFinder.locatePCFile(
7878
name: name,
79-
customSearchPaths: PkgConfig.envSearchPaths + additionalSearchPaths,
79+
customSearchPaths: try PkgConfig.envSearchPaths() + additionalSearchPaths,
8080
fileSystem: fileSystem,
8181
observabilityScope: observabilityScope
8282
)
@@ -121,12 +121,12 @@ public struct PkgConfig {
121121
loadingContext.pkgConfigStack.removeLast();
122122
}
123123

124-
private static var envSearchPaths: [AbsolutePath] {
124+
private static func envSearchPaths() throws -> [AbsolutePath] {
125125
if let configPath = ProcessEnv.vars["PKG_CONFIG_PATH"] {
126126
#if os(Windows)
127-
return configPath.split(separator: ";").map({ AbsolutePath(String($0)) })
127+
return try configPath.split(separator: ";").map({ try AbsolutePath(validating: String($0)) })
128128
#else
129-
return configPath.split(separator: ":").map({ AbsolutePath(String($0)) })
129+
return try configPath.split(separator: ":").map({ try AbsolutePath(validating: String($0)) })
130130
#endif
131131
}
132132
return []
@@ -397,9 +397,9 @@ internal struct PCFileFinder {
397397
).spm_chomp()
398398

399399
#if os(Windows)
400-
PCFileFinder.pkgConfigPaths = searchPaths.split(separator: ";").map({ AbsolutePath(String($0)) })
400+
PCFileFinder.pkgConfigPaths = try searchPaths.split(separator: ";").map({ try AbsolutePath(validating: String($0)) })
401401
#else
402-
PCFileFinder.pkgConfigPaths = searchPaths.split(separator: ":").map({ AbsolutePath(String($0)) })
402+
PCFileFinder.pkgConfigPaths = try searchPaths.split(separator: ":").map({ try AbsolutePath(validating: String($0)) })
403403
#endif
404404
} catch {
405405
PCFileFinder.shouldEmitPkgConfigPathsDiagnostic = true

Sources/PackageLoading/Target+PkgConfig.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ extension SystemPackageProviderDescription {
182182
return []
183183
}
184184
}
185-
return packages.map({ AbsolutePath(brewPrefix).appending(components: "opt", $0, "lib", "pkgconfig") })
185+
return packages.compactMap({ (try? AbsolutePath(validating: brewPrefix))?.appending(components: "opt", $0, "lib", "pkgconfig") })
186186
case .apt:
187187
return []
188188
case .yum:

Sources/PackageModel/Destination.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public struct Destination: Encodable, Equatable {
129129
guard !sdkPathStr.isEmpty else {
130130
throw DestinationError.invalidInstallation("default SDK not found")
131131
}
132-
sdkPath = AbsolutePath(sdkPathStr)
132+
sdkPath = try AbsolutePath(validating: sdkPathStr)
133133
}
134134
#else
135135
sdkPath = nil
@@ -139,7 +139,7 @@ public struct Destination: Encodable, Equatable {
139139
var extraCCFlags: [String] = []
140140
var extraSwiftCFlags: [String] = []
141141
#if os(macOS)
142-
if let sdkPaths = Destination.sdkPlatformFrameworkPaths(environment: environment) {
142+
if let sdkPaths = try Destination.sdkPlatformFrameworkPaths(environment: environment) {
143143
extraCCFlags += ["-F", sdkPaths.fwk.pathString]
144144
extraSwiftCFlags += ["-F", sdkPaths.fwk.pathString]
145145
extraSwiftCFlags += ["-I", sdkPaths.lib.pathString]
@@ -164,7 +164,7 @@ public struct Destination: Encodable, Equatable {
164164
/// Returns macosx sdk platform framework path.
165165
public static func sdkPlatformFrameworkPaths(
166166
environment: EnvironmentVariables = .process()
167-
) -> (fwk: AbsolutePath, lib: AbsolutePath)? {
167+
) throws -> (fwk: AbsolutePath, lib: AbsolutePath)? {
168168
if let path = _sdkPlatformFrameworkPath {
169169
return path
170170
}
@@ -174,11 +174,11 @@ public struct Destination: Encodable, Equatable {
174174

175175
if let platformPath = platformPath, !platformPath.isEmpty {
176176
// For XCTest framework.
177-
let fwk = AbsolutePath(platformPath).appending(
177+
let fwk = try AbsolutePath(validating: platformPath).appending(
178178
components: "Developer", "Library", "Frameworks")
179179

180180
// For XCTest Swift library.
181-
let lib = AbsolutePath(platformPath).appending(
181+
let lib = try AbsolutePath(validating: platformPath).appending(
182182
components: "Developer", "usr", "lib")
183183

184184
_sdkPlatformFrameworkPath = (fwk, lib)

Sources/SPMTestSupport/MockRegistry.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ private struct MockRegistryArchiver: Archiver {
352352
if !self.fileSystem.exists(targetPath.parentDirectory) {
353353
try self.fileSystem.createDirectory(targetPath.parentDirectory, recursive: true)
354354
}
355-
try self.fileSystem.copy(from: AbsolutePath(path), to: targetPath)
355+
try self.fileSystem.copy(from: AbsolutePath(validating: path), to: targetPath)
356356
}
357357
completion(.success(()))
358358
} catch {

Sources/SPMTestSupport/Toolchain.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@ import Workspace
1616
import TSCBasic
1717

1818
#if os(macOS)
19-
private func macOSBundleRoot() -> AbsolutePath {
19+
private func macOSBundleRoot() throws -> AbsolutePath {
2020
for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") {
21-
return AbsolutePath(bundle.bundlePath).parentDirectory
21+
return try AbsolutePath(validating: bundle.bundlePath).parentDirectory
2222
}
2323
fatalError()
2424
}
2525
#endif
2626

27-
private func resolveBinDir() -> AbsolutePath {
27+
private func resolveBinDir() throws -> AbsolutePath {
2828
#if os(macOS)
29-
return macOSBundleRoot()
29+
return try macOSBundleRoot()
3030
#else
3131
return AbsolutePath(CommandLine.arguments[0], relativeTo: localFileSystem.currentWorkingDirectory!).parentDirectory
3232
#endif
@@ -35,8 +35,8 @@ private func resolveBinDir() -> AbsolutePath {
3535
extension UserToolchain {
3636

3737
#if os(macOS)
38-
public var sdkPlatformFrameworksPath: AbsolutePath {
39-
return Destination.sdkPlatformFrameworkPaths()!.fwk
38+
public func sdkPlatformFrameworksPath() throws -> AbsolutePath {
39+
return try Destination.sdkPlatformFrameworkPaths()!.fwk
4040
}
4141
#endif
4242

@@ -45,7 +45,7 @@ extension UserToolchain {
4545
extension Destination {
4646
public static var `default`: Self {
4747
get {
48-
let binDir = resolveBinDir()
48+
let binDir = try! resolveBinDir()
4949
return try! Destination.hostDestination(binDir)
5050
}
5151
}

Sources/Workspace/DefaultPluginScriptRunner.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,9 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
386386
guard let sdkRoot = foundPath?.spm_chomp(), !sdkRoot.isEmpty else {
387387
return nil
388388
}
389-
let path = AbsolutePath(sdkRoot)
389+
guard let path = try? AbsolutePath(validating: sdkRoot) else {
390+
return nil
391+
}
390392
sdkRootPath = path
391393
self.sdkRootCache.put(path)
392394
#endif
@@ -416,7 +418,11 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
416418

417419
// Optionally wrap the command in a sandbox, which places some limits on what it can do. In particular, it blocks network access and restricts the paths to which the plugin can make file system changes. It does allow writing to temporary directories.
418420
if self.enableSandbox {
419-
command = Sandbox.apply(command: command, strictness: .writableTemporaryDirectory, writableDirectories: writableDirectories + [self.cacheDir], readOnlyDirectories: readOnlyDirectories)
421+
do {
422+
command = try Sandbox.apply(command: command, strictness: .writableTemporaryDirectory, writableDirectories: writableDirectories + [self.cacheDir], readOnlyDirectories: readOnlyDirectories)
423+
} catch {
424+
print("error while preparing to send message to plugin: \(error)")
425+
}
420426
}
421427

422428
// Create and configure a Process. We set the working directory to the cache directory, so that relative paths end up there.

Sources/Workspace/Workspace+Configuration.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,10 @@ extension Workspace {
9292
// config locations
9393

9494
/// Path to the local mirrors configuration.
95-
public var localMirrorsConfigurationFile: AbsolutePath {
95+
public func localMirrorsConfigurationFile() throws -> AbsolutePath {
9696
// backwards compatibility
9797
if let customPath = ProcessEnv.vars["SWIFTPM_MIRROR_CONFIG"] {
98-
return AbsolutePath(customPath)
98+
return try AbsolutePath(validating: customPath)
9999
}
100100
return DefaultLocations.mirrorsConfigurationFile(at: self.localConfigurationDirectory)
101101
}

0 commit comments

Comments
 (0)