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.

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

command: fix target file is created despite the download failure #477

Merged
merged 11 commits into from
Aug 22, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
- Fixed a bug where some part of the destination path is removed by `cp` and `sync` subcommands ([#360](https://github.com/peak/s5cmd/issues/360))
- Fixed a bug where proxy is not being used when `--no-verify-ssl` flag is used. ([#445](https://github.com/peak/s5cmd/issues/445))
- Fixed `unknown url format` error when object key also includes `s3://` e.g. `s5cmd ls s3://foo/bar/s3://baz` ([#449](https://github.com/peak/s5cmd/issues/449))
- Fixed a bug where the local file created for the download operation was not deleted if the download fails in Windows. ([#348](https://github.com/peak/s5cmd/issues/348))

## v2.0.0 - 4 Jul 2022

7 changes: 6 additions & 1 deletion command/cp.go
Original file line number Diff line number Diff line change
@@ -522,7 +522,12 @@ func (c Copy) doDownload(ctx context.Context, srcurl *url.URL, dsturl *url.URL)

size, err := srcClient.Get(ctx, srcurl, file, c.concurrency, c.partSize)
if err != nil {
_ = dstClient.Delete(ctx, dsturl)
// file must be closed before deletion
file.Close()
dErr := dstClient.Delete(ctx, dsturl)
if dErr != nil {
printDebug(c.op, dErr, srcurl, dsturl)
}
return err
}

24 changes: 24 additions & 0 deletions e2e/cp_test.go
Original file line number Diff line number Diff line change
@@ -4185,3 +4185,27 @@ func TestCopySingleFileToS3WithNoSuchUploadRetryCount(t *testing.T) {
// assert S3
assert.Assert(t, ensureS3Object(s3client, bucket, filename, content))
}

// Before downloading a file from s3 a local target file is created. If download
// fails the created file should be deleted.
func TestDeleteFileWhenDownloadFailed(t *testing.T) {
t.Parallel()

s3client, s5cmd, cleanup := setup(t)
defer cleanup()

bucket := s3BucketFromTestName(t)
filename := "testfile1.txt"
createBucket(t, s3client, bucket)

// It is going try downloading a nonexistent file from the s3 so it will fail.
// In this case we don't expect to have a local file with the name `filename`.
cmd := s5cmd("cp", "s3://"+bucket+"/"+filename, filename)
result := icmd.RunCmd(cmd)

result.Assert(t, icmd.Expected{ExitCode: 1})

// assert local filesystem does not have any (such) file
expected := fs.Expected(t)
assert.Assert(t, fs.Equal(cmd.Dir, expected))
}