From 3d3823225a2b32939574ec88b562b451e149bf32 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sat, 11 Mar 2023 13:44:58 -0600 Subject: [PATCH 1/2] Add --allow-missing-coverage --- cmd/bindown/cli.go | 41 +++++++++++--------- config.go | 95 ++++++++++++++++++++++++++++++++++++++-------- dependency.go | 40 ++++++++++++------- dependency_test.go | 2 +- 4 files changed, 131 insertions(+), 47 deletions(-) diff --git a/cmd/bindown/cli.go b/cmd/bindown/cli.go index 03d58a87..4bffc44b 100644 --- a/cmd/bindown/cli.go +++ b/cmd/bindown/cli.go @@ -35,6 +35,7 @@ var kongVars = kong.Vars{ "download_force_help": `force download even if the file already exists`, "download_target_file_help": `filename and path for the downloaded file. Default downloads to cache.`, "download_dependency_help": `name of the dependency to download`, + "allow_missing_checksum": `allow missing checksums`, "download_help": `download a dependency but don't extract or install it`, "extract_dependency_help": `name of the dependency to extract`, "extract_help": `download and extract a dependency but don't install it`, @@ -180,10 +181,11 @@ func (d validateCmd) Run(ctx context.Context) error { } type installCmd struct { - Force bool `kong:"help=${install_force_help}"` - Dependency string `kong:"required=true,arg,help=${download_dependency_help},predictor=bin"` - TargetFile string `kong:"type=path,name=output,type=file,help=${install_target_file_help}"` - System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` + Force bool `kong:"help=${install_force_help}"` + Dependency string `kong:"required=true,arg,help=${download_dependency_help},predictor=bin"` + TargetFile string `kong:"type=path,name=output,type=file,help=${install_target_file_help}"` + System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` + AllowMissingChecksum bool `kong:"name=allow-missing-checksum,help=${allow_missing_checksum}"` } func (d *installCmd) Run(kctx *kong.Context) error { @@ -193,8 +195,9 @@ func (d *installCmd) Run(kctx *kong.Context) error { return err } pth, err := config.InstallDependency(d.Dependency, d.System, &bindown.ConfigInstallDependencyOpts{ - TargetPath: d.TargetFile, - Force: d.Force, + TargetPath: d.TargetFile, + Force: d.Force, + AllowMissingChecksum: d.AllowMissingChecksum, }) if err != nil { return err @@ -204,10 +207,11 @@ func (d *installCmd) Run(kctx *kong.Context) error { } type downloadCmd struct { - Force bool `kong:"help=${download_force_help}"` - System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` - Dependency string `kong:"required=true,arg,help=${download_dependency_help},predictor=bin"` - TargetFile string `kong:"name=output,help=${download_target_file_help}"` + Force bool `kong:"help=${download_force_help}"` + System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` + Dependency string `kong:"required=true,arg,help=${download_dependency_help},predictor=bin"` + TargetFile string `kong:"name=output,help=${download_target_file_help}"` + AllowMissingChecksum bool `kong:"name=allow-missing-checksum,help=${allow_missing_checksum}"` } func (d *downloadCmd) Run(ctx context.Context, kctx *kong.Context) error { @@ -216,8 +220,9 @@ func (d *downloadCmd) Run(ctx context.Context, kctx *kong.Context) error { return err } pth, err := config.DownloadDependency(d.Dependency, d.System, &bindown.ConfigDownloadDependencyOpts{ - TargetFile: d.TargetFile, - Force: d.Force, + TargetFile: d.TargetFile, + Force: d.Force, + AllowMissingChecksum: d.AllowMissingChecksum, }) if err != nil { return err @@ -227,9 +232,10 @@ func (d *downloadCmd) Run(ctx context.Context, kctx *kong.Context) error { } type extractCmd struct { - System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` - Dependency string `kong:"required=true,arg,help=${extract_dependency_help},predictor=bin"` - TargetDir string `kong:"name=output,help=${extract_target_dir_help}"` + System bindown.SystemInfo `kong:"name=system,default=${system_default},help=${system_help},predictor=allSystems"` + Dependency string `kong:"required=true,arg,help=${extract_dependency_help},predictor=bin"` + TargetDir string `kong:"name=output,help=${extract_target_dir_help}"` + AllowMissingChecksum bool `kong:"name=allow-missing-checksum,help=${allow_missing_checksum}"` } func (d *extractCmd) Run(ctx context.Context, kctx *kong.Context) error { @@ -238,8 +244,9 @@ func (d *extractCmd) Run(ctx context.Context, kctx *kong.Context) error { return err } pth, err := config.ExtractDependency(d.Dependency, d.System, &bindown.ConfigExtractDependencyOpts{ - TargetDirectory: d.TargetDir, - Force: false, + TargetDirectory: d.TargetDir, + Force: false, + AllowMissingChecksum: d.AllowMissingChecksum, }) if err != nil { return err diff --git a/config.go b/config.go index bc7b3d9a..cd886e82 100644 --- a/config.go +++ b/config.go @@ -259,7 +259,7 @@ func (c *Config) addChecksum(dependencyName string, sysInfo SystemInfo) error { if existingSum != "" { return nil } - sum, err := getURLChecksum(*dep.URL) + sum, err := getURLChecksum(*dep.URL, "") if err != nil { return err } @@ -334,8 +334,9 @@ func (c *Config) validateDep(systems []SystemInfo, depName string) error { // ConfigDownloadDependencyOpts options for Config.DownloadDependency type ConfigDownloadDependencyOpts struct { - TargetFile string - Force bool + TargetFile string + Force bool + AllowMissingChecksum bool } // extractsCacheDir returns the cache directory for an extraction based on the download's checksum and dependency name @@ -357,7 +358,7 @@ func (c *Config) downloadCacheDir(checksum string) (string, error) { } // DownloadDependency downloads a dependency -func (c *Config) DownloadDependency(dependencyName string, sysInfo SystemInfo, opts *ConfigDownloadDependencyOpts) (string, error) { +func (c *Config) DownloadDependency(dependencyName string, sysInfo SystemInfo, opts *ConfigDownloadDependencyOpts) (_ string, errOut error) { if opts == nil { opts = &ConfigDownloadDependencyOpts{} } @@ -366,17 +367,37 @@ func (c *Config) DownloadDependency(dependencyName string, sysInfo SystemInfo, o if err != nil { return "", err } - if dep.URL == nil { - return "", fmt.Errorf("no URL configured") + depURL, err := dep.url() + if err != nil { + return "", err } - checksum, err := c.dependencyChecksum(dependencyName, sysInfo) + tempDir, err := os.MkdirTemp("", "bindown") if err != nil { return "", err } + defer func() { + cleanupErr := os.RemoveAll(tempDir) + if errOut == nil { + errOut = cleanupErr + } + }() + tempFile := filepath.Join(tempDir, "download") + downloadedToTemp := false + checksum, err := c.dependencyChecksum(dependencyName, sysInfo) + if err != nil { + if !opts.AllowMissingChecksum { + return "", err + } + checksum, err = getURLChecksum(depURL, tempFile) + if err != nil { + return "", err + } + downloadedToTemp = true + } if targetFile == "" { - dlFile, err := urlFilename(*dep.URL) + dlFile, err := urlFilename(depURL) if err != nil { return "", err } @@ -386,7 +407,40 @@ func (c *Config) DownloadDependency(dependencyName string, sysInfo SystemInfo, o } targetFile = filepath.Join(cacheDir, dlFile) } - return targetFile, download(strFromPtr(dep.URL), targetFile, checksum, opts.Force) + + if !downloadedToTemp { + return targetFile, download(depURL, targetFile, checksum, opts.Force) + } + + ok, err := fileExistsWithChecksum(targetFile, checksum) + if err != nil { + return "", err + } + if ok { + return targetFile, nil + } + + err = os.MkdirAll(filepath.Dir(targetFile), 0o750) + if err != nil { + return "", err + } + + // copy the file from the temp dir to the target file + out, err := os.Create(targetFile) + if err != nil { + return "", err + } + defer logCloseErr(out) + in, err := os.Open(tempFile) + if err != nil { + return "", err + } + defer logCloseErr(in) + _, err = io.Copy(out, in) + if err != nil { + return "", err + } + return targetFile, nil } func urlFilename(dlURL string) (string, error) { @@ -414,8 +468,9 @@ func (c *Config) dependencyChecksum(dependencyName string, sysInfo SystemInfo) ( // ConfigExtractDependencyOpts options for Config.ExtractDependency type ConfigExtractDependencyOpts struct { - TargetDirectory string - Force bool + TargetDirectory string + Force bool + AllowMissingChecksum bool } // ExtractDependency downloads and extracts a dependency @@ -424,7 +479,8 @@ func (c *Config) ExtractDependency(dependencyName string, sysInfo SystemInfo, op opts = &ConfigExtractDependencyOpts{} } downloadPath, err := c.DownloadDependency(dependencyName, sysInfo, &ConfigDownloadDependencyOpts{ - Force: opts.Force, + Force: opts.Force, + AllowMissingChecksum: opts.AllowMissingChecksum, }) if err != nil { return "", err @@ -443,7 +499,13 @@ func (c *Config) ExtractDependency(dependencyName string, sysInfo SystemInfo, op var checksum string checksum, err = c.dependencyChecksum(dependencyName, sysInfo) if err != nil { - return "", err + if !opts.AllowMissingChecksum { + return "", err + } + checksum, err = fileChecksum(downloadPath) + if err != nil { + return "", err + } } targetDir, err = c.extractsCacheDir(dependencyName, checksum) if err != nil { @@ -465,8 +527,10 @@ func (c *Config) ExtractDependency(dependencyName string, sysInfo SystemInfo, op type ConfigInstallDependencyOpts struct { // TargetPath is the path where the executable should end up TargetPath string - // Force - whether to force the install even if it already exists + // Force - install even if it already exists Force bool + // AllowMissingChecksum - whether to allow missing checksum + AllowMissingChecksum bool } // InstallDependency downloads, extracts and installs a dependency @@ -475,7 +539,8 @@ func (c *Config) InstallDependency(dependencyName string, sysInfo SystemInfo, op opts = &ConfigInstallDependencyOpts{} } extractDir, err := c.ExtractDependency(dependencyName, sysInfo, &ConfigExtractDependencyOpts{ - Force: opts.Force, + Force: opts.Force, + AllowMissingChecksum: opts.AllowMissingChecksum, }) if err != nil { return "", err diff --git a/dependency.go b/dependency.go index 882bb574..646bce1c 100644 --- a/dependency.go +++ b/dependency.go @@ -120,6 +120,13 @@ func varsWithSubstitutions(vars map[string]string, subs map[string]map[string]st return vars } +func (d *Dependency) url() (string, error) { + if d.URL == nil { + return "", fmt.Errorf("no URL configured") + } + return *d.URL, nil +} + func (d *Dependency) clone() *Dependency { dep := *d if d.Vars != nil { @@ -435,24 +442,29 @@ func extract(archivePath, extractDir string) error { return os.WriteFile(extractSumFile, []byte(extractSum), 0o600) } -// getURLChecksum returns the checksum of what is returned from this url -func getURLChecksum(dlURL string) (string, error) { - downloadDir, err := os.MkdirTemp("", "bindown") - if err != nil { - return "", err +// getURLChecksum returns the checksum of the file at dlURL. If tempFile is specified +// it will be used as the temporary file to download the file to and it will be the caller's +// responsibility to clean it up. Otherwise, a temporary file will be created and cleaned up +// automatically. +func getURLChecksum(dlURL, tempFile string) (_ string, errOut error) { + if tempFile == "" { + downloadDir, err := os.MkdirTemp("", "bindown") + if err != nil { + return "", err + } + tempFile = filepath.Join(downloadDir, "foo") + defer func() { + cleanupErr := os.RemoveAll(downloadDir) + if errOut == nil { + errOut = cleanupErr + } + }() } - dlPath := filepath.Join(downloadDir, "foo") - err = downloadFile(dlPath, dlURL) + err := downloadFile(tempFile, dlURL) if err != nil { - _ = os.RemoveAll(downloadDir) //nolint:errcheck // we already have an error to report return "", err } - sum, err := fileChecksum(dlPath) - cleanupErr := os.RemoveAll(downloadDir) - if err == nil { - err = cleanupErr - } - return sum, err + return fileChecksum(tempFile) } func downloadFile(targetPath, url string) error { diff --git a/dependency_test.go b/dependency_test.go index d20c100c..515fb0d5 100644 --- a/dependency_test.go +++ b/dependency_test.go @@ -89,7 +89,7 @@ func Test_downloadFile(t *testing.T) { func TestGetURLChecksum(t *testing.T) { ts := serveFile(t, filepath.Join("testdata", "downloadables", "foo.tar.gz"), "/foo/foo.tar.gz", "") - got, err := getURLChecksum(ts.URL + "/foo/foo.tar.gz") + got, err := getURLChecksum(ts.URL+"/foo/foo.tar.gz", "") require.NoError(t, err) require.Equal(t, fooChecksum, got) } From 510eea09251060051788846a70f5b262a8797ad4 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sat, 11 Mar 2023 13:51:17 -0600 Subject: [PATCH 2/2] lint fix --- config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/config.go b/config.go index cd886e82..5574aebf 100644 --- a/config.go +++ b/config.go @@ -397,11 +397,12 @@ func (c *Config) DownloadDependency(dependencyName string, sysInfo SystemInfo, o } if targetFile == "" { - dlFile, err := urlFilename(depURL) + var dlFile, cacheDir string + dlFile, err = urlFilename(depURL) if err != nil { return "", err } - cacheDir, err := c.downloadCacheDir(checksum) + cacheDir, err = c.downloadCacheDir(checksum) if err != nil { return "", err }