Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2021-01-06 11:53:23 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2021-01-06 12:46:58 +0300
commit94979b06d074bf2cf7e5d2902230edbe6cc1a02d (patch)
treeb5701dd8230b3e4ac758f3ad37b5317409563b3f
parent7402234c9f0a003f84b9c650e2258f16c0c35944 (diff)
git: Convert subcommands to use a struct
The map of subcommands is currently a simple bitfield used for flags. While this has served its purpose, it's inflexible and doesn't allow us to further customize specific commands. While one could do so via flags as well, it is cumbersome and rather indirect. Let's refactor the map to use a struct instead. While it currently only has a `flags` member, it'll get extended in a subsequent commit to also carry command-specific options.
-rw-r--r--internal/git/safecmd.go25
-rw-r--r--internal/git/subcommand.go200
-rw-r--r--internal/git/subcommand_test.go10
3 files changed, 160 insertions, 75 deletions
diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go
index 9c696b8a1..7f559623d 100644
--- a/internal/git/safecmd.go
+++ b/internal/git/safecmd.go
@@ -62,12 +62,13 @@ func (sc SubCmd) Subcommand() string { return sc.Name }
func (sc SubCmd) CommandArgs() ([]string, error) {
var safeArgs []string
- if _, ok := subcommands[sc.Name]; !ok {
+ gitCommand, ok := gitCommands[sc.Name]
+ if !ok {
return nil, fmt.Errorf("invalid sub command name %q: %w", sc.Name, ErrInvalidArg)
}
safeArgs = append(safeArgs, sc.Name)
- commandArgs, err := assembleCommandArgs(sc.Name, sc.Flags, sc.Args, sc.PostSepArgs)
+ commandArgs, err := assembleCommandArgs(gitCommand, sc.Flags, sc.Args, sc.PostSepArgs)
if err != nil {
return nil, err
}
@@ -76,7 +77,7 @@ func (sc SubCmd) CommandArgs() ([]string, error) {
return safeArgs, nil
}
-func assembleCommandArgs(command string, flags []Option, args []string, postSepArgs []string) ([]string, error) {
+func assembleCommandArgs(gitCommand gitCommand, flags []Option, args []string, postSepArgs []string) ([]string, error) {
var commandArgs []string
for _, o := range flags {
@@ -94,7 +95,7 @@ func assembleCommandArgs(command string, flags []Option, args []string, postSepA
commandArgs = append(commandArgs, a)
}
- if supportsEndOfOptions(command) {
+ if gitCommand.supportsEndOfOptions() {
commandArgs = append(commandArgs, "--end-of-options")
}
@@ -144,7 +145,8 @@ var actionRegex = regexp.MustCompile(`^[[:alnum:]]+[-[:alnum:]]*$`)
func (sc SubSubCmd) CommandArgs() ([]string, error) {
var safeArgs []string
- if _, ok := subcommands[sc.Name]; !ok {
+ gitCommand, ok := gitCommands[sc.Name]
+ if !ok {
return nil, fmt.Errorf("invalid sub command name %q: %w", sc.Name, ErrInvalidArg)
}
safeArgs = append(safeArgs, sc.Name)
@@ -154,7 +156,7 @@ func (sc SubSubCmd) CommandArgs() ([]string, error) {
}
safeArgs = append(safeArgs, sc.Action)
- commandArgs, err := assembleCommandArgs(sc.Name, sc.Flags, sc.Args, sc.PostSepArgs)
+ commandArgs, err := assembleCommandArgs(gitCommand, sc.Flags, sc.Args, sc.PostSepArgs)
if err != nil {
return nil, err
}
@@ -316,19 +318,24 @@ var (
)
func handleOpts(ctx context.Context, sc Cmd, cc *cmdCfg, opts []CmdOpt) error {
+ gitCommand, ok := gitCommands[sc.Subcommand()]
+ if !ok {
+ return fmt.Errorf("invalid sub command name %q: %w", sc.Subcommand(), ErrInvalidArg)
+ }
+
for _, opt := range opts {
if err := opt(cc); err != nil {
return err
}
}
- if !cc.hooksConfigured && mayUpdateRef(sc.Subcommand()) {
+ if !cc.hooksConfigured && gitCommand.mayUpdateRef() {
return fmt.Errorf("subcommand %q: %w", sc.Subcommand(), ErrRefHookRequired)
}
- if cc.hooksConfigured && !mayUpdateRef(sc.Subcommand()) {
+ if cc.hooksConfigured && !gitCommand.mayUpdateRef() {
return fmt.Errorf("subcommand %q: %w", sc.Subcommand(), ErrRefHookNotRequired)
}
- if mayGeneratePackfiles(sc.Subcommand()) {
+ if gitCommand.mayGeneratePackfiles() {
cc.globals = append(cc.globals, ConfigPair{
Key: "pack.windowMemory", Value: "100m",
})
diff --git a/internal/git/subcommand.go b/internal/git/subcommand.go
index 527a04a0e..09404ba76 100644
--- a/internal/git/subcommand.go
+++ b/internal/git/subcommand.go
@@ -11,81 +11,155 @@ const (
scGeneratesPackfiles
)
-// subcommands is a curated list of Git command names for special git.SafeCmd
+type gitCommand struct {
+ flags uint
+}
+
+// gitCommands is a curated list of Git command names for special git.SafeCmd
// validation logic
-var subcommands = map[string]uint{
- "apply": scNoRefUpdates,
- "archive": scReadOnly | scNoEndOfOptions,
- "blame": scReadOnly | scNoEndOfOptions,
- "bundle": scReadOnly | scGeneratesPackfiles,
- "cat-file": scReadOnly,
- "checkout": scNoEndOfOptions,
- "clone": scNoEndOfOptions | scGeneratesPackfiles,
- "commit": 0,
- "commit-graph": scNoRefUpdates,
- "config": scNoRefUpdates | scNoEndOfOptions,
- "count-objects": scReadOnly,
- "diff": scReadOnly,
- "diff-tree": scReadOnly,
- "fetch": 0,
- "for-each-ref": scReadOnly | scNoEndOfOptions,
- "format-patch": scReadOnly,
- "fsck": scReadOnly,
- "gc": scNoRefUpdates | scGeneratesPackfiles,
- "grep": scReadOnly | scNoEndOfOptions,
- "hash-object": scNoRefUpdates,
- "init": scNoRefUpdates,
- "linguist": scNoEndOfOptions,
- "log": scReadOnly,
- "ls-remote": scReadOnly,
- "ls-tree": scReadOnly | scNoEndOfOptions,
- "merge-base": scReadOnly,
- "multi-pack-index": scNoRefUpdates,
- "pack-refs": scNoRefUpdates,
- "receive-pack": 0,
- "remote": scNoEndOfOptions,
- "repack": scNoRefUpdates | scGeneratesPackfiles,
- "rev-list": scReadOnly,
- "rev-parse": scReadOnly | scNoEndOfOptions,
- "show": scReadOnly,
- "show-ref": scReadOnly,
- "symbolic-ref": 0,
- "tag": 0,
- "update-ref": 0,
- "upload-archive": scReadOnly | scNoEndOfOptions,
- "upload-pack": scReadOnly | scGeneratesPackfiles,
- "worktree": 0,
+var gitCommands = map[string]gitCommand{
+ "apply": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "archive": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "blame": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "bundle": gitCommand{
+ flags: scReadOnly | scGeneratesPackfiles,
+ },
+ "cat-file": gitCommand{
+ flags: scReadOnly,
+ },
+ "checkout": gitCommand{
+ flags: scNoEndOfOptions,
+ },
+ "clone": gitCommand{
+ flags: scNoEndOfOptions | scGeneratesPackfiles,
+ },
+ "commit": gitCommand{
+ flags: 0,
+ },
+ "commit-graph": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "config": gitCommand{
+ flags: scNoRefUpdates | scNoEndOfOptions,
+ },
+ "count-objects": gitCommand{
+ flags: scReadOnly,
+ },
+ "diff": gitCommand{
+ flags: scReadOnly,
+ },
+ "diff-tree": gitCommand{
+ flags: scReadOnly,
+ },
+ "fetch": gitCommand{
+ flags: 0,
+ },
+ "for-each-ref": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "format-patch": gitCommand{
+ flags: scReadOnly,
+ },
+ "fsck": gitCommand{
+ flags: scReadOnly,
+ },
+ "gc": gitCommand{
+ flags: scNoRefUpdates | scGeneratesPackfiles,
+ },
+ "grep": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "hash-object": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "init": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "linguist": gitCommand{
+ flags: scNoEndOfOptions,
+ },
+ "log": gitCommand{
+ flags: scReadOnly,
+ },
+ "ls-remote": gitCommand{
+ flags: scReadOnly,
+ },
+ "ls-tree": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "merge-base": gitCommand{
+ flags: scReadOnly,
+ },
+ "multi-pack-index": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "pack-refs": gitCommand{
+ flags: scNoRefUpdates,
+ },
+ "receive-pack": gitCommand{
+ flags: 0,
+ },
+ "remote": gitCommand{
+ flags: scNoEndOfOptions,
+ },
+ "repack": gitCommand{
+ flags: scNoRefUpdates | scGeneratesPackfiles,
+ },
+ "rev-list": gitCommand{
+ flags: scReadOnly,
+ },
+ "rev-parse": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "show": gitCommand{
+ flags: scReadOnly,
+ },
+ "show-ref": gitCommand{
+ flags: scReadOnly,
+ },
+ "symbolic-ref": gitCommand{
+ flags: 0,
+ },
+ "tag": gitCommand{
+ flags: 0,
+ },
+ "update-ref": gitCommand{
+ flags: 0,
+ },
+ "upload-archive": gitCommand{
+ flags: scReadOnly | scNoEndOfOptions,
+ },
+ "upload-pack": gitCommand{
+ flags: scReadOnly | scGeneratesPackfiles,
+ },
+ "worktree": gitCommand{
+ flags: 0,
+ },
}
-// mayUpdateRef indicates if a subcommand is known to update references.
+// mayUpdateRef indicates if a gitCommand is known to update references.
// This is useful to determine if a command requires reference hook
// configuration. A non-exhaustive list of commands is consulted to determine if
// refs are updated. When unknown, true is returned to err on the side of
// caution.
-func mayUpdateRef(subcmd string) bool {
- flags, ok := subcommands[subcmd]
- if !ok {
- return true
- }
- return flags&(scReadOnly|scNoRefUpdates) == 0
+func (c gitCommand) mayUpdateRef() bool {
+ return c.flags&(scReadOnly|scNoRefUpdates) == 0
}
-// mayGeneratePackfiles indicates if a subcommand is known to generate
+// mayGeneratePackfiles indicates if a gitCommand is known to generate
// packfiles. This is used in order to inject packfile configuration.
-func mayGeneratePackfiles(subcmd string) bool {
- flags, ok := subcommands[subcmd]
- if !ok {
- return false
- }
- return flags&scGeneratesPackfiles != 0
+func (c gitCommand) mayGeneratePackfiles() bool {
+ return c.flags&scGeneratesPackfiles != 0
}
// supportsEndOfOptions indicates whether a command can handle the
// `--end-of-options` option.
-func supportsEndOfOptions(subcmd string) bool {
- flags, ok := subcommands[subcmd]
- if !ok {
- return true
- }
- return flags&scNoEndOfOptions == 0
+func (c gitCommand) supportsEndOfOptions() bool {
+ return c.flags&scNoEndOfOptions == 0
}
diff --git a/internal/git/subcommand_test.go b/internal/git/subcommand_test.go
index 78dc39f79..e874e817b 100644
--- a/internal/git/subcommand_test.go
+++ b/internal/git/subcommand_test.go
@@ -7,7 +7,11 @@ import (
)
func TestSubcommand_mayGeneratePackfiles(t *testing.T) {
- require.True(t, mayGeneratePackfiles("gc"))
- require.False(t, mayGeneratePackfiles("apply"))
- require.False(t, mayGeneratePackfiles("nonexistent"))
+ gcCmd, ok := gitCommands["gc"]
+ require.True(t, ok)
+ require.True(t, gcCmd.mayGeneratePackfiles())
+
+ applyCmd, ok := gitCommands["apply"]
+ require.True(t, ok)
+ require.False(t, applyCmd.mayGeneratePackfiles())
}