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 <ofabry@cisco.com>

* Remove test

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

* Resolve review feedback

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

---------

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

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()