Add support for zstd compression#249
Add support for zstd compression#249m90 merged 1 commit intooffen:mainfrom michalmiddleton:add_zstd_support
Conversation
m90
left a comment
There was a problem hiding this comment.
Thanks for working on this feature 🎩 I left a few comments inline, nothing major. Let me know what you think about my suggestions.
m90
left a comment
There was a problem hiding this comment.
Thanks for getting this done so fast again. I left another round of nit picky comments, the only one that is really interesting is the one about maybe having a custom type in the config that limits the number of possible compression algorithms. Let me know what you think.
cmd/backup/script.go
Outdated
| return nil, fmt.Errorf("newScript: unable to parse backup file extension template: %w", tErr) | ||
| } | ||
| var bf bytes.Buffer | ||
| if tErr := tmplFileName.Execute(&bf, extMap); tErr != nil { |
There was a problem hiding this comment.
It would also be possible to anonymously define the data being passed here
| if tErr := tmplFileName.Execute(&bf, extMap); tErr != nil { | |
| if tErr := tmplFileName.Execute(&bf, map[string]string{ | |
| "Extension": "tar."+s.c.BackupCompression | |
| }); tErr != nil { |
|
@m90 Thanks for being patient with me :). I pushed amended commit with the changes. |
m90
left a comment
There was a problem hiding this comment.
One more tiny remark, everything else looks fabulous and ready for the next release now.
Hi,
I took a stab at adding zstd support in a way that keeps gzip as default to maintain backward compatibility:
tar.gzortar.zstdepending on compressioncli-zstdPlease let me know if you require any additional changes.
Real world test:
Extract to validate consistency:
(The uncompressed tar was about 17GB in size and zstd compressed archive was about 7GB!)