Skip to content
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

Make local cache importer non-lazy #3493

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Conversation

ktock
Copy link
Collaborator

@ktock ktock commented Jan 11, 2023

Another approach for solving #3229 and docker/buildx#1325

Quote from #3229:

As reported in docker/buildx#1325, there seems a case where a build result with lazy local cache fails to unlazied on the following (or concurrent) build assigned the different session.

The difference between this and #3229 is that this PR fixes that issue by making the local cache importer non-lazy so this is simpler implementation than #3229 . As soon as the cache record is created, it is unlazied using the session provided from the importer.

Signed-off-by: Kohei Tokunaga <[email protected]>
@jedevc
Copy link
Member

jedevc commented Jan 17, 2023

@tonistiigi I think we discussed possibly adding this same fix into the oci-layout:// code? I think it suffers from the same problem here, but can't quite see where that needs to plug-in.

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the behavior with docker/buildx#1325 repro and LGTM thanks @ktock

@sipsma PTAL if you have time 🙏

@kevingentile
Copy link

Does this issue impact v0.10.x @ktock?

@ktock
Copy link
Collaborator Author

ktock commented Apr 1, 2023

EDIT:

I'm not sure docker/buildx#1325 happens on v0.10 as well. Please post an issue if you see it.
This patch isn't backported to v0.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants