Progress reads collapse unread updates by ID, so the raw item count can vary
with scheduling. Assert the final status for each nested writer instead.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Update golangci-lint and adjust code for new gosec diagnostics. Use
root-scoped filesystem operations where appropriate, preserve explicit
user path behavior for SSH keys, and avoid background contexts in
request-scoped cleanup paths.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
In order to print tty output, the termHeight for the vt100
module needs to be between 6 and (current window size - 7).
Signed-off-by: Burt Holzman <burt@fnal.gov>
In the case where the context provided to MultiReader.Reader is
cancelled, the writer will be deleted.
Before this, closeWriter was not called in that codepath, when meant
that MultiReader.handle would never see the writer and never close it.
* I don't have an isolated repro, but when running a few hundred of
Dagger's tests I was seeing several thousand goroutines blocked on
waiting for the progress pipe's context to be done.
Now, we just call closeWriter when it gets deleted so that the pipe's
goroutine doesn't leak.
* After this change, those thousands of leaked goroutines are entirely
gone.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Previously, if this case occured, we would deadlock, so as to make this
case more easily identifiable and debuggable, we should panic instead.
Potentially, we could also just *skip* this case, but given that this is
an internal logical error that should not occur, it would be better to
identify this.
Signed-off-by: Justin Chadwell <me@jedevc.com>
When different LLB vertexes (eg. parallel requests referencing
local sources from different sessions) generate same cache keys
during solve they are merged together into a single operation.
Currently, when this happened the progress for the vertex that
was dropped got lost. This fixes this case by adding the
progressWriter of the redirected vertex as a target to the
source one.
This should also work with multiple levels of merged edges,
just multiple nested multiwriters as well.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
The output window was previously hard-coded to a height of 6;
this patch makes it configurable at run-time by setting the
BUILDKIT_TTY_LOG_LINES environment variable.
Signed-off-by: Burt Holzman <burt@fnal.gov>
The original intention of accepting `console.File` was that the current
uses of this function in the codebase were always files and it seemed
helpful to restrict the API to `console.File` to aid developers into
passing the correct output.
There are circumstances outside of the codebase where the `io.Writer`
versions are used so `NewDisplay` has now been adapted to take an
`io.Writer` to allow for those cases more easily.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This adds an additional display output for the progress indicator to
support a json output. It refators the progressui package a bit to add a
new method that takes in a `SolveStatusDisplay`. This
`SolveStatusDisplay` can be created by the user using `NewDisplay` with
the various modes as input parameters.
The json output will print the raw events as JSON blobs. It will not
throttle the messages or limit the display. It is meant as a pure raw
marshaling of the underlying event stream.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
`MultiWriter` would create an unnecessary map allocation when the
`Write` method was used. The `Write` method would create a progress
object with the meta field initialized to the meta field of the writer
itself. It then invoked its own `WriteRawProgress` method which would
see two maps with the metadata and erroneously believe that they were
different and needed to be merged into a single metadata map.
Since this map is initialized with the metadata of the writer before
`WriteRawProgress` is invoked, this merge was unnecessary and could add
a lot of unnecessary memory allocations during a build.
This changes the `MultiWriter.Write` method to invoke the private
`writeRawProgress` which performs the actual write and avoids the
metadata merge.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
No other parts of the progress rendering modify the inputs, so we should
avoid this as well.
This actually fixes an edge case in pushWithMoby which writes the same
VertexStatus multiple times, modifying the timestamps and similar.
However, if the operation takes long enough the small time difference
can accumulate, and move the Start time far into the past.
Signed-off-by: Justin Chadwell <me@jedevc.com>
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'
...
#5 [2/2] RUN echo -n no newline
#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'
...
#5 [2/2] RUN echo -n no newline
#5 0.268 no newline
#5 DONE 0.3s
...
```
Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
The plain progress has a loop that iterates over each vertex the trace
knows about (rather than just the updated ones). Before this, that loop
was not checking to see if a vertex was supposed to be skipped due to
being in a group or due to being hidden.
This resulted in vertexes being printed unnecessarily. It also led to a
rare case where a vertex with nil lastBlockTime could be set to the
'current' field, which could then result in a nil pointer exception.
The update here just adds the appropriate checks to the loop.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
The previous updates for start/stop intervals only updated the tty
display. Now, corresponding changes have been made to the plain progress
printer too.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
