fix: ls differentiate exit codes for empty and non-existent buckets (#…

…732) Previously, s5cmd ls returned an exit code of 1 for both empty buckets and non-existent buckets, making it difficult to differentiate between the two cases. This change updates the behavior to: - Return exit code 0 for empty buckets, similar to the behavior of s3cmd. - Return exit code 1 for non-existent buckets, providing a clear distinction. Example behavior after the change: `s5cmd ls s3://empty-bucket` returns exit code 0. It does not print anything. `s5cmd ls s3://non-existent-bucket` returns exit code 1 with an appropriate error message. Also, select command with `--all-versions true` flag no longer prints `ERROR "s3://bucket/": no object found` message for empty buckets. Similarly, du command used to print ``` ERROR "du s3://empty-bucket": no object found 0 bytes in 0 objects: s3://empty-bucket ``` Now it will omit the part with error, it will yield: ``` 0 bytes in 0 objects: s3://empty-bucket ``` They both exit with 0 without any change. Resolves #722 --------- Co-authored-by: Tarık Buğra Özyurt <[email protected]> Co-authored-by: Deniz Surmeli <[email protected]> Co-authored-by: İbrahim Güngör <[email protected]>
peak · Jul 10, 2024 · 21fa2da · 21fa2da
1 parent 0adfb9b
commit 21fa2da
Show file tree

Hide file tree

Showing 5 changed files with 151 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,9 @@
 # Changelog
+## v2.3.0
+
+#### Breaking changes
+- Changed the exit code from 1 to 0 for `ls` when used with an empty bucket. Exits with 1 if the bucket is non-existent. ([#722](https://github.com/peak/s5cmd/issues/722))
+
 ## v2.2.2 - 13 Sep 2023 
 
 #### Bugfixes

diff --git a/e2e/du_test.go b/e2e/du_test.go
@@ -300,3 +300,21 @@ func TestDiskUsageByVersionIDAndAllVersions(t *testing.T) {
 		})
 	}
 }
+
+func TestDiskUsageEmptyBucket(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+	createBucket(t, s3client, bucket)
+
+	cmd := s5cmd("du", "s3://"+bucket)
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	assertLines(t, result.Stdout(), map[int]compareFunc{
+		0: suffix(`0 bytes in 0 objects: s3://%v`, bucket),
+	})
+}
diff --git a/e2e/ls_test.go b/e2e/ls_test.go
@@ -786,3 +786,19 @@ func TestListNestedLocalFolders(t *testing.T) {
 		2: match(filepath.ToSlash("file.txt")),
 	}, trimMatch(dateRe), alignment(true))
 }
