Skip to content
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.

Sign up for GitHub

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

Core tool downloader fixes #1028

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Core tool downloader fixes #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/bitrise-io/stepman v0.0.0-20240828074035-6ae1a5f5efde
github.com/go-git/go-git/v5 v5.12.0
github.com/gofrs/uuid v4.3.1+incompatible
github.com/hashicorp/go-retryablehttp v0.7.7
github.com/hashicorp/go-version v1.4.0
github.com/heimdalr/dag v1.4.0
github.com/ryanuber/go-glob v1.0.0
Expand All @@ -39,7 +40,6 @@ require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
Expand Down
31 changes: 8 additions & 23 deletions tools/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package tools
import (
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"runtime"
Expand All @@ -17,10 +16,10 @@ import (
"github.com/bitrise-io/go-utils/pathutil"
stepman "github.com/bitrise-io/stepman/cli"
stepmanModels "github.com/bitrise-io/stepman/models"
"github.com/hashicorp/go-retryablehttp"
"golang.org/x/sys/unix"
)

// UnameGOOS ...
func UnameGOOS() (string, error) {
switch runtime.GOOS {
case "darwin":
Expand All @@ -31,7 +30,6 @@ func UnameGOOS() (string, error) {
return "", fmt.Errorf("Unsupported platform (%s)", runtime.GOOS)
}

// UnameGOARCH ...
func UnameGOARCH() (string, error) {
switch runtime.GOARCH {
case "amd64":
Expand All @@ -42,7 +40,6 @@ func UnameGOARCH() (string, error) {
return "", fmt.Errorf("Unsupported architecture (%s)", runtime.GOARCH)
}

// InstallToolFromGitHub ...
func InstallToolFromGitHub(toolname, githubUser, toolVersion string) error {
unameGOOS, err := UnameGOOS()
if err != nil {
Expand All @@ -57,7 +54,6 @@ func InstallToolFromGitHub(toolname, githubUser, toolVersion string) error {
return InstallFromURL(toolname, downloadURL)
}

// DownloadFile ...
func DownloadFile(downloadURL, targetDirPath string) error {
outFile, err := os.Create(targetDirPath)
defer func() {
Expand All @@ -66,19 +62,22 @@ func DownloadFile(downloadURL, targetDirPath string) error {
}
}()
if err != nil {
return fmt.Errorf("failed to create (%s), error: %s", targetDirPath, err)
return fmt.Errorf("create %s: %s", targetDirPath, err)
}

resp, err := http.Get(downloadURL)
resp, err := retryablehttp.Get(downloadURL)
if err != nil {
return fmt.Errorf("failed to download from (%s), error: %s", downloadURL, err)
return fmt.Errorf("download %s: %s", downloadURL, err)
}
if resp.StatusCode >= 400 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL net/http follows redirects by default, we should not see 3xx status codes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default retryablehttp config does close http response body https://github.com/hashicorp/go-retryablehttp/blob/9dfd949154e507c91ee569367ffa461991b80950/client.go#L811. closing it twice may not be an issue tough

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's why the defer func is not above this error handling...at least this is how I understand the whole thing.

return fmt.Errorf("download %s: %s", downloadURL, resp.Status)
}

defer func() {
if err := resp.Body.Close(); err != nil {
log.Warnf("failed to close (%s) body", downloadURL)
}
}()

_, err = io.Copy(outFile, resp.Body)
if err != nil {
return fmt.Errorf("failed to download from (%s), error: %s", downloadURL, err)
Expand All @@ -87,7 +86,6 @@ func DownloadFile(downloadURL, targetDirPath string) error {
return nil
}

// InstallFromURL ...
func InstallFromURL(toolBinName, downloadURL string) error {
if len(toolBinName) < 1 {
return fmt.Errorf("no Tool (bin) Name provided! URL was: %s", downloadURL)
Expand Down Expand Up @@ -128,13 +126,11 @@ func InstallFromURL(toolBinName, downloadURL string) error {
// ------------------
// --- Stepman

// StepmanSetup ...
func StepmanSetup(collection string) error {
log := log.NewLogger(log.GetGlobalLoggerOpts())
return stepman.Setup(collection, "", log)
}

// StepmanStepInfo ...
func StepmanStepInfo(collection, stepID, stepVersion string) (stepmanModels.StepInfoModel, error) {
log := log.NewLogger(log.GetGlobalLoggerOpts())
return stepman.QueryStepInfo(collection, stepID, stepVersion, log)
Expand All @@ -143,31 +139,26 @@ func StepmanStepInfo(collection, stepID, stepVersion string) (stepmanModels.Step
//
// Share

// StepmanShare ...
func StepmanShare() error {
args := []string{"share", "--toolmode"}
return command.RunCommand("stepman", args...)
}

// StepmanShareAudit ...
func StepmanShareAudit() error {
args := []string{"share", "audit", "--toolmode"}
return command.RunCommand("stepman", args...)
}

// StepmanShareCreate ...
func StepmanShareCreate(tag, git, stepID string) error {
args := []string{"share", "create", "--tag", tag, "--git", git, "--stepid", stepID, "--toolmode"}
return command.RunCommand("stepman", args...)
}

// StepmanShareFinish ...
func StepmanShareFinish() error {
args := []string{"share", "finish", "--toolmode"}
return command.RunCommand("stepman", args...)
}

// StepmanShareStart ...
func StepmanShareStart(collection string) error {
args := []string{"share", "start", "--collection", collection, "--toolmode"}
return command.RunCommand("stepman", args...)
Expand All @@ -176,17 +167,14 @@ func StepmanShareStart(collection string) error {
// ------------------
// --- Envman

// EnvmanInit ...
func EnvmanInit(envStorePth string, clear bool) error {
return envman.InitEnvStore(envStorePth, clear)
}

// EnvmanAdd ...
func EnvmanAdd(envStorePth, key, value string, expand, skipIfEmpty, sensitive bool) error {
return envman.AddEnv(envStorePth, key, value, expand, false, skipIfEmpty, sensitive)
}

// EnvmanAddEnvs ...
func EnvmanAddEnvs(envstorePth string, envsList []envmanModels.EnvironmentItemModel) error {
for _, env := range envsList {
key, value, err := env.GetKeyValuePair()
Expand Down Expand Up @@ -221,12 +209,10 @@ func EnvmanAddEnvs(envstorePth string, envsList []envmanModels.EnvironmentItemMo
return nil
}

// EnvmanReadEnvList ...
func EnvmanReadEnvList(envStorePth string) (envmanModels.EnvsJSONListModel, error) {
return envman.ReadEnvsJSONList(envStorePth, true, false, &envmanEnv.DefaultEnvironmentSource{})
}

// EnvmanClear ...
func EnvmanClear(envStorePth string) error {
return envman.ClearEnvs(envStorePth)
}
Expand All @@ -253,7 +239,6 @@ func GetSecretKeysAndValues(secrets []envmanModels.EnvironmentItemModel) ([]stri
return secretKeys, secretValues
}

// MoveFile ...
func MoveFile(oldpath, newpath string) error {
err := os.Rename(oldpath, newpath)
if err == nil {
Expand Down
Loading