plugin: handle chunks of output correctly

.github/workflows/integration_tests.yml

@@ -34,3 +34,25 @@ jobs:
                   REGISTRY_PORT: 5000
               run: |
                   swift test
+
+    plugin-streaming-output-test:
+        name: Plugin streaming output test
+        runs-on: ubuntu-latest
+        services:
+            registry:
+                image: registry:2
+                ports:
+                    - 5000:5000
+        steps:
+            - name: Checkout repository
+              uses: actions/checkout@v4
+              with:
+                  persist-credentials: false
+
+            - name: Mark the workspace as safe
+              # https://github.com/actions/checkout/issues/766
+              run: git config --global --add safe.directory ${GITHUB_WORKSPACE}
+
+            - name: Check plugin streaming output is reassembled and printed properly
+              run: |
+                  scripts/test-plugin-output-streaming.sh

Plugins/ContainerImageBuilder/main.swift

@@ -99,16 +99,48 @@ extension PluginError: CustomStringConvertible {
 
         await withThrowingTaskGroup(of: Void.self) { group in
             group.addTask {
+                enum LoggingState {
+                    // Normal output is logged at 'progress' level.
+                    case progress
+
+                    // If an error is detected, all output from that point onwards is logged at 'error' level, which is always printed.
+                    // Errors are reported even without the --verbose flag and cause the build to return a nonzero exit code.
+                    case error
+
+                    func log(_ msg: String) {
+                        let trimmed = msg.trimmingCharacters(in: .newlines)
+                        switch self {
+                        case .progress: Diagnostics.progress(trimmed)
+                        case .error: Diagnostics.error(trimmed)
+                        }
+                    }
+                }
+
+                var buf = ""
+                var logger = LoggingState.progress
+
                 for try await line in err.lines {
-                    let errorLabel = "Error: "  // SwiftArgumentParser adds this prefix to all errors which bubble up
-                    let trimmed = line.trimmingCharacters(in: .whitespacesAndNewlines)
+                    buf.append(line)
 
-                    if trimmed.starts(with: errorLabel) {
-                        // Errors are reported even without the --verbose flag and cause the build to fail.
-                        Diagnostics.error(String(trimmed.dropFirst(errorLabel.count)))
-                    } else {
-                        Diagnostics.progress(trimmed)
+                    guard let (before, after) = buf.splitOn(first: "\n") else {
+                        continue
+                    }
+
+                    var msg = before
+                    buf = String(after)
+
+                    let errorLabel = "Error: "  // SwiftArgumentParser adds this prefix to all errors which bubble up
+                    if msg.starts(with: errorLabel) {
+                        logger = .error
+                        msg.trimPrefix(errorLabel)
                     }
+
+                    logger.log(String(msg))
+                }
+
+                // Print any leftover output in the buffer, in case the child exited without sending a final newline.
+                if !buf.isEmpty {
+                    logger.log(buf)
                 }
             }
 
@@ -118,3 +150,13 @@ extension PluginError: CustomStringConvertible {
         }
     }
 }
+
+extension Collection where Element: Equatable {
+    func splitOn(first element: Element) -> (before: SubSequence, after: SubSequence)? {
+        guard let idx = self.firstIndex(of: element) else {
+            return nil
+        }
+
+        return (self[..<idx], self[idx...].dropFirst())
+    }
+}

scripts/test-plugin-output-streaming.sh

@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+# Test that error output streamed from containertool is printed correctly by the plugin. 
+
+set -exo pipefail
+
+log() { printf -- "** %s\n" "$*" >&2; }
+error() { printf -- "** ERROR: %s\n" "$*" >&2; }
+fatal() { error "$@"; exit 1; }
+
+# Work in a temporary directory, deleted after the test finishes
+PKGPATH=$(mktemp -d)
+cleanup() {
+  log "Deleting temporary package $PKGPATH"
+  rm -rf "$PKGPATH"
+}
+trap cleanup EXIT
+
+swift package --package-path "$PKGPATH" init --type executable --name hello
+cat >> "$PKGPATH/Package.swift" <<EOF
+package.dependencies += [
+    .package(path: "$PWD"),
+]
+EOF
+
+# Run the plugin, forgetting a mandatory argument.   Verify that the output is not corrupted.
+# The `swift package` command will return a nonzero exit code.   This is expected, so disable pipefail.
+set +o pipefail
+swift package --package-path "$PKGPATH" --allow-network-connections all build-container-image 2>&1 | tee "$PKGPATH/output"
+set -o pipefail
+
+grep -F -x -e "error: Missing expected argument '--repository <repository>'" \
+           -e "error: Help:  --repository <repository>  Repository path" \
+           -e "error: Usage: containertool [<options>] --repository <repository> <executable>" \
+           -e "error:   See 'containertool --help' for more information." "$PKGPATH/output"
+
+echo Plugin error output: PASSED
+