From 21b5292f48f31e277dacfb99f836d735af5f9e7e Mon Sep 17 00:00:00 2001 From: Ondrej Fabry Date: Fri, 23 Jun 2023 15:55:42 +0200 Subject: [PATCH] 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 * Remove test Signed-off-by: Ondrej Fabry * Resolve review feedback Signed-off-by: Ondrej Fabry --------- Signed-off-by: Ondrej Fabry --- binapigen/generator.go | 2 - binapigen/vppapi.go | 9 ++ binapigen/vppapi/input.go | 28 +++-- binapigen/vppapi/input_test.go | 12 +- binapigen/vppapi/source.go | 6 +- binapigen/vppapi/util.go | 32 +++-- cmd/govpp/cli.go | 108 ++++++++++++++++ cmd/govpp/cmd.go | 22 ++-- cmd/govpp/cmd_cli.go | 2 +- cmd/govpp/cmd_generate.go | 3 +- cmd/govpp/cmd_http.go | 19 +-- cmd/govpp/cmd_vppapi.go | 84 +++++++++---- cmd/govpp/cmd_vppapi_diff.go | 26 ++-- cmd/govpp/cmd_vppapi_export.go | 70 ++++++++--- cmd/govpp/cmd_vppapi_lint.go | 29 +++-- cmd/govpp/cmd_vppapi_ls.go | 142 +++++---------------- cmd/govpp/compare.go | 37 +++++- cmd/govpp/formatter.go | 7 +- cmd/govpp/input.go | 30 ++++- cmd/govpp/linter.go | 220 +++++++++++++++++---------------- cmd/govpp/options.go | 7 +- cmd/govpp/util.go | 15 +++ go.mod | 3 + go.sum | 8 ++ 24 files changed, 564 insertions(+), 357 deletions(-) diff --git a/binapigen/generator.go b/binapigen/generator.go index 9fb40b3a..3a0a983e 100644 --- a/binapigen/generator.go +++ b/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 diff --git a/binapigen/vppapi.go b/binapigen/vppapi.go index 034341a8..51da1f86 100644 --- a/binapigen/vppapi.go +++ b/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 { diff --git a/binapigen/vppapi/input.go b/binapigen/vppapi/input.go index 9da8036f..c81f58db 100644 --- a/binapigen/vppapi/input.go +++ b/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) diff --git a/binapigen/vppapi/input_test.go b/binapigen/vppapi/input_test.go index b6519887..904b8444 100644 --- a/binapigen/vppapi/input_test.go +++ b/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) } diff --git a/binapigen/vppapi/source.go b/binapigen/vppapi/source.go index 74fa4ec9..15c7661f 100644 --- a/binapigen/vppapi/source.go +++ b/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 } diff --git a/binapigen/vppapi/util.go b/binapigen/vppapi/util.go index 577e1ad1..435ea881 100644 --- a/binapigen/vppapi/util.go +++ b/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) diff --git a/cmd/govpp/cli.go b/cmd/govpp/cli.go index fb31a1d8..8707da1b 100644 --- a/cmd/govpp/cli.go +++ b/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 + } +} diff --git a/cmd/govpp/cmd.go b/cmd/govpp/cmd.go index 10ef7692..7a60d6e9 100644 --- a/cmd/govpp/cmd.go +++ b/cmd/govpp/cmd.go @@ -31,20 +31,24 @@ 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, @@ -52,7 +56,7 @@ func newRootCmd() *cobra.Command { 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() diff --git a/cmd/govpp/cmd_cli.go b/cmd/govpp/cmd_cli.go index f40105d4..371438d1 100644 --- a/cmd/govpp/cmd_cli.go +++ b/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, diff --git a/cmd/govpp/cmd_generate.go b/cmd/govpp/cmd_generate.go index 0ce75517..9eea1a9a 100644 --- a/cmd/govpp/cmd_generate.go +++ b/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() diff --git a/cmd/govpp/cmd_http.go b/cmd/govpp/cmd_http.go index 45e3b2fb..b39c6f04 100644 --- a/cmd/govpp/cmd_http.go +++ b/cmd/govpp/cmd_http.go @@ -47,7 +47,7 @@ type HttpCmdOptions struct { Address string } -func newHttpCmd() *cobra.Command { +func newHttpCmd(Cli) *cobra.Command { var opts = HttpCmdOptions{ ApiSocket: socketclient.DefaultSocketName, Address: DefaultHttpServiceAddress, @@ -166,23 +166,6 @@ func reqHandler(apifile *vppapi.File) func(http.ResponseWriter, *http.Request) { } } -/*func showRPC(apifiles []*vppapi.File) { - for _, apifile := range apifiles { - fmt.Printf("%s.api\n", apifile.Name) - if apifile.Service == nil { - continue - } - for _, rpc := range apifile.Service.RPCs { - req := rpc.Request - reply := rpc.Reply - if rpc.Stream { - reply = "stream " + reply - } - fmt.Printf(" rpc (%s) --> (%s)\n", req, reply) - } - } -}*/ - func apiFilesHandler(apifiles []vppapi.File) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, req *http.Request) { b, err := json.MarshalIndent(apifiles, "", " ") diff --git a/cmd/govpp/cmd_vppapi.go b/cmd/govpp/cmd_vppapi.go index 1344439d..4b20409a 100644 --- a/cmd/govpp/cmd_vppapi.go +++ b/cmd/govpp/cmd_vppapi.go @@ -15,55 +15,95 @@ package main import ( + "fmt" + + "github.com/gookit/color" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" + + "go.fd.io/govpp/binapigen" + "go.fd.io/govpp/binapigen/vppapi" ) const exampleVppapi = ` - # Specify VPP API input source - govpp vppapi --input=. COMMAND - govpp vppapi --input=http://github.com/FDio/vpp.git COMMAND - govpp vppapi --input=vppapi.tar.gz COMMAND + # Specify input source of VPP API + govpp vppapi COMMAND [INPUT] + govpp vppapi COMMAND ./vpp + govpp vppapi COMMAND /usr/share/vpp/api + govpp vppapi COMMAND vppapi.tar.gz + govpp vppapi COMMAND http://github.com/FDio/vpp.git - # Browse VPP API files - govpp vppapi --input=INPUT ls - govpp vppapi --input=INPUT ls --show-messages + # List VPP API contents + govpp vppapi ls [INPUT] + govpp vppapi ls --show-contents - # Export VPP API files - govpp vppapi --input=INPUT export --output=vppapi + # Export VPP API files + govpp vppapi export [INPUT] --output vppapi + govpp vppapi export [INPUT] --output vppapi.tar.gz - # Lint VPP API - govpp vppapi --input=INPUT lint + # Lint VPP API definitions + govpp vppapi lint [INPUT] - # Compare VPPPI schemas - govpp vppapi -input=INPUT diff --against=http://github.com/FDio/vpp.git + # Compare VPP API schemas + govpp vppapi diff [INPUT] --against http://github.com/FDio/vpp.git ` type VppApiCmdOptions struct { Input string + Paths []string Format string } -func newVppapiCmd() *cobra.Command { +func newVppapiCmd(cli Cli) *cobra.Command { var ( opts VppApiCmdOptions ) cmd := &cobra.Command{ Use: "vppapi", Short: "Manage VPP API", - Long: "Manage VPP API development", - Example: exampleVppapi, + Long: "Manage VPP API development.", + Example: color.Sprint(exampleVppapi), TraverseChildren: true, } - cmd.PersistentFlags().StringVar(&opts.Input, "input", opts.Input, "Input for VPP API (e.g. path to VPP API directory, local VPP repo)") - cmd.PersistentFlags().StringVar(&opts.Format, "format", "", "Output format (json, yaml, go-template..)") + cmd.PersistentFlags().StringSliceVar(&opts.Paths, "path", nil, "Limit to specific files or directories.\nFor example: \"vpe\" or \"core/\".") + cmd.PersistentFlags().StringVarP(&opts.Format, "format", "f", "", "Format for the output (json, yaml, go-template..)") cmd.AddCommand( - newVppApiLsCmd(&opts), - newVppApiExportCmd(&opts), - newVppApiDiffCmd(&opts), - newVppApiLintCmd(&opts), + newVppApiLsCmd(cli, &opts), + newVppApiExportCmd(cli, &opts), + newVppApiDiffCmd(cli, &opts), + newVppApiLintCmd(cli, &opts), ) return cmd } + +func prepareVppApiFiles(allapifiles []vppapi.File, paths []string, includeImported, sortByName bool) ([]vppapi.File, error) { + // remove imported types + if !includeImported { + binapigen.SortFilesByImports(allapifiles) + for i, apifile := range allapifiles { + f := apifile + binapigen.RemoveImportedTypes(allapifiles, &f) + allapifiles[i] = f + } + } + + apifiles := allapifiles + + // filter files + if len(paths) > 0 { + apifiles = filterFilesByPaths(allapifiles, paths) + if len(apifiles) == 0 { + return nil, fmt.Errorf("no files matching: %q", paths) + } + logrus.Tracef("filter (%d paths) matched %d/%d files", len(paths), len(apifiles), len(allapifiles)) + } + + if sortByName { + binapigen.SortFilesByName(apifiles) + } + + return apifiles, nil +} diff --git a/cmd/govpp/cmd_vppapi_diff.go b/cmd/govpp/cmd_vppapi_diff.go index 32a4551e..d74e6972 100644 --- a/cmd/govpp/cmd_vppapi_diff.go +++ b/cmd/govpp/cmd_vppapi_diff.go @@ -32,20 +32,21 @@ import ( type VppApiDiffCmdOptions struct { *VppApiCmdOptions - Against string - Differences []string + Against string + Differences []string + CommentDiffs bool } -func newVppApiDiffCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { +func newVppApiDiffCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { var ( opts = VppApiDiffCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "diff INPUT --against=AGAINST", + Use: "diff [INPUT] --against AGAINST [--differences DIFF]...", Aliases: []string{"cmp", "compare"}, Short: "Compare VPP API schemas", Long: "Compares two VPP API schemas and lists the differences.", - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { opts.Input = args[0] @@ -54,6 +55,7 @@ func newVppApiDiffCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { }, } + cmd.PersistentFlags().BoolVar(&opts.CommentDiffs, "comments", false, "Include message comment differences") cmd.PersistentFlags().StringSliceVar(&opts.Differences, "differences", nil, "List only specific differences") cmd.PersistentFlags().StringVar(&opts.Against, "against", "", "The VPP API schema to compare against.") must(cobra.MarkFlagRequired(cmd.PersistentFlags(), "against")) @@ -68,10 +70,6 @@ var ( ) func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error { - if opts.Format != "" { - color.Disable() - } - vppInput, err := resolveInput(opts.Input) if err != nil { return err @@ -92,6 +90,16 @@ func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error { diffs := CompareSchemas(&schema1, &schema2) + if !opts.CommentDiffs { + var filtered []Difference + for _, diff := range diffs { + if diff.Type != MessageCommentDifference { + filtered = append(filtered, diff) + } + } + diffs = filtered + } + if len(opts.Differences) > 0 { diffs, err = filterDiffs(diffs, opts.Differences) if err != nil { diff --git a/cmd/govpp/cmd_vppapi_export.go b/cmd/govpp/cmd_vppapi_export.go index 15d5e1bd..b386bb9c 100644 --- a/cmd/govpp/cmd_vppapi_export.go +++ b/cmd/govpp/cmd_vppapi_export.go @@ -30,7 +30,6 @@ import ( ) // TODO: -// - exporting to archive (.tar.gz) // - add option for exporting flat structure // - embed VPP version into export somehow @@ -38,25 +37,31 @@ type VppApiExportCmdOptions struct { *VppApiCmdOptions Output string + Targz bool } -func newVppApiExportCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { +func newVppApiExportCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { var ( opts = VppApiExportCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "export [INPUT] -o OUTPUT", - Short: "Export VPP API files", + Use: "export [INPUT] --output OUTPUT [--targz]", + Short: "Export VPP API", Long: "Export VPP API files from an input location to an output location.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { opts.Input = args[0] } + // auto-detect .tar.gz + if !cmd.Flags().Changed("targz") && strings.HasSuffix(opts.Output, ".tar.gz") { + opts.Targz = true + } return runExportCmd(opts) }, } + cmd.PersistentFlags().BoolVar(&opts.Targz, "targz", false, "Export to gzipped tarball") cmd.PersistentFlags().StringVarP(&opts.Output, "output", "o", "", "Output directory for the exported files") must(cobra.MarkFlagRequired(cmd.PersistentFlags(), "output")) @@ -70,22 +75,43 @@ func runExportCmd(opts VppApiExportCmdOptions) error { } // collect files from input - logrus.Tracef("searching for files in API dir: %s", vppInput.ApiDirectory) + logrus.Tracef("collecting files for export in API dir: %s", vppInput.ApiDirectory) files, err := vppapi.FindFiles(vppInput.ApiDirectory) if err != nil { return err } - logrus.Debugf("found %d files in API dir %s", len(files), vppInput.ApiDirectory) + logrus.Debugf("exporting %d files", len(files)) - if strings.HasSuffix(opts.Output, ".tar.gz") { - err = exportFilesToTarGz(opts.Output, files, vppInput.ApiDirectory) + if opts.Targz { + temp, err := os.MkdirTemp("", "govpp-vppapi-export-*") + if err != nil { + return fmt.Errorf("creating temp dir failed: %w", err) + } + tmpDir := filepath.Join(temp, "vppapi") + err = exportFilesToDir(tmpDir, files, vppInput) + if err != nil { + return fmt.Errorf("exporting to directory failed: %w", err) + } + var files2 = []string{filepath.Join(tmpDir, "VPP_VERSION")} + for _, f := range files { + filerel, err := filepath.Rel(vppInput.ApiDirectory, f) + if err != nil { + filerel = strings.TrimPrefix(f, vppInput.ApiDirectory) + } + filename := filepath.Join(tmpDir, filerel) + files2 = append(files2, filename) + } + err = exportFilesToTarGz(opts.Output, files2, tmpDir) + if err != nil { + return fmt.Errorf("exporting to gzipped tarball failed: %w", err) + } } else { - err = exportFilesToDir(opts.Output, files, vppInput.ApiDirectory) - } - if err != nil { - return fmt.Errorf("exporting failed: %w", err) + err = exportFilesToDir(opts.Output, files, vppInput) + if err != nil { + return fmt.Errorf("exporting failed: %w", err) + } } logrus.Debugf("exported %d files to %s", len(files), opts.Output) @@ -93,8 +119,8 @@ func runExportCmd(opts VppApiExportCmdOptions) error { return nil } -func exportFilesToTarGz(outputFile string, files []string, apiDir string) error { - logrus.Tracef("exporting files into tarball archive: %s", outputFile) +func exportFilesToTarGz(outputFile string, files []string, baseDir string) error { + logrus.Tracef("exporting %d files into tarball archive: %s", len(files), outputFile) // create the output file for writing fw, err := os.Create(outputFile) @@ -132,7 +158,9 @@ func exportFilesToTarGz(outputFile string, files []string, apiDir string) error } // update the name to correctly reflect the desired destination when untaring - header.Name = filepath.Join(filepath.Base(apiDir), strings.TrimPrefix(file, apiDir)) + header.Name = strings.TrimPrefix(file, baseDir) + + logrus.Tracef("- exporting file: %q to: %s", file, header.Name) // write the header if err := tw.WriteHeader(header); err != nil { @@ -152,9 +180,11 @@ func exportFilesToTarGz(outputFile string, files []string, apiDir string) error return nil } -func exportFilesToDir(outputDir string, files []string, apiDir string) error { +func exportFilesToDir(outputDir string, files []string, vppInput *vppapi.VppInput) error { logrus.Tracef("exporting files into directory: %s", outputDir) + apiDir := vppInput.ApiDirectory + // create the output directory for export if err := os.Mkdir(outputDir, 0750); err != nil { return err @@ -184,5 +214,13 @@ func exportFilesToDir(outputDir string, files []string, apiDir string) error { logrus.Tracef("file %s exported (%d bytes) to %s", filerel, len(data), dir) } + + // write version + filename := filepath.Join(outputDir, "VPP_VERSION") + data := []byte(vppInput.Schema.Version) + if err := os.WriteFile(filename, data, 0660); err != nil { + return err + } + return nil } diff --git a/cmd/govpp/cmd_vppapi_lint.go b/cmd/govpp/cmd_vppapi_lint.go index 1ca52e66..9afe07b2 100644 --- a/cmd/govpp/cmd_vppapi_lint.go +++ b/cmd/govpp/cmd_vppapi_lint.go @@ -18,14 +18,12 @@ import ( "fmt" "io" - "github.com/gookit/color" "github.com/olekukonko/tablewriter" "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) // TODO: -// - support file filter (include, exclude..) // - consider adding categories for linter rules type VppApiLintCmdOptions struct { @@ -37,23 +35,25 @@ type VppApiLintCmdOptions struct { ListRules bool } -func newVppApiLintCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { +func newVppApiLintCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { var ( opts = VppApiLintCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "lint", - Short: "Lint VPP API files", + Use: "lint [INPUT] [--rules RULE]...", + Short: "Lint VPP API", Long: "Run linter checks for VPP API files", - Args: cobra.NoArgs, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.Input = args[0] + } return runVppApiLintCmd(cmd.OutOrStdout(), opts) }, } cmd.PersistentFlags().StringSliceVar(&opts.Rules, "rules", nil, "Limit to specific linter rules") cmd.PersistentFlags().StringSliceVar(&opts.Except, "except", nil, "Skip specific linter rules.") - cmd.PersistentFlags().StringVarP(&opts.Format, "format", "f", "", "The format of the output") cmd.PersistentFlags().BoolVar(&opts.ExitCode, "exit-code", false, "Exit with non-zero exit code if any issue is found") cmd.PersistentFlags().BoolVar(&opts.ListRules, "list-rules", false, "List all known linter rules") @@ -61,10 +61,6 @@ func newVppApiLintCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { } func runVppApiLintCmd(out io.Writer, opts VppApiLintCmdOptions) error { - if opts.Format != "" { - color.Disable() - } - if opts.ListRules { rules := LintRules(defaultLintRules...) if opts.Format == "" { @@ -80,6 +76,15 @@ func runVppApiLintCmd(out io.Writer, opts VppApiLintCmdOptions) error { return err } + schema := vppInput.Schema + + // filter files + apifiles := filterFilesByPaths(schema.Files, opts.Paths) + if len(apifiles) == 0 { + return fmt.Errorf("filter matched no files") + } + logrus.Debugf("filtered %d/%d files", len(apifiles), len(schema.Files)) + linter := NewLinter() if len(opts.Rules) > 0 { @@ -91,8 +96,6 @@ func runVppApiLintCmd(out io.Writer, opts VppApiLintCmdOptions) error { linter.Disable(opts.Except...) } - schema := vppInput.Schema - if err := linter.Lint(&schema); err != nil { if errs, ok := err.(LintIssues); ok { if opts.Format == "" { diff --git a/cmd/govpp/cmd_vppapi_ls.go b/cmd/govpp/cmd_vppapi_ls.go index 1c9808d0..38fd2b65 100644 --- a/cmd/govpp/cmd_vppapi_ls.go +++ b/cmd/govpp/cmd_vppapi_ls.go @@ -21,7 +21,6 @@ import ( "os" "path" "path/filepath" - "sort" "strconv" "strings" @@ -29,13 +28,14 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "go.fd.io/govpp/binapigen" "go.fd.io/govpp/binapigen/vppapi" ) type VppApiLsCmdOptions struct { *VppApiCmdOptions + Paths []string + IncludeImported bool IncludeFields bool @@ -47,17 +47,21 @@ type VppApiLsCmdOptions struct { SortByName bool } -func newVppApiLsCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { +func newVppApiLsCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { var ( opts = VppApiLsCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "ls [FILE, ...]", + Use: "ls [INPUT] [--path PATH]... [--show-contents | --show-messages | --show-raw | --show-rpc]", Aliases: []string{"l", "list"}, - Short: "List VPP API files", - Long: "List VPP API files and their contents", + Short: "Show VPP API contents", + Long: "Show VPP API files and their contents", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runVppApiLsCmd(cmd.OutOrStdout(), opts, args) + if len(args) > 0 { + opts.Input = args[0] + } + return runVppApiLsCmd(cmd.OutOrStdout(), opts) }, } @@ -72,64 +76,19 @@ func newVppApiLsCmd(vppapiOpts *VppApiCmdOptions) *cobra.Command { return cmd } -func runVppApiLsCmd(out io.Writer, opts VppApiLsCmdOptions, args []string) error { +func runVppApiLsCmd(out io.Writer, opts VppApiLsCmdOptions) error { vppInput, err := resolveInput(opts.Input) if err != nil { return err } - logrus.Debugf("parsing VPP API directory: %v", vppInput.ApiDirectory) + allapifiles := vppInput.Schema.Files - // parse API directory - allapifiles, err := vppapi.ParseDir(vppInput.ApiDirectory) - if err != nil { - return fmt.Errorf("vppapi.ParseDir: %w", err) - } + logrus.Debugf("processing %d files", len(allapifiles)) - logrus.Debugf("parsed %d files, normalizing data", len(allapifiles)) - - // normalize data - binapigen.SortFilesByImports(allapifiles) - - // filter files - var apifiles []vppapi.File - if len(args) > 0 { - // select only specific files - for _, arg := range args { - var found bool - for _, apifile := range allapifiles { - if apifile.Name == arg { - apifiles = append(apifiles, apifile) - found = true - break - } - } - if !found { - return fmt.Errorf("VPP API file %q not found", arg) - } - } - } else { - // select all files - apifiles = allapifiles - } - - logrus.Debugf("selected %d/%d files", len(apifiles), len(allapifiles)) - - // omit imported types - if !opts.IncludeImported { - for i, apifile := range apifiles { - f := apifile - binapigen.RemoveImportedTypes(allapifiles, &f) - apifiles[i] = f - } - } - - if opts.SortByName { - sort.SliceStable(apifiles, func(i, j int) bool { - a := apifiles[i] - b := apifiles[j] - return a.Name < b.Name - }) + apifiles, err := prepareVppApiFiles(allapifiles, opts.Paths, opts.IncludeImported, opts.SortByName) + if err != nil { + return err } // format output @@ -142,7 +101,8 @@ func runVppApiLsCmd(out io.Writer, opts VppApiLsCmdOptions, args []string) error } else if opts.ShowContents { showVPPAPIContents(out, apifiles) } else if opts.ShowRaw { - showVPPAPIRaw(out, vppInput, args) + rawFiles := getVppApiRawFiles(vppInput.ApiDirectory, apifiles) + showVPPAPIRaw(out, rawFiles) } else { showVPPAPIList(out, apifiles) } @@ -161,64 +121,30 @@ type VppApiRawFile struct { Content string } -func showVPPAPIRaw(out io.Writer, input *vppapi.VppInput, args []string) { - list, err := vppapi.FindFiles(input.ApiDirectory) - if err != nil { - logrus.Errorf("failed to find files: %v", err) - return - } - - var allapifiles []VppApiRawFile - for _, f := range list { - file, err := vppapi.ParseFile(f) +func getVppApiRawFiles(apiDir string, files []vppapi.File) []VppApiRawFile { + var rawFiles []VppApiRawFile + for _, file := range files { + filePath := filepath.Join(apiDir, file.Path) + b, err := os.ReadFile(filePath) if err != nil { - logrus.Debugf("failed to parse file: %v", err) + logrus.Errorf("failed to read file %q: %v", filePath, err) continue } - // use file path relative to apiDir - if p, err := filepath.Rel(input.ApiDirectory, file.Path); err == nil { - file.Path = p - } - // extract file name - base := filepath.Base(f) - name := base[:strings.Index(base, ".")] - b, err := os.ReadFile(f) - if err != nil { - logrus.Errorf("failed to read file %q: %v", f, err) - continue - } - - allapifiles = append(allapifiles, VppApiRawFile{ + rawFiles = append(rawFiles, VppApiRawFile{ Path: file.Path, - Name: name, + Name: file.Name, Content: string(b), }) } + return rawFiles +} - var apifiles []VppApiRawFile - if len(args) > 0 { - // select only specific files - for _, arg := range args { - var found bool - for _, apifile := range allapifiles { - if apifile.Name == arg { - apifiles = append(apifiles, apifile) - found = true - break - } - } - if !found { - logrus.Errorf("VPP API file %q not found", arg) - continue - - } - } - } else { - // select all files - apifiles = allapifiles +func showVPPAPIRaw(out io.Writer, rawFiles []VppApiRawFile) { + if len(rawFiles) == 0 { + logrus.Errorf("no files to show") + return } - - for _, f := range apifiles { + for _, f := range rawFiles { fmt.Fprintf(out, "# %s (%v)\n", f.Name, f.Path) fmt.Fprintf(out, "%s\n", f.Content) fmt.Fprintln(out) diff --git a/cmd/govpp/compare.go b/cmd/govpp/compare.go index 7509c859..0b56bdf5 100644 --- a/cmd/govpp/compare.go +++ b/cmd/govpp/compare.go @@ -40,6 +40,7 @@ const ( MessageAddedDifference DifferenceType = "MessageAdded" MessageRemovedDifference DifferenceType = "MessageRemoved" MessageCrcDifference DifferenceType = "MessageCRC" + MessageCommentDifference DifferenceType = "MessageComment" MsgOptionChangedDifference DifferenceType = "MsgOptionChanged" MsgOptionAddedDifference DifferenceType = "MsgOptionAdded" @@ -219,8 +220,12 @@ func compareFiles(file1, file2 vppapi.File) []Difference { if msg2, ok := msgMap2[msgName]; ok { msgDiffs := compareMessages(msg1, msg2) for _, msgDiff := range msgDiffs { - msgDiff.Value1 = msg1 - msgDiff.Value2 = msg2 + if msgDiff.Value1 == nil { + msgDiff.Value1 = msg1 + } + if msgDiff.Value2 == nil { + msgDiff.Value2 = msg2 + } differences = append(differences, msgDiff) } } else { @@ -233,13 +238,13 @@ func compareFiles(file1, file2 vppapi.File) []Difference { } } // added messages - for msgName := range msgMap2 { - if msg2, ok := msgMap1[msgName]; !ok { + for msgName, msg := range msgMap2 { + if _, ok := msgMap1[msgName]; !ok { differences = append(differences, Difference{ Type: MessageAddedDifference, Description: color.Sprintf("Message added: %s", clrCyan.Sprint(msgName)), Value1: nil, - Value2: msg2, + Value2: msg, }) } } @@ -259,7 +264,27 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference { differences = append(differences, Difference{ Type: MessageCrcDifference, Description: color.Sprintf("Message %s changed CRC from %s to %s", - clrCyan.Sprint(msg1.Name), msg1.CRC, msg2.CRC), + clrCyan.Sprint(msg1.Name), clrWhite.Sprint(msg1.CRC), clrWhite.Sprint(msg2.CRC)), + Value1: msg1.CRC, + Value2: msg2.CRC, + }) + } + + // Compare message comments + if msg1.Comment != msg2.Comment { + desc := color.Sprintf("Message %s comment ", clrCyan.Sprint(msg1.Name)) + if msg1.Comment == "" { + desc += "added" + } else if msg2.Comment == "" { + desc += "removed" + } else { + desc += "changed" + } + differences = append(differences, Difference{ + Type: MessageCommentDifference, + Description: desc, + Value1: msg1.Comment, + Value2: msg2.Comment, }) } diff --git a/cmd/govpp/formatter.go b/cmd/govpp/formatter.go index 7f6d1edd..1895abfa 100644 --- a/cmd/govpp/formatter.go +++ b/cmd/govpp/formatter.go @@ -23,7 +23,6 @@ import ( "text/template" "time" - "github.com/gookit/color" "gopkg.in/yaml.v3" ) @@ -40,7 +39,7 @@ func formatAsTemplate(w io.Writer, format string, data interface{}) error { var b bytes.Buffer switch strings.ToLower(format) { case "json": - s := color.ClearCode(jsonTmpl(data)) + s := jsonTmpl(data) b.WriteString(s) case "yaml", "yml": b.WriteString(yamlTmpl(data)) @@ -60,7 +59,7 @@ func formatAsTemplate(w io.Writer, format string, data interface{}) error { func jsonTmpl(data interface{}) string { b := encodeJson(data, " ") - return string(b) + return clearColorCode(string(b)) } func yamlTmpl(data interface{}) string { @@ -69,7 +68,7 @@ func yamlTmpl(data interface{}) string { if err != nil { panic(err) } - return string(bb) + return clearColorCode(string(bb)) } func encodeJson(data interface{}, ident string) []byte { diff --git a/cmd/govpp/input.go b/cmd/govpp/input.go index b0289a9b..5fa3fc25 100644 --- a/cmd/govpp/input.go +++ b/cmd/govpp/input.go @@ -16,7 +16,9 @@ package main import ( "os" + "path" "path/filepath" + "strings" "github.com/sirupsen/logrus" @@ -72,7 +74,33 @@ func dirExists(dir ...string) bool { if _, err := os.Stat(d); err != nil { return false } - } return true } + +func filterFilesByPaths(allapifiles []vppapi.File, paths []string) []vppapi.File { + var apifiles []vppapi.File + if len(paths) == 0 { + return allapifiles + } + added := make(map[string]bool) + // filter files + for _, arg := range paths { + var found bool + for _, apifile := range allapifiles { + if added[apifile.Path] { + continue + } + dir, file := path.Split(apifile.Path) + if apifile.Name == strings.TrimSuffix(arg, ".api") || apifile.Path == arg || file == arg || path.Clean(dir) == arg { + apifiles = append(apifiles, apifile) + found = true + added[apifile.Path] = true + } + } + if !found { + logrus.Warnf("path %q did not match any file", arg) + } + } + return apifiles +} diff --git a/cmd/govpp/linter.go b/cmd/govpp/linter.go index a0227b88..7a5ca3ce 100644 --- a/cmd/govpp/linter.go +++ b/cmd/govpp/linter.go @@ -42,7 +42,7 @@ type LintRule struct { Id string Purpose string //Categories []string - check func(schema *vppapi.Schema) error + check func(schema *vppapi.Schema) LintIssues } func LintRules(ids ...string) []*LintRule { @@ -68,97 +68,117 @@ var lintRules = map[string]LintRule{ FILE_BASIC: { Id: FILE_BASIC, Purpose: "File must have basic info defined", - check: checkFiles(func(file *vppapi.File) error { - var errs LintIssues - if file.CRC == "" { - errs = errs.Append(fmt.Errorf("file %s must have CRC defined", file.Name)) - } - if file.Options != nil && file.Options[vppapi.OptFileVersion] == "" { - errs = errs.Append(fmt.Errorf("file %s must have version defined", file.Name)) - } - return errs.ToErr() - }), + check: checkFiles(checkFileBasic), }, MESSAGE_DEPRECATE_OLDER_VERSIONS: { Id: MESSAGE_DEPRECATE_OLDER_VERSIONS, Purpose: "Message should be marked as deprecated if newer version exists", - check: checkFiles(func(file *vppapi.File) error { - var errs LintIssues - messageVersions := extractFileMessageVersions(file) - versionMessages := extractMessageVersions(file) - for _, message := range file.Messages { - baseName, version := extractBaseNameAndVersion(message.Name) - // if this is not the latest version of a message - if version < messageVersions[baseName] { - var newer vppapi.Message - if vers, ok := versionMessages[baseName]; ok { - if newVer, ok := vers[version+1]; ok { - newer = newVer - } - } - // older messages should be marked as deprecated (if newer message version is not in progress) - if !isMessageDeprecated(message) && !isMessageInProgress(newer) { - errs = errs.Append(LintIssue{ - File: file.Path, - // TODO: Line: ?, - Object: map[string]any{ - "Message": message, - "Base": baseName, - "Version": version, - "Latest": messageVersions[baseName], - }, - Violation: color.Sprintf("message %s has newer version available (%v) but is not marked as deprecated", - color.Cyan.Sprint(message.Name), color.Cyan.Sprint(newer.Name)), - }) - } - } - } - return errs.ToErr() - }), + check: checkFiles(checkFileMessageDeprecateOldVersions), }, MESSAGE_SAME_STATUS: { Id: MESSAGE_SAME_STATUS, Purpose: "Message request and reply must have the same status", - check: checkFiles(func(file *vppapi.File) error { - var errs LintIssues - for _, message := range file.Messages { - status := getMessageStatus(message) - related := getRelatedMessages(file, message.Name) - for _, rel := range related { - if relMsg, ok := getFileMessage(file, rel); ok { - if relStatus := getMessageStatus(relMsg); relStatus != status { - errs = errs.Append(LintIssue{ - File: file.Path, - Object: map[string]any{ - "Message": message, - "Related": relMsg, - "Status": status, - "RelatedStatus": relStatus, - }, - Violation: color.Sprintf("message %s does not have consistent status (%v) with related message: %v (%v)", - color.Cyan.Sprint(message.Name), clrWhite.Sprint(status), color.Cyan.Sprint(rel), clrWhite.Sprint(relStatus)), - }) - } - } + check: checkFiles(checkFileMessageSameStatus), + }, +} + +func checkFileBasic(file *vppapi.File) LintIssues { + var issues LintIssues + if file.CRC == "" { + issues = issues.Append(fmt.Errorf("file %s must have CRC defined", file.Name)) + } + if file.Options != nil && file.Options[vppapi.OptFileVersion] == "" { + issues = issues.Append(fmt.Errorf("file %s must have version defined", file.Name)) + } + return issues +} + +func checkFileMessageDeprecateOldVersions(file *vppapi.File) LintIssues { + var issues LintIssues + + messageVersions := extractFileMessageVersions(file) + versionMessages := extractMessageVersions(file) + + for _, message := range file.Messages { + baseName, version := extractBaseNameAndVersion(message.Name) + + // if this is not the latest version of a message + if version < messageVersions[baseName] { + var newer vppapi.Message + if vers, ok := versionMessages[baseName]; ok { + if newVer, ok := vers[version+1]; ok { + newer = newVer } } - return errs.ToErr() - }), - }, + + // older messages should be marked as deprecated (if newer message version is not in progress) + if !isMessageDeprecated(message) && !isMessageInProgress(newer) { + issues = issues.Append(LintIssue{ + File: file.Path, + // TODO: Line: ?, + Object: map[string]any{ + "Message": message, + "Base": baseName, + "Version": version, + "Latest": messageVersions[baseName], + }, + Violation: color.Sprintf("message %s has newer version available (%v) but is not marked as deprecated", + color.Cyan.Sprint(message.Name), color.Cyan.Sprint(newer.Name)), + }) + } + } + } + + return issues +} + +func checkFileMessageSameStatus(file *vppapi.File) LintIssues { + var issues LintIssues + + for _, message := range file.Messages { + status := getMessageStatus(message) + relatedMsgs := getRelatedMessages(file, message.Name) + + for _, relatedMsg := range relatedMsgs { + relMsg, ok := getFileMessage(file, relatedMsg) + if !ok { + logrus.Warnf("could not find related message %s in file %s", relatedMsg, file.Path) + continue + } + if relStatus := getMessageStatus(relMsg); relStatus != status { + issues = issues.Append(LintIssue{ + File: file.Path, + Object: map[string]any{ + "Message": message, + "Related": relMsg, + "Status": status, + "RelatedStatus": relStatus, + }, + Violation: color.Sprintf("message %s does not have consistent status (%v) with related message: %v (%v)", + color.Cyan.Sprint(message.Name), clrWhite.Sprint(status), color.Cyan.Sprint(relatedMsg), clrWhite.Sprint(relStatus)), + }) + } + } + } + + return issues } -func checkFiles(checkFn func(file *vppapi.File) error) func(*vppapi.Schema) error { - return func(schema *vppapi.Schema) error { - errs := LintIssues{} +func checkFiles(checkFn func(file *vppapi.File) LintIssues) func(*vppapi.Schema) LintIssues { + return func(schema *vppapi.Schema) LintIssues { + var issues LintIssues + logrus.Tracef("running checkFiles for %d files", len(schema.Files)) + for _, file := range schema.Files { e := checkFn(&file) if e != nil { logrus.Tracef("checked file: %v (%v)", file.Name, e) } - errs = errs.Append(e) + issues = append(issues, e...) } - return errs + + return issues } } @@ -199,43 +219,34 @@ func (l *Linter) Disable(rules ...string) { } func (l *Linter) Lint(schema *vppapi.Schema) error { - logrus.Debugf("linter will run %d rules for %d files (schema version: %v)", len(l.rules), len(schema.Files), schema.Version) + logrus.Debugf("linter will run %d rule checks for %d files (schema version: %v)", len(l.rules), len(schema.Files), schema.Version) - var errs LintIssues + var allIssues LintIssues for _, rule := range l.rules { log := logrus.WithField("rule", rule.Id) - log.Debugf("running linter check for rule (purpose: %v)", rule.Purpose) + log.Debugf("running linter rulecheck (purpose: %v)", rule.Purpose) - err := rule.check(schema) - if err != nil { - log.Tracef("linter check failed: %v", err) + issues := rule.check(schema) + if len(issues) > 0 { + log.Tracef("linter rule check failed, found %d issues", len(issues)) - switch e := err.(type) { - case LintIssue: - if e.RuleId == "" { - e.RuleId = rule.Id - } - errs = errs.Append(e) - case LintIssues: - for _, le := range e { - if le.RuleId == "" { - le.RuleId = rule.Id - } - errs = errs.Append(le) + for _, issue := range issues { + if issue.RuleId == "" { + issue.RuleId = rule.Id } - default: - errs = append(errs, createLintIssue(rule.Id, err)) + allIssues = allIssues.Append(issue) } } else { - log.Tracef("linter check passed") + log.Tracef("linter rule check passed") } } - if len(errs) > 0 { - logrus.Debugf("found %d issues in %d files", len(errs), len(schema.Files)) - return errs + + if len(allIssues) > 0 { + logrus.Debugf("found %d issues in %d files", len(allIssues), len(schema.Files)) + return allIssues } else { - logrus.Debugf("no issues in %d files", len(schema.Files)) + logrus.Debugf("no issues found in %d files", len(schema.Files)) } return nil @@ -281,13 +292,6 @@ func (l LintIssue) Error() string { type LintIssues []LintIssue -func (le LintIssues) ToErr() error { - if len(le) == 0 { - return nil - } - return le -} - func (le LintIssues) Error() string { if len(le) == 0 { return "no issues" @@ -304,8 +308,8 @@ func (le LintIssues) Append(errs ...error) LintIssues { switch e := err.(type) { case LintIssue: r = append(r, e) - case LintIssues: - r = append(r, e...) + //case LintIssues: + // r = append(r, e...) default: r = append(r, createLintIssue("", err)) } @@ -413,7 +417,7 @@ func extractRPCMessages(rpc vppapi.RPC) []string { if m := rpc.Request; m != "" { messages = append(messages, m) } - if m := rpc.Reply; m != "" { + if m := rpc.Reply; m != "" && m != "null" { messages = append(messages, m) } if m := rpc.StreamMsg; m != "" { diff --git a/cmd/govpp/options.go b/cmd/govpp/options.go index 4905216d..ac227c56 100644 --- a/cmd/govpp/options.go +++ b/cmd/govpp/options.go @@ -39,7 +39,7 @@ func (glob *GlobalOptions) InstallFlags(flags *pflag.FlagSet) { flags.StringVar(&glob.Color, "color", "", "Color mode; auto/always/never") } -func InitOptions(opts *GlobalOptions) { +func InitOptions(cli Cli, opts *GlobalOptions) { // override if opts.Color == "" && os.Getenv("NO_COLOR") != "" { // https://no-color.org/ @@ -54,9 +54,9 @@ func InitOptions(opts *GlobalOptions) { switch strings.ToLower(opts.Color) { case "auto", "": - /*if !cli.Out().IsTerminal() { + if !cli.Out().IsTerminal() { color.Disable() - }*/ + } case "on", "enabled", "always", "1", "true": color.Enable = true case "off", "disabled", "never", "0", "false": @@ -78,7 +78,6 @@ func InitOptions(opts *GlobalOptions) { } else if opts.Debug { logrus.SetLevel(logrus.DebugLevel) } else { - logrus.SetLevel(defaultLogLevel) } } diff --git a/cmd/govpp/util.go b/cmd/govpp/util.go index d6988e12..09c062f9 100644 --- a/cmd/govpp/util.go +++ b/cmd/govpp/util.go @@ -16,12 +16,27 @@ package main import ( "fmt" + "regexp" "sort" "strings" "github.com/sirupsen/logrus" ) +const ( + codeSuffix = "[0m" + codeExpr = `(\\u001b|\033)\[[\d;?]+m` +) + +var codeRegex = regexp.MustCompile(codeExpr) + +func clearColorCode(str string) string { + if !strings.Contains(str, codeSuffix) { + return str + } + return codeRegex.ReplaceAllString(str, "") +} + func mapStr(data map[string]string) string { var str string for k, v := range data { diff --git a/go.mod b/go.mod index 712eb64f..c58033ea 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,13 @@ go 1.18 require ( github.com/bennyscetbun/jsongo v1.1.1 + github.com/docker/cli v24.0.2+incompatible github.com/fsnotify/fsnotify v1.4.9 github.com/ftrvxmtrx/fd v0.0.0-20150925145434-c6d800382fff github.com/gookit/color v1.5.2 github.com/lunixbochs/struc v0.0.0-20200521075829-a4cb8d33dbbe github.com/mitchellh/go-ps v1.0.0 + github.com/moby/term v0.5.0 github.com/olekukonko/tablewriter v0.0.5 github.com/onsi/gomega v1.19.0 github.com/pkg/profile v1.5.0 @@ -20,6 +22,7 @@ require ( ) require ( + github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect github.com/kr/pretty v0.1.0 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect diff --git a/go.sum b/go.sum index 0df33c65..f58cf74b 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,14 @@ +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8= +github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/bennyscetbun/jsongo v1.1.1 h1:ZlYkrwGtF5lKUOQh8t2Bn8XjCvwFI5Qdl6eBqZOtddE= github.com/bennyscetbun/jsongo v1.1.1/go.mod h1:j5mIRkqjZ4eEoIKQyfVPQpv56ZX0rn+jPETkD/2dRqA= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/docker/cli v24.0.2+incompatible h1:QdqR7znue1mtkXIJ+ruQMGQhpw2JzMJLRXp6zpzF6tM= +github.com/docker/cli v24.0.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/ftrvxmtrx/fd v0.0.0-20150925145434-c6d800382fff h1:zk1wwii7uXmI0znwU+lqg+wFL9G5+vm5I+9rv2let60= @@ -23,6 +28,8 @@ github.com/mattn/go-runewidth v0.0.9 h1:Lm995f3rfxdpd6TSmuVCHVb/QhupuXlYr8sCI/Qd github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= github.com/mitchellh/go-ps v1.0.0 h1:i6ampVEEF4wQFF+bkYfwYgY+F/uYJDktmvLPf7qIgjc= github.com/mitchellh/go-ps v1.0.0/go.mod h1:J4lOc8z8yJs6vUwklHw2XEIiT4z4C40KtWVN3nvg8Pg= +github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0= +github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/onsi/ginkgo/v2 v2.1.3 h1:e/3Cwtogj0HA+25nMP1jCMDIf8RtRYbGwGGuBIFztkc= @@ -52,6 +59,7 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=