Skip to content

Commit

Permalink
command/copy: drop local->local copy support (#125)
Browse files Browse the repository at this point in the history
Fixes #118
  • Loading branch information
igungor authored Mar 26, 2020
1 parent fc557c7 commit 2079136
Show file tree
Hide file tree
Showing 20 changed files with 97 additions and 460 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ script:
- make check-fmt
- make staticcheck
- make vet
- make unparam

6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ This is a major release with many breaking changes.
- Exit code for errors was `127`. It is `1` now.
- Dropped `exit` command. It was used to change the shell exit code and usually
a part of the nested command usage.
- Dropped local->local copy and move support. ([#118](https://github.com/peak/s5cmd/issues/118))
- All error messages are sent to stderr now.
- `-version` flag is changed to `version` command.
- Dropped `batch-rm` command. It was not listed in the help output. Now that we
support variadic arguments, users can remove multiple objects by providing
wildcards or multiple arguments to `s5cmd rm` command.
wildcards or multiple arguments to `s5cmd rm` command. ([#106](https://github.com/peak/s5cmd/pull/106))
- [Virtual host style bucket name
resolving](https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/)
is enabled by default for S3 and GCS. If you provide a custom endpoint via
Expand All @@ -28,13 +29,14 @@ This is a major release with many breaking changes.
- Listing a non-existent object will return exit code `1`, instead of `0`. ([#23](https://github.com/peak/s5cmd/issues/23))
- `-ds`, `-dw`, `-us` and `-uw` global flags are no longer available. Multipart
concurrency and part size flags are now part of the `cp/mv` command. New
replacement flags are `--concurrency | -c` and `--part-size | -p`.
replacement flags are `--concurrency | -c` and `--part-size | -p`. ([#110](https://github.com/peak/s5cmd/pull/110))

#### Features

- Added `mb` command to make buckets. ([#25](https://github.com/peak/s5cmd/issues/25))
- Added `--json` flag for JSON logging. ([#22](https://github.com/peak/s5cmd/issues/22))
- Added [S3 transfer acceleration](https://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html) support. ([#40](https://github.com/peak/s5cmd/issues/40))
- Added [Google Cloud Storage](https://github.com/peak/s5cmd#google-cloud-storage-support) support. ([#81](https://github.com/peak/s5cmd/issues/81))

#### Bugfixes

Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ check: vet staticcheck unparam check-fmt

.PHONY: staticcheck
staticcheck:
@staticcheck -checks 'inherit,-U1000' ./...
@staticcheck -checks 'all,-U1000,-ST1000,-ST1003' ./...

.PHONY: unparam
unparam:
Expand All @@ -30,6 +30,10 @@ vet:
check-fmt:
@sh -c 'if [ -n "$(go fmt -mod=vendor ./...)" ]; then echo "Go code is not formatted"; exit 1; fi'

.PHONY: mock
mock:
@mockery -inpkg -dir=storage -name=Storage -case=underscore

.PHONY: clean
clean:
@rm -f ./s5cmd
Expand Down
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,6 @@ will return your GCS buckets.
acceleration and GCS. If a custom endpoint is provided, it'll fallback to
path-style.

⚠️ There's an [outstanding issue](https://github.com/peak/s5cmd/issues/81) for
not being able to list objects at GCS. It'll be fixed in upcoming releases.

### Retry logic

`s5cmd` uses an exponential backoff retry mechanism for transient or potential
Expand Down
44 changes: 7 additions & 37 deletions command/cmd_copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ var copyCommandFlags = []cli.Flag{
Name: "parents",
Usage: "create same directory structure of source, starting from the first wildcard",
},
&cli.BoolFlag{
Name: "recursive",
Aliases: []string{"R"},
Usage: "command is performed on all objects under the given source",
},
&cli.StringFlag{
Name: "storage-class",
Usage: "set storage class for target ('STANDARD','REDUCED_REDUNDANCY','GLACIER','STANDARD_IA')",
Expand Down Expand Up @@ -81,7 +76,6 @@ var CopyCommand = &cli.Command{
noClobber: c.Bool("no-clobber"),
ifSizeDiffer: c.Bool("if-size-differ"),
ifSourceNewer: c.Bool("if-source-newer"),
recursive: c.Bool("recursive"),
parents: c.Bool("parents"),
storageClass: storage.LookupClass(c.String("storage-class")),
concurrency: c.Int("concurrency"),
Expand All @@ -104,7 +98,6 @@ type Copy struct {
noClobber bool
ifSizeDiffer bool
ifSourceNewer bool
recursive bool
parents bool
storageClass storage.StorageClass

Expand All @@ -124,16 +117,12 @@ func (c Copy) Run(ctx context.Context) error {
return err
}

// set recursive=true for local->remote copy operations. this
// is required for backwards compatibility.
recursive := c.recursive || (!srcurl.IsRemote() && dsturl.IsRemote())

client, err := storage.NewClient(srcurl)
if err != nil {
return err
}

objch, err := expandSource(ctx, client, srcurl, recursive)
objch, err := expandSource(ctx, client, srcurl)
if err != nil {
return err
}
Expand Down Expand Up @@ -575,7 +564,6 @@ func expandSource(
ctx context.Context,
client storage.Storage,
srcurl *url.URL,
isRecursive bool,
) (<-chan *storage.Object, error) {
var isDir bool
// if the source is local, we send a Stat call to know if we have
Expand All @@ -591,7 +579,7 @@ func expandSource(

// call storage.List for only walking operations.
if srcurl.HasGlob() || isDir {
return client.List(ctx, srcurl, isRecursive), nil
return client.List(ctx, srcurl), nil
}

ch := make(chan *storage.Object, 1)
Expand Down Expand Up @@ -664,39 +652,21 @@ func Validate(c *cli.Context) error {

switch {
case srcurl.Type == dsturl.Type:
return validateCopy(ctx, srcurl, dsturl)
return validateCopy(srcurl, dsturl)
case dsturl.IsRemote():
return validateUpload(ctx, srcurl, dsturl)
default:
return nil
}
}

func validateCopy(ctx context.Context, srcurl, dsturl *url.URL) error {
if srcurl.IsRemote() {
func validateCopy(srcurl, dsturl *url.URL) error {
if srcurl.IsRemote() || dsturl.IsRemote() {
return nil
}

client, err := storage.NewClient(dsturl)
if err != nil {
return err
}

// For local->local copy operations, we can safely stat <dst> to check if
// it is a file or a directory.
obj, err := client.Stat(ctx, dsturl)
if err != nil && err != storage.ErrGivenObjectNotFound {
return err
}

// For local->local copy operations, if <src> has glob, <dst> is expected
// to be a directory. As always, local copy operation will create missing
// directories if <dst> has one.
if obj != nil && !obj.Type.IsDir() {
return fmt.Errorf("destination argument is expected to be a directory")
}

return nil
// we don't support local->local copies
return fmt.Errorf("local->local copy operations are not permitted")
}

func validateUpload(ctx context.Context, srcurl, dsturl *url.URL) error {
Expand Down
4 changes: 1 addition & 3 deletions command/cmd_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ func Delete(
return err
}

// storage.MultiDelete operates on file-like objects. Settings
// recursive=true guarantees returning only file-like objects.
objChan := expandSources(ctx, client, srcurls...)

// do object->url transformation
Expand Down Expand Up @@ -121,7 +119,7 @@ func expandSources(
wg.Add(1)
go func(origSrc *url.URL) {
defer wg.Done()
objch, err := expandSource(ctx, client, origSrc, true)
objch, err := expandSource(ctx, client, origSrc)
if err != nil {
mergech <- &storage.Object{Err: err}
return
Expand Down
5 changes: 2 additions & 3 deletions command/cmd_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/stretchr/testify/mock"

"github.com/peak/s5cmd/mocks"
"github.com/peak/s5cmd/storage"
"github.com/peak/s5cmd/storage/url"
)
Expand Down Expand Up @@ -135,7 +134,7 @@ func Test_expandSources(t *testing.T) {
t.Errorf("unexpected error: %v", err)
}

client := &mocks.Storage{}
client := &storage.MockStorage{}

for src, objects := range tt.src {
srcurl, err := url.New(src)
Expand All @@ -144,7 +143,7 @@ func Test_expandSources(t *testing.T) {
}

ch := generateObjects(objects)
client.On("List", mock.Anything, srcurl, true).Once().Return(ch)
client.On("List", mock.Anything, srcurl).Once().Return(ch)
}

gotChan := expandSources(context.Background(), client, srcurls...)
Expand Down
2 changes: 1 addition & 1 deletion command/cmd_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func List(

var merror error

for object := range client.List(ctx, srcurl, true) {
for object := range client.List(ctx, srcurl) {
if errorpkg.IsCancelation(object.Err) {
continue
}
Expand Down
1 change: 0 additions & 1 deletion command/cmd_move.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ var MoveCommand = &cli.Command{
noClobber: c.Bool("no-clobber"),
ifSizeDiffer: c.Bool("if-size-differ"),
ifSourceNewer: c.Bool("if-source-newer"),
recursive: c.Bool("recursive"),
parents: c.Bool("parents"),
storageClass: storage.LookupClass(c.String("storage-class")),
}
Expand Down
2 changes: 1 addition & 1 deletion command/cmd_size.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func Size(

var merror error

for object := range client.List(ctx, srcurl, true) {
for object := range client.List(ctx, srcurl) {
if object.Type.IsDir() || errorpkg.IsCancelation(object.Err) {
continue
}
Expand Down
Loading

0 comments on commit 2079136

Please sign in to comment.