-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Don't allow duplicate cache exporters #3271
Conversation
I would tend to agree so we are aligned for both import/export but I think we should keep the current behavior for cache imports so we don't break anyone. |
89f85dc
to
f0ff957
Compare
By semver, I think the next release would allow us to change some behavior in a non-backwards compatible way? IMO, it should be alright to error on behaviour that was already broken here. |
Yeah we are still major version zero so I guess it's fine but duplicate cache imports is still a "valid" pattern that BuildKit handles correctly whereas write operations when exporting would fail I think. |
f0ff957
to
2d7967c
Compare
Have added a patch to remove the duplicate cache imports - @tonistiigi WDYT? |
2d7967c
to
fe51108
Compare
In adding support for multiple cache exporters, the Export call was moved into a seperate function that merged that functionality with inline cache. This refactor splits the inline cache metadata generation up from the export, so we can put the exporting back directly into the Solve function. Signed-off-by: Justin Chadwell <me@jedevc.com>
Previously, we attempted to gracefully resolve duplicate cache export options, however, we should explicitly error - a client has made an error if the same options appear twice. If we really want de-duplication, then clients such as buildx should implement it there. Signed-off-by: Justin Chadwell <me@jedevc.com>
fe51108
to
621ab1c
Compare
⬆️ Follow-up to #3024.
Previously, we attempted to gracefully resolve duplicate cache export options, however, we should explicitly error - a client has made an error if the same options appear twice. If we really want de-duplication, then clients such as buildx should implement it there as discussed with @crazy-max and @tonistiigi.
Additionally, duplicate cache imports are modified to cause errors as well, for the same reasons as above.
cc @a-palchikov