Skip to content

Commit d98de63

Browse files
authored
Reduce use of AbsolutePath with precondition failure (#5797)
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 76caa54 commit d98de63

File tree

113 files changed

+2239
-2153
lines changed

Some content is hidden

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

113 files changed

+2239
-2153
lines changed

Examples/package-info/Sources/package-info/example.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct Example {
1616

1717
// We need a package to work with.
1818
// This computes the path of this package root based on the file location
19-
let packagePath = AbsolutePath(#file).parentDirectory.parentDirectory.parentDirectory
19+
let packagePath = try AbsolutePath(validating: #file).parentDirectory.parentDirectory.parentDirectory
2020

2121
// LOADING
2222
// =======

Sources/Basics/FileSystem+Extensions.swift

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ import TSCBasic
2121
extension FileSystem {
2222
/// SwiftPM directory under user's home directory (~/.swiftpm)
2323
public var dotSwiftPM: AbsolutePath {
24-
self.homeDirectory.appending(component: ".swiftpm")
24+
get throws {
25+
return try self.homeDirectory.appending(component: ".swiftpm")
26+
}
2527
}
2628

2729
fileprivate var idiomaticSwiftPMDirectory: AbsolutePath? {
28-
return FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask).first.flatMap { AbsolutePath($0.path) }?.appending(component: "org.swift.swiftpm")
30+
get throws {
31+
return try FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask).first.flatMap { try AbsolutePath(validating: $0.path) }?.appending(component: "org.swift.swiftpm")
32+
}
2933
}
3034
}
3135

@@ -39,33 +43,37 @@ extension FileSystem {
3943

4044
/// SwiftPM cache directory under user's caches directory (if exists)
4145
public var swiftPMCacheDirectory: AbsolutePath {
42-
if let path = self.idiomaticUserCacheDirectory {
43-
return path.appending(component: "org.swift.swiftpm")
44-
} else {
45-
return self.dotSwiftPMCachesDirectory
46+
get throws {
47+
if let path = self.idiomaticUserCacheDirectory {
48+
return path.appending(component: "org.swift.swiftpm")
49+
} else {
50+
return try self.dotSwiftPMCachesDirectory
51+
}
4652
}
4753
}
4854

4955
fileprivate var dotSwiftPMCachesDirectory: AbsolutePath {
50-
return self.dotSwiftPM.appending(component: "cache")
56+
get throws {
57+
return try self.dotSwiftPM.appending(component: "cache")
58+
}
5159
}
5260
}
5361

5462
extension FileSystem {
5563
public func getOrCreateSwiftPMCacheDirectory() throws -> AbsolutePath {
56-
let idiomaticCacheDirectory = self.swiftPMCacheDirectory
64+
let idiomaticCacheDirectory = try self.swiftPMCacheDirectory
5765
// Create idiomatic if necessary
5866
if !self.exists(idiomaticCacheDirectory) {
5967
try self.createDirectory(idiomaticCacheDirectory, recursive: true)
6068
}
6169
// Create ~/.swiftpm if necessary
62-
if !self.exists(self.dotSwiftPM) {
70+
if !self.exists(try self.dotSwiftPM) {
6371
try self.createDirectory(self.dotSwiftPM, recursive: true)
6472
}
6573
// Create ~/.swiftpm/cache symlink if necessary
6674
// locking ~/.swiftpm to protect from concurrent access
6775
try self.withLock(on: self.dotSwiftPM, type: .exclusive) {
68-
if !self.exists(self.dotSwiftPMCachesDirectory, followSymlink: false) {
76+
if !self.exists(try self.dotSwiftPMCachesDirectory, followSymlink: false) {
6977
try self.createSymbolicLink(dotSwiftPMCachesDirectory, pointingAt: idiomaticCacheDirectory, relative: false)
7078
}
7179
}
@@ -78,21 +86,25 @@ extension FileSystem {
7886
extension FileSystem {
7987
/// SwiftPM config directory under user's config directory (if exists)
8088
public var swiftPMConfigurationDirectory: AbsolutePath {
81-
if let path = self.idiomaticSwiftPMDirectory {
82-
return path.appending(component: "configuration")
83-
} else {
84-
return self.dotSwiftPMConfigurationDirectory
89+
get throws {
90+
if let path = try self.idiomaticSwiftPMDirectory {
91+
return path.appending(component: "configuration")
92+
} else {
93+
return try self.dotSwiftPMConfigurationDirectory
94+
}
8595
}
8696
}
8797

8898
fileprivate var dotSwiftPMConfigurationDirectory: AbsolutePath {
89-
return self.dotSwiftPM.appending(component: "configuration")
99+
get throws {
100+
return try self.dotSwiftPM.appending(component: "configuration")
101+
}
90102
}
91103
}
92104

93105
extension FileSystem {
94106
public func getOrCreateSwiftPMConfigurationDirectory(warningHandler: @escaping (String) -> Void) throws -> AbsolutePath {
95-
let idiomaticConfigurationDirectory = self.swiftPMConfigurationDirectory
107+
let idiomaticConfigurationDirectory = try self.swiftPMConfigurationDirectory
96108

97109
// temporary 5.6, remove on next version: transition from previous configuration location
98110
if !self.exists(idiomaticConfigurationDirectory) {
@@ -116,7 +128,7 @@ extension FileSystem {
116128
}
117129

118130
// in the case where ~/.swiftpm/configuration is not the idiomatic location (eg on macOS where its /Users/<user>/Library/org.swift.swiftpm/configuration)
119-
if idiomaticConfigurationDirectory != self.dotSwiftPMConfigurationDirectory {
131+
if try idiomaticConfigurationDirectory != self.dotSwiftPMConfigurationDirectory {
120132
// copy the configuration files from old location (eg /Users/<user>/Library/org.swift.swiftpm) to new one (eg /Users/<user>/Library/org.swift.swiftpm/configuration)
121133
// but leave them there for backwards compatibility (eg older xcode)
122134
let oldConfigDirectory = idiomaticConfigurationDirectory.parentDirectory
@@ -130,7 +142,7 @@ extension FileSystem {
130142
} else {
131143
// copy the configuration files from old location (~/.swiftpm/config) to new one (~/.swiftpm/configuration)
132144
// but leave them there for backwards compatibility (eg older toolchain)
133-
let oldConfigDirectory = self.dotSwiftPM.appending(component: "config")
145+
let oldConfigDirectory = try self.dotSwiftPM.appending(component: "config")
134146
if self.exists(oldConfigDirectory, followSymlink: false) && self.isDirectory(oldConfigDirectory) {
135147
let configurationFiles = try self.getDirectoryContents(oldConfigDirectory)
136148
.map{ oldConfigDirectory.appending(component: $0) }
@@ -145,13 +157,13 @@ extension FileSystem {
145157
try self.createDirectory(idiomaticConfigurationDirectory, recursive: true)
146158
}
147159
// Create ~/.swiftpm if necessary
148-
if !self.exists(self.dotSwiftPM) {
160+
if !self.exists(try self.dotSwiftPM) {
149161
try self.createDirectory(self.dotSwiftPM, recursive: true)
150162
}
151163
// Create ~/.swiftpm/configuration symlink if necessary
152164
// locking ~/.swiftpm to protect from concurrent access
153165
try self.withLock(on: self.dotSwiftPM, type: .exclusive) {
154-
if !self.exists(self.dotSwiftPMConfigurationDirectory, followSymlink: false) {
166+
if !self.exists(try self.dotSwiftPMConfigurationDirectory, followSymlink: false) {
155167
try self.createSymbolicLink(dotSwiftPMConfigurationDirectory, pointingAt: idiomaticConfigurationDirectory, relative: false)
156168
}
157169
}
@@ -165,26 +177,30 @@ extension FileSystem {
165177
extension FileSystem {
166178
/// SwiftPM security directory under user's security directory (if exists)
167179
public var swiftPMSecurityDirectory: AbsolutePath {
168-
if let path = self.idiomaticSwiftPMDirectory {
169-
return path.appending(component: "security")
170-
} else {
171-
return self.dotSwiftPMSecurityDirectory
180+
get throws {
181+
if let path = try self.idiomaticSwiftPMDirectory {
182+
return path.appending(component: "security")
183+
} else {
184+
return try self.dotSwiftPMSecurityDirectory
185+
}
172186
}
173187
}
174188

175189
fileprivate var dotSwiftPMSecurityDirectory: AbsolutePath {
176-
return self.dotSwiftPM.appending(component: "security")
190+
get throws {
191+
return try self.dotSwiftPM.appending(component: "security")
192+
}
177193
}
178194
}
179195

180196
extension FileSystem {
181197
public func getOrCreateSwiftPMSecurityDirectory() throws -> AbsolutePath {
182-
let idiomaticSecurityDirectory = self.swiftPMSecurityDirectory
198+
let idiomaticSecurityDirectory = try self.swiftPMSecurityDirectory
183199

184200
// temporary 5.6, remove on next version: transition from ~/.swiftpm/security to idiomatic location + symbolic link
185-
if idiomaticSecurityDirectory != self.dotSwiftPMSecurityDirectory &&
186-
self.exists(self.dotSwiftPMSecurityDirectory) &&
187-
self.isDirectory(self.dotSwiftPMSecurityDirectory) {
201+
if try idiomaticSecurityDirectory != self.dotSwiftPMSecurityDirectory &&
202+
self.exists(try self.dotSwiftPMSecurityDirectory) &&
203+
self.isDirectory(try self.dotSwiftPMSecurityDirectory) {
188204
try self.removeFileTree(self.dotSwiftPMSecurityDirectory)
189205
}
190206
// ~temporary 5.6 migration
@@ -194,13 +210,13 @@ extension FileSystem {
194210
try self.createDirectory(idiomaticSecurityDirectory, recursive: true)
195211
}
196212
// Create ~/.swiftpm if necessary
197-
if !self.exists(self.dotSwiftPM) {
213+
if !self.exists(try self.dotSwiftPM) {
198214
try self.createDirectory(self.dotSwiftPM, recursive: true)
199215
}
200216
// Create ~/.swiftpm/security symlink if necessary
201217
// locking ~/.swiftpm to protect from concurrent access
202218
try self.withLock(on: self.dotSwiftPM, type: .exclusive) {
203-
if !self.exists(self.dotSwiftPMSecurityDirectory, followSymlink: false) {
219+
if !self.exists(try self.dotSwiftPMSecurityDirectory, followSymlink: false) {
204220
try self.createSymbolicLink(dotSwiftPMSecurityDirectory, pointingAt: idiomaticSecurityDirectory, relative: false)
205221
}
206222
}

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: 7 additions & 7 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

@@ -111,7 +111,7 @@ fileprivate func macOSSandboxProfile(
111111
if readOnlyDirectories.count > 0 {
112112
contents += "(deny file-write*\n"
113113
for path in readOnlyDirectories {
114-
contents += " (subpath \(resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
114+
contents += " (subpath \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
115115
}
116116
contents += ")\n"
117117
}
@@ -120,7 +120,7 @@ fileprivate func macOSSandboxProfile(
120120
if writableDirectories.count > 0 {
121121
contents += "(allow file-write*\n"
122122
for path in writableDirectories {
123-
contents += " (subpath \(resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
123+
contents += " (subpath \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
124124
}
125125
contents += ")\n"
126126
}
@@ -139,6 +139,6 @@ fileprivate extension AbsolutePath {
139139
}
140140

141141
extension TSCUtility.Platform {
142-
fileprivate static let threadSafeDarwinCacheDirectories = ThreadSafeArrayStore<AbsolutePath>(Self.darwinCacheDirectories())
142+
fileprivate static let threadSafeDarwinCacheDirectories = ThreadSafeArrayStore<AbsolutePath>((try? Self.darwinCacheDirectories()) ?? [])
143143
}
144144
#endif

Sources/Basics/TemporaryFile.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,13 @@ private func createTemporaryDirectory(fileSystem: FileSystem, dir: AbsolutePath?
9393

9494
let randomSuffix = String((0..<6).map { _ in letters.randomElement()! })
9595

96-
let tempDirectory = dir ?? fileSystem.tempDirectory
96+
let tempDirectory = try dir ?? fileSystem.tempDirectory
9797
guard fileSystem.isDirectory(tempDirectory) else {
9898
throw TempFileError.couldNotFindTmpDir(tempDirectory.pathString)
9999
}
100100

101101
// Construct path to the temporary directory.
102-
let templatePath = AbsolutePath(prefix + ".\(randomSuffix)", relativeTo: tempDirectory)
102+
let templatePath = try AbsolutePath(validating: prefix + ".\(randomSuffix)", relativeTo: tempDirectory)
103103

104104
try fileSystem.createDirectory(templatePath, recursive: true)
105105
return templatePath

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: 6 additions & 5 deletions
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() })
@@ -445,7 +445,7 @@ public final class BuildExecutionContext {
445445
.appending(component: "libIndexStore.dll")
446446
#else
447447
let ext = buildParameters.hostTriple.dynamicLibraryExtension
448-
let indexStoreLib = buildParameters.toolchain.toolchainLibDir.appending(component: "libIndexStore" + ext)
448+
let indexStoreLib = try buildParameters.toolchain.toolchainLibDir.appending(component: "libIndexStore" + ext)
449449
#endif
450450
return try IndexStoreAPI(dylib: indexStoreLib)
451451
}
@@ -489,8 +489,8 @@ final class CopyCommand: CustomLLBuildCommand {
489489
throw StringError("command \(command.name) not registered")
490490
}
491491

492-
let input = AbsolutePath(tool.inputs[0].name)
493-
let output = AbsolutePath(tool.outputs[0].name)
492+
let input = try AbsolutePath(validating: tool.inputs[0].name)
493+
let output = try AbsolutePath(validating: tool.outputs[0].name)
494494
try self.context.fileSystem.createDirectory(output.parentDirectory, recursive: true)
495495
try self.context.fileSystem.removeFileTree(output)
496496
try self.context.fileSystem.copy(from: input, to: output)
@@ -903,7 +903,8 @@ fileprivate struct CommandTaskTracker {
903903
switch message.name {
904904
case "compile":
905905
if let sourceFile = info.inputs.first {
906-
return "Compiling \(targetName) \(AbsolutePath(sourceFile).components.last!)"
906+
let sourceFilePath = try! AbsolutePath(validating: sourceFile)
907+
return "Compiling \(targetName) \(sourceFilePath.components.last!)"
907908
}
908909
case "link":
909910
return "Linking \(targetName)"

0 commit comments

Comments
 (0)