dap: improve determination of the proper parent for certain ops #3366
Conversation
21fa920 to
c4eeb0a
Compare
dap/thread.go
Outdated
| candidates[int(action.Input)] = struct{}{} | ||
| } | ||
|
|
||
| if len(candidates) != 1 { |
There was a problem hiding this comment.
Multiple actions are possible, even in Dockerfile. For example COPY with multiple source paths. Non-negative input of first action should be ok.
There was a problem hiding this comment.
I figured it was best to be cautious when it came to the inputs. Is it possible for the multiple source paths in dockerfile to also have multiple different input indices?
There was a problem hiding this comment.
The way it works is that input keys in action point first to the input array of the op and after that is exhausted then then index is result of another action https://github.com/moby/buildkit/blob/master/solver/pb/ops.proto#L310. So if you have two copy actions after each other it is smth like
fileop {
inputs = [ A , B ]
actions = [
copy{input: 0, secondaryInput: 1},
copy{input: 2, secondaryInput: 1}
where the 2 means the result of the first action len(inputs)+0
There was a problem hiding this comment.
I see what you mean. While unlikely, I'm still concerned about what to do if there are multiple copies with inputs that are below index 2 in your example. Maybe I can just update this so it filters out any input index that references something within the action?
There was a problem hiding this comment.
I've updated this to just use the first suitable input from the list of file actions.
27a491d to
fbcf69b
Compare
Improves the determination of the proper parent for exec and file ops. With file ops, it will only consider inputs and ignore secondary inputs. This prevents the following case: ``` FROM busybox AS build1 RUN echo foo > /hello FROM scratch COPY --from=build1 /hello . ``` Previously, `build1` would be considered the parent of the copy instruction. Now, copy properly does not have a parent. If there are multiple file ops and the operations disagree on the canonical "parent", we give up on trying to find a canonical parent and assume there is none. For exec operations, whichever input is associated with the root mount is considered the primary parent. For all other operations, the first parent is considered the primary parent if it exists. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
fbcf69b to
5c97696
Compare
Improves the determination of the proper parent for exec and file ops.
With file ops, it will only consider inputs and ignore secondary inputs.
This prevents the following case:
Previously,
build1would be considered the parent of the copyinstruction. Now, copy properly does not have a parent.
If there are multiple file ops and the operations disagree on the
canonical "parent", we give up on trying to find a canonical parent and
assume there is none.
For exec operations, whichever input is associated with the root mount
is considered the primary parent.
For all other operations, the first parent is considered the primary
parent if it exists.