+
+func TestEmptyBucket(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+	createBucket(t, s3client, bucket)
+
+	cmd := s5cmd("ls", "s3://"+bucket)
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	assertLines(t, result.Stdout(), nil)
+}
diff --git a/e2e/select_test.go b/e2e/select_test.go
@@ -153,7 +153,7 @@ func TestSelectCommand(t *testing.T) {
 				expectedValue: "id0\n",
 			},
 			{
-				name: "input:json-lines,output:csv,compression:gzip,select:with-where",
+				name: "input:json-lines,output:csv,select:with-where",
 				cmd: []string{
 					"select", "json",
 					"--output-format", "csv",
@@ -200,11 +200,11 @@ func TestSelectCommand(t *testing.T) {
 				},
 				informat:      "json",
 				structure:     "document",
-				outformat:     "json",
+				outformat:     "csv",
 				expectedValue: "id0\n",
 			},
 			{
-				name: "input:json-document,output:json,compression:gzip,select:with-document-access",
+				name: "input:json-document,output:csv,compression:gzip,select:with-document-access",
 				cmd: []string{
 					"select", "json",
 					"--structure", "document",
@@ -215,13 +215,24 @@ func TestSelectCommand(t *testing.T) {
 				informat:      "json",
 				compression:   true,
 				structure:     "document",
-				outformat:     "json",
+				outformat:     "csv",
 				expectedValue: "id0\n",
 			},
 		},
 		"csv": {
 			{
-				name: "input:csv,output:csv,delimiter:comma",
+				name: "input:csv,output:json,delimiter:comma",
+				cmd: []string{
+					"select", "csv",
+					"--output-format", "json",
+					"--query", query,
+				},
+				informat:  "csv",
+				structure: ",",
+				outformat: "json",
+			},
+			{
+				name: "input:csv,output:csv,delimiter:comma,extra-flag:false",
 				cmd: []string{
 					"select", "csv",
 					"--query", query,
@@ -231,7 +242,7 @@ func TestSelectCommand(t *testing.T) {
 				outformat: "csv",
 			},
 			{
-				name: "input:csv,output:csv,delimiter:comma,compression:gzip",
+				name: "input:csv,output:csv,delimiter:comma,compression:gzip,extra-flag:false",
 				cmd: []string{
 					"select", "csv",
 					"--compression", "gzip",
@@ -242,6 +253,19 @@ func TestSelectCommand(t *testing.T) {
 				structure:   ",",
 				outformat:   "csv",
 			},
+			{
+				name: "input:csv,output:json,delimiter:comma,compression:gzip",
+				cmd: []string{
+					"select", "csv",
+					"--compression", "gzip",
+					"--output-format", "json",
+					"--query", query,
+				},
+				informat:    "csv",
+				compression: true,
+				structure:   ",",
+				outformat:   "json",
+			},
 			{
 				name: "input:csv,output:csv,delimiter:comma,compression:gzip",
 				cmd: []string{
@@ -267,7 +291,7 @@ func TestSelectCommand(t *testing.T) {
 				outformat: "csv",
 			},
 			{
-				name: "input:csv,output:csv,delimiter:tab",
+				name: "input:csv,output:csv,delimiter:tab,extra-flag:false",
 				cmd: []string{
 					"select", "csv",
 					"--delimiter", "\t",
@@ -277,6 +301,18 @@ func TestSelectCommand(t *testing.T) {
 				structure: "\t",
 				outformat: "csv",
 			},
+			{
+				name: "input:csv,output:json,delimiter:tab",
+				cmd: []string{
+					"select", "csv",
+					"--delimiter", "\t",
+					"--output-format", "json",
+					"--query", query,
+				},
+				informat:  "csv",
+				structure: "\t",
+				outformat: "json",
+			},
 			{
 				name: "input:csv,output:csv,delimiter:tab",
 				cmd: []string{
@@ -372,7 +408,7 @@ func TestSelectCommand(t *testing.T) {
 				informat:      "csv",
 				compression:   true,
 				structure:     ",",
-				outformat:     "csv",
+				outformat:     "json",
 				expectedValue: "{\"id\":\"id0\"}\n",
 			},
 			{
@@ -386,7 +422,7 @@ func TestSelectCommand(t *testing.T) {
 				},
 				informat:      "csv",
 				structure:     "\t",
-				outformat:     "csv",
+				outformat:     "json",
 				expectedValue: "{\"id\":\"id0\"}\n",
 			},
 			{
@@ -402,7 +438,7 @@ func TestSelectCommand(t *testing.T) {
 				informat:      "csv",
 				compression:   true,
 				structure:     "\t",
-				outformat:     "csv",
+				outformat:     "json",
 				expectedValue: "{\"id\":\"id0\"}\n",
 			},
 		},
@@ -471,6 +507,7 @@ func TestSelectCommand(t *testing.T) {
 
 					s3client, s5cmd := setup(t, withEndpointURL(endpoint), withRegion(region), withAccessKeyID(accessKeyID), withSecretKey(secretKey))
 					createBucket(t, s3client, bucket)
+
 					putFile(t, s3client, bucket, filename, contents)
 
 					tc.cmd = append(tc.cmd, src)
@@ -486,6 +523,68 @@ func TestSelectCommand(t *testing.T) {
 	}
 }
 
+func TestSelectCommandEmptyBucket(t *testing.T) {
+	t.Parallel()
+
+	const (
+		region      = "us-east-1"
+		accessKeyID = "minioadmin"
+		secretKey   = "minioadmin"
+
+		query = "SELECT * FROM s3object s"
+	)
+
+	endpoint := os.Getenv(s5cmdTestEndpointEnv)
+	if endpoint == "" {
+		t.Skipf("skipping the test because %v environment variable is empty", s5cmdTestEndpointEnv)
+	}
+
+	type testcase struct {
+		name string
+		cmd  []string
+	}
+
+	testcases := []testcase{
+		{
+			name: "input:json-lines,output:json,all-versions:true,empty-bucket:true",
+			cmd: []string{
+				"select", "json",
+				"--all-versions",
+				"--query", query,
+			},
+		},
+		{
+			name: "input:csv,output:csv,delimiter:comma,all-versions:true,empty-bucket:true",
+			cmd: []string{
+				"select", "csv",
+				"--all-versions",
+				"--query", query,
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			bucket := s3BucketFromTestName(t)
+
+			s3client, s5cmd := setup(t, withEndpointURL(endpoint), withRegion(region), withAccessKeyID(accessKeyID), withSecretKey(secretKey))
+			createBucket(t, s3client, bucket)
+
+			src := fmt.Sprintf("s3://%s/", bucket)
+			tc.cmd = append(tc.cmd, src)
+			cmd := s5cmd(tc.cmd...)
+
+			result := icmd.RunCmd(cmd, withEnv("AWS_ACCESS_KEY_ID", accessKeyID), withEnv("AWS_SECRET_ACCESS_KEY", secretKey))
+
+			result.Assert(t, icmd.Success)
+			assert.DeepEqual(t, "", result.Stdout())
+		})
+	}
+}
+
 func TestSelectWithParquet(t *testing.T) {
 	// NOTE(deniz): We are skipping this test until the image we use in the
 	// service container releases parquet support for select api.

diff --git a/storage/s3.go b/storage/s3.go
@@ -288,7 +288,7 @@ func (s *S3) listObjectVersions(ctx context.Context, url *url.URL) <-chan *Objec
 			return
 		}
 
-		if !objectFound {
+		if !objectFound && !url.IsBucket() {
 			objCh <- &Object{Err: ErrNoObjectFound}
 		}
 	}()
@@ -378,7 +378,7 @@ func (s *S3) listObjectsV2(ctx context.Context, url *url.URL) <-chan *Object {
 			return
 		}
 
-		if !objectFound {
+		if !objectFound && !url.IsBucket() {
 			objCh <- &Object{Err: ErrNoObjectFound}
 		}
 	}()
@@ -470,7 +470,7 @@ func (s *S3) listObjects(ctx context.Context, url *url.URL) <-chan *Object {
 			return
 		}
 
-		if !objectFound {
+		if !objectFound && !url.IsBucket() {
 			objCh <- &Object{Err: ErrNoObjectFound}
 		}
 	}()