Merge pull request #1407 from afbjorklund/download-description

AkihiroSuda · web-flow · commit 22aa98e74aed · 2023-03-19T22:56:27.000+09:00
Show a user-readable description of the download
diff --git a/pkg/downloader/downloader.go b/pkg/downloader/downloader.go
@@ -38,6 +38,7 @@ type Result struct {
 
 type options struct {
 	cacheDir       string // default: empty (disables caching)
+	description    string // default: url
 	expectedDigest digest.Digest
 }
 
@@ -64,6 +65,14 @@ func WithCacheDir(cacheDir string) Opt {
 	}
 }
 
+// WithDecription adds a user description of the download.
+func WithDescription(description string) Opt {
+	return func(o *options) error {
+		o.description = description
+		return nil
+	}
+}
+
 // WithExpectedDigest is used to validate the downloaded file against the expected digest.
 //
 // The digest is not verified in the following cases:
@@ -134,7 +143,7 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {
 	}
 
 	if IsLocal(remote) {
-		if err := copyLocal(localPath, remote, o.expectedDigest); err != nil {
+		if err := copyLocal(localPath, remote, o.description, o.expectedDigest); err != nil {
 			return nil, err
 		}
 		res := &Result{
@@ -145,7 +154,7 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {
 	}
 
 	if o.cacheDir == "" {
-		if err := downloadHTTP(localPath, remote, o.expectedDigest); err != nil {
+		if err := downloadHTTP(localPath, remote, o.description, o.expectedDigest); err != nil {
 			return nil, err
 		}
 		res := &Result{
@@ -174,11 +183,11 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {
 			if o.expectedDigest.String() != shadDigestS {
 				return nil, fmt.Errorf("expected digest %q does not match the cached digest %q", o.expectedDigest.String(), shadDigestS)
 			}
-			if err := copyLocal(localPath, shadData, ""); err != nil {
+			if err := copyLocal(localPath, shadData, "", ""); err != nil {
 				return nil, err
 			}
 		} else {
-			if err := copyLocal(localPath, shadData, o.expectedDigest); err != nil {
+			if err := copyLocal(localPath, shadData, o.description, o.expectedDigest); err != nil {
 				return nil, err
 			}
 		}
@@ -199,11 +208,11 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {
 	if err := os.WriteFile(shadURL, []byte(remote), 0644); err != nil {
 		return nil, err
 	}
-	if err := downloadHTTP(shadData, remote, o.expectedDigest); err != nil {
+	if err := downloadHTTP(shadData, remote, o.description, o.expectedDigest); err != nil {
 		return nil, err
 	}
 	// no need to pass the digest to copyLocal(), as we already verified the digest
-	if err := copyLocal(localPath, shadData, ""); err != nil {
+	if err := copyLocal(localPath, shadData, "", ""); err != nil {
 		return nil, err
 	}
 	if shadDigest != "" && o.expectedDigest != "" {
@@ -244,7 +253,7 @@ func canonicalLocalPath(s string) (string, error) {
 	return localpathutil.Expand(s)
 }
 
-func copyLocal(dst, src string, expectedDigest digest.Digest) error {
+func copyLocal(dst, src string, description string, expectedDigest digest.Digest) error {
 	srcPath, err := canonicalLocalPath(src)
 	if err != nil {
 		return err
@@ -262,6 +271,9 @@ func copyLocal(dst, src string, expectedDigest digest.Digest) error {
 	if err != nil {
 		return err
 	}
+	if description != "" {
+		// TODO: progress bar for copy
+	}
 	return fs.CopyFile(dstPath, srcPath)
 }
 
@@ -316,7 +328,7 @@ func createBar(size int64) (*pb.ProgressBar, error) {
 	return bar, nil
 }
 
-func downloadHTTP(localPath, url string, expectedDigest digest.Digest) error {
+func downloadHTTP(localPath, url string, description string, expectedDigest digest.Digest) error {
 	if localPath == "" {
 		return fmt.Errorf("downloadHTTP: got empty localPath")
 	}
@@ -357,6 +369,12 @@ func downloadHTTP(localPath, url string, expectedDigest digest.Digest) error {
 	}
 	multiWriter := io.MultiWriter(writers...)
 
+	if !HideProgress {
+		if description == "" {
+			description = url
+		}
+		fmt.Printf("Downloading %s\n", description)
+	}
 	bar.Start()
 	if _, err := io.Copy(multiWriter, bar.NewProxyReader(resp.Body)); err != nil {
 		return err
diff --git a/pkg/fileutils/download.go b/pkg/fileutils/download.go
@@ -2,23 +2,27 @@ package fileutils
 
 import (
 	"fmt"
+	"path"
 
 	"github.com/lima-vm/lima/pkg/downloader"
 	"github.com/lima-vm/lima/pkg/limayaml"
 	"github.com/sirupsen/logrus"
 )
 
-func DownloadFile(dest string, f limayaml.File, description string, expectedArch limayaml.Arch) error {
+// DownloadFile downloads a file to the cache, optionally copying it to the destination. Returns path in cache.
+func DownloadFile(dest string, f limayaml.File, description string, expectedArch limayaml.Arch) (string, error) {
 	if f.Arch != expectedArch {
-		return fmt.Errorf("unsupported arch: %q", f.Arch)
+		return "", fmt.Errorf("unsupported arch: %q", f.Arch)
 	}
-	logrus.WithField("digest", f.Digest).Infof("Attempting to download %s from %q", description, f.Location)
+	fields := logrus.Fields{"location": f.Location, "arch": f.Arch, "digest": f.Digest}
+	logrus.WithFields(fields).Infof("Attempting to download %s", description)
 	res, err := downloader.Download(dest, f.Location,
 		downloader.WithCache(),
+		downloader.WithDescription(fmt.Sprintf("%s (%s)", description, path.Base(f.Location))),
 		downloader.WithExpectedDigest(f.Digest),
 	)
 	if err != nil {
-		return fmt.Errorf("failed to download %q: %w", f.Location, err)
+		return "", fmt.Errorf("failed to download %q: %w", f.Location, err)
 	}
 	logrus.Debugf("res.ValidatedDigest=%v", res.ValidatedDigest)
 	switch res.Status {
@@ -29,5 +33,5 @@ func DownloadFile(dest string, f limayaml.File, description string, expectedArch
 	default:
 		logrus.Warnf("Unexpected result from downloader.Download(): %+v", res)
 	}
-	return nil
+	return res.CachePath, nil
 }
diff --git a/pkg/qemu/qemu.go b/pkg/qemu/qemu.go
@@ -50,12 +50,12 @@ func EnsureDisk(cfg Config) error {
 		var ensuredBaseDisk bool
 		errs := make([]error, len(cfg.LimaYAML.Images))
 		for i, f := range cfg.LimaYAML.Images {
-			if err := fileutils.DownloadFile(baseDisk, f.File, "the image", *cfg.LimaYAML.Arch); err != nil {
+			if _, err := fileutils.DownloadFile(baseDisk, f.File, "the image", *cfg.LimaYAML.Arch); err != nil {
 				errs[i] = err
 				continue
 			}
 			if f.Kernel != nil {
-				if err := fileutils.DownloadFile(kernel, f.Kernel.File, "the kernel", *cfg.LimaYAML.Arch); err != nil {
+				if _, err := fileutils.DownloadFile(kernel, f.Kernel.File, "the kernel", *cfg.LimaYAML.Arch); err != nil {
 					errs[i] = err
 					continue
 				}
@@ -67,7 +67,7 @@ func EnsureDisk(cfg Config) error {
 				}
 			}
 			if f.Initrd != nil {
-				if err := fileutils.DownloadFile(initrd, *f.Initrd, "the initrd", *cfg.LimaYAML.Arch); err != nil {
+				if _, err := fileutils.DownloadFile(initrd, *f.Initrd, "the initrd", *cfg.LimaYAML.Arch); err != nil {
 					errs[i] = err
 					continue
 				}
diff --git a/pkg/start/start.go b/pkg/start/start.go
@@ -16,6 +16,7 @@ import (
 	"github.com/lima-vm/lima/pkg/driverutil"
 
 	"github.com/lima-vm/lima/pkg/downloader"
+	"github.com/lima-vm/lima/pkg/fileutils"
 	hostagentevents "github.com/lima-vm/lima/pkg/hostagent/events"
 	"github.com/lima-vm/lima/pkg/limayaml"
 	"github.com/lima-vm/lima/pkg/store"
@@ -37,33 +38,19 @@ func ensureNerdctlArchiveCache(y *limayaml.LimaYAML) (string, error) {
 	}
 
 	errs := make([]error, len(y.Containerd.Archives))
-	for i := range y.Containerd.Archives {
-		f := &y.Containerd.Archives[i]
-		if f.Arch != *y.Arch {
-			errs[i] = fmt.Errorf("unsupported arch: %q", f.Arch)
-			continue
-		}
-		logrus.WithField("digest", f.Digest).Infof("Attempting to download the nerdctl archive from %q", f.Location)
-		res, err := downloader.Download("", f.Location, downloader.WithCache(), downloader.WithExpectedDigest(f.Digest))
+	for i, f := range y.Containerd.Archives {
+		path, err := fileutils.DownloadFile("", f, "the nerdctl archive", *y.Arch)
 		if err != nil {
-			errs[i] = fmt.Errorf("failed to download %q: %w", f.Location, err)
+			errs[i] = err
 			continue
 		}
-		switch res.Status {
-		case downloader.StatusDownloaded:
-			logrus.Infof("Downloaded the nerdctl archive from %q", f.Location)
-		case downloader.StatusUsedCache:
-			logrus.Infof("Using cache %q", res.CachePath)
-		default:
-			logrus.Warnf("Unexpected result from downloader.Download(): %+v", res)
-		}
-		if res.CachePath == "" {
+		if path == "" {
 			if downloader.IsLocal(f.Location) {
 				return f.Location, nil
 			}
 			return "", fmt.Errorf("cache did not contain %q", f.Location)
 		}
-		return res.CachePath, nil
+		return path, nil
 	}
 
 	return "", fmt.Errorf("failed to download the nerdctl archive, attempted %d candidates, errors=%v",
diff --git a/pkg/vz/disk.go b/pkg/vz/disk.go
@@ -29,7 +29,7 @@ func EnsureDisk(driver *driver.BaseDriver) error {
 		var ensuredBaseDisk bool
 		errs := make([]error, len(driver.Yaml.Images))
 		for i, f := range driver.Yaml.Images {
-			if err := fileutils.DownloadFile(baseDisk, f.File, "the image", *driver.Yaml.Arch); err != nil {
+			if _, err := fileutils.DownloadFile(baseDisk, f.File, "the image", *driver.Yaml.Arch); err != nil {
 				errs[i] = err
 				continue
 			}

Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,7 @@ type Result struct {`
`38`	`38`
`39`	`39`	`type options struct {`
`40`	`40`	`cacheDir string // default: empty (disables caching)`
	`41`	`+ description string // default: url`
`41`	`42`	`expectedDigest digest.Digest`
`42`	`43`	`}`
`43`	`44`
`@@ -64,6 +65,14 @@ func WithCacheDir(cacheDir string) Opt {`
`64`	`65`	`}`
`65`	`66`	`}`
`66`	`67`
	`68`	`+// WithDecription adds a user description of the download.`
	`69`	`+func WithDescription(description string) Opt {`
	`70`	`+ return func(o *options) error {`
	`71`	`+ o.description = description`
	`72`	`+ return nil`
	`73`	`+ }`
	`74`	`+}`
	`75`	`+`
`67`	`76`	`// WithExpectedDigest is used to validate the downloaded file against the expected digest.`
`68`	`77`	`//`
`69`	`78`	`// The digest is not verified in the following cases:`
`@@ -134,7 +143,7 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {`
`134`	`143`	`}`
`135`	`144`
`136`	`145`	`if IsLocal(remote) {`
`137`		`- if err := copyLocal(localPath, remote, o.expectedDigest); err != nil {`
	`146`	`+ if err := copyLocal(localPath, remote, o.description, o.expectedDigest); err != nil {`
`138`	`147`	`return nil, err`
`139`	`148`	`}`
`140`	`149`	`res := &Result{`
`@@ -145,7 +154,7 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {`
`145`	`154`	`}`
`146`	`155`
`147`	`156`	`if o.cacheDir == "" {`
`148`		`- if err := downloadHTTP(localPath, remote, o.expectedDigest); err != nil {`
	`157`	`+ if err := downloadHTTP(localPath, remote, o.description, o.expectedDigest); err != nil {`
`149`	`158`	`return nil, err`
`150`	`159`	`}`
`151`	`160`	`res := &Result{`
`@@ -174,11 +183,11 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {`
`174`	`183`	`if o.expectedDigest.String() != shadDigestS {`
`175`	`184`	`return nil, fmt.Errorf("expected digest %q does not match the cached digest %q", o.expectedDigest.String(), shadDigestS)`
`176`	`185`	`}`
`177`		`- if err := copyLocal(localPath, shadData, ""); err != nil {`
	`186`	`+ if err := copyLocal(localPath, shadData, "", ""); err != nil {`
`178`	`187`	`return nil, err`
`179`	`188`	`}`
`180`	`189`	`} else {`
`181`		`- if err := copyLocal(localPath, shadData, o.expectedDigest); err != nil {`
	`190`	`+ if err := copyLocal(localPath, shadData, o.description, o.expectedDigest); err != nil {`
`182`	`191`	`return nil, err`
`183`	`192`	`}`
`184`	`193`	`}`
`@@ -199,11 +208,11 @@ func Download(local, remote string, opts ...Opt) (*Result, error) {`
`199`	`208`	`if err := os.WriteFile(shadURL, []byte(remote), 0644); err != nil {`
`200`	`209`	`return nil, err`
`201`	`210`	`}`
`202`		`- if err := downloadHTTP(shadData, remote, o.expectedDigest); err != nil {`
	`211`	`+ if err := downloadHTTP(shadData, remote, o.description, o.expectedDigest); err != nil {`
`203`	`212`	`return nil, err`
`204`	`213`	`}`
`205`	`214`	`// no need to pass the digest to copyLocal(), as we already verified the digest`
`206`		`- if err := copyLocal(localPath, shadData, ""); err != nil {`
	`215`	`+ if err := copyLocal(localPath, shadData, "", ""); err != nil {`
`207`	`216`	`return nil, err`
`208`	`217`	`}`
`209`	`218`	`if shadDigest != "" && o.expectedDigest != "" {`
`@@ -244,7 +253,7 @@ func canonicalLocalPath(s string) (string, error) {`
`244`	`253`	`return localpathutil.Expand(s)`
`245`	`254`	`}`
`246`	`255`
`247`		`-func copyLocal(dst, src string, expectedDigest digest.Digest) error {`
	`256`	`+func copyLocal(dst, src string, description string, expectedDigest digest.Digest) error {`
`248`	`257`	`srcPath, err := canonicalLocalPath(src)`
`249`	`258`	`if err != nil {`
`250`	`259`	`return err`
`@@ -262,6 +271,9 @@ func copyLocal(dst, src string, expectedDigest digest.Digest) error {`
`262`	`271`	`if err != nil {`
`263`	`272`	`return err`
`264`	`273`	`}`
	`274`	`+ if description != "" {`
	`275`	`+ // TODO: progress bar for copy`
	`276`	`+ }`
`265`	`277`	`return fs.CopyFile(dstPath, srcPath)`
`266`	`278`	`}`
`267`	`279`
`@@ -316,7 +328,7 @@ func createBar(size int64) (*pb.ProgressBar, error) {`
`316`	`328`	`return bar, nil`
`317`	`329`	`}`
`318`	`330`
`319`		`-func downloadHTTP(localPath, url string, expectedDigest digest.Digest) error {`
	`331`	`+func downloadHTTP(localPath, url string, description string, expectedDigest digest.Digest) error {`
`320`	`332`	`if localPath == "" {`
`321`	`333`	`return fmt.Errorf("downloadHTTP: got empty localPath")`
`322`	`334`	`}`
`@@ -357,6 +369,12 @@ func downloadHTTP(localPath, url string, expectedDigest digest.Digest) error {`
`357`	`369`	`}`
`358`	`370`	`multiWriter := io.MultiWriter(writers...)`
`359`	`371`
	`372`	`+ if !HideProgress {`
	`373`	`+ if description == "" {`
	`374`	`+ description = url`
	`375`	`+ }`
	`376`	`+ fmt.Printf("Downloading %s\n", description)`
	`377`	`+ }`
`360`	`378`	`bar.Start()`
`361`	`379`	`if _, err := io.Copy(multiWriter, bar.NewProxyReader(resp.Body)); err != nil {`
`362`	`380`	`return err`
Original file line number	Diff line number	Diff line change
`@@ -50,12 +50,12 @@ func EnsureDisk(cfg Config) error {`
`50`	`50`	`var ensuredBaseDisk bool`
`51`	`51`	`errs := make([]error, len(cfg.LimaYAML.Images))`
`52`	`52`	`for i, f := range cfg.LimaYAML.Images {`
`53`		`- if err := fileutils.DownloadFile(baseDisk, f.File, "the image", *cfg.LimaYAML.Arch); err != nil {`
	`53`	`+ if _, err := fileutils.DownloadFile(baseDisk, f.File, "the image", *cfg.LimaYAML.Arch); err != nil {`
`54`	`54`	`errs[i] = err`
`55`	`55`	`continue`
`56`	`56`	`}`
`57`	`57`	`if f.Kernel != nil {`
`58`		`- if err := fileutils.DownloadFile(kernel, f.Kernel.File, "the kernel", *cfg.LimaYAML.Arch); err != nil {`
	`58`	`+ if _, err := fileutils.DownloadFile(kernel, f.Kernel.File, "the kernel", *cfg.LimaYAML.Arch); err != nil {`
`59`	`59`	`errs[i] = err`
`60`	`60`	`continue`
`61`	`61`	`}`
`@@ -67,7 +67,7 @@ func EnsureDisk(cfg Config) error {`
`67`	`67`	`}`
`68`	`68`	`}`
`69`	`69`	`if f.Initrd != nil {`
`70`		`- if err := fileutils.DownloadFile(initrd, f.Initrd, "the initrd", cfg.LimaYAML.Arch); err != nil {`
	`70`	`+ if _, err := fileutils.DownloadFile(initrd, f.Initrd, "the initrd", cfg.LimaYAML.Arch); err != nil {`
`71`	`71`	`errs[i] = err`
`72`	`72`	`continue`
`73`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ func EnsureDisk(driver *driver.BaseDriver) error {`
`29`	`29`	`var ensuredBaseDisk bool`
`30`	`30`	`errs := make([]error, len(driver.Yaml.Images))`
`31`	`31`	`for i, f := range driver.Yaml.Images {`
`32`		`- if err := fileutils.DownloadFile(baseDisk, f.File, "the image", *driver.Yaml.Arch); err != nil {`
	`32`	`+ if _, err := fileutils.DownloadFile(baseDisk, f.File, "the image", *driver.Yaml.Arch); err != nil {`
`33`	`33`	`errs[i] = err`
`34`	`34`	`continue`
`35`	`35`	`}`