Skip to content

Commit 81c1d6e

Browse files
committed
Remove throws, use completion pattern exclusively
1 parent d9998c2 commit 81c1d6e

File tree

1 file changed

+107
-95
lines changed

1 file changed

+107
-95
lines changed

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 107 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
713713
) {
714714
do {
715715
if localFileSystem.isFile(manifestPath) {
716-
try self.evaluateManifest(
716+
self.evaluateManifest(
717717
at: manifestPath,
718718
packageIdentity: packageIdentity,
719719
toolsVersion: toolsVersion,
@@ -723,7 +723,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
723723
} else {
724724
try withTemporaryFile(suffix: ".swift") { tempFile, cleanupTempFile in
725725
try localFileSystem.writeFileContents(tempFile.path, bytes: ByteString(manifestContents))
726-
try self.evaluateManifest(
726+
self.evaluateManifest(
727727
at: tempFile.path,
728728
packageIdentity: packageIdentity,
729729
toolsVersion: toolsVersion,
@@ -746,7 +746,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
746746
toolsVersion: ToolsVersion,
747747
delegateQueue: DispatchQueue,
748748
completion: @escaping (Result<EvaluationResult, Error>) -> Void
749-
) throws {
749+
) {
750750
var evaluationResult = EvaluationResult()
751751

752752
delegateQueue.async {
@@ -806,10 +806,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
806806
Triple.getHostTriple(usingSwiftCompiler: self.toolchain.swiftCompilerPath)
807807
}
808808

809-
let version = try Self._packageDescriptionMinimumDeploymentTarget.memoize {
810-
(try MinimumDeploymentTarget.computeMinimumDeploymentTarget(of: macOSPackageDescriptionPath, platform: .macOS))?.versionString ?? "10.15"
809+
do {
810+
let version = try Self._packageDescriptionMinimumDeploymentTarget.memoize {
811+
(try MinimumDeploymentTarget.computeMinimumDeploymentTarget(of: macOSPackageDescriptionPath, platform: .macOS))?.versionString ?? "10.15"
812+
}
813+
cmd += ["-target", "\(triple.tripleString(forPlatformVersion: version))"]
814+
} catch {
815+
return completion(.failure(error))
811816
}
812-
cmd += ["-target", "\(triple.tripleString(forPlatformVersion: version))"]
813817
#endif
814818

815819
// Add any extra flags required as indicated by the ManifestLoader.
@@ -824,115 +828,123 @@ public final class ManifestLoader: ManifestLoaderProtocol {
824828
if self.serializedDiagnostics, let databaseCacheDir = self.databaseCacheDir {
825829
let diaDir = databaseCacheDir.appending(component: "ManifestLoading")
826830
let diagnosticFile = diaDir.appending(component: "\(packageIdentity).dia")
827-
try localFileSystem.createDirectory(diaDir, recursive: true)
828-
cmd += ["-Xfrontend", "-serialize-diagnostics-path", "-Xfrontend", diagnosticFile.pathString]
829-
evaluationResult.diagnosticFile = diagnosticFile
831+
do {
832+
try localFileSystem.createDirectory(diaDir, recursive: true)
833+
cmd += ["-Xfrontend", "-serialize-diagnostics-path", "-Xfrontend", diagnosticFile.pathString]
834+
evaluationResult.diagnosticFile = diagnosticFile
835+
} catch {
836+
return completion(.failure(error))
837+
}
830838
}
831839

832840
cmd += [manifestPath.pathString]
833841

834842
cmd += self.extraManifestFlags
835843

836-
try withTemporaryDirectory { tmpDir, cleanupTmpDir in
837-
// Set path to compiled manifest executable.
838-
#if os(Windows)
839-
let executableSuffix = ".exe"
840-
#else
841-
let executableSuffix = ""
842-
#endif
843-
let compiledManifestFile = tmpDir.appending(component: "\(packageIdentity)-manifest\(executableSuffix)")
844-
cmd += ["-o", compiledManifestFile.pathString]
845-
846-
// Compile the manifest.
847-
Process.popen(arguments: cmd, environment: toolchain.swiftCompilerEnvironment, queue: delegateQueue) { result in
848-
var cleanupIfError = DelayableAction(target: tmpDir, action: cleanupTmpDir)
849-
defer { cleanupIfError.perform() }
850-
851-
let compilerResult : ProcessResult
852-
do {
853-
compilerResult = try result.get()
854-
evaluationResult.compilerOutput = try (compilerResult.utf8Output() + compilerResult.utf8stderrOutput()).spm_chuzzle()
855-
} catch {
856-
return completion(.failure(error))
857-
}
858-
859-
// Return now if there was an error.
860-
if compilerResult.exitStatus != .terminated(code: 0) {
861-
return completion(.success(evaluationResult))
862-
}
863-
864-
// Pass an open file descriptor of a file to which the JSON representation of the manifest will be written.
865-
let jsonOutputFile = tmpDir.appending(component: "\(packageIdentity)-output.json")
866-
guard let jsonOutputFileDesc = fopen(jsonOutputFile.pathString, "w") else {
867-
return completion(.failure(StringError("couldn't create the manifest's JSON output file")))
868-
}
869-
870-
cmd = [compiledManifestFile.pathString]
844+
do {
845+
try withTemporaryDirectory { tmpDir, cleanupTmpDir in
846+
// Set path to compiled manifest executable.
871847
#if os(Windows)
872-
// NOTE: `_get_osfhandle` returns a non-owning, unsafe,
873-
// unretained HANDLE. DO NOT invoke `CloseHandle` on `hFile`.
874-
let hFile: Int = _get_osfhandle(_fileno(jsonOutputFileDesc))
875-
cmd += ["-handle", "\(String(hFile, radix: 16))"]
848+
let executableSuffix = ".exe"
876849
#else
877-
cmd += ["-fileno", "\(fileno(jsonOutputFileDesc))"]
850+
let executableSuffix = ""
878851
#endif
852+
let compiledManifestFile = tmpDir.appending(component: "\(packageIdentity)-manifest\(executableSuffix)")
853+
cmd += ["-o", compiledManifestFile.pathString]
879854

880-
do {
881-
let packageDirectory = manifestPath.parentDirectory.pathString
882-
let contextModel = ContextModel(packageDirectory: packageDirectory)
883-
cmd += ["-context", try contextModel.encode()]
884-
} catch {
885-
return completion(.failure(error))
886-
}
855+
// Compile the manifest.
856+
Process.popen(arguments: cmd, environment: toolchain.swiftCompilerEnvironment, queue: delegateQueue) { result in
857+
var cleanupIfError = DelayableAction(target: tmpDir, action: cleanupTmpDir)
858+
defer { cleanupIfError.perform() }
887859

888-
// If enabled, run command in a sandbox.
889-
// This provides some safety against arbitrary code execution when parsing manifest files.
890-
// We only allow the permissions which are absolutely necessary.
891-
if self.isManifestSandboxEnabled {
892-
let cacheDirectories = [self.databaseCacheDir, moduleCachePath].compactMap{ $0 }
893-
let strictness: Sandbox.Strictness = toolsVersion < .v5_3 ? .manifest_pre_53 : .default
894-
cmd = Sandbox.apply(command: cmd, writableDirectories: cacheDirectories, strictness: strictness)
895-
}
860+
let compilerResult : ProcessResult
861+
do {
862+
compilerResult = try result.get()
863+
evaluationResult.compilerOutput = try (compilerResult.utf8Output() + compilerResult.utf8stderrOutput()).spm_chuzzle()
864+
} catch {
865+
return completion(.failure(error))
866+
}
896867

897-
// Run the compiled manifest.
898-
var environment = ProcessEnv.vars
899-
#if os(Windows)
900-
let windowsPathComponent = runtimePath.pathString.replacingOccurrences(of: "/", with: "\\")
901-
environment["Path"] = "\(windowsPathComponent);\(environment["Path"] ?? "")"
902-
#endif
868+
// Return now if there was an error.
869+
if compilerResult.exitStatus != .terminated(code: 0) {
870+
return completion(.success(evaluationResult))
871+
}
872+
873+
// Pass an open file descriptor of a file to which the JSON representation of the manifest will be written.
874+
let jsonOutputFile = tmpDir.appending(component: "\(packageIdentity)-output.json")
875+
guard let jsonOutputFileDesc = fopen(jsonOutputFile.pathString, "w") else {
876+
return completion(.failure(StringError("couldn't create the manifest's JSON output file")))
877+
}
878+
879+
cmd = [compiledManifestFile.pathString]
880+
#if os(Windows)
881+
// NOTE: `_get_osfhandle` returns a non-owning, unsafe,
882+
// unretained HANDLE. DO NOT invoke `CloseHandle` on `hFile`.
883+
let hFile: Int = _get_osfhandle(_fileno(jsonOutputFileDesc))
884+
cmd += ["-handle", "\(String(hFile, radix: 16))"]
885+
#else
886+
cmd += ["-fileno", "\(fileno(jsonOutputFileDesc))"]
887+
#endif
903888

904-
let cleanupAfterRunning = cleanupIfError.delay()
905-
Process.popen(arguments: cmd, environment: environment, queue: delegateQueue) { result in
906-
defer { cleanupAfterRunning.perform() }
907-
fclose(jsonOutputFileDesc)
908-
909889
do {
910-
let runResult = try result.get()
911-
if let runOutput = try (runResult.utf8Output() + runResult.utf8stderrOutput()).spm_chuzzle() {
912-
// Append the runtime output to any compiler output we've received.
913-
evaluationResult.compilerOutput = (evaluationResult.compilerOutput ?? "") + runOutput
914-
}
890+
let packageDirectory = manifestPath.parentDirectory.pathString
891+
let contextModel = ContextModel(packageDirectory: packageDirectory)
892+
cmd += ["-context", try contextModel.encode()]
893+
} catch {
894+
return completion(.failure(error))
895+
}
915896

916-
// Return now if there was an error.
917-
if runResult.exitStatus != .terminated(code: 0) {
918-
// TODO: should this simply be an error?
919-
// return completion(.failure(ProcessResult.Error.nonZeroExit(runResult)))
920-
evaluationResult.errorOutput = evaluationResult.compilerOutput
921-
return completion(.success(evaluationResult))
922-
}
897+
// If enabled, run command in a sandbox.
898+
// This provides some safety against arbitrary code execution when parsing manifest files.
899+
// We only allow the permissions which are absolutely necessary.
900+
if self.isManifestSandboxEnabled {
901+
let cacheDirectories = [self.databaseCacheDir, moduleCachePath].compactMap{ $0 }
902+
let strictness: Sandbox.Strictness = toolsVersion < .v5_3 ? .manifest_pre_53 : .default
903+
cmd = Sandbox.apply(command: cmd, writableDirectories: cacheDirectories, strictness: strictness)
904+
}
923905

924-
// Read the JSON output that was emitted by libPackageDescription.
925-
guard let jsonOutput = try localFileSystem.readFileContents(jsonOutputFile).validDescription else {
926-
return completion(.failure(StringError("the manifest's JSON output has invalid encoding")))
927-
}
928-
evaluationResult.manifestJSON = jsonOutput
906+
// Run the compiled manifest.
907+
var environment = ProcessEnv.vars
908+
#if os(Windows)
909+
let windowsPathComponent = runtimePath.pathString.replacingOccurrences(of: "/", with: "\\")
910+
environment["Path"] = "\(windowsPathComponent);\(environment["Path"] ?? "")"
911+
#endif
912+
913+
let cleanupAfterRunning = cleanupIfError.delay()
914+
Process.popen(arguments: cmd, environment: environment, queue: delegateQueue) { result in
915+
defer { cleanupAfterRunning.perform() }
916+
fclose(jsonOutputFileDesc)
929917

930-
completion(.success(evaluationResult))
931-
} catch {
932-
completion(.failure(error))
918+
do {
919+
let runResult = try result.get()
920+
if let runOutput = try (runResult.utf8Output() + runResult.utf8stderrOutput()).spm_chuzzle() {
921+
// Append the runtime output to any compiler output we've received.
922+
evaluationResult.compilerOutput = (evaluationResult.compilerOutput ?? "") + runOutput
923+
}
924+
925+
// Return now if there was an error.
926+
if runResult.exitStatus != .terminated(code: 0) {
927+
// TODO: should this simply be an error?
928+
// return completion(.failure(ProcessResult.Error.nonZeroExit(runResult)))
929+
evaluationResult.errorOutput = evaluationResult.compilerOutput
930+
return completion(.success(evaluationResult))
931+
}
932+
933+
// Read the JSON output that was emitted by libPackageDescription.
934+
guard let jsonOutput = try localFileSystem.readFileContents(jsonOutputFile).validDescription else {
935+
return completion(.failure(StringError("the manifest's JSON output has invalid encoding")))
936+
}
937+
evaluationResult.manifestJSON = jsonOutput
938+
939+
completion(.success(evaluationResult))
940+
} catch {
941+
completion(.failure(error))
942+
}
933943
}
934944
}
935945
}
946+
} catch {
947+
return completion(.failure(error))
936948
}
937949
}
938950

0 commit comments

Comments
 (0)