Improvements for GoVPP CLI (#135)

* Improvements for GoVPP CLI

- added new linter rule MessageSameStatus
- fixed issue with cache of cloned repository
- added option --targz for vppapi export command
- fixed issues with color mode when not using terminal
- added option to vppapi diff command to include comment differences
- added --paths option to filter out specific API files or paths

Signed-off-by: Ondrej Fabry <[email protected]>

* Remove test

Signed-off-by: Ondrej Fabry <[email protected]>

* Resolve review feedback

Signed-off-by: Ondrej Fabry <[email protected]>

---------

Signed-off-by: Ondrej Fabry <[email protected]>

Author: ondrej-fabry

binapigen/generator.go

@@ -50,8 +50,6 @@ type Generator struct {
 	opts Options
 
 	vppapiSchema *vppapi.Schema
-	//apiFiles   []vppapi.File
-	//vppVersion string
 
 	Files       []*File
 	FilesByName map[string]*File

binapigen/vppapi.go

@@ -94,6 +94,15 @@ func normalizeImport(imp string) string {
 	return imp
 }
 
+// SortFilesByName sorts list of files by their name.
+func SortFilesByName(apifiles []vppapi.File) {
+	sort.SliceStable(apifiles, func(i, j int) bool {
+		a := apifiles[i]
+		b := apifiles[j]
+		return a.Name < b.Name
+	})
+}
+
 // SortFilesByImports sorts list of files by their imports.
 func SortFilesByImports(apifiles []vppapi.File) {
 	dependsOn := func(file *vppapi.File, dep string) bool {

binapigen/vppapi/input.go

@@ -106,7 +106,7 @@ const (
 	cacheDir = "./.cache"
 )
 
-func cloneRepoLocally(repo string, commit string, depth int) (string, error) {
+func cloneRepoLocally(repo string, commit string, branch string, depth int) (string, error) {
 	repoPath := strings.ReplaceAll(repo, "/", "_")
 	repoPath = strings.ReplaceAll(repoPath, ":", "_")
 	cachePath := filepath.Join(cacheDir, repoPath)
@@ -116,36 +116,40 @@ func cloneRepoLocally(repo string, commit string, depth int) (string, error) {
 		if err := os.MkdirAll(cacheDir, 0755); err != nil {
 			return "", fmt.Errorf("failed to create cache directory: %w", err)
 		}
-		logrus.Tracef("Cloning repository %q", repo)
 
-		var args []string
+		args := []string{"--single-branch"}
 		if depth > 0 {
 			args = append(args, fmt.Sprintf("--depth=%d", depth))
 		}
 		args = append(args, repo, cachePath)
+		logrus.Debugf("cloning repo: %v", args)
 		cmd := exec.Command("git", append([]string{"clone"}, args...)...)
 		if output, err := cmd.CombinedOutput(); err != nil {
 			return "", fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, output)
 		}
 	} else if err != nil {
 		return "", fmt.Errorf("failed to check if cache exists: %w", err)
-	} else {
-		logrus.Debugf("Fetching %q", commit)
-		cmd := exec.Command("git", "fetch", "origin", commit)
-		cmd.Dir = cachePath
-		if output, err := cmd.CombinedOutput(); err != nil {
-			return "", fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, output)
-		}
+	}
+	logrus.Debugf("local repo dir: %q, fetching %q", cachePath, commit)
+
+	cmd := exec.Command("git", "fetch", "-f", "origin", commit)
+	cmd.Dir = cachePath
+	if output, err := cmd.CombinedOutput(); err != nil {
+		return "", fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, output)
 	}
 
 	// Resolve the commit hash for the given branch/tag
-	commitHash, err := resolveCommitHash(cachePath, commit)
+	ref := commit
+	if branch != "" {
+		ref = "origin/" + branch
+	}
+	commitHash, err := resolveCommitHash(cachePath, ref)
 	if err != nil {
 		return "", fmt.Errorf("failed to resolve commit hash: %w", err)
 	}
 
 	// Check out the repository at the resolved commit
-	cmd := exec.Command("git", "checkout", commitHash)
+	cmd = exec.Command("git", "checkout", commitHash)
 	cmd.Dir = cachePath
 	if output, err := cmd.CombinedOutput(); err != nil {
 		return "", fmt.Errorf("failed to check out repository: %w\nOutput: %s", err, output)

binapigen/vppapi/input_test.go

@@ -40,21 +40,11 @@ func Test_runCommandInRepo(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		{
-			name: "tag",
-			args: args{
-				repo:    "https://github.com/FDio/vpp.git",
-				commit:  "v23.02",
-				command: "bash",
-				args:    []string{"-exc", "bash ./src/scripts/version; make json-api-files && ls ./build-root/install-vpp-native/vpp/share/vpp/api"},
-			},
-			wantErr: false,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			//, tt.args.command, tt.args.args...
-			_, err := cloneRepoLocally(tt.args.repo, tt.args.commit, 0)
+			_, err := cloneRepoLocally(tt.args.repo, tt.args.commit, "", 0)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("cloneRepoLocally() error = %v, wantErr %v", err, tt.wantErr)
 			}

binapigen/vppapi/source.go

@@ -65,7 +65,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) {
 		return nil, fmt.Errorf("invalid path in input reference")
 	}
 
-	logrus.Tracef("retrieving input:\n%v", input)
+	logrus.Tracef("retrieving input: %+v", input)
 
 	switch input.Format {
 	case FormatNone:
@@ -90,7 +90,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) {
 			return nil, fmt.Errorf("cannot set both branch and tag")
 		} else if branch != "" || tag != "" {
 			if ref != "" {
-				return nil, fmt.Errorf("cannot set ref if branch or tag is set")
+				return nil, fmt.Errorf("cannot set rev if branch or tag is set")
 			}
 			if branch != "" {
 				ref = branch
@@ -115,7 +115,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) {
 
 		logrus.Debugf("updating local repo %s to %s", input.Path, commit)
 
-		repoDir, err := cloneRepoLocally(input.Path, commit, cloneDepth)
+		repoDir, err := cloneRepoLocally(input.Path, commit, branch, cloneDepth)
 		if err != nil {
 			return nil, err
 		}

binapigen/vppapi/util.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -53,7 +54,10 @@ func ResolveApiDir(dir string) string {
 		// check if the API directory exists
 		_, err := os.Stat(apiDirPath)
 		if err == nil {
-			logrus.Tracef("returning %q as api dir", localVPPBuildApiDir)
+			logrus.Tracef("api dir %q exists, running 'make json-api-files'", localVPPBuildApiDir)
+			if err := makeJsonApiFiles(dir); err != nil {
+				logrus.Warnf("make json-api-files failed: %v", err)
+			}
 			return apiDirPath
 		} else if errors.Is(err, os.ErrNotExist) {
 			logrus.Tracef("api dir %q does not exist, running 'make json-api-files'", localVPPBuildApiDir)
@@ -94,12 +98,6 @@ func makeJsonApiFiles(dir string) error {
 //
 // Version resolved here can be overriden by setting VPP_VERSION env var.
 func ResolveVPPVersion(apidir string) string {
-	// check env variable override
-	if ver := os.Getenv(VPPVersionEnvVar); ver != "" {
-		logrus.Debugf("VPP version was manually set to %q via %s env var", ver, VPPVersionEnvVar)
-		return ver
-	}
-
 	// check if inside VPP repo
 	repoDir, err := findGitRepoRootDir(apidir)
 	if err != nil {
@@ -118,12 +116,28 @@ func ResolveVPPVersion(apidir string) string {
 		// try to read VPP_VERSION file
 		data, err := os.ReadFile(path.Join(repoDir, "VPP_VERSION"))
 		if err == nil {
-			return strings.TrimSpace(string(data))
+			ver := strings.TrimSpace(string(data))
+			logrus.Debugf("VPP version was resolved to %q from VPP_VERSION file", ver)
+			return ver
 		}
 	}
 
+	// try to read VPP_VERSION file
+	data, err := os.ReadFile(path.Join(apidir, "VPP_VERSION"))
+	if err == nil {
+		ver := strings.TrimSpace(string(data))
+		logrus.Debugf("VPP version was resolved to %q from VPP_VERSION file", ver)
+		return ver
+	}
+
+	// check env variable override
+	if ver := os.Getenv(VPPVersionEnvVar); ver != "" {
+		logrus.Debugf("VPP version was manually set to %q via %s env var", ver, VPPVersionEnvVar)
+		return ver
+	}
+
 	// assuming VPP package is installed
-	if _, err := exec.LookPath("vpp"); err == nil {
+	if filepath.Clean(apidir) == DefaultDir {
 		version, err := GetVPPVersionInstalled()
 		if err != nil {
 			logrus.Debugf("ERR: failed to resolve VPP version from installed package: %v", err)

cmd/govpp/cli.go

@@ -13,3 +13,111 @@
 //  limitations under the License.
 
 package main
+
+import (
+	"io"
+
+	"github.com/docker/cli/cli/streams"
+	"github.com/moby/term"
+)
+
+type Cli interface {
+	Out() *streams.Out
+	Err() io.Writer
+	In() *streams.In
+	Apply(...CliOption) error
+}
+
+type AppCli struct {
+	out *streams.Out
+	err io.Writer
+	in  *streams.In
+}
+
+func NewCli(opt ...CliOption) (Cli, error) {
+	cli := new(AppCli)
+	if err := cli.Apply(opt...); err != nil {
+		return nil, err
+	}
+	if cli.out == nil || cli.in == nil || cli.err == nil {
+		stdin, stdout, stderr := term.StdStreams()
+		if cli.in == nil {
+			cli.in = streams.NewIn(stdin)
+		}
+		if cli.out == nil {
+			cli.out = streams.NewOut(stdout)
+		}
+		if cli.err == nil {
+			cli.err = stderr
+		}
+	}
+	return cli, nil
+}
+
+func (cli *AppCli) Out() *streams.Out {
+	return cli.out
+}
+
+func (cli *AppCli) Err() io.Writer {
+	return cli.err
+}
+
+func (cli *AppCli) In() *streams.In {
+	return cli.in
+}
+
+func (cli *AppCli) Apply(opt ...CliOption) error {
+	for _, o := range opt {
+		if err := o(cli); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+type CliOption func(cli *AppCli) error
+
+// WithStandardStreams sets a cli in, out and err streams with the standard streams.
+func WithStandardStreams() CliOption {
+	return func(cli *AppCli) error {
+		// Set terminal emulation based on platform as required.
+		stdin, stdout, stderr := term.StdStreams()
+		cli.in = streams.NewIn(stdin)
+		cli.out = streams.NewOut(stdout)
+		cli.err = stderr
+		return nil
+	}
+}
+
+// WithCombinedStreams uses the same stream for the output and error streams.
+func WithCombinedStreams(combined io.Writer) CliOption {
+	return func(cli *AppCli) error {
+		cli.out = streams.NewOut(combined)
+		cli.err = combined
+		return nil
+	}
+}
+
+// WithInputStream sets a cli input stream.
+func WithInputStream(in io.ReadCloser) CliOption {
+	return func(cli *AppCli) error {
+		cli.in = streams.NewIn(in)
+		return nil
+	}
+}
+
+// WithOutputStream sets a cli output stream.
+func WithOutputStream(out io.Writer) CliOption {
+	return func(cli *AppCli) error {
+		cli.out = streams.NewOut(out)
+		return nil
+	}
+}
+
+// WithErrorStream sets a cli error stream.
+func WithErrorStream(err io.Writer) CliOption {
+	return func(cli *AppCli) error {
+		cli.err = err
+		return nil
+	}
+}

cmd/govpp/cmd.go

@@ -31,28 +31,32 @@ const logo = `
 `
 
 func Execute() {
-	root := newRootCmd()
+	cli, err := NewCli()
+	if err != nil {
+		logrus.Fatalf("CLI init error: %v", err)
+	}
+	root := newRootCmd(cli)
 
 	if err := root.Execute(); err != nil {
 		logrus.Fatalf("ERROR: %v", err)
 	}
 }
 
-func newRootCmd() *cobra.Command {
+func newRootCmd(cli Cli) *cobra.Command {
 	var (
 		glob GlobalOptions
 	)
 	cmd := &cobra.Command{
-		Use:               "govpp [options] command",
-		Short:             "GoVPP CLI app",
+		Use:               "govpp [OPTIONS] COMMAND",
+		Short:             "GoVPP CLI",
 		Long:              color.Sprintf(logo, version.Short(), version.BuiltBy(), version.BuildTime()),
 		Version:           version.String(),
 		SilenceUsage:      true,
 		SilenceErrors:     true,
 		TraverseChildren:  true,
 		CompletionOptions: cobra.CompletionOptions{HiddenDefaultCmd: true},
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			InitOptions(&glob)
+			InitOptions(cli, &glob)
 			logrus.Tracef("global options: %+v", glob)
 
 			logrus.Tracef("args: %+v", args)
@@ -68,10 +72,10 @@ func newRootCmd() *cobra.Command {
 	glob.InstallFlags(cmd.PersistentFlags())
 
 	cmd.AddCommand(
-		newGenerateCmd(),
-		newVppapiCmd(),
-		newHttpCmd(),
-		newCliCommand(),
+		newGenerateCmd(cli),
+		newVppapiCmd(cli),
+		newHttpCmd(cli),
+		newCliCommand(cli),
 	)
 
 	cmd.InitDefaultVersionFlag()

cmd/govpp/cmd_cli.go

@@ -59,7 +59,7 @@ type CliOptions struct {
 	Stdout io.Writer
 }
 
-func newCliCommand() *cobra.Command {
+func newCliCommand(Cli) *cobra.Command {
 	var (
 		opts = CliOptions{
 			ApiSocket: socketclient.DefaultSocketName,

cmd/govpp/cmd_generate.go

@@ -34,7 +34,7 @@ type GenerateOptions struct {
 	RunPlugins    []string
 }
 
-func newGenerateCmd() *cobra.Command {
+func newGenerateCmd(cli Cli) *cobra.Command {
 	var (
 		opts = GenerateOptions{
 			RunPlugins: []string{"rpc"},
@@ -44,6 +44,7 @@ func newGenerateCmd() *cobra.Command {
 		Use:     "generate [apifile...]",
 		Aliases: []string{"gen"},
 		Short:   "Generate code",
+		Long:    "Generates Go bindings for VPP API",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if opts.Input == "" {
 				opts.Input = detectVppApiInput()