Skip to content

plugin: handle chunks of output correctly #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2025

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented Mar 25, 2025

Motivation

containertool produces its normal output in single writes of less than PIPE_BUF bytes, which should be delivered atomically through the pipe to the plugin:

Reading or writing pipe data is atomic if the size of data written is not greater than PIPE_BUF. This means that the data transfer seems to be an instantaneous unit, in that nothing else in the system can observe a state in which it is partially complete. Atomic I/O may not begin right away (it may need to wait for buffer space or for data), but once it does begin it finishes immediately.

https://www.gnu.org/software/libc/manual/html_node/Pipe-Atomicity.html

Progress messages written in this way are displayed correctly. Unfortunately error output from swift-argument-parser is delivered in smaller chunks which are often broken across several lines by the plugin:

% swift package --allow-network-connections all  build-container-image       
Building for debugging...
[1/1] Write swift-version-2380AA06D3543E1B.txt
Build of product 'containertool' complete! (1.74s)
Building for debugging...
[0/3] Write swift-version-2380AA06D3543E1B.txt
Build of product 'hello-world' complete! (2.04s)
[ContainerImageBuilder] itory <repository>'
Help:  --repository <
[ContainerImageBuilder] repository>  Repository path
[ContainerImageBuilder] Usage: containertoo
[ContainerImageBuilder] l [<optio
[ContainerImageBuilder] ns>] --re
[ContainerImageBuilder] posito
[ContainerImageBuilder] ry <rep
error: Missing expected argument '--repos
[ContainerImageBuilder] ository
[ContainerImageBuilder] > <ex
[ContainerImageBuilder] ecutable>
  Se
[ContainerImageBuilder] e 'containertool --help' for more inf
[ContainerImageBuilder] ormation
[ContainerImageBuilder] .

At one point the plugin reassembled output from the pipe and re-split it on newlines, but the code triggered strict concurrency warnings with the Swift 6 language mode so it was removed in favour of PIPE_BUF write atomicity. This commit restores the pipe output reassembly.

Modifications

  • Plugin reassembles output from containertool and splits it on newlines
  • All output after an error is now printed using Diagnostics.error. Otherwise error and progress output would be interleaved.

Result

Error output from containertool will be presented correctly by the plugin.

Test Plan

Existing tests continue to pass.
A new integration test checks that error output is not broken into multiple lines.

@euanh euanh force-pushed the plugin-streaming-output branch from 49becc2 to fca18e0 Compare March 25, 2025 08:48
@euanh euanh force-pushed the plugin-streaming-output branch from a664fee to d2f15ab Compare March 25, 2025 15:09
@euanh euanh merged commit 5051545 into apple:main Mar 25, 2025
20 checks passed
@euanh euanh deleted the plugin-streaming-output branch March 25, 2025 15:16
@euanh euanh added kind/bug Something isn't working semver/patch No public API change. labels Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant