Merge pull request #21864 from l0rd/compress-refactoring-v5-plus-plus

Machine decompress.go refactoring follow-up

go.mod

@@ -41,7 +41,6 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hugelgupf/p9 v0.3.1-0.20230822151754-54f5c5530921
 	github.com/json-iterator/go v1.1.12
-	github.com/klauspost/compress v1.17.7
 	github.com/linuxkit/virtsock v0.0.0-20220523201153-1a23e78aa7a2
 	github.com/mattn/go-shellwords v1.0.12
 	github.com/mattn/go-sqlite3 v1.14.22
@@ -65,7 +64,6 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
 	github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635
-	github.com/ulikunitz/xz v0.5.11
 	github.com/vbauerster/mpb/v8 v8.7.2
 	github.com/vishvananda/netlink v1.2.1-beta.2
 	go.etcd.io/bbolt v1.3.9
@@ -149,6 +147,7 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/jinzhu/copier v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
+	github.com/klauspost/compress v1.17.7 // indirect
 	github.com/klauspost/cpuid/v2 v2.2.6 // indirect
 	github.com/klauspost/pgzip v1.2.6 // indirect
 	github.com/kr/fs v0.1.0 // indirect
@@ -198,6 +197,7 @@ require (
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
 	github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect
 	github.com/ugorji/go/codec v1.2.12 // indirect
+	github.com/ulikunitz/xz v0.5.11 // indirect
 	github.com/vbatts/tar-split v0.11.5 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
 	github.com/yusufpapurcu/wmi v1.2.4 // indirect

pkg/machine/compression/compression_test.go

@@ -112,6 +112,8 @@ func Test_Decompress(t *testing.T) {
 	}{
 		{name: "zip", args: args{src: "./testdata/sample.zip", dst: "./testdata/hellozip"}, want: "zip\n"},
 		{name: "zip with trailing zeros", args: args{src: "./testdata/sample-withzeros.zip", dst: "./testdata/hellozip-withzeros"}, want: "zip\n\x00\x00\x00\x00\x00\x00"},
+		{name: "xz", args: args{src: "./testdata/sample.xz", dst: "./testdata/helloxz"}, want: "xz\n"},
+		{name: "xz with trailing zeros", args: args{src: "./testdata/sample-withzeros.xz", dst: "./testdata/helloxz-withzeros"}, want: "xz\n\x00\x00\x00\x00\x00\x00\x00"},
 		{name: "gzip", args: args{src: "./testdata/sample.gz", dst: "./testdata/hellogz"}, want: "gzip\n"},
 		{name: "gzip with trailing zeros", args: args{src: "./testdata/sample-withzeros.gz", dst: "./testdata/hellogzip-withzeros"}, want: "gzip\n\x00\x00\x00\x00\x00"},
 		{name: "bzip2", args: args{src: "./testdata/sample.bz2", dst: "./testdata/hellobz2"}, want: "bzip2\n"},

pkg/machine/compression/decompress.go

@@ -4,7 +4,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 
 	"github.com/containers/podman/v5/pkg/machine/define"
@@ -46,24 +45,15 @@ func Decompress(compressedVMFile *define.VMFile, decompressedFilePath string) er
 
 func newDecompressor(compressedFilePath string, compressedFileMagicNum []byte) (decompressor, error) {
 	compressionType := archive.DetectCompression(compressedFileMagicNum)
-	os := runtime.GOOS
 	hasZipSuffix := strings.HasSuffix(compressedFilePath, zipExt)
 
 	switch {
-	case compressionType == archive.Xz:
-		return newXzDecompressor(compressedFilePath)
 	// Zip files are not guaranteed to have a magic number at the beginning
 	// of the file, so we need to use the file name to detect them.
 	case compressionType == archive.Uncompressed && hasZipSuffix:
 		return newZipDecompressor(compressedFilePath)
 	case compressionType == archive.Uncompressed:
 		return newUncompressedDecompressor(compressedFilePath)
-	// Using special compressors on MacOS because default ones
-	// in c/image/pkg/compression are slow with sparse files.
-	case compressionType == archive.Gzip && os == macOs:
-		return newGzipDecompressor(compressedFilePath)
-	case compressionType == archive.Zstd && os == macOs:
-		return newZstdDecompressor(compressedFilePath)
 	default:
 		return newGenericDecompressor(compressedFilePath)
 	}

pkg/machine/compression/generic.go

@@ -4,6 +4,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"runtime"
 
 	"github.com/containers/image/v5/pkg/compression"
 	"github.com/sirupsen/logrus"
@@ -54,7 +55,15 @@ func (d *genericDecompressor) decompress(w io.WriteSeeker, r io.Reader) error {
 		}
 	}()
 
-	_, err = io.Copy(w, decompressedFileReader)
+	// Use sparse-optimized copy for macOS as applehv,
+	// macOS native hypervisor, uses large raw VM disk
+	// files mostly empty (~2GB of content ~8GB empty).
+	if runtime.GOOS == macOs {
+		err = d.sparseOptimizedCopy(w, decompressedFileReader)
+	} else {
+		_, err = io.Copy(w, decompressedFileReader)
+	}
+
 	return err
 }
 

pkg/machine/compression/gzip.go

@@ -1,31 +0,0 @@
-package compression
-
-import (
-	"io"
-
-	image "github.com/containers/image/v5/pkg/compression"
-	"github.com/sirupsen/logrus"
-)
-
-type gzipDecompressor struct {
-	genericDecompressor
-}
-
-func newGzipDecompressor(compressedFilePath string) (*gzipDecompressor, error) {
-	d, err := newGenericDecompressor(compressedFilePath)
-	return &gzipDecompressor{*d}, err
-}
-
-func (d *gzipDecompressor) decompress(w io.WriteSeeker, r io.Reader) error {
-	gzReader, err := image.GzipDecompressor(r)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		if err := gzReader.Close(); err != nil {
-			logrus.Errorf("Unable to close gz file: %q", err)
-		}
-	}()
-
-	return d.sparseOptimizedCopy(w, gzReader)
-}

