dap: support evaluate request to invoke a container #3257
Conversation
jsternberg
force-pushed
the
dap-invoke
branch
2 times, most recently
from
07d9e5a to
bac335d
Compare
| cobrautil.MarkCommandExperimental(cmd) | ||
|
|
||
| flags := cmd.Flags() | ||
| flags.StringVar(&options.OnFlag, "on", "error", "When to pause the adapter ([always, error])") |
There was a problem hiding this comment.
Isn't this ExceptionBreakMode sent via DAP protocol itself?
There was a problem hiding this comment.
Not currently implemented but yes ideally we would remove this and use ExceptionBreakMode to set the behavior.
commands/dap.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (d *adapterProtocolDebugger) Out() console.File { |
There was a problem hiding this comment.
Why is this returning console.File? I assume detecting the progress mode can be done outside without leaking this into dap layer.
There was a problem hiding this comment.
I think this is cited somewhere else in the PR but it's mostly because the NewPrinter takes a console.File even though it shouldn't. progressui.NewDisplay takes an io.Writer and creates a console.File from that so NewPrinter is just going console.File -> io.Writer -> console.File.
We can change NewPrinter to take an io.Writer and then this can change to an io.Writer.
| type ( | ||
| EvaluateFunc func(ctx context.Context, name string, c gateway.Client, res *gateway.Result) error | ||
| Handler struct { | ||
| Evaluate EvaluateFunc |
commands/build.go
Outdated
| @@ -1210,3 +1126,24 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *BuildOptions, inSt | |||
| } | |||
| return resp[defaultTargetName], inputs, nil | |||
| } | |||
|
|
|||
| // debuggerOptions will start a debuggerOptions instance. | |||
There was a problem hiding this comment.
Looks like this should be in commands/debug.
| name string | ||
|
|
||
| paused chan struct{} | ||
| rCtx *build.ResultHandle |
There was a problem hiding this comment.
follow-up: this ResultHandle seems incorrectly named. I'm not sure if we need it at all but if we do then it should be called something else and should not be defined in the build pkg, as it is basically the transformation of the Evaluate() parameters into struct form and therefore duplicate in the context of build pkg.
Ctx variable naming also makes it confusing as it looks like some form of context.Context.
There was a problem hiding this comment.
Agreed. It's mostly been kept around because the code exists and it was useful to reuse it. It can be renamed and probably should be renamed.
commands/dap.go
Outdated
|
|
||
| cobrautil.MarkFlagsExperimental(flags, "on") | ||
|
|
||
| cmd.AddCommand(buildCmd(dockerCli, rootOpts, &options)) |
There was a problem hiding this comment.
What's the use case of the build subcommand?
There was a problem hiding this comment.
The command is invoked with docker buildx dap build and the bake version will be docker buildx dap bake.
jsternberg
force-pushed
the
dap-invoke
branch
2 times, most recently
from
6ed539d to
eacb06c
Compare
Adds a simple implementation of the debug adapter that supports the very basics of a debug adapter. It supports the launch request, the configuration done request, the creation of threads, stopping, resuming, and disconnecting from server. It does not support custom breakpoints, stack traces, or variable inspection yet. These are planned to be added in the future. It also does not support sending output through the debug adapter yet. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Supports using the `evaluate` request in REPL mode to start a container with the `exec` command. Presently doesn't support any arguments. This improves the dap server so it is capable of sending reverse requests and receiving the response. It also adds a hidden command `dap attach` that attaches to the socket created by `evaluate`. This requires the client to support `runInTerminal`. Likely needs some additional work to make sure resources are cleaned up cleanly especially when the build is unpaused or terminated, but it should work as a decent base. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Supports using the
evaluaterequest in REPL mode to start a containerwith the
execcommand. Presently doesn't support any arguments.This improves the dap server so it is capable of sending reverse
requests and receiving the response. It also adds a hidden command
dap attachthat attaches to the socket created byevaluate.This requires the client to support
runInTerminal.Likely needs some additional work to make sure resources are cleaned up
cleanly especially when the build is unpaused or terminated, but it
should work as a decent base.