Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix no-newline-at-end-of-RUN output issue #3072

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

tianon
Copy link
Member

@tianon tianon commented Aug 30, 2022

This edge case was already handled for CANCELED or ERROR, but not DONE (or CACHED, which shouldn't have logs, but 馃し).

Before:

$ docker buildx build --progress=plain --no-cache - <<<$'FROM bash\nRUN echo -n no newline'
...
#5 [2/2] RUN echo -n no newline
#5 0.268 no newline#5 DONE 0.3s
...

After:

$ docker buildx build --progress=plain --no-cache - <<<$'FROM bash\nRUN echo -n no newline'
...
#5 [2/2] RUN echo -n no newline
#5 0.268 no newline
#5 DONE 0.3s
...

This edge case was already handled for `CANCELED` or `ERROR`, but not `DONE` (or `CACHED`, which shouldn't have logs, but 馃し).

Before:

```console
$ docker buildx build --progress=plain --no-cache - <<<$'FROM bash\nRUN echo -n no newline'
...
moby#5 [2/2] RUN echo -n no newline
moby#5 0.268 no newline#5 DONE 0.3s
...
```

After:

```console
$ docker buildx build --progress=plain --no-cache - <<<$'FROM bash\nRUN echo -n no newline'
...
moby#5 [2/2] RUN echo -n no newline
moby#5 0.268 no newline
moby#5 DONE 0.3s
...
```

Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
@@ -170,10 +170,10 @@ func (p *textMux) printVtx(t *trace, dgst digest.Digest) {
p.current = ""
v.count = 0

if v.logsPartial {
fmt.Fprintln(p.w, "")
}
if v.Error != "" {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My justification for this specific fix is that every single exit path of this if chain ends with some variation of fmt.Fprintf(p.w, "#%d ..., so it seems correct to make sure that every single path clears any partial line before outputting (even in the case of CACHED where I think logsPartial shouldn't ever be true).

@tonistiigi tonistiigi merged commit f1f9bde into moby:master Sep 2, 2022
@tianon tianon deleted the newline-at-eorun branch September 2, 2022 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants