-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
add support for xz compression #6523 #6534
Conversation
Thanks for the quick response! The new dep is missing from the Makefile, I applied this patch which allowed me to build and test. One thing I noticed is that while packer told me it was compressing the image with 8 core it only pegged one core to %100. |
Cool! I'll see if I can fix those things up! |
Looks like this particular xz implementation is single core only ulikunitz/xz#18 The pros of the above library is that it is is pure golang so we don't have any runtime deps. I also saw: https://github.com/danielrh/go-xz/ which seems to call into the C libs which gained multithreading around 2014 https://www.phoronix.com/forums/forum/software/general-linux-open-source/47329-xz-5-2-adds-new-multi-threaded-options The third option would be to write the code to shell out to the command line utilities, but nothing else does that from what I can tell in this compress post-processor. So this sounds like a trade off for the packer project to choose. I can fix up the printed message for now. |
Normally we use govendor to manage dependencies, rather than the makefile. So you'd go into your local copy of Packer, then run |
re: library choice: I think I'd rather single-threading and no runtime dependencies. |
Yep! I eventually figured this out in the contributing doc. I can remove the line from the Makefile.
Cool cool, sounds good. |
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.
Nice one, thanks !
I found just a tiny minor change and I think it's ready to merge.
@@ -273,6 +280,14 @@ func makeLZ4Writer(output io.WriteCloser, compressionLevel int) (io.WriteCloser, | |||
return lzwriter, nil | |||
} | |||
|
|||
func makeXZWriter(output io.WriteCloser, compressionLevel int) (io.WriteCloser, error) { |
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.
🌵here I'd use _ instead of compressionLevel since it's not used
Howdy @ChrisLundquist, And I forgot to test try it locally !
🙂 |
@azr don't think running |
Hey @rickard-von-essen, Yes, tried running a windows .iso but it crashed my machine. But I also tried it using
And for this try I was on the wrong branch :
Do you have an example of a real usage-case of compression ? |
I guess all of those above |
Yup, all empty, good catch, thanks. Trying with docker then 🙂 |
Okay, so I tried with docker and works; It creates an XZ file. The behaviour is the same as with gz but with XZ. |
Okay so it sounds like this is ready to merge after the one small change to the makeXZWriter function signature. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Closes #6523
Seems like it works, but I haven't used this before.
This pulls in a new dep, so I may need some help on how to pull that in.