dap: add hint to copy when there is no parent #6117
Conversation
github-actions
bot
added
area/dockerfile
area/hack
jsternberg
force-pushed
the
dap-hint-noparent
branch
2 times, most recently
from
2354286 to
5428a47
Compare
jsternberg
force-pushed
the
dap-hint-noparent
branch
from
5428a47 to
f2502fb
Compare
| if d.state.Output() == nil { | ||
| // Give a hint to DAP that this copy has no real parent. | ||
| fileOpt = append(fileOpt, llb.WithDescription(map[string]string{ | ||
| "dap.hint.noparent": "true", |
There was a problem hiding this comment.
I think we should be generic for the key and use the same format as
buildkit/frontend/dockerfile/dockerfile2llb/convert.go
Lines 1950 to 1952 in ee216b9
Maybe:
| "dap.hint.noparent": "true", | |
| "com.docker.dockerfile.v1.copy.noparent": "true", |
There was a problem hiding this comment.
The main reason for the specific annotation was because I thought this might be a generic annotation for DAP that other frontends could implement. I like the naming here, but it also means it's very specific to dockerfile and the DAP adapter would have to be changed to look for it instead of the other way around.
Maybe com.docker.dap.v1.hint.noparent?
There was a problem hiding this comment.
I thought this might be a generic annotation for DAP that other frontends could implement.
...
I like the naming here, but it also means it's very specific to dockerfile
I think it's specific to the dockerfile frontend as it lies in its own package imo but I understand the intent.
hack/composefiles/compose.yaml
Outdated
There was a problem hiding this comment.
These changes don't look related
There was a problem hiding this comment.
Yea I was just fixing the compose file to remove a bunch of cruft at the same time. Do you want me to separate it into a different PR?
There was a problem hiding this comment.
Yeah if you can, I was just confused 😅
Also I found the collector quite useful in the compose file from last time I used it. Are there any issues with this stack?
There was a problem hiding this comment.
I'll separate that out into a separate PR. I think the current compose file is broken so it needs some work.
jsternberg
force-pushed
the
dap-hint-noparent
branch
from
f2502fb to
2e6c463
Compare
When a copy has no real parent, we add an annotation of the name `com.docker.dap.v1.hint.noparent=true` to the node to signal to DAP that the LLB node has no real parent so it doesn't follow the first index assuming that index is the direct ancestor. Signed-off-by: Jonathan A. Sternberg <[email protected]>
jsternberg
force-pushed
the
dap-hint-noparent
branch
from
2e6c463 to
6409aa0
Compare
|
This is unneeded since this can be done in a way that already exists. @tonistiigi pointed this out to me in a recent conversation so I'm going to close this. The proper solution to this is here. |
When a copy has no real parent, we add an annotation of the name
com.docker.dap.v1.hints.noparent=trueto the node to signal to DAP that the LLB nodehas no real parent so it doesn't follow the first index assuming that
index is the direct ancestor.