pkg/machine/compression/xz.go

@@ -1,66 +0,0 @@
-package compression
-
-import (
-	"bufio"
-	"io"
-	"os"
-	"os/exec"
-
-	"github.com/sirupsen/logrus"
-	"github.com/ulikunitz/xz"
-)
-
-type xzDecompressor struct {
-	genericDecompressor
-}
-
-func newXzDecompressor(compressedFilePath string) (*xzDecompressor, error) {
-	d, err := newGenericDecompressor(compressedFilePath)
-	return &xzDecompressor{*d}, err
-}
-
-// Will error out if file without .Xz already exists
-// Maybe extracting then renaming is a good idea here..
-// depends on Xz: not pre-installed on mac, so it becomes a brew dependency
-func (*xzDecompressor) decompress(w io.WriteSeeker, r io.Reader) error {
-	var cmd *exec.Cmd
-	var read io.Reader
-
-	// Prefer Xz utils for fastest performance, fallback to go xi2 impl
-	if _, err := exec.LookPath("xz"); err == nil {
-		cmd = exec.Command("xz", "-d", "-c")
-		cmd.Stdin = r
-		read, err = cmd.StdoutPipe()
-		if err != nil {
-			return err
-		}
-		cmd.Stderr = os.Stderr
-	} else {
-		// This XZ implementation is reliant on buffering. It is also 3x+ slower than XZ utils.
-		// Consider replacing with a faster implementation (e.g. xi2) if podman machine is
-		// updated with a larger image for the distribution base.
-		buf := bufio.NewReader(r)
-		read, err = xz.NewReader(buf)
-		if err != nil {
-			return err
-		}
-	}
-
-	done := make(chan bool)
-	go func() {
-		if _, err := io.Copy(w, read); err != nil {
-			logrus.Error(err)
-		}
-		done <- true
-	}()
-
-	if cmd != nil {
-		err := cmd.Start()
-		if err != nil {
-			return err
-		}
-		return cmd.Wait()
-	}
-	<-done
-	return nil
-}

pkg/machine/compression/zstd.go

@@ -1,26 +0,0 @@
-package compression
-
-import (
-	"io"
-
-	"github.com/klauspost/compress/zstd"
-)
-
-type zstdDecompressor struct {
-	genericDecompressor
-}
-
-func newZstdDecompressor(compressedFilePath string) (*zstdDecompressor, error) {
-	d, err := newGenericDecompressor(compressedFilePath)
-	return &zstdDecompressor{*d}, err
-}
-
-func (d *zstdDecompressor) decompress(w io.WriteSeeker, r io.Reader) error {
-	zstdReader, err := zstd.NewReader(r)
-	if err != nil {
-		return err
-	}
-	defer zstdReader.Close()
-
-	return d.sparseOptimizedCopy(w, zstdReader)
-}