diff options
author | John Cai <jcai@gitlab.com> | 2022-02-23 18:20:48 +0300 |
---|---|---|
committer | Toon Claes <toon@gitlab.com> | 2022-02-25 17:41:06 +0300 |
commit | 191663b47e837b223135999b87c19d401f219fe4 (patch) | |
tree | 2422dacdea3719999054b809a2d444c0f871dbc5 /internal/gitaly/service | |
parent | 1e442ff0382ed147e7bc4682cd9c278a56163a42 (diff) |
git: Refactor CheckRefFormat
CheckRefFormat calls git-check-ref-format(1) and if there is an error
from the command, returns a separate CheckRefFormat error. This doesn't
help much however. It swallows up the context of the original error. But
also, the implementation of git-check-ref-format(1) is such that if the
format is not valid, it will return with an exit code of 1.
Clean up the interface of this function by instead, checking if the
exit code of git-check-ref-format(1) is 1. If it is, then we know the
format is invalid. If the exit code is anything else, something went
wrong so we bubble up the error to be handled by the caller. We can also
get rid of CheckRefFormatError.
Update the one callsite that checks for CheckRefFormatError
Diffstat (limited to 'internal/gitaly/service')
-rw-r--r-- | internal/gitaly/service/operations/tags.go | 33 |
1 files changed, 16 insertions, 17 deletions
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go index cb36bdb29..68acb036a 100644 --- a/internal/gitaly/service/operations/tags.go +++ b/internal/gitaly/service/operations/tags.go @@ -121,6 +121,14 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR var updateRefError updateref.Error if errors.As(err, &updateRefError) { refNameOK, err := git.CheckRefFormat(ctx, s.gitCmdFactory, referenceName.String()) + if err != nil { + // Should only be reachable if "git + // check-ref-format"'s invocation is + // incorrect, or if it segfaults on startup + // etc. + return nil, status.Error(codes.Internal, err.Error()) + } + if refNameOK { // The tag might not actually exist, // perhaps update-ref died for some @@ -134,23 +142,14 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR }, nil } - var CheckRefFormatError git.CheckRefFormatError - if errors.As(err, &CheckRefFormatError) { - // It doesn't make sense either to - // tell the user to retry with an - // invalid ref name, but ditto on the - // Ruby bug-for-bug emulation. - return &gitalypb.UserCreateTagResponse{ - Tag: nil, - Exists: true, - }, status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", req.TagName) - } - - // Should only be reachable if "git - // check-ref-format"'s invocation is - // incorrect, or if it segfaults on startup - // etc. - return nil, status.Error(codes.Internal, err.Error()) + // It doesn't make sense either to + // tell the user to retry with an + // invalid ref name, but ditto on the + // Ruby bug-for-bug emulation. + return &gitalypb.UserCreateTagResponse{ + Tag: nil, + Exists: true, + }, status.Errorf(codes.Unknown, "Gitlab::Git::CommitError: Could not update refs/tags/%s. Please refresh and try again.", req.TagName) } // The Ruby code did not return this, but always an |