While it would make sense to add "from file" to complement "from env,"
(in the common case of `--file` or using the default), it wouldn't
provide any real value.
A simpler solution would have been looking for the existence of the
variable at the point where printing happens. It felt wrong
duplicating the logic. Executing the same logic (if it was extracted)
wouldn't be as bad, but still not ideal.
A 'correct' solution would be to explicitly track the source of each
definition, which would be clearer and more future-proof. It didn't
seem like this feature warranted that amount of engineering (with no
known features that might make use of it).
This implementation seemed like a fair compromise; none of the functions
are exported, and all have only one caller.
I also considered converting prefixing environment values with `env://`
so they could be thought of (and processed like) `cmd://` values. I
didn't think it would be viewed as a good solution.
Co-authored-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
Some minor fixes to the printer and how bake invokes it. Bake previously
had a race condition that could result in the display not updating on an
error condition, but it was much rarer because the channel communication
was much closer. The refactor added a proxy for the status channel so
there was more of an opportunity to surface the race condition.
When bake exits with an error when reading the bakefiles, it doesn't
wait for the printer to finish so it is possible for the printer to
update the display after an error is printed. This adds an extra `Wait`
in a defer to make sure the printer is finished.
`Wait` has also been fixed to allow it to be called multiple times and
have the same behavior. Previously, it only waited for the done channel
once so only the first wait would block.
The `onclose` method is now called every time the display is paused or
stopped. That was the previous behavior and it's been restored here.
The display only gets refreshed if we aren't exiting. There's no point
in initializing another display if we're about to exit.
The metric writer attached to the printer was erroneously removed. It is
now assigned properly.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The environment variable `BUILDX_BAKE_FILE` (and optional variable
`BUILDX_BAKE_FILE_SEPARATOR`) can be used to specify one or more bake
files (similar to `compose`). This is mutually exclusive with`--file`
(which takes precedence).
This is done very early to ensure the values are treated just like
`--file`, e.g., participate in telemetry. This includes leaving
relative paths as-is, which deviates from `compose` (which makes them
absolute).
Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
The package just causes the entire flow to be more complicated as build
has to pretend it doesn't know about debug options and the debugger has
to pretend it doesn't know about the build.
This abstraction has been difficult when integrating a DAP command into
this same workflow so I don't think this abstraction has much of a
value.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This changes the build handler to customize the behavior of evaluate
rather than onresult and also simplifies the `ResultHandle`. The
`ResultHandle` is now only valid within the gateway callback and can be
used to start containers from the handler.
`Evaluate` now executes inside of the gateway callback rather than
having a separate implementation that executes or re-invokes the build.
This keeps the gateway callback session open until the debugger has
returned.
The `ErrReload` for monitor has now been moved into the `build` package
and been renamed to `ErrRestart`. This is because it restarts the build
so the name makes a bit more sense. The actual use of this functionality
is still tied to the monitor reload.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Removes all references to the controller and moves the remaining
sections of code to other packages.
Processes has been moved to monitor where it is used and the data
structs have been removed so buildflags is used directly. The controller
build function has been moved to the commands package.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This creates a `Monitor` type that keeps the global state between
monitor invocations and allows the monitor to exist during the build so
it can be utilized for callbacks.
The result handler is now registered with the monitor during the build
and `Run` will use the result if it is present and the configuration
intends the monitor to be invoked with the given result.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The build now happens in a loop and the monitor is run after every
build. The monitor can return `ErrReload` to signal to the main thread
that it should reload the build result.
This will be used in the future to move the monitor into a callback
rather than as a separate existence. It allows the monitor to not
control the build itself which now makes it possible to completely
remove the controller.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The monitor needs stdin to run and isn't compatible with loading a
context or dockerfile from stdin. We already disallow this combination
and, with the removal of the remote controller, there's no way to use
stdin during the build when invoke is configured.
This just removes the extra code to allow forwarding stdin to the build
when the monitor is configured to simplify that section of code.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The controller interface is removed and the local controller is used for
only the initial build, invoke, and rebuilds.
Process control has been moved to the monitor.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The `--keep-storage` flag was changed to `--reserved-space`. Before it was
changed to that name, it was changed to `--max-storage`. This flag never
made it into a release as the name was changed before release, but the
update to the flag in buildx forgot to update the deprecation notice.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
If a type was explicitly provided, it will be displayed in the variable
listing. Inferred type names are not displayed, as they likely would
not match the user's intent.
Previously only `string` and `bool` default values were displayed in the
listing. All default values, regardless of type, are now displayed.
Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
While lsBuilder has a field called Current that gets lost
because the embedded struct implements custom MarshalJSON method.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Remove the protobuf files associated with controller/errdefs.
This doesn't completely remove the type as the monitor still uses it as
a signal to start the monitor.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Remove the controller grpc service along with associated code related to
sessions or remote controllers.
Data types that are still used with complicated dependency chains have
been kept in the same package for a future refactor.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Allow builds to be exported into .dockerbuild bundles
that can be shared and imported into Docker Desktop.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
User might pass a value that they don't expect to
be kept in trace storage. For example some cache backends
allow passing authentication tokens with a flag.
Instead use known primary config values as attributes
of the root span.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Some commands (logs/open) were still missing offset handling.
Now all commands use the same reference parsing/sort.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
