-
Notifications
You must be signed in to change notification settings - Fork 1.4k
frontend: add required paths to LLB and use it with --parents #6229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6b635b4 to
9c72837
Compare
d03c28e to
8b24130
Compare
8b24130 to
3d8ef37
Compare
3d8ef37 to
f616837
Compare
|
Need to fix the tests but I think this method of fixing it is probably the easiest. |
9e6f6ef to
2f4d2b1
Compare
0bb04e7 to
dba28b3
Compare
dba28b3 to
03ea339
Compare
| Wildcard bool | ||
| IncludePatterns []string | ||
| ExcludePatterns []string | ||
| RequiredPaths []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why this needs to take paths, instead of just being boolean? If it is boolean then I guess if no paths match then it returns a constant "zero checksum" that would be possible to check on the caller side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little fuzzy on why I went this direction exactly compared to the boolean, but I think there were two reasons.
- The error message that gave the path was really difficult to figure out without putting the paths explicitly.
- Wildcards complicated the boolean method because the correct prefix wasn't necessarily trivial to determine from the checksum path.
Adding the required paths as an explicit argument took away the guesswork involved with this. The main problem is we needed information both about the pivot point and the non-wildcard section of the pattern to determine which path we needed so it wasn't as simple as just saying "if things are empty it's invalid" because the directory being empty could still be valid.
This adds an additional `RequiredPaths` that is primarily intended for use with `COPY --parents`. This parameter specifies expected directories or files that should exist when performing the checksum. A not found error will be produced if one of these paths is missing. This fixes an issue with `COPY --parents` where a non existent directory that was intended to be copied would be ignored. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
03ea339 to
ac10b41
Compare
This adds an additional
RequiredPathsthat is primarily intended foruse with
COPY --parents. This parameter specifies expected directoriesor files that should exist when performing the checksum. A not found
error will be produced if one of these paths is missing.
This fixes an issue with
COPY --parentswhere a non existent directorythat was intended to be copied would be ignored.
Fixes #4900.