-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix OCI format, GZIP files can be smaller than 1024 bytes #511
base: main
Are you sure you want to change the base?
Conversation
Tested locally on my linux-amd64 with command: |
@wagoodman can you merge this? I'm unable to use dive lately due to this bug |
// Not a gzipped entry | ||
unwrappedReader = io.MultiReader(bytes.NewReader(buffer[:n]), tarReader) | ||
} | ||
// Try reading a GZIP |
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.
Fwiw, the comment above (line 97) about Docker not gzipping very small layers might have to be removed (unfortunately, GitHub doesn't let me put that comment on that line 🤷)
@wagoodman I'm also not able to use dive because of this bug, and I'm running a compiled version of @Maddog2050's Is there anything blocking the PR? It would be great if you / we could get it sorted and pushed. Thanks! |
Does anyone know if there is an easy way to ask homebrew to install @Maddog2050 's fork? |
Hmmm
Well that was a deep rabbit hole. |
Not that I know of... I "just" did: git clone https://github.com/Maddog2050/dive.git
git co fix-oci-format
make bootstrap
make build Then, I Hopefully this fix will get merged soon and we can just use the official binary! :) |
Fix works for me too :) I hope this gets merged soon! 🤞 |
Worked for me - this should get merged |
I'm running into this problem as well. Any reason the fix has been sitting around since it was approved back in March? |
Since Docker Desktop 4.34 https://docs.docker.com/desktop/release-notes/#4340
Can we merge it and release a new version? I checked this PR and it works fine. Thank you for this utility, @wagoodman ! |
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.
Fix works
Thx. 👍 |
work on latest Docker Desktop 4.35.1 on Mac M2! thanks~~ |
Fixes wagoodman#507 Fixes wagoodman#510 Fixes wagoodman#526 Fixes wagoodman#534 Co-authored-by: Maddog2050 <17902029+Maddog2050@users.noreply.github.com>
Fixes wagoodman#507 Fixes wagoodman#510 Fixes wagoodman#526 Fixes wagoodman#534 Co-authored-by: Maddog2050 <17902029+Maddog2050@users.noreply.github.com>
Fixes wagoodman#507 Fixes wagoodman#510 Fixes wagoodman#526 Fixes wagoodman#534 Co-authored-by: Maddog2050 <17902029+Maddog2050@users.noreply.github.com>
If anybody is interested, I built an image for this PR and pushed it here: https://github.com/users/solidDoWant/packages/container/dive/306957285?tag=v0.12.0-1-g5dd9ba6 |
The fix was exactly what we needed here, thanks! |
@wagoodman Hey, first of all thanks for this great tool. Unfortunately it stopped working for me due to this issue. Can you have a look at this PR and approve/merge/release a new version? Thanks in advance |
@pascal-hofmann unfortunately it seems that Dive is currently unmaintained. If you're looking for a release with this PR merged in, you can find one in Joschi's fork |
That's sad. Maybe we should create an org and try to maintain it there together? @joschi What do you think? |
@pascal-hofmann that is under consideration, you can find it here It does not seem many have responded, and Joschi does not want to actively maintain it himself either I'd have offered assistance myself, but I do not know Go, and couldn't even get the makefile working 😅 |
Here is a simple fix for the OCI format. When debugging I found that the GZIP size can be less than 1024 bytes when using containerd to pull the image.
I have tested with and without the "Use containerd for pulling and storing images" option in docker desktop and it appears to have fixed the issue. I also noticed issue #507 and downloading the example file it appears to resolve this issue also.
Fixes: #510 #507