Skip to content

Commit

Permalink
make Get return GetResult (#231)
Browse files Browse the repository at this point in the history
To allow to tell what operation was done by the go getter; this will allow the go-getter to 'make a choice' based on a set of arguments.

For example:

* Decide of a random temporary folder/file in which to put the wanted object and then tell us where it was put.
* just tell that this file was not being copied but simply referenced because it is already in the drive ( this one is usefull to Packer )
  • Loading branch information
azr authored Feb 4, 2020
1 parent ec2266f commit 8bdb487
Show file tree
Hide file tree
Showing 10 changed files with 178 additions and 65 deletions.
2 changes: 1 addition & 1 deletion checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
46 changes: 26 additions & 20 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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()

Expand All @@ -75,15 +81,15 @@ 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
}

g, ok := c.Getters[force]
if !ok {
return fmt.Errorf(
return nil, fmt.Errorf(
"download not supported for scheme '%s'", force)
}

Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}
}
Expand All @@ -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
Expand All @@ -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
}
}

Expand All @@ -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")
}

Expand All @@ -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
}
6 changes: 3 additions & 3 deletions client_option_progress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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{
Expand All @@ -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)
}

Expand Down
7 changes: 5 additions & 2 deletions cmd/go-getter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion folder_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions get_git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")
}
Expand Down
5 changes: 3 additions & 2 deletions get_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions get_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit 8bdb487

Please sign in to comment.