From 4067e01561e16832b12dd21abe197a00d08b8a7f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 13 Jan 2021 17:10:58 +0100 Subject: git: Rename SafeBareCmd() to NewCommandWithoutRepository() Back when we introduced the Git DSL, we still had conflicting sets of safe and unsafe functions. Because of this legacy, our safe set of functions is still has the "Safe" prefix. This commit now ends that chapter and renames `SafeBareCmd()`. The previous name has been quite confusing, given that "bare" in the world of git typically means a repository without a working directory. That was not the intended meaning though, but instead that the command is not run with an associated repository. It's thus renamed to `NewCommandWithoutRepository()`. --- cmd/gitaly-ssh/upload_pack_test.go | 2 +- internal/git/objectpool/clone.go | 2 +- internal/git/objectpool/pool.go | 2 +- internal/git/reference.go | 2 +- internal/git/safecmd.go | 8 +++++--- internal/git/safecmd_test.go | 2 +- internal/gitaly/service/operations/squash.go | 2 +- internal/gitaly/service/remote/remotes.go | 2 +- internal/gitaly/service/repository/clone_from_pool_internal.go | 2 +- internal/gitaly/service/repository/create.go | 2 +- internal/gitaly/service/repository/create_from_bundle.go | 4 ++-- internal/gitaly/service/repository/create_from_url.go | 2 +- internal/gitaly/service/repository/fork.go | 2 +- internal/gitaly/service/smarthttp/inforefs.go | 2 +- internal/gitaly/service/smarthttp/receive_pack.go | 2 +- internal/gitaly/service/smarthttp/upload_pack.go | 2 +- internal/gitaly/service/ssh/monitor_stdin_command.go | 2 +- internal/gitaly/service/ssh/receive_pack.go | 2 +- 18 files changed, 23 insertions(+), 21 deletions(-) diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go index 2a6b7e677..126561bed 100644 --- a/cmd/gitaly-ssh/upload_pack_test.go +++ b/cmd/gitaly-ssh/upload_pack_test.go @@ -86,7 +86,7 @@ func TestVisibilityOfHiddenRefs(t *testing.T) { } stdout := &bytes.Buffer{} - cmd, err := git.SafeBareCmd(ctx, nil, git.SubCmd{ + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "ls-remote", Args: []string{ fmt.Sprintf("%s:%s", "git@localhost", testRepoPath), diff --git a/internal/git/objectpool/clone.go b/internal/git/objectpool/clone.go index 255018482..dd7b60350 100644 --- a/internal/git/objectpool/clone.go +++ b/internal/git/objectpool/clone.go @@ -17,7 +17,7 @@ func (o *ObjectPool) clone(ctx context.Context, repo *gitalypb.Repository) error return err } - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "clone", Flags: []git.Option{ diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go index 6e2fbf64f..9e6ad046c 100644 --- a/internal/git/objectpool/pool.go +++ b/internal/git/objectpool/pool.go @@ -124,7 +124,7 @@ func (o *ObjectPool) Init(ctx context.Context) (err error) { return nil } - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "init", Flags: []git.Option{ diff --git a/internal/git/reference.go b/internal/git/reference.go index 12a2eb828..269f3db69 100644 --- a/internal/git/reference.go +++ b/internal/git/reference.go @@ -48,7 +48,7 @@ func CheckRefFormat(ctx context.Context, refName string) (bool, error) { stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} - cmd, err := SafeBareCmd(ctx, nil, + cmd, err := NewCommandWithoutRepo(ctx, nil, SubCmd{ Name: "check-ref-format", Args: []string{refName}, diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go index 79e81691e..8c4e29dce 100644 --- a/internal/git/safecmd.go +++ b/internal/git/safecmd.go @@ -374,9 +374,11 @@ func SafeCmd(ctx context.Context, repo repository.GitRepo, globals []GlobalOptio }, repo, args...) } -// SafeBareCmd creates a git.Command with the given args. It -// validates the arguments in the command before executing. -func SafeBareCmd(ctx context.Context, globals []GlobalOption, sc Cmd, opts ...CmdOpt) (*command.Command, error) { +// NewCommandWithoutRepo creates a command.Command with the given args. It is not +// connected to any specific git repository. It should only be used for +// commands which do not require a git repository or which accept a repository +// path as parameter like e.g. git-upload-pack(1). +func NewCommandWithoutRepo(ctx context.Context, globals []GlobalOption, sc Cmd, opts ...CmdOpt) (*command.Command, error) { cc := &cmdCfg{} if err := handleOpts(ctx, sc, cc, opts); err != nil { diff --git a/internal/git/safecmd_test.go b/internal/git/safecmd_test.go index 26099a2bd..3b0b2ba2d 100644 --- a/internal/git/safecmd_test.go +++ b/internal/git/safecmd_test.go @@ -340,7 +340,7 @@ func TestSafeCmdValid(t *testing.T) { // ignore first 3 indeterministic args (executable path and repo args) require.Equal(t, tt.expectArgs, cmd.Args()[3:]) - cmd, err = SafeBareCmd(ctx, tt.globals, tt.subCmd, opts...) + cmd, err = NewCommandWithoutRepo(ctx, tt.globals, tt.subCmd, opts...) require.NoError(t, err) // ignore first indeterministic arg (executable path) require.Equal(t, tt.expectArgs, cmd.Args()[1:]) diff --git a/internal/gitaly/service/operations/squash.go b/internal/gitaly/service/operations/squash.go index d5a5fc89f..77105d99e 100644 --- a/internal/gitaly/service/operations/squash.go +++ b/internal/gitaly/service/operations/squash.go @@ -167,7 +167,7 @@ func (s *Server) runUserSquashGo(ctx context.Context, req *gitalypb.UserSquashRe func (s *Server) diffFiles(ctx context.Context, env []string, repoPath string, req *gitalypb.UserSquashRequest) ([]byte, error) { var stdout, stderr bytes.Buffer - cmd, err := git.SafeBareCmd(ctx, + cmd, err := git.NewCommandWithoutRepo(ctx, []git.GlobalOption{git.ValueFlag{Name: "--git-dir", Value: repoPath}}, git.SubCmd{ Name: "diff", diff --git a/internal/gitaly/service/remote/remotes.go b/internal/gitaly/service/remote/remotes.go index 7bb8eeae2..31489dad0 100644 --- a/internal/gitaly/service/remote/remotes.go +++ b/internal/gitaly/service/remote/remotes.go @@ -75,7 +75,7 @@ func (s *server) FindRemoteRepository(ctx context.Context, req *gitalypb.FindRem return nil, status.Error(codes.InvalidArgument, "FindRemoteRepository: empty remote can't be checked.") } - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "ls-remote", Args: []string{ diff --git a/internal/gitaly/service/repository/clone_from_pool_internal.go b/internal/gitaly/service/repository/clone_from_pool_internal.go index 0609c9d73..53b212a57 100644 --- a/internal/gitaly/service/repository/clone_from_pool_internal.go +++ b/internal/gitaly/service/repository/clone_from_pool_internal.go @@ -125,7 +125,7 @@ func (s *server) cloneFromPool(ctx context.Context, objectPoolRepo *gitalypb.Obj return fmt.Errorf("expected *gitlaypb.Repository but got %T", repo) } - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "clone", Flags: []git.Option{git.Flag{Name: "--bare"}, git.Flag{Name: "--shared"}}, diff --git a/internal/gitaly/service/repository/create.go b/internal/gitaly/service/repository/create.go index 13bfdf5f4..ca6f201cc 100644 --- a/internal/gitaly/service/repository/create.go +++ b/internal/gitaly/service/repository/create.go @@ -21,7 +21,7 @@ func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepos } stderr := &bytes.Buffer{} - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "init", Flags: []git.Option{ diff --git a/internal/gitaly/service/repository/create_from_bundle.go b/internal/gitaly/service/repository/create_from_bundle.go index 36e0e593b..858a50a00 100644 --- a/internal/gitaly/service/repository/create_from_bundle.go +++ b/internal/gitaly/service/repository/create_from_bundle.go @@ -71,7 +71,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr } stderr := bytes.Buffer{} - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "clone", Flags: []git.Option{ @@ -94,7 +94,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr // We do a fetch to get all refs including keep-around refs stderr.Reset() - cmd, err = git.SafeBareCmd(ctx, + cmd, err = git.NewCommandWithoutRepo(ctx, []git.GlobalOption{git.ValueFlag{Name: "-C", Value: repoPath}}, git.SubCmd{ Name: "fetch", diff --git a/internal/gitaly/service/repository/create_from_url.go b/internal/gitaly/service/repository/create_from_url.go index 3bbbcab9c..10948c86e 100644 --- a/internal/gitaly/service/repository/create_from_url.go +++ b/internal/gitaly/service/repository/create_from_url.go @@ -47,7 +47,7 @@ func (s *server) cloneFromURLCommand(ctx context.Context, repo *gitalypb.Reposit globalFlags = append(globalFlags, git.ConfigPair{Key: "http.extraHeader", Value: authHeader}) } - return git.SafeBareCmd(ctx, globalFlags, + return git.NewCommandWithoutRepo(ctx, globalFlags, git.SubCmd{ Name: "clone", Flags: cloneFlags, diff --git a/internal/gitaly/service/repository/fork.go b/internal/gitaly/service/repository/fork.go index a9948fe7b..721b07c2f 100644 --- a/internal/gitaly/service/repository/fork.go +++ b/internal/gitaly/service/repository/fork.go @@ -53,7 +53,7 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest return nil, err } - cmd, err := git.SafeBareCmd(ctx, nil, + cmd, err := git.NewCommandWithoutRepo(ctx, nil, git.SubCmd{ Name: "clone", Flags: []git.Option{ diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go index 3b3646f8a..2ec6b3269 100644 --- a/internal/gitaly/service/smarthttp/inforefs.go +++ b/internal/gitaly/service/smarthttp/inforefs.go @@ -58,7 +58,7 @@ func (s *server) handleInfoRefs(ctx context.Context, service string, req *gitaly globalOpts[i] = git.ValueFlag{"-c", o} } - cmd, err := git.SafeBareCmd(ctx, globalOpts, git.SubCmd{ + cmd, err := git.NewCommandWithoutRepo(ctx, globalOpts, git.SubCmd{ Name: service, Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}, git.Flag{Name: "--advertise-refs"}}, Args: []string{repoPath}, diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go index b7d46205a..f03d4736e 100644 --- a/internal/gitaly/service/smarthttp/receive_pack.go +++ b/internal/gitaly/service/smarthttp/receive_pack.go @@ -48,7 +48,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac globalOpts[i] = git.ValueFlag{"-c", o} } - cmd, err := git.SafeBareCmd(ctx, globalOpts, + cmd, err := git.NewCommandWithoutRepo(ctx, globalOpts, git.SubCmd{ Name: "receive-pack", Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}}, diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index f57a84715..ec124549a 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -77,7 +77,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS globalOpts[i] = git.ValueFlag{"-c", o} } - cmd, err := git.SafeBareCmd(ctx, globalOpts, git.SubCmd{ + cmd, err := git.NewCommandWithoutRepo(ctx, globalOpts, git.SubCmd{ Name: "upload-pack", Flags: []git.Option{git.Flag{Name: "--stateless-rpc"}}, Args: []string{repoPath}, diff --git a/internal/gitaly/service/ssh/monitor_stdin_command.go b/internal/gitaly/service/ssh/monitor_stdin_command.go index a21c4d223..f9fea54c0 100644 --- a/internal/gitaly/service/ssh/monitor_stdin_command.go +++ b/internal/gitaly/service/ssh/monitor_stdin_command.go @@ -16,7 +16,7 @@ func monitorStdinCommand(ctx context.Context, stdin io.Reader, stdout, stderr io return nil, nil, fmt.Errorf("create monitor: %v", err) } - cmd, err := git.SafeBareCmd(ctx, globals, sc, append([]git.CmdOpt{ + cmd, err := git.NewCommandWithoutRepo(ctx, globals, sc, append([]git.CmdOpt{ git.WithStdin(stdinPipe), git.WithStdout(stdout), git.WithStderr(stderr), }, opts...)...) stdinPipe.Close() // this now belongs to cmd diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go index 54a779894..328aa141f 100644 --- a/internal/gitaly/service/ssh/receive_pack.go +++ b/internal/gitaly/service/ssh/receive_pack.go @@ -66,7 +66,7 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, globalOpts[i] = git.ValueFlag{"-c", o} } - cmd, err := git.SafeBareCmd(ctx, globalOpts, + cmd, err := git.NewCommandWithoutRepo(ctx, globalOpts, git.SubCmd{ Name: "receive-pack", Args: []string{repoPath}, -- cgit v1.2.3