diff options
44 files changed, 613 insertions, 574 deletions
diff --git a/internal/git/config.go b/internal/git/config.go index 5171d573b..e824fc8fd 100644 --- a/internal/git/config.go +++ b/internal/git/config.go @@ -53,15 +53,6 @@ type ConfigAddOpts struct { Type ConfigType } -func (opts ConfigAddOpts) buildFlags() []Option { - var flags []Option - if opts.Type != ConfigTypeDefault { - flags = append(flags, Flag{Name: opts.Type.String()}) - } - - return flags -} - // ConfigGetRegexpOpts is used to configure invocation of the 'git config --get-regexp' command. type ConfigGetRegexpOpts struct { // Type allows to specify an expected type for the configuration. @@ -72,23 +63,6 @@ type ConfigGetRegexpOpts struct { ShowScope bool } -func (opts ConfigGetRegexpOpts) buildFlags() []Option { - var flags []Option - if opts.Type != ConfigTypeDefault { - flags = append(flags, Flag{Name: opts.Type.String()}) - } - - if opts.ShowOrigin { - flags = append(flags, Flag{Name: "--show-origin"}) - } - - if opts.ShowScope { - flags = append(flags, Flag{Name: "--show-scope"}) - } - - return flags -} - // ConfigUnsetOpts allows to configure fetching of the configurations using regexp. type ConfigUnsetOpts struct { // All controls if all values associated with the key needs to be unset. @@ -97,11 +71,3 @@ type ConfigUnsetOpts struct { // or in case multiple values exist for a given key and All option is not set. NotStrict bool } - -func (opts ConfigUnsetOpts) buildFlags() []Option { - if opts.All { - return []Option{Flag{Name: "--unset-all"}} - } - - return []Option{Flag{Name: "--unset"}} -} diff --git a/internal/git/config_test.go b/internal/git/config_test.go deleted file mode 100644 index da59ad3b7..000000000 --- a/internal/git/config_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package git - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestConfigAddOpts_buildFlags(t *testing.T) { - for _, tc := range []struct { - desc string - opts ConfigAddOpts - exp []Option - }{ - { - desc: "none", - opts: ConfigAddOpts{}, - exp: nil, - }, - { - desc: "all set", - opts: ConfigAddOpts{Type: ConfigTypeBoolOrInt}, - exp: []Option{Flag{Name: "--bool-or-int"}}, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - require.Equal(t, tc.exp, tc.opts.buildFlags()) - }) - } -} - -func TestConfigGetRegexpOpts_buildFlags(t *testing.T) { - for _, tc := range []struct { - desc string - opts ConfigGetRegexpOpts - exp []Option - }{ - { - desc: "none", - opts: ConfigGetRegexpOpts{}, - exp: nil, - }, - { - desc: "all set", - opts: ConfigGetRegexpOpts{Type: ConfigTypeInt, ShowOrigin: true, ShowScope: true}, - exp: []Option{ - Flag{Name: "--int"}, - Flag{Name: "--show-origin"}, - Flag{Name: "--show-scope"}, - }, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - require.Equal(t, tc.exp, tc.opts.buildFlags()) - }) - } -} - -func TestConfigUnsetOpts_buildFlags(t *testing.T) { - for _, tc := range []struct { - desc string - opts ConfigUnsetOpts - exp []Option - }{ - { - desc: "none", - opts: ConfigUnsetOpts{}, - exp: []Option{Flag{Name: "--unset"}}, - }, - { - desc: "all set", - opts: ConfigUnsetOpts{All: true, NotStrict: true}, - exp: []Option{Flag{Name: "--unset-all"}}, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - require.Equal(t, tc.exp, tc.opts.buildFlags()) - }) - } -} diff --git a/internal/git/localrepo.go b/internal/git/localrepo.go deleted file mode 100644 index 1519d690e..000000000 --- a/internal/git/localrepo.go +++ /dev/null @@ -1,47 +0,0 @@ -package git - -import ( - "context" - "fmt" - - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" -) - -// LocalRepository represents a local Git repository. -type LocalRepository struct { - repository.GitRepo - commandFactory *ExecCommandFactory - cfg config.Cfg -} - -// NewRepository creates a new Repository from its protobuf representation. -func NewRepository(repo repository.GitRepo, cfg config.Cfg) *LocalRepository { - return &LocalRepository{ - GitRepo: repo, - cfg: cfg, - commandFactory: NewExecCommandFactory(cfg), - } -} - -// command creates a Git Command with the given args and Repository, executed -// in the Repository. It validates the arguments in the command before -// executing. -func (repo *LocalRepository) command(ctx context.Context, globals []GlobalOption, cmd Cmd, opts ...CmdOpt) (*command.Command, error) { - return repo.commandFactory.newCommand(ctx, repo, "", globals, cmd, opts...) -} - -// Config returns executor of the 'config' sub-command. -func (repo *LocalRepository) Config() Config { - return LocalRepositoryConfig{repo: repo} -} - -// Remote returns executor of the 'remote' sub-command. -func (repo *LocalRepository) Remote() Remote { - return LocalRepositoryRemote{repo: repo} -} - -func errorWithStderr(err error, stderr []byte) error { - return fmt.Errorf("%w, stderr: %q", err, stderr) -} diff --git a/internal/git/localrepo_config.go b/internal/git/localrepo/config.go index fea2e4fed..911d6cc66 100644 --- a/internal/git/localrepo_config.go +++ b/internal/git/localrepo/config.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "bufio" @@ -10,22 +10,23 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" ) -// LocalRepositoryConfig provides functionality of the 'config' git sub-command. -type LocalRepositoryConfig struct { - repo *LocalRepository +// Config provides functionality of the 'config' git sub-command. +type Config struct { + repo *Repo } // Add adds a new entry to the repository's configuration. -func (cfg LocalRepositoryConfig) Add(ctx context.Context, name, value string, opts ConfigAddOpts) error { +func (cfg Config) Add(ctx context.Context, name, value string, opts git.ConfigAddOpts) error { if err := validateNotBlank(name, "name"); err != nil { return err } - cmd, err := cfg.repo.command(ctx, nil, SubCmd{ + cmd, err := cfg.repo.command(ctx, nil, git.SubCmd{ Name: "config", - Flags: append(opts.buildFlags(), Flag{Name: "--add"}), + Flags: append(buildConfigAddOptsFlags(opts), git.Flag{Name: "--add"}), Args: []string{name, value}, }) if err != nil { @@ -37,10 +38,10 @@ func (cfg LocalRepositoryConfig) Add(ctx context.Context, name, value string, op switch { case isExitWithCode(err, 1): // section or key is invalid - return fmt.Errorf("%w: bad section or name", ErrInvalidArg) + return fmt.Errorf("%w: bad section or name", git.ErrInvalidArg) case isExitWithCode(err, 2): // no section or name was provided - return fmt.Errorf("%w: missing section or name", ErrInvalidArg) + return fmt.Errorf("%w: missing section or name", git.ErrInvalidArg) } return err @@ -49,8 +50,17 @@ func (cfg LocalRepositoryConfig) Add(ctx context.Context, name, value string, op return nil } +func buildConfigAddOptsFlags(opts git.ConfigAddOpts) []git.Option { + var flags []git.Option + if opts.Type != git.ConfigTypeDefault { + flags = append(flags, git.Flag{Name: opts.Type.String()}) + } + + return flags +} + // GetRegexp gets all config entries which whose keys match the given regexp. -func (cfg LocalRepositoryConfig) GetRegexp(ctx context.Context, nameRegexp string, opts ConfigGetRegexpOpts) ([]ConfigPair, error) { +func (cfg Config) GetRegexp(ctx context.Context, nameRegexp string, opts git.ConfigGetRegexpOpts) ([]git.ConfigPair, error) { if err := validateNotBlank(nameRegexp, "nameRegexp"); err != nil { return nil, err } @@ -63,16 +73,16 @@ func (cfg LocalRepositoryConfig) GetRegexp(ctx context.Context, nameRegexp strin return cfg.parseConfig(data, opts) } -func (cfg LocalRepositoryConfig) getRegexp(ctx context.Context, nameRegexp string, opts ConfigGetRegexpOpts) ([]byte, error) { +func (cfg Config) getRegexp(ctx context.Context, nameRegexp string, opts git.ConfigGetRegexpOpts) ([]byte, error) { var stderr bytes.Buffer cmd, err := cfg.repo.command(ctx, nil, - SubCmd{ + git.SubCmd{ Name: "config", // '--null' is used to support proper parsing of the multiline config values - Flags: append(opts.buildFlags(), Flag{Name: "--null"}, Flag{Name: "--get-regexp"}), + Flags: append(buildConfigGetRegexpOptsFlags(opts), git.Flag{Name: "--null"}, git.Flag{Name: "--get-regexp"}), Args: []string{nameRegexp}, }, - WithStderr(&stderr), + git.WithStderr(&stderr), ) if err != nil { return nil, err @@ -90,10 +100,10 @@ func (cfg LocalRepositoryConfig) getRegexp(ctx context.Context, nameRegexp strin return nil, nil case isExitWithCode(err, 6): // use of invalid regexp - return nil, fmt.Errorf("%w: regexp has a bad format", ErrInvalidArg) + return nil, fmt.Errorf("%w: regexp has a bad format", git.ErrInvalidArg) default: if strings.Contains(stderr.String(), "invalid unit") { - return nil, fmt.Errorf("%w: fetched result doesn't correspond to requested type", ErrInvalidArg) + return nil, fmt.Errorf("%w: fetched result doesn't correspond to requested type", git.ErrInvalidArg) } } @@ -103,8 +113,25 @@ func (cfg LocalRepositoryConfig) getRegexp(ctx context.Context, nameRegexp strin return data, nil } -func (cfg LocalRepositoryConfig) parseConfig(data []byte, opts ConfigGetRegexpOpts) ([]ConfigPair, error) { - var res []ConfigPair +func buildConfigGetRegexpOptsFlags(opts git.ConfigGetRegexpOpts) []git.Option { + var flags []git.Option + if opts.Type != git.ConfigTypeDefault { + flags = append(flags, git.Flag{Name: opts.Type.String()}) + } + + if opts.ShowOrigin { + flags = append(flags, git.Flag{Name: "--show-origin"}) + } + + if opts.ShowScope { + flags = append(flags, git.Flag{Name: "--show-scope"}) + } + + return flags +} + +func (cfg Config) parseConfig(data []byte, opts git.ConfigGetRegexpOpts) ([]git.ConfigPair, error) { + var res []git.ConfigPair var err error for reader := bufio.NewReader(bytes.NewReader(data)); ; { @@ -134,7 +161,7 @@ func (cfg LocalRepositoryConfig) parseConfig(data []byte, opts ConfigGetRegexpOp return nil, fmt.Errorf("bad format of the config: %q", pair) } - res = append(res, ConfigPair{ + res = append(res, git.ConfigPair{ Key: string(parts[0]), Value: chompNul(parts[1]), Origin: chompNul(origin), @@ -150,10 +177,10 @@ func (cfg LocalRepositoryConfig) parseConfig(data []byte, opts ConfigGetRegexpOp } // Unset unsets the given config entry. -func (cfg LocalRepositoryConfig) Unset(ctx context.Context, name string, opts ConfigUnsetOpts) error { - cmd, err := cfg.repo.command(ctx, nil, SubCmd{ +func (cfg Config) Unset(ctx context.Context, name string, opts git.ConfigUnsetOpts) error { + cmd, err := cfg.repo.command(ctx, nil, git.SubCmd{ Name: "config", - Flags: opts.buildFlags(), + Flags: buildConfigUnsetOptsFlags(opts), Args: []string{name}, }) if err != nil { @@ -165,17 +192,17 @@ func (cfg LocalRepositoryConfig) Unset(ctx context.Context, name string, opts Co switch { case isExitWithCode(err, 1): // section or key is invalid - return fmt.Errorf("%w: bad section or name", ErrInvalidArg) + return fmt.Errorf("%w: bad section or name", git.ErrInvalidArg) case isExitWithCode(err, 2): // no section or name was provided - return fmt.Errorf("%w: missing section or name", ErrInvalidArg) + return fmt.Errorf("%w: missing section or name", git.ErrInvalidArg) case isExitWithCode(err, 5): // unset an option which does not exist if opts.NotStrict { return nil } - return ErrNotFound + return git.ErrNotFound } return err } @@ -183,6 +210,14 @@ func (cfg LocalRepositoryConfig) Unset(ctx context.Context, name string, opts Co return nil } +func buildConfigUnsetOptsFlags(opts git.ConfigUnsetOpts) []git.Option { + if opts.All { + return []git.Option{git.Flag{Name: "--unset-all"}} + } + + return []git.Option{git.Flag{Name: "--unset"}} +} + func chompNul(b []byte) string { return string(bytes.Trim(b, "\x00")) } diff --git a/internal/git/localrepo_config_test.go b/internal/git/localrepo/config_test.go index b152f7e68..9d0f55980 100644 --- a/internal/git/localrepo_config_test.go +++ b/internal/git/localrepo/config_test.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "errors" @@ -8,23 +8,47 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) -func TestLocalRepository_Config(t *testing.T) { +func TestRepo_Config(t *testing.T) { bareRepo, _, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(bareRepo, config.Config) - require.Equal(t, LocalRepositoryConfig{repo: repo}, repo.Config()) + repo := New(bareRepo, config.Config) + require.Equal(t, Config{repo: repo}, repo.Config()) } -func TestLocalRepositoryConfig_Add(t *testing.T) { +func TestBuildConfigAddOptsFlags(t *testing.T) { + for _, tc := range []struct { + desc string + opts git.ConfigAddOpts + exp []git.Option + }{ + { + desc: "none", + opts: git.ConfigAddOpts{}, + exp: nil, + }, + { + desc: "all set", + opts: git.ConfigAddOpts{Type: git.ConfigTypeBoolOrInt}, + exp: []git.Option{git.Flag{Name: "--bool-or-int"}}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.exp, buildConfigAddOptsFlags(tc.opts)) + }) + } +} + +func TestConfig_Add(t *testing.T) { repoProto, repoPath, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() @@ -32,22 +56,22 @@ func TestLocalRepositoryConfig_Add(t *testing.T) { config := repo.Config() t.Run("ok", func(t *testing.T) { - require.NoError(t, config.Add(ctx, "key.one", "1", ConfigAddOpts{})) + require.NoError(t, config.Add(ctx, "key.one", "1", git.ConfigAddOpts{})) actual := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "key.one")) require.Equal(t, "1", actual) }) t.Run("appends to an old value", func(t *testing.T) { - require.NoError(t, config.Add(ctx, "key.two", "2", ConfigAddOpts{})) - require.NoError(t, config.Add(ctx, "key.two", "3", ConfigAddOpts{})) + require.NoError(t, config.Add(ctx, "key.two", "2", git.ConfigAddOpts{})) + require.NoError(t, config.Add(ctx, "key.two", "3", git.ConfigAddOpts{})) actual := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--get-all", "key.two")) require.Equal(t, "2\n3", actual) }) t.Run("options are passed", func(t *testing.T) { - require.NoError(t, config.Add(ctx, "key.three", "3", ConfigAddOpts{Type: ConfigTypeInt})) + require.NoError(t, config.Add(ctx, "key.three", "3", git.ConfigAddOpts{Type: git.ConfigTypeInt})) actual := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--int", "key.three")) require.Equal(t, "3", actual) @@ -63,19 +87,19 @@ func TestLocalRepositoryConfig_Add(t *testing.T) { { desc: "empty name", name: "", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, expMsg: `"name" is blank or empty`, }, { desc: "invalid name", name: "`.\n", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, expMsg: "bad section or name", }, { desc: "no section or name", name: "missing", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, expMsg: "missing section or name", }, } { @@ -84,7 +108,7 @@ func TestLocalRepositoryConfig_Add(t *testing.T) { defer cancel() config := repo.Config() - err := config.Add(ctx, tc.name, "some", ConfigAddOpts{}) + err := config.Add(ctx, tc.name, "some", git.ConfigAddOpts{}) require.Error(t, err) require.True(t, errors.Is(err, tc.expErr), err.Error()) require.Contains(t, err.Error(), tc.expMsg) @@ -93,10 +117,37 @@ func TestLocalRepositoryConfig_Add(t *testing.T) { }) } -func TestLocalRepositoryConfig_GetRegexp(t *testing.T) { +func TestBuildConfigGetRegexpOptsFlags(t *testing.T) { + for _, tc := range []struct { + desc string + opts git.ConfigGetRegexpOpts + exp []git.Option + }{ + { + desc: "none", + opts: git.ConfigGetRegexpOpts{}, + exp: nil, + }, + { + desc: "all set", + opts: git.ConfigGetRegexpOpts{Type: git.ConfigTypeInt, ShowOrigin: true, ShowScope: true}, + exp: []git.Option{ + git.Flag{Name: "--int"}, + git.Flag{Name: "--show-origin"}, + git.Flag{Name: "--show-scope"}, + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.exp, buildConfigGetRegexpOptsFlags(tc.opts)) + }) + } +} + +func TestConfig_GetRegexp(t *testing.T) { repoProto, repoPath, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() @@ -108,27 +159,27 @@ func TestLocalRepositoryConfig_GetRegexp(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.two", "2") testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.three", "!@#$%^&") - vals, err := config.GetRegexp(ctx, "^key\\..*o", ConfigGetRegexpOpts{}) + vals, err := config.GetRegexp(ctx, "^key\\..*o", git.ConfigGetRegexpOpts{}) require.NoError(t, err) - require.Equal(t, []ConfigPair{{Key: "key.one", Value: "one"}, {Key: "key.two", Value: "2"}}, vals) + require.Equal(t, []git.ConfigPair{{Key: "key.one", Value: "one"}, {Key: "key.two", Value: "2"}}, vals) }) t.Run("show origin and scope", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.four", "4") testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.five", "five") - exp := []ConfigPair{ + exp := []git.ConfigPair{ {Key: "key.four", Value: "4", Origin: "file:" + filepath.Join(repoPath, "config"), Scope: "local"}, {Key: "key.five", Value: "five", Origin: "file:" + filepath.Join(repoPath, "config"), Scope: "local"}, } - vals, err := config.GetRegexp(ctx, "^key\\.f", ConfigGetRegexpOpts{ShowScope: true, ShowOrigin: true}) + vals, err := config.GetRegexp(ctx, "^key\\.f", git.ConfigGetRegexpOpts{ShowScope: true, ShowOrigin: true}) require.NoError(t, err) require.Equal(t, exp, vals) }) t.Run("none found", func(t *testing.T) { - vals, err := config.GetRegexp(ctx, "nonexisting", ConfigGetRegexpOpts{}) + vals, err := config.GetRegexp(ctx, "nonexisting", git.ConfigGetRegexpOpts{}) require.NoError(t, err) require.Empty(t, vals) }) @@ -136,9 +187,9 @@ func TestLocalRepositoryConfig_GetRegexp(t *testing.T) { t.Run("bad combination of regexp and type", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.six", "key-six") - _, err := config.GetRegexp(ctx, "^key\\.six$", ConfigGetRegexpOpts{Type: ConfigTypeBool}) + _, err := config.GetRegexp(ctx, "^key\\.six$", git.ConfigGetRegexpOpts{Type: git.ConfigTypeBool}) require.Error(t, err) - require.True(t, errors.Is(err, ErrInvalidArg)) + require.True(t, errors.Is(err, git.ErrInvalidArg)) require.Contains(t, err.Error(), "fetched result doesn't correspond to requested type") }) @@ -152,18 +203,18 @@ func TestLocalRepositoryConfig_GetRegexp(t *testing.T) { { desc: "empty regexp", regexp: "", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, expMsg: `"nameRegexp" is blank or empty`, }, { desc: "invalid regexp", regexp: "{4", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, expMsg: "regexp has a bad format", }, } { t.Run(tc.desc, func(t *testing.T) { - _, err := config.GetRegexp(ctx, tc.regexp, ConfigGetRegexpOpts{}) + _, err := config.GetRegexp(ctx, tc.regexp, git.ConfigGetRegexpOpts{}) require.Error(t, err) require.True(t, errors.Is(err, tc.expErr), err.Error()) require.Contains(t, err.Error(), tc.expMsg) @@ -172,7 +223,30 @@ func TestLocalRepositoryConfig_GetRegexp(t *testing.T) { }) } -func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { +func TestBuildConfigUnsetOptsFlags(t *testing.T) { + for _, tc := range []struct { + desc string + opts git.ConfigUnsetOpts + exp []git.Option + }{ + { + desc: "none", + opts: git.ConfigUnsetOpts{}, + exp: []git.Option{git.Flag{Name: "--unset"}}, + }, + { + desc: "all set", + opts: git.ConfigUnsetOpts{All: true, NotStrict: true}, + exp: []git.Option{git.Flag{Name: "--unset-all"}}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.exp, buildConfigUnsetOptsFlags(tc.opts)) + }) + } +} + +func TestConfig_UnsetAll(t *testing.T) { configContains := func(t *testing.T, repoPath string) func(t *testing.T, val string, contains bool) { data, err := ioutil.ReadFile(filepath.Join(repoPath, "config")) require.NoError(t, err) @@ -184,7 +258,7 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { repoProto, repoPath, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() @@ -194,7 +268,7 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { t.Run("unset single value", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.one", "key-one") - require.NoError(t, config.Unset(ctx, "key.one", ConfigUnsetOpts{})) + require.NoError(t, config.Unset(ctx, "key.one", git.ConfigUnsetOpts{})) contains := configContains(t, repoPath) contains(t, "key-one", false) @@ -204,7 +278,7 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.two", "key-two-1") testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.two", "key-two-2") - require.NoError(t, config.Unset(ctx, "key.two", ConfigUnsetOpts{All: true})) + require.NoError(t, config.Unset(ctx, "key.two", git.ConfigUnsetOpts{All: true})) contains := configContains(t, repoPath) contains(t, "key-two-1", false) @@ -215,8 +289,8 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.two", "key-two-1") testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.two", "key-two-2") - err := config.Unset(ctx, "key.two", ConfigUnsetOpts{}) - require.Equal(t, ErrNotFound, err) + err := config.Unset(ctx, "key.two", git.ConfigUnsetOpts{}) + require.Equal(t, git.ErrNotFound, err) contains := configContains(t, repoPath) contains(t, "key-two-1", true) @@ -226,8 +300,8 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { t.Run("config key doesn't exist - is strict (by default)", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.three", "key-three") - err := config.Unset(ctx, "some.stub", ConfigUnsetOpts{}) - require.Equal(t, ErrNotFound, err) + err := config.Unset(ctx, "some.stub", git.ConfigUnsetOpts{}) + require.Equal(t, git.ErrNotFound, err) contains := configContains(t, repoPath) contains(t, "key-three", true) @@ -236,7 +310,7 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { t.Run("config key doesn't exist - not strict", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--add", "key.four", "key-four") - require.NoError(t, config.Unset(ctx, "some.stub", ConfigUnsetOpts{NotStrict: true})) + require.NoError(t, config.Unset(ctx, "some.stub", git.ConfigUnsetOpts{NotStrict: true})) contains := configContains(t, repoPath) contains(t, "key-four", true) @@ -251,21 +325,21 @@ func TestLocalRepositoryConfig_UnsetAll(t *testing.T) { { desc: "empty name", name: "", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, }, { desc: "invalid name", name: "`.\n", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, }, { desc: "no section or name", name: "bad", - expErr: ErrInvalidArg, + expErr: git.ErrInvalidArg, }, } { t.Run(tc.desc, func(t *testing.T) { - err := config.Unset(ctx, tc.name, ConfigUnsetOpts{}) + err := config.Unset(ctx, tc.name, git.ConfigUnsetOpts{}) require.Error(t, err) require.True(t, errors.Is(err, tc.expErr), err.Error()) }) diff --git a/internal/git/localrepo_objects.go b/internal/git/localrepo/objects.go index b0c4d9c3f..28c75820b 100644 --- a/internal/git/localrepo_objects.go +++ b/internal/git/localrepo/objects.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "bytes" @@ -8,27 +8,29 @@ import ( "strings" "time" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper/text" ) // WriteBlob writes a blob to the repository's object database and // returns its object ID. Path is used by git to decide which filters to // run on the content. -func (repo *LocalRepository) WriteBlob(ctx context.Context, path string, content io.Reader) (string, error) { +func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) (string, error) { stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} cmd, err := repo.command(ctx, nil, - SubCmd{ + git.SubCmd{ Name: "hash-object", - Flags: []Option{ - ValueFlag{Name: "--path", Value: path}, - Flag{Name: "--stdin"}, Flag{Name: "-w"}, + Flags: []git.Option{ + git.ValueFlag{Name: "--path", Value: path}, + git.Flag{Name: "--stdin"}, + git.Flag{Name: "-w"}, }, }, - WithStdin(content), - WithStdout(stdout), - WithStderr(stderr), + git.WithStdin(content), + git.WithStdout(stdout), + git.WithStderr(stderr), ) if err != nil { return "", err @@ -105,7 +107,7 @@ func (e MktagError) Error() string { // // It's important that this be git-mktag and not git-hash-object due // to its fsck sanity checking semantics. -func (repo *LocalRepository) WriteTag(ctx context.Context, objectID, objectType string, tagName, userName, userEmail, tagBody []byte, committerDate time.Time) (string, error) { +func (repo *Repo) WriteTag(ctx context.Context, objectID, objectType string, tagName, userName, userEmail, tagBody []byte, committerDate time.Time) (string, error) { stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} @@ -117,12 +119,12 @@ func (repo *LocalRepository) WriteTag(ctx context.Context, objectID, objectType content := strings.NewReader(tagBuf) cmd, err := repo.command(ctx, nil, - SubCmd{ + git.SubCmd{ Name: "mktag", }, - WithStdin(content), - WithStdout(stdout), - WithStderr(stderr), + git.WithStdin(content), + git.WithStdout(stdout), + git.WithStderr(stderr), ) if err != nil { return "", err @@ -142,19 +144,19 @@ func (err InvalidObjectError) Error() string { return fmt.Sprintf("invalid objec // ReadObject reads an object from the repository's object database. InvalidObjectError // is returned if the oid does not refer to a valid object. -func (repo *LocalRepository) ReadObject(ctx context.Context, oid string) ([]byte, error) { +func (repo *Repo) ReadObject(ctx context.Context, oid string) ([]byte, error) { const msgInvalidObject = "fatal: Not a valid object name " stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} cmd, err := repo.command(ctx, nil, - SubCmd{ + git.SubCmd{ Name: "cat-file", - Flags: []Option{Flag{"-p"}}, + Flags: []git.Option{git.Flag{"-p"}}, Args: []string{oid}, }, - WithStdout(stdout), - WithStderr(stderr), + git.WithStdout(stdout), + git.WithStderr(stderr), ) if err != nil { return nil, err diff --git a/internal/git/localrepo_objects_test.go b/internal/git/localrepo/objects_test.go index 2dae8951a..fed40893e 100644 --- a/internal/git/localrepo_objects_test.go +++ b/internal/git/localrepo/objects_test.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "fmt" @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -21,14 +22,14 @@ type ReaderFunc func([]byte) (int, error) func (fn ReaderFunc) Read(b []byte) (int, error) { return fn(b) } -func TestLocalRepository_WriteBlob(t *testing.T) { +func TestRepo_WriteBlob(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() pbRepo, repoPath, clean := testhelper.InitBareRepo(t) defer clean() - repo := NewRepository(pbRepo, config.Config) + repo := New(pbRepo, config.Config) for _, tc := range []struct { desc string @@ -147,14 +148,14 @@ func TestFormatTag(t *testing.T) { } } -func TestLocalRepository_WriteTag(t *testing.T) { +func TestRepo_WriteTag(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() pbRepo, repoPath, clean := testhelper.NewTestRepo(t) defer clean() - repo := NewRepository(pbRepo, config.Config) + repo := New(pbRepo, config.Config) for _, tc := range []struct { desc string @@ -198,14 +199,14 @@ func TestLocalRepository_WriteTag(t *testing.T) { } } -func TestLocalRepository_ReadObject(t *testing.T) { +func TestRepo_ReadObject(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) for _, tc := range []struct { desc string @@ -215,8 +216,8 @@ func TestLocalRepository_ReadObject(t *testing.T) { }{ { desc: "invalid object", - oid: ZeroOID.String(), - error: InvalidObjectError(ZeroOID.String()), + oid: git.ZeroOID.String(), + error: InvalidObjectError(git.ZeroOID.String()), }, { desc: "valid object", diff --git a/internal/git/localrepo_refs.go b/internal/git/localrepo/refs.go index a30cf6a56..5a9db28f8 100644 --- a/internal/git/localrepo_refs.go +++ b/internal/git/localrepo/refs.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "bufio" @@ -11,15 +11,16 @@ import ( "os/exec" "strings" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" ) // HasRevision checks if a revision in the repository exists. This will not // verify whether the target object exists. To do so, you can peel the revision // to a given object type, e.g. by passing `refs/heads/master^{commit}`. -func (repo *LocalRepository) HasRevision(ctx context.Context, revision Revision) (bool, error) { +func (repo *Repo) HasRevision(ctx context.Context, revision git.Revision) (bool, error) { if _, err := repo.ResolveRevision(ctx, revision); err != nil { - if errors.Is(err, ErrReferenceNotFound) { + if errors.Is(err, git.ErrReferenceNotFound) { return false, nil } return false, err @@ -32,16 +33,16 @@ func (repo *LocalRepository) HasRevision(ctx context.Context, revision Revision) // reference to a given object type, e.g. by passing // `refs/heads/master^{commit}`. Returns an ErrReferenceNotFound error in case // the revision does not exist. -func (repo *LocalRepository) ResolveRevision(ctx context.Context, revision Revision) (ObjectID, error) { +func (repo *Repo) ResolveRevision(ctx context.Context, revision git.Revision) (git.ObjectID, error) { if revision.String() == "" { return "", errors.New("repository cannot contain empty reference name") } - cmd, err := repo.command(ctx, nil, SubCmd{ + cmd, err := repo.command(ctx, nil, git.SubCmd{ Name: "rev-parse", - Flags: []Option{Flag{Name: "--verify"}}, + Flags: []git.Option{git.Flag{Name: "--verify"}}, Args: []string{revision.String()}, - }, WithStderr(ioutil.Discard)) + }, git.WithStderr(ioutil.Discard)) if err != nil { return "", err } @@ -51,13 +52,13 @@ func (repo *LocalRepository) ResolveRevision(ctx context.Context, revision Revis if err := cmd.Wait(); err != nil { if _, ok := err.(*exec.ExitError); ok { - return "", ErrReferenceNotFound + return "", git.ErrReferenceNotFound } return "", err } hex := strings.TrimSpace(stdout.String()) - oid, err := NewObjectIDFromHex(hex) + oid, err := git.NewObjectIDFromHex(hex) if err != nil { return "", fmt.Errorf("unsupported object hash %q: %w", hex, err) } @@ -67,14 +68,14 @@ func (repo *LocalRepository) ResolveRevision(ctx context.Context, revision Revis // GetReference looks up and returns the given reference. Returns a // ReferenceNotFound error if the reference was not found. -func (repo *LocalRepository) GetReference(ctx context.Context, reference ReferenceName) (Reference, error) { +func (repo *Repo) GetReference(ctx context.Context, reference git.ReferenceName) (git.Reference, error) { refs, err := repo.GetReferences(ctx, reference.String()) if err != nil { - return Reference{}, err + return git.Reference{}, err } if len(refs) == 0 { - return Reference{}, ErrReferenceNotFound + return git.Reference{}, git.ErrReferenceNotFound } return refs[0], nil @@ -82,20 +83,20 @@ func (repo *LocalRepository) GetReference(ctx context.Context, reference Referen // HasBranches determines whether there is at least one branch in the // repository. -func (repo *LocalRepository) HasBranches(ctx context.Context) (bool, error) { +func (repo *Repo) HasBranches(ctx context.Context) (bool, error) { refs, err := repo.getReferences(ctx, "refs/heads/", 1) return len(refs) > 0, err } // GetReferences returns references matching the given pattern. -func (repo *LocalRepository) GetReferences(ctx context.Context, pattern string) ([]Reference, error) { +func (repo *Repo) GetReferences(ctx context.Context, pattern string) ([]git.Reference, error) { return repo.getReferences(ctx, pattern, 0) } -func (repo *LocalRepository) getReferences(ctx context.Context, pattern string, limit uint) ([]Reference, error) { - flags := []Option{Flag{Name: "--format=%(refname)%00%(objectname)%00%(symref)"}} +func (repo *Repo) getReferences(ctx context.Context, pattern string, limit uint) ([]git.Reference, error) { + flags := []git.Option{git.Flag{Name: "--format=%(refname)%00%(objectname)%00%(symref)"}} if limit > 0 { - flags = append(flags, Flag{Name: fmt.Sprintf("--count=%d", limit)}) + flags = append(flags, git.Flag{Name: fmt.Sprintf("--count=%d", limit)}) } var args []string @@ -103,7 +104,7 @@ func (repo *LocalRepository) getReferences(ctx context.Context, pattern string, args = []string{pattern} } - cmd, err := repo.command(ctx, nil, SubCmd{ + cmd, err := repo.command(ctx, nil, git.SubCmd{ Name: "for-each-ref", Flags: flags, Args: args, @@ -114,7 +115,7 @@ func (repo *LocalRepository) getReferences(ctx context.Context, pattern string, scanner := bufio.NewScanner(cmd) - var refs []Reference + var refs []git.Reference for scanner.Scan() { line := bytes.SplitN(scanner.Bytes(), []byte{0}, 3) if len(line) != 3 { @@ -122,9 +123,9 @@ func (repo *LocalRepository) getReferences(ctx context.Context, pattern string, } if len(line[2]) == 0 { - refs = append(refs, NewReference(ReferenceName(line[0]), string(line[1]))) + refs = append(refs, git.NewReference(git.ReferenceName(line[0]), string(line[1]))) } else { - refs = append(refs, NewSymbolicReference(ReferenceName(line[0]), string(line[1]))) + refs = append(refs, git.NewSymbolicReference(git.ReferenceName(line[0]), string(line[1]))) } } @@ -139,7 +140,7 @@ func (repo *LocalRepository) getReferences(ctx context.Context, pattern string, } // GetBranches returns all branches. -func (repo *LocalRepository) GetBranches(ctx context.Context) ([]Reference, error) { +func (repo *Repo) GetBranches(ctx context.Context) ([]git.Reference, error) { return repo.GetReferences(ctx, "refs/heads/") } @@ -147,14 +148,14 @@ func (repo *LocalRepository) GetBranches(ctx context.Context) ([]Reference, erro // non-empty string, the update will fail it the reference is not currently at // that revision. If newValue is the ZeroOID, the reference will be deleted. // If oldValue is the ZeroOID, the reference will created. -func (repo *LocalRepository) UpdateRef(ctx context.Context, reference ReferenceName, newValue, oldValue ObjectID) error { +func (repo *Repo) UpdateRef(ctx context.Context, reference git.ReferenceName, newValue, oldValue git.ObjectID) error { cmd, err := repo.command(ctx, nil, - SubCmd{ + git.SubCmd{ Name: "update-ref", - Flags: []Option{Flag{Name: "-z"}, Flag{Name: "--stdin"}}, + Flags: []git.Option{git.Flag{Name: "-z"}, git.Flag{Name: "--stdin"}}, }, - WithStdin(strings.NewReader(fmt.Sprintf("update %s\x00%s\x00%s\x00", reference, newValue.String(), oldValue.String()))), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), repo.cfg), + git.WithStdin(strings.NewReader(fmt.Sprintf("update %s\x00%s\x00%s\x00", reference, newValue.String(), oldValue.String()))), + git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(repo), repo.cfg), ) if err != nil { return err diff --git a/internal/git/localrepo_refs_test.go b/internal/git/localrepo/refs_test.go index edd46d95b..dc55b717a 100644 --- a/internal/git/localrepo_refs_test.go +++ b/internal/git/localrepo/refs_test.go @@ -1,27 +1,28 @@ -package git +package localrepo import ( "errors" "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) const ( - masterOID = ObjectID("1e292f8fedd741b75372e19097c76d327140c312") - nonexistentOID = ObjectID("ba4f184e126b751d1bffad5897f263108befc780") + masterOID = git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312") + nonexistentOID = git.ObjectID("ba4f184e126b751d1bffad5897f263108befc780") ) -func TestLocalRepository_ContainsRef(t *testing.T) { +func TestRepo_ContainsRef(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testcases := []struct { desc string @@ -47,54 +48,54 @@ func TestLocalRepository_ContainsRef(t *testing.T) { for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { - contained, err := repo.HasRevision(ctx, Revision(tc.ref)) + contained, err := repo.HasRevision(ctx, git.Revision(tc.ref)) require.NoError(t, err) require.Equal(t, tc.contained, contained) }) } } -func TestLocalRepository_GetReference(t *testing.T) { +func TestRepo_GetReference(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testcases := []struct { desc string ref string - expected Reference + expected git.Reference }{ { desc: "fully qualified master branch", ref: "refs/heads/master", - expected: NewReference("refs/heads/master", masterOID.String()), + expected: git.NewReference("refs/heads/master", masterOID.String()), }, { desc: "unqualified master branch fails", ref: "master", - expected: Reference{}, + expected: git.Reference{}, }, { desc: "nonexistent branch", ref: "refs/heads/nonexistent", - expected: Reference{}, + expected: git.Reference{}, }, { desc: "nonexistent branch", ref: "nonexistent", - expected: Reference{}, + expected: git.Reference{}, }, } for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { - ref, err := repo.GetReference(ctx, ReferenceName(tc.ref)) + ref, err := repo.GetReference(ctx, git.ReferenceName(tc.ref)) if tc.expected.Name == "" { - require.True(t, errors.Is(err, ErrReferenceNotFound)) + require.True(t, errors.Is(err, git.ErrReferenceNotFound)) } else { require.NoError(t, err) require.Equal(t, tc.expected, ref) @@ -103,47 +104,47 @@ func TestLocalRepository_GetReference(t *testing.T) { } } -func TestLocalRepository_GetReferences(t *testing.T) { +func TestRepo_GetReferences(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testcases := []struct { desc string pattern string - match func(t *testing.T, refs []Reference) + match func(t *testing.T, refs []git.Reference) }{ { desc: "master branch", pattern: "refs/heads/master", - match: func(t *testing.T, refs []Reference) { - require.Equal(t, []Reference{ - NewReference("refs/heads/master", masterOID.String()), + match: func(t *testing.T, refs []git.Reference) { + require.Equal(t, []git.Reference{ + git.NewReference("refs/heads/master", masterOID.String()), }, refs) }, }, { desc: "all references", pattern: "", - match: func(t *testing.T, refs []Reference) { + match: func(t *testing.T, refs []git.Reference) { require.Len(t, refs, 94) }, }, { desc: "branches", pattern: "refs/heads/", - match: func(t *testing.T, refs []Reference) { + match: func(t *testing.T, refs []git.Reference) { require.Len(t, refs, 91) }, }, { desc: "branches", pattern: "refs/heads/nonexistent", - match: func(t *testing.T, refs []Reference) { + match: func(t *testing.T, refs []git.Reference) { require.Empty(t, refs) }, }, @@ -158,21 +159,21 @@ func TestLocalRepository_GetReferences(t *testing.T) { } } -func TestLocalRepository_GetBranches(t *testing.T) { +func TestRepo_GetBranches(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) refs, err := repo.GetBranches(ctx) require.NoError(t, err) require.Len(t, refs, 91) } -func TestLocalRepository_UpdateRef(t *testing.T) { +func TestRepo_UpdateRef(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() @@ -184,22 +185,22 @@ func TestLocalRepository_UpdateRef(t *testing.T) { }(config.Config.Ruby.Dir) config.Config.Ruby.Dir = "/var/empty" - otherRef, err := NewRepository(testRepo, config.Config).GetReference(ctx, "refs/heads/gitaly-test-ref") + otherRef, err := New(testRepo, config.Config).GetReference(ctx, "refs/heads/gitaly-test-ref") require.NoError(t, err) testcases := []struct { desc string ref string - newValue ObjectID - oldValue ObjectID - verify func(t *testing.T, repo *LocalRepository, err error) + newValue git.ObjectID + oldValue git.ObjectID + verify func(t *testing.T, repo *Repo, err error) }{ { desc: "successfully update master", ref: "refs/heads/master", - newValue: ObjectID(otherRef.Target), + newValue: git.ObjectID(otherRef.Target), oldValue: masterOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -209,9 +210,9 @@ func TestLocalRepository_UpdateRef(t *testing.T) { { desc: "update fails with stale oldValue", ref: "refs/heads/master", - newValue: ObjectID(otherRef.Target), + newValue: git.ObjectID(otherRef.Target), oldValue: nonexistentOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -223,7 +224,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/master", newValue: nonexistentOID, oldValue: masterOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -233,9 +234,9 @@ func TestLocalRepository_UpdateRef(t *testing.T) { { desc: "successfully update master with empty oldValue", ref: "refs/heads/master", - newValue: ObjectID(otherRef.Target), + newValue: git.ObjectID(otherRef.Target), oldValue: "", - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -245,9 +246,9 @@ func TestLocalRepository_UpdateRef(t *testing.T) { { desc: "updating unqualified branch fails", ref: "master", - newValue: ObjectID(otherRef.Target), + newValue: git.ObjectID(otherRef.Target), oldValue: masterOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -257,9 +258,9 @@ func TestLocalRepository_UpdateRef(t *testing.T) { { desc: "deleting master succeeds", ref: "refs/heads/master", - newValue: ZeroOID, + newValue: git.ZeroOID, oldValue: masterOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) _, err = repo.GetReference(ctx, "refs/heads/master") require.Error(t, err) @@ -269,8 +270,8 @@ func TestLocalRepository_UpdateRef(t *testing.T) { desc: "creating new branch succeeds", ref: "refs/heads/new", newValue: masterOID, - oldValue: ZeroOID, - verify: func(t *testing.T, repo *LocalRepository, err error) { + oldValue: git.ZeroOID, + verify: func(t *testing.T, repo *Repo, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/new") require.NoError(t, err) @@ -285,8 +286,8 @@ func TestLocalRepository_UpdateRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) - err := repo.UpdateRef(ctx, ReferenceName(tc.ref), tc.newValue, tc.oldValue) + repo := New(testRepo, config.Config) + err := repo.UpdateRef(ctx, git.ReferenceName(tc.ref), tc.newValue, tc.oldValue) tc.verify(t, repo, err) }) diff --git a/internal/git/localrepo_remote.go b/internal/git/localrepo/remote.go index eab81928c..75a68a3a8 100644 --- a/internal/git/localrepo_remote.go +++ b/internal/git/localrepo/remote.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "bytes" @@ -8,17 +8,18 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" ) -// LocalRepositoryRemote provides functionality of the 'remote' git sub-command. -type LocalRepositoryRemote struct { - repo *LocalRepository +// Remote provides functionality of the 'remote' git sub-command. +type Remote struct { + repo *Repo } // Add adds a new remote to the repository. -func (remote LocalRepositoryRemote) Add(ctx context.Context, name, url string, opts RemoteAddOpts) error { +func (remote Remote) Add(ctx context.Context, name, url string, opts git.RemoteAddOpts) error { if err := validateNotBlank(name, "name"); err != nil { return err } @@ -29,14 +30,14 @@ func (remote LocalRepositoryRemote) Add(ctx context.Context, name, url string, o stderr := bytes.Buffer{} cmd, err := remote.repo.command(ctx, nil, - SubSubCmd{ + git.SubSubCmd{ Name: "remote", Action: "add", - Flags: opts.buildFlags(), + Flags: buildRemoteAddOptsFlags(opts), Args: []string{name, url}, }, - WithStderr(&stderr), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), + git.WithStderr(&stderr), + git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), ) if err != nil { return err @@ -50,32 +51,57 @@ func (remote LocalRepositoryRemote) Add(ctx context.Context, name, url string, o if status == 3 { // In Git v2.30.0 and newer (https://gitlab.com/git-vcs/git/commit/9144ba4cf52) - return ErrAlreadyExists + return git.ErrAlreadyExists } if status == 128 && bytes.HasPrefix(stderr.Bytes(), []byte("fatal: remote "+name+" already exists")) { // ..in older versions we parse stderr - return ErrAlreadyExists + return git.ErrAlreadyExists } } return nil } +func buildRemoteAddOptsFlags(opts git.RemoteAddOpts) []git.Option { + var flags []git.Option + for _, b := range opts.RemoteTrackingBranches { + flags = append(flags, git.ValueFlag{Name: "-t", Value: b}) + } + + if opts.DefaultBranch != "" { + flags = append(flags, git.ValueFlag{Name: "-m", Value: opts.DefaultBranch}) + } + + if opts.Fetch { + flags = append(flags, git.Flag{Name: "-f"}) + } + + if opts.Tags != git.RemoteAddOptsTagsDefault { + flags = append(flags, git.Flag{Name: opts.Tags.String()}) + } + + if opts.Mirror != git.RemoteAddOptsMirrorDefault { + flags = append(flags, git.ValueFlag{Name: "--mirror", Value: opts.Mirror.String()}) + } + + return flags +} + // Remove removes a named remote from the repository configuration. -func (remote LocalRepositoryRemote) Remove(ctx context.Context, name string) error { +func (remote Remote) Remove(ctx context.Context, name string) error { if err := validateNotBlank(name, "name"); err != nil { return err } var stderr bytes.Buffer cmd, err := remote.repo.command(ctx, nil, - SubSubCmd{ + git.SubSubCmd{ Name: "remote", Action: "remove", Args: []string{name}, }, - WithStderr(&stderr), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), + git.WithStderr(&stderr), + git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), ) if err != nil { return err @@ -89,11 +115,11 @@ func (remote LocalRepositoryRemote) Remove(ctx context.Context, name string) err if status == 2 { // In Git v2.30.0 and newer (https://gitlab.com/git-vcs/git/commit/9144ba4cf52) - return ErrNotFound + return git.ErrNotFound } if status == 128 && strings.HasPrefix(stderr.String(), "fatal: No such remote") { // ..in older versions we parse stderr - return ErrNotFound + return git.ErrNotFound } } @@ -101,7 +127,7 @@ func (remote LocalRepositoryRemote) Remove(ctx context.Context, name string) err } // SetURL sets the URL for a given remote. -func (remote LocalRepositoryRemote) SetURL(ctx context.Context, name, url string, opts SetURLOpts) error { +func (remote Remote) SetURL(ctx context.Context, name, url string, opts git.SetURLOpts) error { if err := validateNotBlank(name, "name"); err != nil { return err } @@ -112,14 +138,14 @@ func (remote LocalRepositoryRemote) SetURL(ctx context.Context, name, url string var stderr bytes.Buffer cmd, err := remote.repo.command(ctx, nil, - SubSubCmd{ + git.SubSubCmd{ Name: "remote", Action: "set-url", - Flags: opts.buildFlags(), + Flags: buildSetURLOptsFlags(opts), Args: []string{name, url}, }, - WithStderr(&stderr), - WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), + git.WithStderr(&stderr), + git.WithRefTxHook(ctx, helper.ProtoRepoFromRepo(remote.repo), config.Config), ) if err != nil { return err @@ -133,17 +159,25 @@ func (remote LocalRepositoryRemote) SetURL(ctx context.Context, name, url string if status == 2 { // In Git v2.30.0 and newer (https://gitlab.com/git-vcs/git/commit/9144ba4cf52) - return ErrNotFound + return git.ErrNotFound } if status == 128 && strings.HasPrefix(stderr.String(), "fatal: No such remote") { // ..in older versions we parse stderr - return ErrNotFound + return git.ErrNotFound } } return err } +func buildSetURLOptsFlags(opts git.SetURLOpts) []git.Option { + if opts.Push { + return []git.Option{git.Flag{Name: "--push"}} + } + + return nil +} + // FetchOptsTags controls what tags needs to be imported on fetch. type FetchOptsTags string @@ -165,7 +199,7 @@ type FetchOpts struct { // Env is a list of env vars to pass to the cmd. Env []string // Global is a list of global flags to use with 'git' command. - Global []GlobalOption + Global []git.GlobalOption // Prune if set fetch removes any remote-tracking references that no longer exist on the remote. // https://git-scm.com/docs/git-fetch#Documentation/git-fetch.txt---prune Prune bool @@ -187,20 +221,20 @@ type FetchOpts struct { } // FetchRemote fetches changes from the specified remote. -func (repo *LocalRepository) FetchRemote(ctx context.Context, remoteName string, opts FetchOpts) error { +func (repo *Repo) FetchRemote(ctx context.Context, remoteName string, opts FetchOpts) error { if err := validateNotBlank(remoteName, "remoteName"); err != nil { return err } - cmd, err := NewCommand(ctx, repo, opts.Global, - SubCmd{ + cmd, err := git.NewCommand(ctx, repo, opts.Global, + git.SubCmd{ Name: "fetch", Flags: opts.buildFlags(), Args: []string{remoteName}, }, - WithEnv(opts.Env...), - WithStderr(opts.Stderr), - WithDisabledHooks(), + git.WithEnv(opts.Env...), + git.WithStderr(opts.Stderr), + git.WithDisabledHooks(), ) if err != nil { return err @@ -209,23 +243,23 @@ func (repo *LocalRepository) FetchRemote(ctx context.Context, remoteName string, return cmd.Wait() } -func (opts FetchOpts) buildFlags() []Option { - flags := []Option{} +func (opts FetchOpts) buildFlags() []git.Option { + flags := []git.Option{} if !opts.Verbose { - flags = append(flags, Flag{Name: "--quiet"}) + flags = append(flags, git.Flag{Name: "--quiet"}) } if opts.Prune { - flags = append(flags, Flag{Name: "--prune"}) + flags = append(flags, git.Flag{Name: "--prune"}) } if opts.Force { - flags = append(flags, Flag{Name: "--force"}) + flags = append(flags, git.Flag{Name: "--force"}) } if opts.Tags != FetchOptsTagsDefault { - flags = append(flags, Flag{Name: opts.Tags.String()}) + flags = append(flags, git.Flag{Name: opts.Tags.String()}) } return flags @@ -233,7 +267,7 @@ func (opts FetchOpts) buildFlags() []Option { func validateNotBlank(val, name string) error { if strings.TrimSpace(val) == "" { - return fmt.Errorf("%w: %q is blank or empty", ErrInvalidArg, name) + return fmt.Errorf("%w: %q is blank or empty", git.ErrInvalidArg, name) } return nil } diff --git a/internal/git/localrepo_remote_test.go b/internal/git/localrepo/remote_test.go index e1f0c56c5..5ab46895e 100644 --- a/internal/git/localrepo_remote_test.go +++ b/internal/git/localrepo/remote_test.go @@ -1,4 +1,4 @@ -package git +package localrepo import ( "bytes" @@ -10,23 +10,64 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) -func TestLocalRepository_Remote(t *testing.T) { +func TestRepo_Remote(t *testing.T) { repository := &gitalypb.Repository{StorageName: "stub", RelativePath: "/stub"} - repo := NewRepository(repository, config.Config) - require.Equal(t, LocalRepositoryRemote{repo: repo}, repo.Remote()) + repo := New(repository, config.Config) + require.Equal(t, Remote{repo: repo}, repo.Remote()) } -func TestLocalRepositoryRemote_Add(t *testing.T) { +func TestBuildRemoteAddOptsFlags(t *testing.T) { + for _, tc := range []struct { + desc string + opts git.RemoteAddOpts + exp []git.Option + }{ + { + desc: "none", + exp: nil, + }, + { + desc: "all set", + opts: git.RemoteAddOpts{ + Tags: git.RemoteAddOptsTagsNone, + Fetch: true, + RemoteTrackingBranches: []string{"branch-1", "branch-2"}, + DefaultBranch: "develop", + Mirror: git.RemoteAddOptsMirrorPush, + }, + exp: []git.Option{ + git.ValueFlag{Name: "-t", Value: "branch-1"}, + git.ValueFlag{Name: "-t", Value: "branch-2"}, + git.ValueFlag{Name: "-m", Value: "develop"}, + git.Flag{Name: "-f"}, + git.Flag{Name: "--no-tags"}, + git.ValueFlag{Name: "--mirror", Value: "push"}, + }, + }, + { + desc: "with tags", + opts: git.RemoteAddOpts{Tags: git.RemoteAddOptsTagsAll}, + exp: []git.Option{git.Flag{Name: "--tags"}}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.exp, buildRemoteAddOptsFlags(tc.opts)) + }) + } +} + +func TestRemote_Add(t *testing.T) { repoProto, repoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) _, remoteRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() @@ -63,16 +104,16 @@ func TestLocalRepositoryRemote_Add(t *testing.T) { }, } { t.Run(tc.desc, func(t *testing.T) { - err := remote.Add(ctx, tc.name, tc.url, RemoteAddOpts{}) + err := remote.Add(ctx, tc.name, tc.url, git.RemoteAddOpts{}) require.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidArg)) + assert.True(t, errors.Is(err, git.ErrInvalidArg)) assert.Contains(t, err.Error(), tc.errMsg) }) } }) t.Run("fetch", func(t *testing.T) { - require.NoError(t, remote.Add(ctx, "first", remoteRepoPath, RemoteAddOpts{Fetch: true})) + require.NoError(t, remote.Add(ctx, "first", remoteRepoPath, git.RemoteAddOpts{Fetch: true})) remotes := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "remote", "--verbose")) require.Equal(t, @@ -85,31 +126,31 @@ func TestLocalRepositoryRemote_Add(t *testing.T) { }) t.Run("default branch", func(t *testing.T) { - require.NoError(t, remote.Add(ctx, "second", "http://some.com.git", RemoteAddOpts{DefaultBranch: "wip"})) + require.NoError(t, remote.Add(ctx, "second", "http://some.com.git", git.RemoteAddOpts{DefaultBranch: "wip"})) defaultRemote := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "symbolic-ref", "refs/remotes/second/HEAD")) require.Equal(t, "refs/remotes/second/wip", defaultRemote) }) t.Run("remote tracking branches", func(t *testing.T) { - require.NoError(t, remote.Add(ctx, "third", "http://some.com.git", RemoteAddOpts{RemoteTrackingBranches: []string{"a", "b"}})) + require.NoError(t, remote.Add(ctx, "third", "http://some.com.git", git.RemoteAddOpts{RemoteTrackingBranches: []string{"a", "b"}})) defaultRemote := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "config", "--get-all", "remote.third.fetch")) require.Equal(t, "+refs/heads/a:refs/remotes/third/a\n+refs/heads/b:refs/remotes/third/b", defaultRemote) }) t.Run("already exists", func(t *testing.T) { - require.NoError(t, remote.Add(ctx, "fourth", "http://some.com.git", RemoteAddOpts{})) + require.NoError(t, remote.Add(ctx, "fourth", "http://some.com.git", git.RemoteAddOpts{})) - err := remote.Add(ctx, "fourth", "http://some.com.git", RemoteAddOpts{}) - require.Equal(t, ErrAlreadyExists, err) + err := remote.Add(ctx, "fourth", "http://some.com.git", git.RemoteAddOpts{}) + require.Equal(t, git.ErrAlreadyExists, err) }) } -func TestLocalRepositoryRemote_Remove(t *testing.T) { +func TestRemote_Remove(t *testing.T) { repoProto, repoPath, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() @@ -127,21 +168,43 @@ func TestLocalRepositoryRemote_Remove(t *testing.T) { t.Run("not found", func(t *testing.T) { err := remote.Remove(ctx, "second") - require.Equal(t, ErrNotFound, err) + require.Equal(t, git.ErrNotFound, err) }) t.Run("invalid argument: name", func(t *testing.T) { err := remote.Remove(ctx, " ") require.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidArg)) + assert.True(t, errors.Is(err, git.ErrInvalidArg)) assert.Contains(t, err.Error(), `"name" is blank or empty`) }) } -func TestLocalRepositoryRemote_SetURL(t *testing.T) { +func TestBuildSetURLOptsFlags(t *testing.T) { + for _, tc := range []struct { + desc string + opts git.SetURLOpts + exp []git.Option + }{ + { + desc: "none", + exp: nil, + }, + { + desc: "all set", + opts: git.SetURLOpts{Push: true}, + exp: []git.Option{git.Flag{Name: "--push"}}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require.Equal(t, tc.exp, buildSetURLOptsFlags(tc.opts)) + }) + } +} + +func TestRemote_SetURL(t *testing.T) { repoProto, repoPath, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(repoProto, config.Config) + repo := New(repoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() @@ -167,9 +230,9 @@ func TestLocalRepositoryRemote_SetURL(t *testing.T) { } { t.Run(tc.desc, func(t *testing.T) { remote := repo.Remote() - err := remote.SetURL(ctx, tc.name, tc.url, SetURLOpts{}) + err := remote.SetURL(ctx, tc.name, tc.url, git.SetURLOpts{}) require.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidArg)) + assert.True(t, errors.Is(err, git.ErrInvalidArg)) assert.Contains(t, err.Error(), tc.errMsg) }) } @@ -178,8 +241,8 @@ func TestLocalRepositoryRemote_SetURL(t *testing.T) { t.Run("ok", func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "remote", "add", "first", "file:/"+repoPath) - remote := LocalRepositoryRemote{repo: repo} - require.NoError(t, remote.SetURL(ctx, "first", "http://some.com.git", SetURLOpts{Push: true})) + remote := Remote{repo: repo} + require.NoError(t, remote.SetURL(ctx, "first", "http://some.com.git", git.SetURLOpts{Push: true})) remotes := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "remote", "--verbose")) require.Equal(t, @@ -190,20 +253,20 @@ func TestLocalRepositoryRemote_SetURL(t *testing.T) { }) t.Run("doesnt exist", func(t *testing.T) { - remote := LocalRepositoryRemote{repo: repo} - err := remote.SetURL(ctx, "second", "http://some.com.git", SetURLOpts{}) - require.True(t, errors.Is(err, ErrNotFound), err) + remote := Remote{repo: repo} + err := remote.SetURL(ctx, "second", "http://some.com.git", git.SetURLOpts{}) + require.True(t, errors.Is(err, git.ErrNotFound), err) }) } -func TestLocalRepository_FetchRemote(t *testing.T) { +func TestRepo_FetchRemote(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() _, remoteRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - initBareWithRemote := func(t *testing.T, remote string) (*LocalRepository, string, testhelper.Cleanup) { + initBareWithRemote := func(t *testing.T, remote string) (*Repo, string, testhelper.Cleanup) { t.Helper() testRepo, testRepoPath, cleanup := testhelper.InitBareRepo(t) @@ -216,7 +279,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { t.FailNow() } - return NewRepository(testRepo, config.Config), testRepoPath, cleanup + return New(testRepo, config.Config), testRepoPath, cleanup } defer func(oldValue string) { @@ -225,10 +288,10 @@ func TestLocalRepository_FetchRemote(t *testing.T) { config.Config.Ruby.Dir = "/var/empty" t.Run("invalid name", func(t *testing.T) { - repo := NewRepository(nil, config.Config) + repo := New(nil, config.Config) err := repo.FetchRemote(ctx, " ", FetchOpts{}) - require.True(t, errors.Is(err, ErrInvalidArg)) + require.True(t, errors.Is(err, git.ErrInvalidArg)) require.Contains(t, err.Error(), `"remoteName" is blank or empty`) }) @@ -236,7 +299,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, _, cleanup := testhelper.InitBareRepo(t) defer cleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) var stderr bytes.Buffer err := repo.FetchRemote(ctx, "stub", FetchOpts{Stderr: &stderr}) require.Error(t, err) @@ -259,9 +322,9 @@ func TestLocalRepository_FetchRemote(t *testing.T) { require.Contains(t, fetchHead, "e56497bb5f03a90a51293fc6d516788730953899 not-for-merge branch ''test''") require.Contains(t, fetchHead, "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b not-for-merge tag 'v1.1.0'") - sha, err := repo.ResolveRevision(ctx, Revision("refs/remotes/origin/master^{commit}")) + sha, err := repo.ResolveRevision(ctx, git.Revision("refs/remotes/origin/master^{commit}")) require.NoError(t, err, "the object from remote should exists in local after fetch done") - require.Equal(t, ObjectID("1e292f8fedd741b75372e19097c76d327140c312"), sha) + require.Equal(t, git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312"), sha) }) t.Run("with env", func(t *testing.T) { @@ -271,7 +334,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) var stderr bytes.Buffer @@ -286,7 +349,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) @@ -298,11 +361,11 @@ func TestLocalRepository_FetchRemote(t *testing.T) { ctx, "source", FetchOpts{ - Global: []GlobalOption{ConfigPair{Key: "fetch.prune", Value: "true"}}, + Global: []git.GlobalOption{git.ConfigPair{Key: "fetch.prune", Value: "true"}}, }), ) - contains, err := repo.HasRevision(ctx, Revision("refs/remotes/source/markdown")) + contains, err := repo.HasRevision(ctx, git.Revision("refs/remotes/source/markdown")) require.NoError(t, err) require.False(t, contains, "remote tracking branch should be pruned as it no longer exists on the remote") }) @@ -314,7 +377,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { testRepo, testRepoPath, testCleanup := testhelper.NewTestRepo(t) defer testCleanup() - repo := NewRepository(testRepo, config.Config) + repo := New(testRepo, config.Config) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", "source", sourceRepoPath) require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{})) @@ -324,7 +387,7 @@ func TestLocalRepository_FetchRemote(t *testing.T) { require.NoError(t, repo.FetchRemote(ctx, "source", FetchOpts{Prune: true})) - contains, err := repo.HasRevision(ctx, Revision("refs/remotes/source/markdown")) + contains, err := repo.HasRevision(ctx, git.Revision("refs/remotes/source/markdown")) require.NoError(t, err) require.False(t, contains, "remote tracking branch should be pruned as it no longer exists on the remote") }) @@ -341,11 +404,11 @@ func TestLocalRepository_FetchRemote(t *testing.T) { tagsAfter := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", "--list") require.Empty(t, tagsAfter) - containsBranches, err := repo.HasRevision(ctx, Revision("'test'")) + containsBranches, err := repo.HasRevision(ctx, git.Revision("'test'")) require.NoError(t, err) require.False(t, containsBranches) - containsTags, err := repo.HasRevision(ctx, Revision("v1.1.0")) + containsTags, err := repo.HasRevision(ctx, git.Revision("v1.1.0")) require.NoError(t, err) require.False(t, containsTags) }) diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go new file mode 100644 index 000000000..9959116ce --- /dev/null +++ b/internal/git/localrepo/repo.go @@ -0,0 +1,48 @@ +package localrepo + +import ( + "context" + "fmt" + + "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/repository" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" +) + +// Repo represents a local Git repository. +type Repo struct { + repository.GitRepo + commandFactory *git.ExecCommandFactory + cfg config.Cfg +} + +// New creates a new Repo from its protobuf representation. +func New(repo repository.GitRepo, cfg config.Cfg) *Repo { + return &Repo{ + GitRepo: repo, + cfg: cfg, + commandFactory: git.NewExecCommandFactory(cfg), + } +} + +// command creates a Git Command with the given args and Repository, executed +// in the Repository. It validates the arguments in the command before +// executing. +func (repo *Repo) command(ctx context.Context, globals []git.GlobalOption, cmd git.Cmd, opts ...git.CmdOpt) (*command.Command, error) { + return repo.commandFactory.New(ctx, repo, globals, cmd, opts...) +} + +// Config returns executor of the 'config' sub-command. +func (repo *Repo) Config() Config { + return Config{repo: repo} +} + +// Remote returns executor of the 'remote' sub-command. +func (repo *Repo) Remote() Remote { + return Remote{repo: repo} +} + +func errorWithStderr(err error, stderr []byte) error { + return fmt.Errorf("%w, stderr: %q", err, stderr) +} diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go new file mode 100644 index 000000000..931213a58 --- /dev/null +++ b/internal/git/localrepo/repo_test.go @@ -0,0 +1,16 @@ +package localrepo + +import ( + "testing" + + "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" +) + +func TestRepo(t *testing.T) { + git.TestRepository(t, func(t testing.TB, pbRepo *gitalypb.Repository) git.Repository { + t.Helper() + return New(pbRepo, config.Config) + }) +} diff --git a/internal/git/localrepo/testhelper_test.go b/internal/git/localrepo/testhelper_test.go new file mode 100644 index 000000000..e38907063 --- /dev/null +++ b/internal/git/localrepo/testhelper_test.go @@ -0,0 +1,19 @@ +package localrepo + +import ( + "os" + "testing" + + "gitlab.com/gitlab-org/gitaly/internal/testhelper" +) + +func TestMain(m *testing.M) { + os.Exit(testMain(m)) +} + +func testMain(m *testing.M) int { + defer testhelper.MustHaveNoChildProcess() + cleanup := testhelper.Configure() + defer cleanup() + return m.Run() +} diff --git a/internal/git/localrepo_test.go b/internal/git/localrepo_test.go deleted file mode 100644 index 7032ddddc..000000000 --- a/internal/git/localrepo_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package git - -import ( - "testing" - - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" -) - -func TestLocalRepository(t *testing.T) { - TestRepository(t, func(t testing.TB, pbRepo *gitalypb.Repository) Repository { - t.Helper() - return NewRepository(pbRepo, config.Config) - }) -} diff --git a/internal/git/remote.go b/internal/git/remote.go index 9c831ecca..9eafb3994 100644 --- a/internal/git/remote.go +++ b/internal/git/remote.go @@ -72,41 +72,8 @@ type RemoteAddOpts struct { Mirror RemoteAddOptsMirror } -func (opts RemoteAddOpts) buildFlags() []Option { - var flags []Option - for _, b := range opts.RemoteTrackingBranches { - flags = append(flags, ValueFlag{Name: "-t", Value: b}) - } - - if opts.DefaultBranch != "" { - flags = append(flags, ValueFlag{Name: "-m", Value: opts.DefaultBranch}) - } - - if opts.Fetch { - flags = append(flags, Flag{Name: "-f"}) - } - - if opts.Tags != RemoteAddOptsTagsDefault { - flags = append(flags, Flag{Name: opts.Tags.String()}) - } - - if opts.Mirror != RemoteAddOptsMirrorDefault { - flags = append(flags, ValueFlag{Name: "--mirror", Value: opts.Mirror.String()}) - } - - return flags -} - // SetURLOpts are the options for SetURL. type SetURLOpts struct { // Push URLs are manipulated instead of fetch URLs. Push bool } - -func (opts SetURLOpts) buildFlags() []Option { - if opts.Push { - return []Option{Flag{Name: "--push"}} - } - - return nil -} diff --git a/internal/git/remote_test.go b/internal/git/remote_test.go deleted file mode 100644 index fa475253e..000000000 --- a/internal/git/remote_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package git - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestAddRemoteOpts_buildFlags(t *testing.T) { - for _, tc := range []struct { - desc string - opts RemoteAddOpts - exp []Option - }{ - { - desc: "none", - exp: nil, - }, - { - desc: "all set", - opts: RemoteAddOpts{ - Tags: RemoteAddOptsTagsNone, - Fetch: true, - RemoteTrackingBranches: []string{"branch-1", "branch-2"}, - DefaultBranch: "develop", - Mirror: RemoteAddOptsMirrorPush, - }, - exp: []Option{ - ValueFlag{Name: "-t", Value: "branch-1"}, - ValueFlag{Name: "-t", Value: "branch-2"}, - ValueFlag{Name: "-m", Value: "develop"}, - Flag{Name: "-f"}, - Flag{Name: "--no-tags"}, - ValueFlag{Name: "--mirror", Value: "push"}, - }, - }, - { - desc: "with tags", - opts: RemoteAddOpts{Tags: RemoteAddOptsTagsAll}, - exp: []Option{Flag{Name: "--tags"}}, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - require.Equal(t, tc.exp, tc.opts.buildFlags()) - }) - } -} - -func TestRemoteSetURLOpts_buildFlags(t *testing.T) { - for _, tc := range []struct { - desc string - opts SetURLOpts - exp []Option - }{ - { - desc: "none", - exp: nil, - }, - { - desc: "all set", - opts: SetURLOpts{Push: true}, - exp: []Option{Flag{Name: "--push"}}, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - require.Equal(t, tc.exp, tc.opts.buildFlags()) - }) - } -} diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 4e5ac51d4..4b9570266 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -137,7 +138,7 @@ func TestBulkOperation(t *testing.T) { require.NoError(t, updater.Wait()) - refs, err := git.NewRepository(testRepo, config.Config).GetReferences(ctx, "refs/") + refs, err := localrepo.New(testRepo, config.Config).GetReferences(ctx, "refs/") require.NoError(t, err) require.Greater(t, len(refs), 1000, "At least 1000 refs should be present") } diff --git a/internal/git2go/apply_test.go b/internal/git2go/apply_test.go index 07680ee0e..b782cf13e 100644 --- a/internal/git2go/apply_test.go +++ b/internal/git2go/apply_test.go @@ -8,7 +8,7 @@ import ( "time" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -17,7 +17,7 @@ func TestExecutor_Apply(t *testing.T) { pbRepo, repoPath, clean := testhelper.InitBareRepo(t) defer clean() - repo := git.NewRepository(pbRepo, config.Config) + repo := localrepo.New(pbRepo, config.Config) executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"), config.Config.Git.BinPath) ctx, cancel := testhelper.Context() diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go index 8dd3dd3a2..1e4243c20 100644 --- a/internal/git2go/commit_test.go +++ b/internal/git2go/commit_test.go @@ -13,7 +13,7 @@ import ( "time" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -55,7 +55,7 @@ func TestExecutor_Commit(t *testing.T) { pbRepo, repoPath, clean := testhelper.InitBareRepo(t) defer clean() - repo := git.NewRepository(pbRepo, config.Config) + repo := localrepo.New(pbRepo, config.Config) originalFile, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("original")) require.NoError(t, err) @@ -495,7 +495,7 @@ func TestExecutor_Commit(t *testing.T) { } } -func getCommit(t testing.TB, ctx context.Context, repo *git.LocalRepository, oid string) commit { +func getCommit(t testing.TB, ctx context.Context, repo *localrepo.Repo, oid string) commit { t.Helper() data, err := repo.ReadObject(ctx, oid) diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go index 903801544..921e1f561 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -71,7 +72,7 @@ func TestApplyBfgObjectMapStreamSuccess(t *testing.T) { require.NoError(t, err) // Ensure that the internal refs are gone, but the others still exist - refs, err := git.NewRepository(testRepo, config.Config).GetReferences(ctx, "refs/") + refs, err := localrepo.New(testRepo, config.Config).GetReferences(ctx, "refs/") require.NoError(t, err) refNames := make([]string, len(refs)) diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go index 923fb0bed..7a4d811ec 100644 --- a/internal/gitaly/service/commit/list_files.go +++ b/internal/gitaly/service/commit/list_files.go @@ -8,6 +8,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" @@ -44,7 +45,7 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit revision = string(defaultBranch) } - contained, err := git.NewRepository(in.Repository, s.cfg).HasRevision(stream.Context(), git.Revision(revision)) + contained, err := localrepo.New(in.Repository, s.cfg).HasRevision(stream.Context(), git.Revision(revision)) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go index 4bdadf8d5..dac9e1525 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files.go +++ b/internal/gitaly/service/conflicts/list_conflict_files.go @@ -8,6 +8,7 @@ import ( "unicode/utf8" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -21,7 +22,7 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s return helper.ErrInvalidArgument(err) } - repo := git.NewRepository(request.Repository, s.cfg) + repo := localrepo.New(request.Repository, s.cfg) ours, err := repo.ResolveRevision(ctx, git.Revision(request.OurCommitOid+"^{commit}")) if err != nil { diff --git a/internal/gitaly/service/conflicts/list_conflict_files_test.go b/internal/gitaly/service/conflicts/list_conflict_files_test.go index 01c2ef17b..396aeb3d3 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files_test.go +++ b/internal/gitaly/service/conflicts/list_conflict_files_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -209,7 +210,7 @@ func buildCommit(t *testing.T, ctx context.Context, repo *gitalypb.Repository, r testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "commit", "-m", "message") - oid, err := git.NewRepository(repo, config.Config).ResolveRevision(ctx, git.Revision("HEAD")) + oid, err := localrepo.New(repo, config.Config).ResolveRevision(ctx, git.Revision("HEAD")) require.NoError(t, err) testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "reset", "--hard", "HEAD~") diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go index 495f411cb..3d1a93145 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts.go @@ -15,6 +15,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/conflict" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" @@ -225,7 +226,7 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader return err } - if err := git.NewRepository(header.GetRepository(), s.cfg).UpdateRef( + if err := localrepo.New(header.GetRepository(), s.cfg).UpdateRef( stream.Context(), git.ReferenceName("refs/heads/"+string(header.GetSourceBranch())), commitOID, @@ -269,7 +270,7 @@ func sameRepo(left, right *gitalypb.Repository) bool { func (s *server) repoWithBranchCommit(ctx context.Context, srcRepo, targetRepo *gitalypb.Repository, srcBranch, targetBranch []byte) error { const peelCommit = "^{commit}" - src := git.NewRepository(srcRepo, s.cfg) + src := localrepo.New(srcRepo, s.cfg) if sameRepo(srcRepo, targetRepo) { _, err := src.ResolveRevision(ctx, git.Revision(string(targetBranch)+peelCommit)) return err diff --git a/internal/gitaly/service/diff/find_changed_paths.go b/internal/gitaly/service/diff/find_changed_paths.go index 99789e6db..64e621811 100644 --- a/internal/gitaly/service/diff/find_changed_paths.go +++ b/internal/gitaly/service/diff/find_changed_paths.go @@ -9,6 +9,7 @@ import ( "github.com/golang/protobuf/proto" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -135,7 +136,7 @@ func (s *server) validateFindChangedPathsRequestParams(ctx context.Context, in * return err } - gitRepo := git.NewRepository(repo, config.Config) + gitRepo := localrepo.New(repo, config.Config) for _, commit := range in.GetCommits() { if commit == "" { diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index a7cac180d..4a7c26658 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -7,6 +7,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" @@ -38,7 +39,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB // like BranchName. See // https://gitlab.com/gitlab-org/gitaly/-/issues/3331 // - // startPointReference, err := git.NewRepository(req.Repository).GetReference(ctx, "refs/heads/"+string(req.StartPoint)) + // startPointReference, err := localrepo.New(req.Repository).GetReference(ctx, "refs/heads/"+string(req.StartPoint)) // startPointCommit, err := log.GetCommit(ctx, req.Repository, startPointReference.Target) startPointCommit, err := log.GetCommit(ctx, s.locator, req.Repository, git.Revision(req.StartPoint)) // END TODO @@ -47,7 +48,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB } referenceName := fmt.Sprintf("refs/heads/%s", req.BranchName) - _, err = git.NewRepository(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) + _, err = localrepo.New(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) if err == nil { return nil, status.Errorf(codes.FailedPrecondition, "Could not update %s. Please refresh and try again.", req.BranchName) } else if !errors.Is(err, git.ErrReferenceNotFound) { @@ -180,7 +181,7 @@ func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteB referenceFmt = "%s" } referenceName := fmt.Sprintf(referenceFmt, req.BranchName) - referenceValue, err := git.NewRepository(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) + referenceValue, err := localrepo.New(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "branch not found: %s", req.BranchName) } diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 814559628..bfb5445f9 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -14,6 +14,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" @@ -183,7 +184,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi remoteRepo = nil } - localRepo := git.NewRepository(header.Repository, s.cfg) + localRepo := localrepo.New(header.Repository, s.cfg) targetBranchName := "refs/heads/" + string(header.BranchName) targetBranchCommit, err := localRepo.ResolveRevision(ctx, git.Revision(targetBranchName+"^{commit}")) @@ -430,7 +431,7 @@ func (s *Server) resolveParentCommit(ctx context.Context, local git.Repository, } func (s *Server) fetchMissingCommit(ctx context.Context, local, remote *gitalypb.Repository, commitID string) error { - if _, err := git.NewRepository(local, s.cfg).ResolveRevision(ctx, git.Revision(commitID+"^{commit}")); err != nil { + if _, err := localrepo.New(local, s.cfg).ResolveRevision(ctx, git.Revision(commitID+"^{commit}")); err != nil { if !errors.Is(err, git.ErrReferenceNotFound) || remote == nil { return fmt.Errorf("lookup parent commit: %w", err) } diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 4ceb8be2b..8fc6af5e8 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -10,6 +10,7 @@ import ( "github.com/golang/protobuf/ptypes" "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" @@ -58,7 +59,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc } branch := "refs/heads/" + text.ChompBytes(firstRequest.Branch) - revision, err := git.NewRepository(repo, s.cfg).ResolveRevision(ctx, git.Revision(branch)) + revision, err := localrepo.New(repo, s.cfg).ResolveRevision(ctx, git.Revision(branch)) if err != nil { return err } @@ -158,7 +159,7 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ } branch := fmt.Sprintf("refs/heads/%s", in.Branch) - revision, err := git.NewRepository(in.Repository, s.cfg).ResolveRevision(ctx, git.Revision(branch)) + revision, err := localrepo.New(in.Repository, s.cfg).ResolveRevision(ctx, git.Revision(branch)) if err != nil { return nil, helper.ErrInvalidArgument(err) } @@ -248,7 +249,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge return nil, err } - repo := git.NewRepository(request.Repository, s.cfg) + repo := localrepo.New(request.Repository, s.cfg) revision := git.Revision(request.Branch) if request.FirstParentRef != nil { diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index d08ff4b85..33e229b37 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -14,6 +14,7 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" @@ -339,7 +340,7 @@ func TestUserMergeBranch_ambiguousReference(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", mergeBranchName, mergeBranchHeadBefore) - repo := git.NewRepository(testRepo, config.Config) + repo := localrepo.New(testRepo, config.Config) masterOID, err := repo.ResolveRevision(ctx, "refs/heads/master") require.NoError(t, err) diff --git a/internal/gitaly/service/operations/revert.go b/internal/gitaly/service/operations/revert.go index 16946feca..26716afa5 100644 --- a/internal/gitaly/service/operations/revert.go +++ b/internal/gitaly/service/operations/revert.go @@ -8,6 +8,7 @@ import ( "github.com/golang/protobuf/ptypes" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" @@ -29,7 +30,7 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest return nil, err } - localRepo := git.NewRepository(req.Repository, s.cfg) + localRepo := localrepo.New(req.Repository, s.cfg) repoHadBranches, err := localRepo.HasBranches(ctx) if err != nil { return nil, err @@ -161,7 +162,7 @@ func (s *Server) fetchStartRevision(ctx context.Context, req *gitalypb.UserRever return startRevision.String(), nil } - _, err = git.NewRepository(req.Repository, s.cfg).ResolveRevision(ctx, startRevision.Revision()+"^{commit}") + _, err = localrepo.New(req.Repository, s.cfg).ResolveRevision(ctx, startRevision.Revision()+"^{commit}") if errors.Is(err, git.ErrReferenceNotFound) { if err := s.fetchRemoteObject(ctx, req.Repository, req.StartRepository, startRevision.String()); err != nil { return "", helper.ErrInternalf("fetch start: %w", err) diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go index fd707ee23..287dd4ba7 100644 --- a/internal/gitaly/service/operations/submodules.go +++ b/internal/gitaly/service/operations/submodules.go @@ -11,6 +11,7 @@ import ( "github.com/golang/protobuf/ptypes" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git2go" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -77,7 +78,7 @@ func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest } func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpdateSubmoduleRequest) (*gitalypb.UserUpdateSubmoduleResponse, error) { - repo := git.NewRepository(req.GetRepository(), s.cfg) + repo := localrepo.New(req.GetRepository(), s.cfg) branches, err := repo.GetBranches(ctx) if err != nil { return nil, fmt.Errorf("%s: get branches: %w", userUpdateSubmoduleName, err) diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go index a3185b2cf..88e46f4b8 100644 --- a/internal/gitaly/service/operations/tags.go +++ b/internal/gitaly/service/operations/tags.go @@ -11,6 +11,7 @@ import ( "github.com/golang/protobuf/ptypes" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -30,7 +31,7 @@ func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagR } referenceName := fmt.Sprintf("refs/tags/%s", req.TagName) - revision, err := git.NewRepository(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) + revision, err := localrepo.New(req.Repository, s.cfg).GetReference(ctx, git.ReferenceName(referenceName)) if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "tag not found: %s", req.TagName) } @@ -166,7 +167,7 @@ func (s *Server) userCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTa refObjectID := targetObjectID var tagObject *gitalypb.Tag if makingTag { - localRepo := git.NewRepository(repo, s.cfg) + localRepo := localrepo.New(repo, s.cfg) committerTime := time.Now() if req.Timestamp != nil { @@ -178,12 +179,12 @@ func (s *Server) userCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTa tagObjectID, err := localRepo.WriteTag(ctx, targetObjectID, targetObjectType, req.TagName, req.User.Name, req.User.Email, req.Message, committerTime) if err != nil { - var FormatTagError git.FormatTagError + var FormatTagError localrepo.FormatTagError if errors.As(err, &FormatTagError) { return nil, status.Errorf(codes.Unknown, "Rugged::InvalidError: failed to parse signature - expected prefix doesn't match actual") } - var MktagError git.MktagError + var MktagError localrepo.MktagError if errors.As(err, &MktagError) { return nil, status.Errorf(codes.NotFound, "Gitlab::Git::CommitError: %s", err.Error()) } diff --git a/internal/gitaly/service/operations/update_with_hooks_test.go b/internal/gitaly/service/operations/update_with_hooks_test.go index f9cc9e2c3..2f7f667e7 100644 --- a/internal/gitaly/service/operations/update_with_hooks_test.go +++ b/internal/gitaly/service/operations/update_with_hooks_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" hookservice "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" @@ -271,12 +272,12 @@ func TestUpdateReferenceWithHooks(t *testing.T) { } if tc.expectedRefDeletion { - contained, err := git.NewRepository(repo, config.Config).HasRevision(ctx, git.Revision("refs/heads/master")) + contained, err := localrepo.New(repo, config.Config).HasRevision(ctx, git.Revision("refs/heads/master")) require.NoError(t, err) require.False(t, contained, "branch should have been deleted") testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "branch", "master", oldRev) } else { - ref, err := git.NewRepository(repo, config.Config).GetReference(ctx, "refs/heads/master") + ref, err := localrepo.New(repo, config.Config).GetReference(ctx, "refs/heads/master") require.NoError(t, err) require.Equal(t, ref.Target, oldRev) } diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index 6c1733666..eb23e6f7e 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -5,6 +5,7 @@ import ( "errors" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -20,7 +21,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest repo := req.GetRepository() branchName := git.NewBranchReferenceName(string(req.GetName())) - branchRef, err := git.NewRepository(repo, config.Config).GetReference(ctx, branchName) + branchRef, err := localrepo.New(repo, config.Config).GetReference(ctx, branchName) if err != nil { if errors.Is(err, git.ErrReferenceNotFound) { return &gitalypb.FindBranchResponse{}, nil diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go index 5941e9fc9..2bbf6bfc1 100644 --- a/internal/gitaly/service/ref/delete_refs.go +++ b/internal/gitaly/service/ref/delete_refs.go @@ -7,6 +7,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -60,7 +61,7 @@ func refsToRemove(ctx context.Context, req *gitalypb.DeleteRefsRequest) ([]git.R prefixes[i] = string(prefix) } - existingRefs, err := git.NewRepository(req.GetRepository(), config.Config).GetReferences(ctx, "") + existingRefs, err := localrepo.New(req.GetRepository(), config.Config).GetReferences(ctx, "") if err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/delete_refs_test.go b/internal/gitaly/service/ref/delete_refs_test.go index 2db9737bd..49436fc9d 100644 --- a/internal/gitaly/service/ref/delete_refs_test.go +++ b/internal/gitaly/service/ref/delete_refs_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -55,7 +55,7 @@ func TestSuccessfulDeleteRefs(t *testing.T) { require.NoError(t, err) // Ensure that the internal refs are gone, but the others still exist - refs, err := git.NewRepository(repo, config.Config).GetReferences(ctx, "refs/") + refs, err := localrepo.New(repo, config.Config).GetReferences(ctx, "refs/") require.NoError(t, err) refNames := make([]string, len(refs)) diff --git a/internal/gitaly/service/ref/pack_refs_test.go b/internal/gitaly/service/ref/pack_refs_test.go index a1f4e6c68..1d7706d0e 100644 --- a/internal/gitaly/service/ref/pack_refs_test.go +++ b/internal/gitaly/service/ref/pack_refs_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -32,7 +33,7 @@ func TestPackRefsSuccessfulRequest(t *testing.T) { packedRefs := linesInPackfile(t, testRepoPath) - repo := git.NewRepository(testRepo, config.Config) + repo := localrepo.New(testRepo, config.Config) // creates some new heads newBranches := 10 diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go index d77a40ac2..99aa18181 100644 --- a/internal/gitaly/service/repository/fetch.go +++ b/internal/gitaly/service/repository/fetch.go @@ -6,6 +6,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/internal/gitalyssh" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -21,7 +22,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc return nil, helper.ErrInvalidArgument(err) } - targetRepo := git.NewRepository(req.GetRepository(), s.cfg) + targetRepo := localrepo.New(req.GetRepository(), s.cfg) sourceRepo, err := remoterepo.New(ctx, req.GetSourceRepository(), s.conns) if err != nil { diff --git a/internal/gitaly/service/repository/fetch_remote.go b/internal/gitaly/service/repository/fetch_remote.go index bfc351802..b82f61d66 100644 --- a/internal/gitaly/service/repository/fetch_remote.go +++ b/internal/gitaly/service/repository/fetch_remote.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/errors" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -29,19 +30,19 @@ func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteReque } var stderr bytes.Buffer - opts := git.FetchOpts{ + opts := localrepo.FetchOpts{ Stderr: &stderr, Force: req.Force, Prune: !req.NoPrune, - Tags: git.FetchOptsTagsAll, + Tags: localrepo.FetchOptsTagsAll, Verbose: req.GetCheckTagsChanged(), } if req.GetNoTags() { - opts.Tags = git.FetchOptsTagsNone + opts.Tags = localrepo.FetchOptsTagsNone } - repo := git.NewRepository(req.GetRepository(), s.cfg) + repo := localrepo.New(req.GetRepository(), s.cfg) remoteName := req.GetRemote() if params := req.GetRemoteParams(); params != nil { @@ -226,7 +227,7 @@ func (s *server) getRefspecs(refmaps []string) []string { return refspecs } -func (s *server) setRemote(ctx context.Context, repo *git.LocalRepository, name, url string) error { +func (s *server) setRemote(ctx context.Context, repo *localrepo.Repo, name, url string) error { if err := repo.Remote().Remove(ctx, name); err != nil { if err != git.ErrNotFound { return fmt.Errorf("remove remote: %w", err) @@ -240,7 +241,7 @@ func (s *server) setRemote(ctx context.Context, repo *git.LocalRepository, name, return nil } -func (s *server) removeRemote(ctx context.Context, repo *git.LocalRepository, name string) error { +func (s *server) removeRemote(ctx context.Context, repo *localrepo.Repo, name string) error { if err := repo.Remote().Remove(ctx, name); err != nil { if err != git.ErrNotFound { return fmt.Errorf("remove remote: %w", err) diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go index 84c049c5d..4a64615c6 100644 --- a/internal/gitaly/service/repository/fetch_remote_test.go +++ b/internal/gitaly/service/repository/fetch_remote_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/storage" @@ -84,13 +85,13 @@ func TestFetchRemote_withDefaultRefmaps(t *testing.T) { sourceRepoProto, sourceRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - sourceRepo := git.NewRepository(sourceRepoProto, config.Config) + sourceRepo := localrepo.New(sourceRepoProto, config.Config) targetRepoProto, targetRepoPath := copyRepoWithNewRemote(t, sourceRepoProto, locator, "my-remote") defer func() { require.NoError(t, os.RemoveAll(targetRepoPath)) }() - targetRepo := git.NewRepository(targetRepoProto, config.Config) + targetRepo := localrepo.New(targetRepoProto, config.Config) port, stopGitServer := testhelper.GitServer(t, config.Config, sourceRepoPath, nil) defer func() { require.NoError(t, stopGitServer()) }() @@ -214,7 +215,7 @@ func TestFetchRemote_prune(t *testing.T) { defer func() { require.NoError(t, os.RemoveAll(targetRepoPath)) }() - targetRepo := git.NewRepository(targetRepoProto, config.Config) + targetRepo := localrepo.New(targetRepoProto, config.Config) ctx, cancel := testhelper.Context() defer cancel() diff --git a/internal/gitaly/service/repository/gc_test.go b/internal/gitaly/service/repository/gc_test.go index 34f8956f6..5dc274964 100644 --- a/internal/gitaly/service/repository/gc_test.go +++ b/internal/gitaly/service/repository/gc_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/gittest" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -58,7 +59,7 @@ func TestGarbageCollectCommitGraph(t *testing.T) { require.NoError(t, err) defer cfgF.Close() - cfg, err := git.NewRepository(testRepo, config.Config).Config().GetRegexp(ctx, "core.commitgraph", git.ConfigGetRegexpOpts{}) + cfg, err := localrepo.New(testRepo, config.Config).Config().GetRegexp(ctx, "core.commitgraph", git.ConfigGetRegexpOpts{}) require.NoError(t, err) require.Equal(t, []git.ConfigPair{{Key: "core.commitgraph", Value: "true"}}, cfg) } diff --git a/internal/gitaly/service/repository/midx_test.go b/internal/gitaly/service/repository/midx_test.go index 5f2456298..ad13386da 100644 --- a/internal/gitaly/service/repository/midx_test.go +++ b/internal/gitaly/service/repository/midx_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/git/stats" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -46,7 +47,7 @@ func TestMidxWrite(t *testing.T) { require.NoError(t, err) defer cfgF.Close() - cfg, err := git.NewRepository(testRepo, config.Config).Config().GetRegexp(ctx, "core.multipackindex", git.ConfigGetRegexpOpts{}) + cfg, err := localrepo.New(testRepo, config.Config).Config().GetRegexp(ctx, "core.multipackindex", git.ConfigGetRegexpOpts{}) require.NoError(t, err) require.Equal(t, []git.ConfigPair{{Key: "core.multipackindex", Value: "true"}}, cfg) } diff --git a/internal/gitaly/service/repository/repository.go b/internal/gitaly/service/repository/repository.go index 7ff317996..9c53c6004 100644 --- a/internal/gitaly/service/repository/repository.go +++ b/internal/gitaly/service/repository/repository.go @@ -3,7 +3,7 @@ package repository import ( "context" - "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -26,7 +26,7 @@ func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryEx } func (s *server) HasLocalBranches(ctx context.Context, in *gitalypb.HasLocalBranchesRequest) (*gitalypb.HasLocalBranchesResponse, error) { - hasBranches, err := git.NewRepository(in.Repository, s.cfg).HasBranches(ctx) + hasBranches, err := localrepo.New(in.Repository, s.cfg).HasBranches(ctx) if err != nil { return nil, helper.ErrInternal(err) } |