feat: [code-2296] minor improvements on command package (#3046)

* Merge remote-tracking branch 'origin/main' into eb/code-2296
* requested changes
* added unit test and description
* refactor command parser
* minor improvements on command package

git/api/service_pack.go

@@ -65,6 +65,10 @@ func (g *Git) InfoRefs(
 	return nil
 }
 
+type ServicePackConfig struct {
+	UploadPackHook string
+}
+
 type ServicePackOptions struct {
 	Service      enum.GitServiceType
 	Timeout      int // seconds
@@ -74,6 +78,7 @@ type ServicePackOptions struct {
 	Stderr       io.Writer
 	Env          []string
 	Protocol     string
+	Config       ServicePackConfig
 }
 
 func (g *Git) ServicePack(
@@ -94,6 +99,10 @@ func (g *Git) ServicePack(
 		cmd.Add(command.WithEnv("GIT_PROTOCOL", options.Protocol))
 	}
 
+	if options.Config.UploadPackHook != "" {
+		cmd.Add(command.WithConfig("uploadpack.packObjectsHook", options.Config.UploadPackHook))
+	}
+
 	err := cmd.Run(ctx,
 		command.WithDir(repoPath),
 		command.WithStdout(options.Stdout),

git/command/builder.go

@@ -29,6 +29,7 @@ const (
 
 type builder struct {
 	flags                  uint
+	actions                map[string]uint
 	validatePositionalArgs func([]string) error
 }
 
@@ -196,6 +197,16 @@ var descriptions = map[string]builder{
 		// While git-remote(1)'s `add` subcommand does support `--end-of-options`,
 		// `remove` doesn't.
 		flags: NoEndOfOptions,
+		actions: map[string]uint{
+			"add":          0,
+			"rename":       0,
+			"remove":       0,
+			"set-head":     0,
+			"set-branches": 0,
+			"get-url":      0,
+			"set-url":      0,
+			"prune":        0,
+		},
 	},
 	"repack": {
 		flags: NoRefUpdates,
@@ -269,8 +280,12 @@ var descriptions = map[string]builder{
 }
 
 // args validates the given flags and arguments and, if valid, returns the complete command line.
-func (b builder) args(flags []string, args []string, postSepArgs []string) ([]string, error) {
-	var cmdArgs []string
+func (b builder) args(
+	flags []string,
+	args []string,
+	postSepArgs []string,
+) ([]string, error) {
+	cmdArgs := make([]string, 0, len(flags)+len(args)+len(postSepArgs))
 
 	cmdArgs = append(cmdArgs, flags...)
 

git/command/command.go

@@ -32,6 +32,10 @@ var (
 
 // Command contains options for running a git command.
 type Command struct {
+	// Globals is the number of optional flags to pass before command name.
+	// example: git --shallow-file pack-objects ...
+	Globals []string
+
 	// Name is the name of the Git command to run, e.g. "log", "cat-file" or "worktree".
 	Name string
 
@@ -79,6 +83,9 @@ func New(name string, options ...CmdOptionFunc) *Command {
 
 // Clone clones the command object.
 func (c *Command) Clone() *Command {
+	globals := make([]string, len(c.Globals))
+	copy(globals, c.Globals)
+
 	flags := make([]string, len(c.Flags))
 	copy(flags, c.Flags)
 
@@ -176,6 +183,11 @@ func (c *Command) Run(ctx context.Context, opts ...RunOptionFunc) (err error) {
 func (c *Command) makeArgs() ([]string, error) {
 	var safeArgs []string
 
+	// add globals
+	if len(c.Globals) > 0 {
+		safeArgs = append(safeArgs, c.Globals...)
+	}
+
 	commandDescription, ok := descriptions[c.Name]
 	if !ok {
 		return nil, fmt.Errorf("invalid sub command name %q: %w", c.Name, ErrInvalidArg)

git/command/option.go

@@ -23,6 +23,13 @@ import (
 
 type CmdOptionFunc func(c *Command)
 
+// WithGlobal set the global optional flag of the Git command.
+func WithGlobal(flags ...string) CmdOptionFunc {
+	return func(c *Command) {
+		c.Globals = append(c.Globals, flags...)
+	}
+}
+
 // WithAction set the action of the Git command, e.g. "set-url" in `git remote set-url`.
 func WithAction(action string) CmdOptionFunc {
 	return func(c *Command) {

git/command/parser.go

@@ -0,0 +1,123 @@
+// Copyright 2023 Harness, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package command
+
+import (
+	"strings"
+)
+
+// Parse os args to Command object.
+// This is very basic parser which doesn't care about
+// flags or positional args values it just injects into proper
+// slice of command struct. Every git command can contain
+// globals:
+//
+//	git --help
+//
+// command:
+//
+//	git version
+//	git diff
+//
+// action:
+//
+//	git remote set-url ...
+//
+// command or action flags:
+//
+//	git diff --shortstat
+//
+// command or action args:
+//
+//	git diff --shortstat main...dev
+//
+// post args:
+//
+//	git diff main...dev -- file1
+func Parse(args ...string) *Command {
+	actions := map[string]uint{}
+	c := &Command{}
+
+	globalPos := -1
+	namePos := -1
+	actionPos := -1
+	flagsPos := -1
+	argsPos := -1
+	postPos := -1
+
+	if len(args) == 0 {
+		return c
+	}
+
+	if strings.ToLower(args[0]) == "git" {
+		args = args[1:]
+	}
+
+	for i, arg := range args {
+		isFlag := arg != "--" && strings.HasPrefix(arg, "-")
+		b, isCommand := descriptions[arg]
+		_, isAction := actions[arg]
+		switch {
+		case globalPos == -1 && namePos == -1 && isFlag:
+			globalPos = i
+		case namePos == -1 && isCommand:
+			namePos = i
+			actions = b.actions
+		case actionPos == -1 && isAction && !isFlag:
+			actionPos = i
+		case flagsPos == -1 && (namePos >= 0 || actionPos > 0) && isFlag:
+			flagsPos = i
+		case argsPos == -1 && (namePos >= 0 || actionPos > 0) && !isFlag:
+			argsPos = i
+		case postPos == -1 && arg == "--":
+			postPos = i
+		}
+	}
+
+	end := len(args)
+
+	if globalPos >= 0 {
+		c.Globals = args[globalPos:cmpPos(namePos, end)]
+	}
+
+	if namePos >= 0 {
+		c.Name = args[namePos]
+	}
+
+	if actionPos > 0 {
+		c.Action = args[actionPos]
+	}
+
+	if flagsPos > 0 {
+		c.Flags = args[flagsPos:cmpPos(argsPos, end)]
+	}
+
+	if argsPos > 0 {
+		c.Args = args[argsPos:cmpPos(postPos, end)]
+	}
+
+	if postPos > 0 {
+		c.PostSepArgs = args[postPos+1:]
+	}
+
+	return c
+}
+
+func cmpPos(check, or int) int {
+	if check == -1 {
+		return or
+	}
+	return check
+}

git/command/parser_test.go

@@ -0,0 +1,162 @@
+// Copyright 2023 Harness, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package command
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParse(t *testing.T) {
+	type args struct {
+		args []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want *Command
+	}{
+		{
+			name: "git version test",
+			args: args{
+				args: []string{
+					"git",
+					"version",
+				},
+			},
+			want: &Command{
+				Name: "version",
+			},
+		},
+		{
+			name: "git help test",
+			args: args{
+				args: []string{
+					"git",
+					"--help",
+				},
+			},
+			want: &Command{
+				Globals: []string{"--help"},
+			},
+		},
+		{
+			name: "diff basic test",
+			args: args{
+				args: []string{
+					"git",
+					"diff",
+					"main...dev",
+				},
+			},
+			want: &Command{
+				Name: "diff",
+				Args: []string{"main...dev"},
+			},
+		},
+		{
+			name: "diff path test",
+			args: args{
+				args: []string{
+					"git",
+					"diff",
+					"--shortstat",
+					"main...dev",
+					"--",
+					"file1",
+					"file2",
+				},
+			},
+			want: &Command{
+				Name:  "diff",
+				Flags: []string{"--shortstat"},
+				Args:  []string{"main...dev"},
+				PostSepArgs: []string{
+					"file1",
+					"file2",
+				},
+			},
+		},
+		{
+			name: "diff path test",
+			args: args{
+				args: []string{
+					"git",
+					"diff",
+					"--shortstat",
+					"main...dev",
+					"--",
+				},
+			},
+			want: &Command{
+				Name:        "diff",
+				Flags:       []string{"--shortstat"},
+				Args:        []string{"main...dev"},
+				PostSepArgs: []string{},
+			},
+		},
+		{
+			name: "git remote basic test",
+			args: args{
+				args: []string{
+					"git",
+					"remote",
+					"set-url",
+					"origin",
+					"http://reponame",
+				},
+			},
+			want: &Command{
+				Name:   "remote",
+				Action: "set-url",
+				Args:   []string{"origin", "http://reponame"},
+			},
+		},
+		{
+			name: "pack object test",
+			args: args{
+				args: []string{
+					"git",
+					"--shallow-file",
+					"",
+					"pack-objects",
+					"--revs",
+					"--thin",
+					"--stdout",
+					"--progress",
+					"--delta-base-offset",
+				},
+			},
+			want: &Command{
+				Globals: []string{"--shallow-file", ""},
+				Name:    "pack-objects",
+				Flags: []string{
+					"--revs",
+					"--thin",
+					"--stdout",
+					"--progress",
+					"--delta-base-offset",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := Parse(tt.args.args...); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Parse() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}