Determine when CreateBundleFromRefList would generate an empty bundle

In order to gracefully handle incremental backups where nothing has
changed, we need to determine when `git bundle create` failed because
the bundle was empty. This isn't an error for incremental backups, it
simply means there were no updates.

The FailedPrecondition GRPC code was chosen here since it best matches
the litmus test:

>  (c) Use FailedPrecondition if the client should not retry until
>      the system state has been explicitly fixed. E.g., if an "rmdir"
>      fails because the directory is non-empty, FailedPrecondition
>      should be returned since the client should not retry unless
>      they have first fixed up the directory by deleting files from it.

Changelog: changed

5 files changed, 41 insertions, 15 deletions

diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
index 2829b4da7..6bc15efd1 100644
--- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go
+++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
@@ -68,7 +68,10 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat
 		return status.Errorf(codes.Internal, "stream writer failed: %v", err)
 	}
 
-	if err := cmd.Wait(); err != nil {
+	err = cmd.Wait()
+	if isExitWithCode(err, 128) && bytes.HasPrefix(stderr.Bytes(), []byte("fatal: Refusing to create empty bundle.")) {
+		return status.Errorf(codes.FailedPrecondition, "cmd wait failed: refusing to create empty bundle")
+	} else if err != nil {
 		return status.Errorf(codes.Internal, "cmd wait failed: %v, stderr: %q", err, stderr.String())
 	}
 
diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go b/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go
index 2174d891d..382fce605 100644
--- a/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go
+++ b/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go
@@ -71,17 +71,30 @@ func TestCreateBundleFromRefList_success(t *testing.T) {
 
 func TestCreateBundleFromRefList_validations(t *testing.T) {
 	t.Parallel()
-	_, client := setupRepositoryServiceWithoutRepo(t)
+	_, repo, _, client := setupRepositoryService(t)
 
 	testCases := []struct {
-		desc    string
-		request *gitalypb.CreateBundleFromRefListRequest
-		code    codes.Code
+		desc         string
+		request      *gitalypb.CreateBundleFromRefListRequest
+		expectedErr  string
+		expectedCode codes.Code
 	}{
 		{
-			desc:    "empty repository",
-			request: &gitalypb.CreateBundleFromRefListRequest{Patterns: [][]byte{[]byte("master")}},
-			code:    codes.InvalidArgument,
+			desc: "empty repository",
+			request: &gitalypb.CreateBundleFromRefListRequest{
+				Patterns: [][]byte{[]byte("master")},
+			},
+			expectedErr:  "empty Repository",
+			expectedCode: codes.InvalidArgument,
+		},
+		{
+			desc: "empty bundle",
+			request: &gitalypb.CreateBundleFromRefListRequest{
+				Repository: repo,
+				Patterns:   [][]byte{[]byte("master"), []byte("^master")},
+			},
+			expectedErr:  "cmd wait failed: refusing to create empty bundle",
+			expectedCode: codes.FailedPrecondition,
 		},
 	}
 
@@ -96,9 +109,15 @@ func TestCreateBundleFromRefList_validations(t *testing.T) {
 			require.NoError(t, stream.Send(testCase.request))
 			require.NoError(t, stream.CloseSend())
 
-			_, err = stream.Recv()
-			require.NotEqual(t, io.EOF, err)
-			testhelper.RequireGrpcError(t, err, testCase.code)
+			for {
+				_, err = stream.Recv()
+				if err != nil {
+					break
+				}
+			}
+			require.Error(t, err)
+			require.Contains(t, err.Error(), testCase.expectedErr)
+			testhelper.RequireGrpcError(t, err, testCase.expectedCode)
 		})
 	}
 }
diff --git a/proto/go/gitalypb/repository-service_grpc.pb.go b/proto/go/gitalypb/repository-service_grpc.pb.go
index 6f434d4d1..50da376fa 100644
--- a/proto/go/gitalypb/repository-service_grpc.pb.go
+++ b/proto/go/gitalypb/repository-service_grpc.pb.go
@@ -42,7 +42,8 @@ type RepositoryServiceClient interface {
 	CreateRepositoryFromURL(ctx context.Context, in *CreateRepositoryFromURLRequest, opts ...grpc.CallOption) (*CreateRepositoryFromURLResponse, error)
 	// CreateBundle creates a bundle from all refs
 	CreateBundle(ctx context.Context, in *CreateBundleRequest, opts ...grpc.CallOption) (RepositoryService_CreateBundleClient, error)
-	// CreateBundleFromRefList creates a bundle from a stream of ref patterns
+	// CreateBundleFromRefList creates a bundle from a stream of ref patterns.
+	// When the bundle would be empty the FailedPrecondition error code is returned.
 	CreateBundleFromRefList(ctx context.Context, opts ...grpc.CallOption) (RepositoryService_CreateBundleFromRefListClient, error)
 	// FetchBundle fetches references from a bundle into the local repository.
 	// Refs will be mirrored to the target repository with the refspec
@@ -798,7 +799,8 @@ type RepositoryServiceServer interface {
 	CreateRepositoryFromURL(context.Context, *CreateRepositoryFromURLRequest) (*CreateRepositoryFromURLResponse, error)
 	// CreateBundle creates a bundle from all refs
 	CreateBundle(*CreateBundleRequest, RepositoryService_CreateBundleServer) error
-	// CreateBundleFromRefList creates a bundle from a stream of ref patterns
+	// CreateBundleFromRefList creates a bundle from a stream of ref patterns.
+	// When the bundle would be empty the FailedPrecondition error code is returned.
 	CreateBundleFromRefList(RepositoryService_CreateBundleFromRefListServer) error
 	// FetchBundle fetches references from a bundle into the local repository.
 	// Refs will be mirrored to the target repository with the refspec
diff --git a/proto/repository-service.proto b/proto/repository-service.proto
index 33ac2ebf0..61416421f 100644
--- a/proto/repository-service.proto
+++ b/proto/repository-service.proto
@@ -113,7 +113,8 @@ service RepositoryService {
     };
   }
 
-  // CreateBundleFromRefList creates a bundle from a stream of ref patterns
+  // CreateBundleFromRefList creates a bundle from a stream of ref patterns.
+  // When the bundle would be empty the FailedPrecondition error code is returned.
   rpc CreateBundleFromRefList(stream CreateBundleFromRefListRequest) returns (stream CreateBundleFromRefListResponse) {
     option (op_type) = {
       op: ACCESSOR
diff --git a/ruby/proto/gitaly/repository-service_services_pb.rb b/ruby/proto/gitaly/repository-service_services_pb.rb
index 68de3267c..f7451e2fe 100644
--- a/ruby/proto/gitaly/repository-service_services_pb.rb
+++ b/ruby/proto/gitaly/repository-service_services_pb.rb
@@ -38,7 +38,8 @@ module Gitaly
       rpc :CreateRepositoryFromURL, Gitaly::CreateRepositoryFromURLRequest, Gitaly::CreateRepositoryFromURLResponse
       # CreateBundle creates a bundle from all refs
       rpc :CreateBundle, Gitaly::CreateBundleRequest, stream(Gitaly::CreateBundleResponse)
-      # CreateBundleFromRefList creates a bundle from a stream of ref patterns
+      # CreateBundleFromRefList creates a bundle from a stream of ref patterns.
+      # When the bundle would be empty the FailedPrecondition error code is returned.
       rpc :CreateBundleFromRefList, stream(Gitaly::CreateBundleFromRefListRequest), stream(Gitaly::CreateBundleFromRefListResponse)
       # FetchBundle fetches references from a bundle into the local repository.
       # Refs will be mirrored to the target repository with the refspec