monitor: breakpoint debugger on monitor and on IDEs (via DAP) #1656
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ktock
force-pushed
the
monitor-continue-r-r
branch
3 times, most recently
from
2f1e4ad to
94aabcf
Compare
Collaborator
Author
|
CI passed about the codes. |
Member
https://github.com/docker/buildx/actions/runs/4299321437/jobs/7494431087#step:7:38 @ktock Yes looks linked to the new internal command you just added. Can you add an extra commit replacing buildx/hack/dockerfiles/docs.Dockerfile Line 25 in 5e685c0 with |
ktock
force-pushed
the
monitor-continue-r-r
branch
2 times, most recently
from
3e50a48 to
35cab95
Compare
Collaborator
Author
|
Thank you for the comment! but it looks still failing with that change: |
ktock
force-pushed
the
monitor-continue-r-r
branch
from
35cab95 to
01f0a3d
Compare
Member
Thanks I will take a look |
ktock
force-pushed
the
monitor-continue-r-r
branch
from
01f0a3d to
65c5759
Compare
ktock
force-pushed
the
monitor-continue-r-r
branch
2 times, most recently
from
1b2034f to
648e37c
Compare
Member
|
@ktock I try to find the root cause for the docs-upstream issue. I keep you posted when I have smth. Edit: looks good now |
crazy-max
force-pushed
the
monitor-continue-r-r
branch
2 times, most recently
from
1c91613 to
dfbdef7
Compare
Collaborator
Author
|
@crazy-max Thank you for fixing it. |
Collaborator
|
See #1640 (comment) 👀 I know there's a lot of vendor changes here as well, but there's still a lot going on 🎉 I think it might even be worth splitting out the DAP into a separate PR (though maybe once #1640 is merged, to prevent too much PR chaining), it feels like a separate feature, with it's own complexities to me. |
Collaborator
|
What's the intended way to use the debugger? I am starting it using When using I also don't see |
ktock
force-pushed
the
monitor-continue-r-r
branch
2 times, most recently
from
371f2bb to
2c5432b
Compare
Collaborator
Author
|
@jedevc Thank you for trying this patch.
Could you use
Could you post the reproducer? |
Collaborator
|
Hm, so I can reproduce by taking a fresh clone of buildkit and then modifying the last part of the diff --git a/Dockerfile b/Dockerfile
index b489426d4..b11637145 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -302,3 +302,4 @@ ENTRYPOINT ["rootlesskit", "buildkitd"]
# buildkit builds the buildkit container image
FROM buildkit-$TARGETOS AS buildkit
+RUN skdfjalsdkfjThen I can build with |
ktock
force-pushed
the
monitor-continue-r-r
branch
from
2c5432b to
b72d7d5
Compare
Collaborator
Author
|
Thank you for the reproducer. Fixed the patch not to panic. |
ktock
force-pushed
the
monitor-continue-r-r
branch
from
b72d7d5 to
ebb58d4
Compare
ktock
force-pushed
the
monitor-continue-r-r
branch
from
ebb58d4 to
f64a39a
Compare
ktock
force-pushed
the
monitor-continue-r-r
branch
4 times, most recently
from
c8bc942 to
2fed3fe
Compare
Collaborator
Author
|
Rebased. |
ktock
force-pushed
the
monitor-continue-r-r
branch
from
2fed3fe to
1afe466
Compare
This commit adds BuildOptions.Debug that allows skipping the evaluation of the ResultHandler. The ResultHandler is created without evaluated, so calling Build API with this flag always returns an error with the reference ID of that (errored) ResultHandler. Note that this state of ResultHandler doesn't contain result nor error so any operation on that ResultHandler will result in an error. Following commit will allow user to do further operations (e.g. evaluation of a bulid) using the buildkit client held by that ResultHandler. Signed-off-by: Kohei Tokunaga <[email protected]>
This commit adds Solve API to the controller. This receives a build definition, performs that build using ResultHandler held by that session. After Solve completes, the client can debug the result using other APIs including Invoke. Note that the ResultHandle provided by Solve overwrites the ResultHandle previously stored in that session (possibly generated by the past Build or Solve API call). Using this API, user can perform build-and-debugging loop on the same session. Signed-off-by: Kohei Tokunaga <[email protected]>
This commit adds two fields to the response of Inspect API. - Definition: Build definition of the first build executed by Build API. - CurrentDefinition: Build definition of the latest build executed by Build or Solve API. The client can use these information for debugging the build deeply. Signed-off-by: Kohei Tokunaga <[email protected]>
Walker is an utility for inspecting and debugging each vertex in an LLB graph. Signed-off-by: Kohei Tokunaga <[email protected]>
This commit adds a set of commands to monitor for enabling breakpoint debugger. This is implemented based on the walker utility for step-by-step LLB inspection. For each vertex and breakpoint, monitor calls Solve API so the user can enter to the debugger container on each vertex for inspection. User can enter to the breakpoint debugger mode by --invoke=debug-step flag. Signed-off-by: Kohei Tokunaga <[email protected]>
This commit adds the IDE-integrated breakpoint debugger based on walker. Now buildx provides DAP (Debug Adapter Protocol) API to IDEs so DAP-aware IDEs can call buildx and allow users to perform breakpoint-based debugging on the IDE's UI/UX. Signed-off-by: Kohei Tokunaga <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: Kohei Tokunaga <[email protected]>
ktock
force-pushed
the
monitor-continue-r-r
branch
from
1afe466 to
335acca
Compare
Collaborator
Author
|
rebased |
Member
|
@ktock Do you think walking the definitionOp from build result is enough for all the cases we would want to support in the DAP debugger or would we need to add some of the functionality directly to frontends. If not making the frontends understand DAP protocol itself then would it help to add some requests there that help buildx do the correct things on client side. We have the subrequests support in frontends so theoretically this can be added in backward compatible manner. I also wonder what to do with frontend that chain multiple long builds (evaluate in the middle). With the current structure, we can only debug the result of the final solve request that was returned. Another case would be when for example, I have Dockerfile with 2 stages. I add a breakpoint to the beginning of first stage but I have an error in the base image of the second stage. Now my build will fail before it ever gets to debugging although visually this was later in my code and I only want to debug the first stage atm. before getting to the second one. But because images need to be resolved before Is there a good way we could also show the LLB operations that do not map directly to the source code but still participate in the execution path? Maybe a "fake" source file to LLB definition (like gdb shows machine code if no source I believe). What should the "call stack" mean in our case? Should we put the whole LLB parent chain there? I guess that we have multiple inputs for steps is problematic for this data structure as well. I haven't fully investigated what are all the capabilities in DAP (or vscode extension). Is there a point in trying to combine the "Buildx Interactive Debugger" command and monitor mode commands or are they completely different?
What do you mean by "other features" in here. Generally, I agree on taking the extension under buildx umbrella, but we can do that after the quick prototype phase. |
Collaborator
Author
So far I haven't encountered this kind of problems for Dockerfile debugging but if we want to handle this case (e.g. for non-Dockerfile sources?), then I think walker needs to be implemented in the frontend. To make DAP server work on that frontend, it needs to provide breakpoint-related APIs like implemented by the current
SGTM.
I'm not sure LLB has "call stack" as it doesn't have stack nor call instruction. Some frontends (e.g. HLB) seem to have their own "function" feature so maybe these source languages have their own definition of call stack?
I've added the interactive debugger to monitor because both of them are debugging-related features. But having the interactive debugger as a separated CLI/subcommand sounds also ok to me.
I don't have a concrete proposal for this as of now but what this patch doesn't cover is LSP-based extensions. (But there are already LSP extensions for Dockerfile in community so I'm not sure if there are unique features we can provide.) |
Member
I was thinking something like:
I guess in this blocked mode all solves should be serial as well and not run in parallel.
Technically every stage in Dockerfile could be a separate debug thread and all serial commands in a stage could be part of the stack. Maybe this is too much information though. Otoh step-over should go next line in Dockerfile, step-in could go to the first command of the inner stage for commands like
Are these directly related to debugger? Some examples what you think in here? Question: in a classical debugger, breakpoint in a line runs before that line runs. In Dockerfile I think both cases could make sense. Eg. if I have a line like |
Collaborator
|
We're currently developing an alternative implementation of this that does not utilize the now removed controller API. This PR was very helpful as a prototype and your help on reviews for the PRs has been invaluable. Thank you. I'm going to close this PR since it is now superseded by these other implementations. #3235 It might be helpful to keep the original branch around for a little bit in case it would be helpful for comparison. There are some implementations like |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related: #1104
This commit adds an interactive breakpoint debugger on monitor.
This adds a
Solvecontroller API for requesting building a specific vertex.Please see each commit's commit message for details.
Monitor walks the LLB graph with support for breakpoints and can exec into each step's container using Solve and Invoke API. Monitor provides DAP (Debug Adapter Protocol) so that IDE's debugger can use the breakpoints on their rich UI. DAP implementation is mostly inheited from buildg.
Debugger on the monitor
showcommand shows the source file with breakpoint information.continuestarts the build.=>shows the position where the build is pausing.breakcommand sets a breakpointThe position of the breakpoint is shown by
*on theshowcommand output.continueresumes the build and stops when it encounters a breakpoint.C-a-cswitches IO to the shell.show: shows the Dockerfilebreak: set a breakpoint at the specified linebreakpoints: list key-value pairs of available breakpointsclear: clear the breakpoint specified by the keyclearall: clear all breakpointsnext: proceed to the next linecontinue: resume the build until the next breakpointDebugger on IDEs
monitor provides DAP (Debug Adapter Protocol) and allows controlling the monitor on IDEs.
Editors can use buildx monitor via that standard protocol.
Please see
docs/guides/breakpoints.mdfor details about configuration.vscode-buildx-debuggerextension can be used https://github.com/ktock/vscode-buildx-debugger