diff --git a/checksum.go b/checksum.go index d17e8a905..705d5576d 100644 --- a/checksum.go +++ b/checksum.go @@ -211,7 +211,7 @@ func (c *Client) ChecksumFromFile(ctx context.Context, checksumURL, checksummedP Dst: tempfile, // ProgressListener: c.ProgressListener, TODO(adrien): pass progress bar ? } - if err = c.Get(ctx, req); err != nil { + if _, err = c.Get(ctx, req); err != nil { return nil, fmt.Errorf( "Error downloading checksum file: %s", err) } diff --git a/client.go b/client.go index 82216f2df..7a43d492e 100644 --- a/client.go +++ b/client.go @@ -33,10 +33,16 @@ type Client struct { Getters map[string]Getter } +// GetResult is the result of a Client.Get +type GetResult struct { + // Local destination of the gotten object. + Dst string +} + // Get downloads the configured source to the destination. -func (c *Client) Get(ctx context.Context, req *Request) error { +func (c *Client) Get(ctx context.Context, req *Request) (*GetResult, error) { if err := c.configure(); err != nil { - return err + return nil, err } // Store this locally since there are cases we swap this @@ -51,7 +57,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { var err error req.Src, err = Detect(req.Src, req.Pwd, c.Detectors) if err != nil { - return err + return nil, err } var force string @@ -65,7 +71,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { if subDir != "" { td, tdcloser, err := safetemp.Dir("", "getter") if err != nil { - return err + return nil, err } defer tdcloser.Close() @@ -75,7 +81,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { req.u, err = urlhelper.Parse(req.Src) if err != nil { - return err + return nil, err } if force == "" { force = req.u.Scheme @@ -83,7 +89,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { g, ok := c.Getters[force] if !ok { - return fmt.Errorf( + return nil, fmt.Errorf( "download not supported for scheme '%s'", force) } @@ -126,7 +132,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // this at the end of everything. td, err := ioutil.TempDir("", "getter") if err != nil { - return fmt.Errorf( + return nil, fmt.Errorf( "Error creating temporary directory for archive: %s", err) } defer os.RemoveAll(td) @@ -142,7 +148,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // Determine checksum if we have one checksum, err := c.extractChecksum(ctx, req.u) if err != nil { - return fmt.Errorf("invalid checksum: %s", err) + return nil, fmt.Errorf("invalid checksum: %s", err) } // Delete the query parameter if we have it. @@ -153,7 +159,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // Ask the getter which client mode to use req.Mode, err = g.ClientMode(ctx, req.u) if err != nil { - return err + return nil, err } // Destination is the base name of the URL path in "any" mode when @@ -187,12 +193,12 @@ func (c *Client) Get(ctx context.Context, req *Request) error { if getFile { err := g.GetFile(ctx, req) if err != nil { - return err + return nil, err } if checksum != nil { if err := checksum.checksum(req.Dst); err != nil { - return err + return nil, err } } } @@ -202,7 +208,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // into the final destination with the proper mode. err := decompressor.Decompress(decompressDst, req.Dst, decompressDir) if err != nil { - return err + return nil, err } // Swap the information back @@ -218,7 +224,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // if we were unarchiving. If we're still only Get-ing a file, then // we're done. if req.Mode == ClientModeFile { - return nil + return &GetResult{req.Dst}, nil } } @@ -230,7 +236,7 @@ func (c *Client) Get(ctx context.Context, req *Request) error { // If we're getting a directory, then this is an error. You cannot // checksum a directory. TODO: test if checksum != nil { - return fmt.Errorf( + return nil, fmt.Errorf( "checksum cannot be specified for directory download") } @@ -239,27 +245,27 @@ func (c *Client) Get(ctx context.Context, req *Request) error { err := g.Get(ctx, req) if err != nil { err = fmt.Errorf("error downloading '%s': %s", req.Src, err) - return err + return nil, err } } // If we have a subdir, copy that over if subDir != "" { if err := os.RemoveAll(realDst); err != nil { - return err + return nil, err } if err := os.MkdirAll(realDst, 0755); err != nil { - return err + return nil, err } // Process any globs subDir, err := SubdirGlob(req.Dst, subDir) if err != nil { - return err + return nil, err } - return copyDir(ctx, realDst, subDir, false) + return &GetResult{realDst}, copyDir(ctx, realDst, subDir, false) } - return nil + return &GetResult{req.Dst}, nil } diff --git a/client_option_progress_test.go b/client_option_progress_test.go index ba9556354..a7b644dc2 100644 --- a/client_option_progress_test.go +++ b/client_option_progress_test.go @@ -41,7 +41,7 @@ func TestGet_progress(t *testing.T) { { // dl without tracking dst := tempTestFile(t) defer os.RemoveAll(filepath.Dir(dst)) - if err := GetFile(ctx, dst, s.URL+"/file?thig=this&that"); err != nil { + if _, err := GetFile(ctx, dst, s.URL+"/file?thig=this&that"); err != nil { t.Fatalf("download failed: %v", err) } } @@ -56,7 +56,7 @@ func TestGet_progress(t *testing.T) { ProgressListener: p, Dir: false, } - if err := DefaultClient.Get(ctx, req); err != nil { + if _, err := DefaultClient.Get(ctx, req); err != nil { t.Fatalf("download failed: %v", err) } req = &Request{ @@ -65,7 +65,7 @@ func TestGet_progress(t *testing.T) { ProgressListener: p, Dir: false, } - if err := DefaultClient.Get(ctx, req); err != nil { + if _, err := DefaultClient.Get(ctx, req); err != nil { t.Fatalf("download failed: %v", err) } diff --git a/cmd/go-getter/main.go b/cmd/go-getter/main.go index 61b5c848c..9c03f1ec2 100644 --- a/cmd/go-getter/main.go +++ b/cmd/go-getter/main.go @@ -55,13 +55,17 @@ func main() { wg := sync.WaitGroup{} wg.Add(1) + errChan := make(chan error, 2) go func() { defer wg.Done() defer cancel() - if err := getter.DefaultClient.Get(ctx, req); err != nil { + res, err := getter.DefaultClient.Get(ctx, req) + if err != nil { errChan <- err + return } + log.Printf("-> %s", res.Dst) }() c := make(chan os.Signal) @@ -75,7 +79,6 @@ func main() { log.Printf("signal %v", sig) case <-ctx.Done(): wg.Wait() - log.Printf("success!") case err := <-errChan: wg.Wait() log.Fatalf("Error downloading: %s", err) diff --git a/folder_storage.go b/folder_storage.go index 4481ee27a..891eda956 100644 --- a/folder_storage.go +++ b/folder_storage.go @@ -55,7 +55,8 @@ func (s *FolderStorage) Get(ctx context.Context, key string, source string, upda } // Get the source. This always forces an update. - return Get(ctx, dir, source) + _, err := Get(ctx, dir, source) + return err } // dir returns the directory name internally that we'll use to map to diff --git a/get.go b/get.go index 2738bc4ce..5f9ce7afd 100644 --- a/get.go +++ b/get.go @@ -87,7 +87,7 @@ func init() { // // src is a URL, whereas dst is always just a file path to a folder. This // folder doesn't need to exist. It will be created if it doesn't exist. -func Get(ctx context.Context, dst, src string) error { +func Get(ctx context.Context, dst, src string) (*GetResult, error) { req := &Request{ Src: src, Dst: dst, @@ -102,7 +102,7 @@ func Get(ctx context.Context, dst, src string) error { // dst must be a directory. If src is a file, it will be downloaded // into dst with the basename of the URL. If src is a directory or // archive, it will be unpacked directly into dst. -func GetAny(ctx context.Context, dst, src string) error { +func GetAny(ctx context.Context, dst, src string) (*GetResult, error) { req := &Request{ Src: src, Dst: dst, @@ -113,7 +113,7 @@ func GetAny(ctx context.Context, dst, src string) error { // GetFile downloads the file specified by src into the path specified by // dst. -func GetFile(ctx context.Context, dst, src string) error { +func GetFile(ctx context.Context, dst, src string) (*GetResult, error) { req := &Request{ Src: src, Dst: dst, diff --git a/get_git_test.go b/get_git_test.go index 02f9e49f7..fde606863 100644 --- a/get_git_test.go +++ b/get_git_test.go @@ -420,7 +420,7 @@ func TestGitGetter_sshSCPStyle(t *testing.T) { }, } - if err := client.Get(ctx, req); err != nil { + if _, err := client.Get(ctx, req); err != nil { t.Fatalf("client.Get failed: %s", err) } @@ -464,7 +464,7 @@ func TestGitGetter_sshExplicitPort(t *testing.T) { }, } - if err := client.Get(ctx, req); err != nil { + if _, err := client.Get(ctx, req); err != nil { t.Fatalf("client.Get failed: %s", err) } @@ -508,7 +508,7 @@ func TestGitGetter_sshSCPStyleInvalidScheme(t *testing.T) { }, } - err := client.Get(ctx, req) + _, err := client.Get(ctx, req) if err == nil { t.Fatalf("get succeeded; want error") } diff --git a/get_http.go b/get_http.go index cfcc6b669..a80ad2502 100644 --- a/get_http.go +++ b/get_http.go @@ -122,7 +122,8 @@ func (g *HttpGetter) Get(ctx context.Context, req *Request) error { Dst: req.Dst, } if subDir == "" { - return DefaultClient.Get(ctx, req) + _, err = DefaultClient.Get(ctx, req) + return err } // We have a subdir, time to jump some hoops return g.getSubdir(ctx, req.Dst, source, subDir) @@ -230,7 +231,7 @@ func (g *HttpGetter) getSubdir(ctx context.Context, dst, source, subDir string) defer tdcloser.Close() // Download that into the given directory - if err := Get(ctx, td, source); err != nil { + if _, err := Get(ctx, td, source); err != nil { return err } diff --git a/get_http_test.go b/get_http_test.go index d1fd3c012..b60448317 100644 --- a/get_http_test.go +++ b/get_http_test.go @@ -241,7 +241,7 @@ func TestHttpGetter_resume(t *testing.T) { ctx := context.Background() // Finish getting it! - if err := GetFile(ctx, dst, u.String()); err != nil { + if _, err := GetFile(ctx, dst, u.String()); err != nil { t.Fatalf("finishing download should not error: %v", err) } @@ -255,7 +255,7 @@ func TestHttpGetter_resume(t *testing.T) { } // Get it again - if err := GetFile(ctx, dst, u.String()); err != nil { + if _, err := GetFile(ctx, dst, u.String()); err != nil { t.Fatalf("should not error: %v", err) } } @@ -298,7 +298,7 @@ func TestHttpGetter_resumeNoRange(t *testing.T) { ctx := context.Background() // Finish getting it! - if err := GetFile(ctx, dst, u.String()); err != nil { + if _, err := GetFile(ctx, dst, u.String()); err != nil { t.Fatalf("finishing download should not error: %v", err) } diff --git a/get_test.go b/get_test.go index 58d610a05..8a23b47eb 100644 --- a/get_test.go +++ b/get_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/google/go-cmp/cmp" ) func TestGet_badSchema(t *testing.T) { @@ -15,9 +17,13 @@ func TestGet_badSchema(t *testing.T) { u := testModule("basic") u = strings.Replace(u, "file", "nope", -1) - if err := Get(ctx, dst, u); err == nil { + op, err := Get(ctx, dst, u) + if err == nil { t.Fatal("should error") } + if op != nil { + t.Fatal("op should be nil") + } } func TestGet_file(t *testing.T) { @@ -26,9 +32,13 @@ func TestGet_file(t *testing.T) { dst := tempDir(t) u := testModule("basic") - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -43,9 +53,13 @@ func TestGet_fileDecompressorExt(t *testing.T) { dst := tempDir(t) u := testModule("basic-tgz") - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -60,9 +74,13 @@ func TestGet_filePercent2F(t *testing.T) { dst := tempDir(t) u := testModule("basic%2Ftest") - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -92,9 +110,13 @@ func TestGet_fileDetect(t *testing.T) { t.Fatalf("configure: %s", err) } - if err := client.Get(ctx, req); err != nil { + op, err := client.Get(ctx, req) + if err != nil { t.Fatalf("get: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -109,9 +131,13 @@ func TestGet_fileForced(t *testing.T) { u := testModule("basic") u = "file::" + u - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -125,9 +151,13 @@ func TestGet_fileSubdir(t *testing.T) { dst := tempDir(t) u := testModule("basic//subdir") - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "sub.tf") if _, err := os.Stat(mainPath); err != nil { @@ -142,9 +172,13 @@ func TestGet_archive(t *testing.T) { u := filepath.Join("./testdata", "archive.tar.gz") u, _ = filepath.Abs(u) - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -159,9 +193,13 @@ func TestGetAny_archive(t *testing.T) { u := filepath.Join("./testdata", "archive.tar.gz") u, _ = filepath.Abs(u) - if err := GetAny(ctx, dst, u); err != nil { + op, err := GetAny(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "main.tf") if _, err := os.Stat(mainPath); err != nil { @@ -174,9 +212,13 @@ func TestGet_archiveRooted(t *testing.T) { dst := tempDir(t) u := testModule("archive-rooted/archive.tar.gz") - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "root", "hello.txt") if _, err := os.Stat(mainPath); err != nil { @@ -190,9 +232,13 @@ func TestGet_archiveSubdirWild(t *testing.T) { dst := tempDir(t) u := testModule("archive-rooted/archive.tar.gz") u += "//*" - if err := Get(ctx, dst, u); err != nil { + op, err := Get(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } mainPath := filepath.Join(dst, "hello.txt") if _, err := os.Stat(mainPath); err != nil { @@ -206,10 +252,17 @@ func TestGet_archiveSubdirWildMultiMatch(t *testing.T) { dst := tempDir(t) u := testModule("archive-rooted-multi/archive.tar.gz") u += "//*" - if err := Get(ctx, dst, u); err == nil { + op, err := Get(ctx, dst, u) + switch err { + case nil: t.Fatal("should error") - } else if !strings.Contains(err.Error(), "multiple") { - t.Fatalf("err: %s", err) + default: + if !strings.Contains(err.Error(), "multiple") { + t.Fatalf("err: %s", err) + } + if op != nil { + t.Fatal("operation should be nil") + } } } @@ -219,7 +272,7 @@ func TestGetAny_file(t *testing.T) { dst := tempDir(t) u := testModule("basic-file/foo.txt") - if err := GetAny(ctx, dst, u); err != nil { + if _, err := GetAny(ctx, dst, u); err != nil { t.Fatalf("err: %s", err) } @@ -236,9 +289,13 @@ func TestGetAny_dir(t *testing.T) { u := filepath.Join("./testdata", "basic") u, _ = filepath.Abs(u) - if err := GetAny(ctx, dst, u); err != nil { + op, err := GetAny(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } check := []string{ "main.tf", @@ -260,9 +317,13 @@ func TestGetFile(t *testing.T) { defer os.RemoveAll(filepath.Dir(dst)) u := testModule("basic-file/foo.txt") - if err := GetFile(ctx, dst, u); err != nil { + op, err := GetFile(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } // Verify the main file exists assertContents(t, dst, "Hello\n") @@ -275,9 +336,13 @@ func TestGetFile_archive(t *testing.T) { defer os.RemoveAll(filepath.Dir(dst)) u := testModule("basic-file-archive/archive.tar.gz") - if err := GetFile(ctx, dst, u); err != nil { + op, err := GetFile(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } // Verify the main file exists assertContents(t, dst, "Hello\n") @@ -291,9 +356,13 @@ func TestGetFile_archiveChecksum(t *testing.T) { u := testModule( "basic-file-archive/archive.tar.gz?checksum=md5:fbd90037dacc4b1ab40811d610dde2f0") - if err := GetFile(ctx, dst, u); err != nil { + op, err := GetFile(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } // Verify the main file exists assertContents(t, dst, "Hello\n") @@ -307,9 +376,13 @@ func TestGetFile_archiveNoUnarchive(t *testing.T) { u := testModule("basic-file-archive/archive.tar.gz") u += "?archive=false" - if err := GetFile(ctx, dst, u); err != nil { + op, err := GetFile(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } // Verify the main file exists actual := testMD5(t, dst) @@ -394,9 +467,15 @@ func TestGetFile_checksum(t *testing.T) { func() { dst := tempTestFile(t) defer os.RemoveAll(filepath.Dir(dst)) - if err := GetFile(ctx, dst, u); (err != nil) != tc.Err { + op, err := GetFile(ctx, dst, u) + if (err != nil) != tc.Err { t.Fatalf("append: %s\n\nerr: %s", tc.Append, err) } + if err == nil { + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected dst: %s", diff) + } + } // Verify the main file exists assertContents(t, dst, "Hello\n") @@ -477,9 +556,15 @@ func TestGetFile_checksum_from_file(t *testing.T) { dst := tempTestFile(t) defer os.RemoveAll(filepath.Dir(dst)) - if err := GetFile(ctx, dst, u); (err != nil) != tc.WantErr { + op, err := GetFile(ctx, dst, u) + if (err != nil) != tc.WantErr { t.Fatalf("append: %s\n\nerr: %s", tc.Append, err) } + if err == nil { + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected dst: %s", diff) + } + } if tc.WantTransfer { // Verify the main file exists @@ -508,9 +593,13 @@ func TestGetFile_checksumURL(t *testing.T) { }, } - if err := client.Get(ctx, req); err != nil { + op, err := client.Get(ctx, req) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } if v := getter.GetFileURL.Query().Get("checksum"); v != "" { t.Fatalf("bad: %s", v) @@ -524,12 +613,17 @@ func TestGetFile_filename(t *testing.T) { u := testModule("basic-file/foo.txt") u += "?filename=bar.txt" + realDst := filepath.Join(dst, "bar.txt") - if err := GetAny(ctx, dst, u); err != nil { + op, err := GetAny(ctx, dst, u) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(realDst, op.Dst); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } - mainPath := filepath.Join(dst, "bar.txt") + mainPath := realDst if _, err := os.Stat(mainPath); err != nil { t.Fatalf("err: %s", err) } @@ -555,9 +649,13 @@ func TestGetFile_checksumSkip(t *testing.T) { } // get the file - if err := client.Get(ctx, req); err != nil { + op, err := client.Get(ctx, req) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } if v := getter.GetFileURL.Query().Get("checksum"); v != "" { t.Fatalf("bad: %s", v) @@ -567,9 +665,13 @@ func TestGetFile_checksumSkip(t *testing.T) { getter.Proxy = nil getter.GetFileCalled = false - if err := client.Get(ctx, req); err != nil { + op, err = client.Get(ctx, req) + if err != nil { t.Fatalf("err: %s", err) } + if diff := cmp.Diff(&GetResult{Dst: dst}, op); diff != "" { + t.Fatalf("unexpected op: %s", diff) + } if getter.GetFileCalled { t.Fatalf("get should not have been called")