Make building executables as dylibs more robust (now parses SwiftPM b…

…uild plan instead of verbose build output)

Package.resolved

@@ -275,8 +275,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/jpsim/Yams.git",
       "state" : {
-        "revision" : "0d9ee7ea8c4ebd4a489ad7a73d5c6cad55d6fed3",
-        "version" : "5.0.6"
+        "revision" : "9234124cff5e22e178988c18d8b95a8ae8007f76",
+        "version" : "5.1.2"
       }
     }
   ],

Package.swift

@@ -23,6 +23,7 @@ let package = Package(
     .package(url: "https://github.com/apple/swift-format", exact: "510.0.1"),
     .package(url: "https://github.com/pointfreeco/swift-overture", from: "0.5.0"),
     .package(url: "https://github.com/stackotter/Socket", from: "0.3.3"),
+    .package(url: "https://github.com/jpsim/Yams.git", from: "5.1.2"),
 
     // File watcher dependencies
     .package(url: "https://github.com/sersoft-gmbh/swift-inotify", from: "0.4.0"),
@@ -42,6 +43,7 @@ let package = Package(
         "Socket",
         "HotReloadingProtocol",
         "FileSystemWatcher",
+        "Yams",
         .product(name: "XcodeGenKit", package: "XcodeGen"),
         .product(name: "ProjectSpec", package: "XcodeGen"),
         .product(name: "SwiftSyntax", package: "swift-syntax"),

Sources/swift-bundler/Bundler/Runner/Runner.swift

@@ -268,7 +268,6 @@ enum Runner {
     arguments: [String],
     environmentVariables: [String: String]
   ) -> Result<Void, RunnerError> {
-    print("Creating")
     let process = Process.create(
       bundle.path,
       arguments: arguments,

Sources/swift-bundler/Bundler/StringCatalogCompiler.swift

@@ -386,7 +386,6 @@ enum StringCatalogCompiler {
         if formatSpecifierOrder[currentOrder] == nil {
           formatSpecifierOrder[currentOrder] = (String(formatSpecifierType), formatSpecifier)
         } else if formatSpecifierOrder[currentOrder]?.0 != String(formatSpecifierType) {
-          print(formatSpecifierOrder[currentOrder] ?? "nil")
           return .failure(.invalidNonMatchingFormatString(URL(fileURLWithPath: ""), string))
         }
         currentOrder += 1

Sources/swift-bundler/Bundler/SwiftPackageManager/BuildPlan.swift

@@ -0,0 +1,16 @@
+import Foundation
+
+/// A SwiftPM build plan as seen in `.build/debug.yaml` and co.
+struct BuildPlan: Codable {
+  var commands: [String: Command]
+
+  struct Command: Codable {
+    var tool: String
+    var arguments: [String]?
+
+    enum CodingKeys: String, CodingKey {
+      case tool
+      case arguments = "args"
+    }
+  }
+}

Sources/swift-bundler/Bundler/SwiftPackageManager/SwiftPackageManager.swift

@@ -2,6 +2,7 @@ import Foundation
 import Parsing
 import StackOtterArgParser
 import Version
+import Yams
 
 /// A utility for interacting with the Swift package manager and performing some other package
 /// related operations.
@@ -139,8 +140,6 @@ enum SwiftPackageManager {
     hotReloadingEnabled: Bool = false
   ) -> Result<URL, SwiftPackageManagerError> {
     #if os(macOS)
-      log.info("Starting \(configuration.rawValue) build")
-
       // TODO: Package up 'build options' into a struct so that it can be passed around
       //   more easily
       let productsDirectory: URL
@@ -158,56 +157,65 @@ enum SwiftPackageManager {
       }
       let dylibFile = productsDirectory.appendingPathComponent("lib\(product).dylib")
 
-      return createBuildArguments(
+      return build(
         product: product,
+        packageDirectory: packageDirectory,
         configuration: configuration,
         architectures: architectures,
         platform: platform,
-        platformVersion: platformVersion
-      ).flatMap { arguments in
-        let process = Process.create(
-          "swift",
-          arguments: arguments + ["-v"],
-          directory: packageDirectory,
-          runSilentlyWhenNotVerbose: false
-        )
-        if hotReloadingEnabled {
-          process.addEnvironmentVariables([
-            "SWIFT_BUNDLER_HOT_RELOADING": "1"
-          ])
+        platformVersion: platformVersion,
+        hotReloadingEnabled: hotReloadingEnabled
+      ).flatMap { _ in
+        let buildPlanFile = packageDirectory.appendingPathComponent(".build/\(configuration).yaml")
+        let buildPlanString: String
+        do {
+          buildPlanString = try String(contentsOf: buildPlanFile)
+        } catch {
+          return .failure(.failedToReadBuildPlan(path: buildPlanFile, error))
         }
 
-        return process.getOutputData().mapError { error in
-          return .failedToRunSwiftBuild(
-            command: "swift \(arguments.joined(separator: " "))",
-            error
-          )
-        }
-      }.flatMap { (verboseOutput: Data) in
-        guard let string = String(data: verboseOutput, encoding: .utf8) else {
-          return .failure(.failedToParseBuildCommandSteps(details: "Invalid UTF-8"))
+        let buildPlan: BuildPlan
+        do {
+          buildPlan = try YAMLDecoder().decode(BuildPlan.self, from: buildPlanString)
+        } catch {
+          return .failure(.failedToDecodeBuildPlan(error))
         }
 
-        let lines = string.split(separator: "\n")
+        let commandName = "C.\(product)-\(configuration).exe"
         guard
-          let linkCommandString = lines.last(where: { line in
-            line.hasPrefix("/")
-          })
+          let linkCommand = buildPlan.commands[commandName],
+          linkCommand.tool == "shell",
+          let commandExecutable = linkCommand.arguments?.first,
+          let arguments = linkCommand.arguments?.dropFirst()
         else {
-          return .failure(.failedToParseBuildCommandSteps(details: "Couldn't locate link command"))
+          return .failure(
+            .failedToComputeLinkingCommand(
+              details: "Couldn't find valid command for \(commandName)"
+            )
+          )
         }
-        let linkCommand = CommandLine.lenientParse(String(linkCommandString))
 
-        guard linkCommand.arguments.count >= 1 else {
-          return .failure(.failedToParseBuildCommandSteps(details: "No arguments"))
+        var modifiedArguments = Array(arguments)
+        guard
+          let index = modifiedArguments.firstIndex(of: "-o"),
+          index < modifiedArguments.count - 1
+        else {
+          return .failure(
+            .failedToComputeLinkingCommand(details: "Couldn't find '-o' argument to replace")
+          )
         }
 
-        let modifiedArguments =
-          Array(linkCommand.arguments.dropLast()) + [dylibFile.path, "-dynamiclib"]
+        modifiedArguments.remove(at: index)
+        modifiedArguments.remove(at: index)
+        modifiedArguments.append(contentsOf: [
+          "-o",
+          dylibFile.path,
+          "-Xcc",
+          "-dynamiclib",
+        ])
 
-        print("Running linking command")
         let process = Process.create(
-          linkCommand.command,
+          commandExecutable,
           arguments: modifiedArguments,
           directory: packageDirectory,
           runSilentlyWhenNotVerbose: false
@@ -216,8 +224,17 @@ enum SwiftPackageManager {
         return process.runAndWait()
           .map { _ in dylibFile }
           .mapError { error in
-            return .failedToRunSwiftBuild(
-              command: linkCommand.description,
+            // TODO: Make a more robust way of converting commands to strings for display (keeping
+            //   correctness in mind in case users want to copy-paste commands from errors).
+            return .failedToRunLinkingCommand(
+              command: ([commandExecutable]
+                + modifiedArguments.map { argument in
+                  if argument.contains(" ") {
+                    return "\"\(argument)\""
+                  } else {
+                    return argument
+                  }
+                }).joined(separator: " "),
               error
             )
           }

Sources/swift-bundler/Bundler/SwiftPackageManager/SwiftPackageManagerError.swift

@@ -18,7 +18,10 @@ enum SwiftPackageManagerError: LocalizedError {
   case failedToExecutePackageManifest(Error)
   case failedToParsePackageManifestOutput(json: String, Error?)
   case failedToParsePackageManifestToolsVersion(Error?)
-  case failedToParseBuildCommandSteps(details: String)
+  case failedToReadBuildPlan(path: URL, Error)
+  case failedToDecodeBuildPlan(Error)
+  case failedToComputeLinkingCommand(details: String)
+  case failedToRunLinkingCommand(command: String, Error)
 
   var errorDescription: String? {
     switch self {
@@ -63,8 +66,15 @@ enum SwiftPackageManagerError: LocalizedError {
         return """
           Failed to parse package manifest tools version: \(error?.localizedDescription ?? "Unknown error")
           """
-      case .failedToParseBuildCommandSteps(let details):
-        return "Failed to parse build command steps: \(details)"
+      case .failedToReadBuildPlan(let path, let error):
+        return
+          "Failed to read build plan file at '\(path.relativePath(from: URL(fileURLWithPath: ".")) ?? path.path))': \(error.localizedDescription)"
+      case .failedToDecodeBuildPlan(let error):
+        return "Failed to decode build plain: \(error.localizedDescription)"
+      case .failedToComputeLinkingCommand(let details):
+        return "Failed to compute linking command: \(details)"
+      case .failedToRunLinkingCommand(let command, let error):
+        return "Failed to run linking commmand '\(command)': \(error.localizedDescription)"
     }
   }
 }

Sources/swift-bundler/Commands/RunCommand.swift

@@ -141,7 +141,6 @@ struct RunCommand: AsyncCommand {
         }
 
         // TODO: Avoid loading manifest twice
-        log.info("Building 'lib\(appConfiguration.product).dylib'")
         let manifest = try await SwiftPackageManager.loadPackageManifest(from: packageDirectory)
           .unwrap()
 
@@ -160,6 +159,7 @@ struct RunCommand: AsyncCommand {
         try await FileSystemWatcher.watch(
           paths: [packageDirectory.appendingPathComponent("Sources").path],
           with: {
+            log.info("Building 'lib\(appConfiguration.product).dylib'")
             let client = client
             Task {
               do {
@@ -173,16 +173,16 @@ struct RunCommand: AsyncCommand {
                   platformVersion: platformVersion,
                   hotReloadingEnabled: true
                 ).unwrap()
-                log.info("Successfully built dylib to '\(dylibFile.relativeString)'")
+                log.info("Successfully built dylib")
 
                 try await Packet.reloadDylib(path: dylibFile).write(to: &client)
               } catch {
-                print("Hot reloading: \(error)")
+                log.error("Hot reloading: \(error.localizedDescription)")
               }
             }
           },
           errorHandler: { error in
-            print("Hot reloading: \(error)")
+            log.error("Hot reloading: \(error.localizedDescription)")
           })
       }
 

Sources/swift-bundler/Extensions/Process.swift

@@ -26,29 +26,53 @@ extension Process {
   }
 
   /// Gets the process's stdout and stderr as `Data`.
-  /// - Parameter excludeStdError: If `true`, only stdout is returned.
+  /// - Parameters:
+  ///   - excludeStdError: If `true`, only stdout is returned.
+  ///   - handleLine: A handler to run every time that a line is received.
   /// - Returns: The process's stdout and stderr. If an error occurs, a failure is returned.
-  func getOutputData(excludeStdError: Bool = false) -> Result<Data, ProcessError> {
+  func getOutputData(
+    excludeStdError: Bool = false,
+    handleLine: ((String) -> Void)? = nil
+  ) -> Result<Data, ProcessError> {
     let pipe = Pipe()
     setOutputPipe(pipe, excludeStdError: excludeStdError)
 
     // Thanks Martin! https://forums.swift.org/t/the-problem-with-a-frozen-process-in-swift-process-class/39579/6
     var output = Data()
+    var currentLine: String?
     let group = DispatchGroup()
     group.enter()
     pipe.fileHandleForReading.readabilityHandler = { fh in
+      // TODO: All of this Process code is getting pretty ridiculous and janky, we should switch to
+      //   the experimental proposed Subprocess API (swift-experimental-subprocess)
       let newData = fh.availableData
       if newData.isEmpty {
         pipe.fileHandleForReading.readabilityHandler = nil
         group.leave()
       } else {
         output.append(contentsOf: newData)
+        if let handleLine = handleLine, let string = String(data: newData, encoding: .utf8) {
+          let lines = ((currentLine ?? "") + string).split(
+            separator: "\n", omittingEmptySubsequences: false)
+          if let lastLine = lines.last, lastLine != "" {
+            currentLine = String(lastLine)
+          } else {
+            currentLine = nil
+          }
+
+          for line in lines.dropLast() {
+            handleLine(String(line))
+          }
+        }
       }
     }
 
     return runAndWait()
       .map { _ in
         group.wait()
+        if let currentLine = currentLine {
+          handleLine?(currentLine)
+        }
         return output
       }
       .mapError { error in