From bc51fdca29f882a07ff3a0cddc49096e4bae6230 Mon Sep 17 00:00:00 2001 From: Ondrej Fabry Date: Thu, 31 Aug 2023 14:24:21 +0200 Subject: [PATCH] Improvements GoVPP CLI (#156) * Improvements GoVPP CLI Changes: - improve output format for diff command - add new linter check to detect unused messages - add format option to lint command - general code cleanup - improved log messages Signed-off-by: Ondrej Fabry * Update Signed-off-by: Ondrej Fabry --------- Signed-off-by: Ondrej Fabry --- .github/workflows/ci.yaml | 24 ++-- binapigen/vppapi/input.go | 33 +++-- binapigen/vppapi/source.go | 227 ++++++++++++++++---------------- binapigen/vppapi/util.go | 48 ++++--- binapigen/vppapi/vppapi.go | 54 +++++--- binapigen/vppapi/vppapi_test.go | 111 +++++++++++++++- cmd/binapi-generator/main.go | 9 +- cmd/govpp/cmd.go | 68 ++++++++-- cmd/govpp/cmd_cli.go | 20 +-- cmd/govpp/cmd_generate.go | 2 +- cmd/govpp/cmd_http.go | 2 +- cmd/govpp/cmd_vppapi.go | 40 +++--- cmd/govpp/cmd_vppapi_diff.go | 93 +++++++++---- cmd/govpp/cmd_vppapi_export.go | 166 ++++++++++++----------- cmd/govpp/cmd_vppapi_lint.go | 90 +++++++------ cmd/govpp/cmd_vppapi_ls.go | 109 ++++++++++----- cmd/govpp/compare.go | 134 ++++++++++++++----- cmd/govpp/input.go | 38 ++++-- cmd/govpp/linter.go | 187 +++++++++++++++----------- cmd/govpp/options.go | 6 +- cmd/govpp/util.go | 7 + 21 files changed, 945 insertions(+), 523 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b5e16bfc..8d9ab69a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -27,18 +27,18 @@ jobs: config_file: '.github/ci/yamllint.yml' go-mod: + name: "Check go.mod" strategy: matrix: go: [ '1.20' ] - name: "Check go.mod" runs-on: ubuntu-latest steps: + - name: "Checkout" + uses: actions/checkout@v3 - name: "Setup Go" uses: actions/setup-go@v4 with: go-version: ${{ matrix.go }} - - name: "Checkout" - uses: actions/checkout@v3 - name: "Run go mod tidy" run: go mod tidy -v - name: "Check go.mod" @@ -49,20 +49,20 @@ jobs: git diff --exit-code go.sum binapi: + name: "Check generated binapi" strategy: matrix: go: [ '1.20' ] env: VERSION: v0.8.0 - name: "Check generated binapi" runs-on: ubuntu-latest steps: + - name: "Checkout" + uses: actions/checkout@v3 - name: "Setup Go" uses: actions/setup-go@v4 with: go-version: ${{ matrix.go }} - - name: "Checkout" - uses: actions/checkout@v3 - name: "Generate binapi" run: make gen-binapi-docker - name: "Check binapi" @@ -70,13 +70,15 @@ jobs: git diff --exit-code binapi build-test: + name: "Build and test" strategy: matrix: go: [ '1.20' ] os: [ ubuntu-latest ] - name: "Build and test" runs-on: ${{ matrix.os }} steps: + - name: "Checkout" + uses: actions/checkout@v3 - name: "Setup Go" uses: actions/setup-go@v4 with: @@ -85,8 +87,6 @@ jobs: uses: autero1/action-gotestsum@v2.0.0 with: gotestsum_version: 1.9.0 - - name: "Checkout" - uses: actions/checkout@v3 - name: "Go Build" run: go build -v ./... - name: "Go Test" @@ -98,21 +98,21 @@ jobs: test-results: test.json golangci: + name: "GolangCI" strategy: matrix: go: [ '1.20' ] - name: "GolangCI" runs-on: ubuntu-latest permissions: contents: read pull-requests: read steps: + - name: "Checkout" + uses: actions/checkout@v3 - name: "Setup Go" uses: actions/setup-go@v4 with: go-version: ${{ matrix.go }} - - name: "Checkout" - uses: actions/checkout@v3 - name: "Run golangci" uses: golangci/golangci-lint-action@v3 # docs: https://github.com/golangci/golangci-lint-action with: diff --git a/binapigen/vppapi/input.go b/binapigen/vppapi/input.go index c81f58db..af3524ba 100644 --- a/binapigen/vppapi/input.go +++ b/binapigen/vppapi/input.go @@ -64,6 +64,8 @@ import ( // - `compression`: compression to use (`gzip`) // - `subdir`: subdirectory to use as base directory // - 'strip': strip first N directories, applied before `subdir` +// +// Returns VppInput on success. func ResolveVppInput(input string) (*VppInput, error) { inputRef, err := ParseInputRef(input) if err != nil { @@ -84,20 +86,21 @@ func resolveVppInputFromDir(path string) (*VppInput, error) { apidir := ResolveApiDir(path) vppInput.ApiDirectory = apidir - logrus.Debugf("path %q resolved to api dir: %v", path, apidir) + logrus.WithField("path", path).Tracef("resolved API dir: %q", apidir) apiFiles, err := ParseDir(apidir) if err != nil { - logrus.Warnf("vppapi parsedir error: %v", err) - } else { - vppInput.Schema.Files = apiFiles - logrus.Debugf("resolved %d apifiles", len(apiFiles)) + //logrus.Warnf("vppapi parsedir error: %v", err) + return nil, fmt.Errorf("parsing API dir %s failed: %w", apidir, err) } + vppInput.Schema.Files = apiFiles + logrus.Tracef("resolved %d apifiles", len(apiFiles)) - vppInput.Schema.Version = ResolveVPPVersion(path) - if vppInput.Schema.Version == "" { - vppInput.Schema.Version = "unknown" + vppVersion := ResolveVPPVersion(path) + if vppVersion == "" { + vppVersion = "unknown" } + vppInput.Schema.Version = vppVersion return vppInput, nil } @@ -132,10 +135,16 @@ func cloneRepoLocally(repo string, commit string, branch string, depth int) (str } logrus.Debugf("local repo dir: %q, fetching %q", cachePath, commit) - cmd := exec.Command("git", "fetch", "-f", "origin", commit) + cmd := exec.Command("git", "fetch", "--tags", "origin") cmd.Dir = cachePath if output, err := cmd.CombinedOutput(); err != nil { - return "", fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, output) + logrus.Debugf("ERROR: failed to fetch tags: %v\nOutput: %s", err, output) + } + + cmd = exec.Command("git", "fetch", "-f", "origin", commit) + cmd.Dir = cachePath + if output, err := cmd.CombinedOutput(); err != nil { + return "", fmt.Errorf("failed to fetch commit: %w\nOutput: %s", err, output) } // Resolve the commit hash for the given branch/tag @@ -163,9 +172,11 @@ func resolveCommitHash(repoPath, ref string) (string, error) { cmd.Dir = repoPath outputBytes, err := cmd.Output() - logrus.Tracef("command %q output: %s", cmd, outputBytes) if err != nil { + logrus.Tracef("[ERR] command %q output: %s", cmd, outputBytes) return "", fmt.Errorf("failed to run command: %w", err) + } else { + logrus.Tracef("[OK] command %q output: %s", cmd, outputBytes) } return strings.TrimSpace(string(outputBytes)), nil diff --git a/binapigen/vppapi/source.go b/binapigen/vppapi/source.go index 15c7661f..5aee17d2 100644 --- a/binapigen/vppapi/source.go +++ b/binapigen/vppapi/source.go @@ -15,8 +15,10 @@ package vppapi import ( + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "strconv" @@ -25,14 +27,18 @@ import ( "github.com/sirupsen/logrus" ) +// TODO: +// - validate format-specific options parsed by input ref +// - add support for zip format +// - add support for json format to allow using JSON-formatted schema as input + type InputFormat string const ( - FormatNone InputFormat = "" - FormatDir InputFormat = "dir" - FormatGit InputFormat = "git" - FormatTar InputFormat = "tar" - FormatZip InputFormat = "zip" + FormatDir InputFormat = "dir" + FormatGit InputFormat = "git" + FormatTar InputFormat = "tar" + FormatZip InputFormat = "zip" ) const ( @@ -60,52 +66,131 @@ type InputRef struct { Options map[string]string } -func (input *InputRef) Retrieve() (*VppInput, error) { - if input.Path == "" { - return nil, fmt.Errorf("invalid path in input reference") +func ParseInputRef(input string) (*InputRef, error) { + logrus.Tracef("parsing input string: %q", input) + + path, options := parsePathAndOptions(input) + + var inputFormat InputFormat + format, ok := options[OptionFormat] + if ok { + inputFormat = InputFormat(format) + delete(options, OptionFormat) + } else { + inputFormat = detectFormatFromPath(path) + logrus.Tracef("detected format: %q", inputFormat) + } + + // Use current working dir by default + if inputFormat == FormatDir && path == "" { + path = "." + } + + ref := &InputRef{ + Format: inputFormat, + Path: path, + Options: options, + } + logrus.Tracef("parsed input ref: %+v", ref) + + return ref, nil + +} + +func parsePathAndOptions(input string) (path string, options map[string]string) { + // Split into path and options + parts := strings.Split(input, "#") + path = parts[0] + options = make(map[string]string) + if len(parts) > 1 { + // Split into key-value pairs + optionsList := strings.Split(parts[1], ",") + for _, option := range optionsList { + // Split each option into key and value + keyValue := strings.SplitN(option, "=", 2) + key := keyValue[0] + value := "" + if len(keyValue) > 1 { + value = keyValue[1] + } + options[key] = value + } + } + return path, options +} + +func detectFormatFromPath(path string) InputFormat { + // By suffix + if strings.HasSuffix(path, ".tar") || strings.HasSuffix(path, ".tar.gz") || strings.HasSuffix(path, ".tgz") { + return FormatTar + } + if strings.HasSuffix(path, ".zip") { + return FormatZip + } + if strings.HasSuffix(path, ".git") { + return FormatGit + } + + // By prefix + if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "ssh://") || strings.HasPrefix(path, "git://") { + return FormatGit } - logrus.Tracef("retrieving input: %+v", input) + // By default + return FormatDir +} - switch input.Format { - case FormatNone: +func (ref *InputRef) Retrieve() (*VppInput, error) { + if ref.Path == "" { + return nil, fmt.Errorf("missing path") + } + if ref.Format == "" { return nil, fmt.Errorf("undefined format") + } + logrus.Tracef("retrieving from ref: %+v", ref) + + switch ref.Format { case FormatDir: - info, err := os.Stat(input.Path) + info, err := os.Stat(ref.Path) if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("directory path %q does not exist", ref.Path) + } else if errors.Is(err, fs.ErrPermission) { + return nil, fmt.Errorf("directory path %q does not have sufficient permission", ref.Path) + } return nil, fmt.Errorf("path error: %v", err) } if !info.IsDir() { return nil, fmt.Errorf("path must be a directory") } - return resolveVppInputFromDir(input.Path) + return resolveVppInputFromDir(ref.Path) case FormatGit: - branch := input.Options[OptionGitBranch] - tag := input.Options[OptionGitTag] - ref := input.Options[OptionGitRef] + branch := ref.Options[OptionGitBranch] + tag := ref.Options[OptionGitTag] + rev := ref.Options[OptionGitRef] if branch != "" && tag != "" { return nil, fmt.Errorf("cannot set both branch and tag") } else if branch != "" || tag != "" { - if ref != "" { + if rev != "" { return nil, fmt.Errorf("cannot set rev if branch or tag is set") } if branch != "" { - ref = branch + rev = branch } else if tag != "" { - ref = tag + rev = tag } } - commit := ref + commit := rev if commit == "" { commit = defaultGitRef } cloneDepth := 0 - if depth := input.Options[OptionGitDepth]; depth != "" { + if depth := ref.Options[OptionGitDepth]; depth != "" { d, err := strconv.Atoi(depth) if err != nil { return nil, fmt.Errorf("invalid depth: %w", err) @@ -113,15 +198,15 @@ func (input *InputRef) Retrieve() (*VppInput, error) { cloneDepth = d } - logrus.Debugf("updating local repo %s to %s", input.Path, commit) + logrus.Debugf("updating local repo %s to %s", ref.Path, commit) - repoDir, err := cloneRepoLocally(input.Path, commit, branch, cloneDepth) + repoDir, err := cloneRepoLocally(ref.Path, commit, branch, cloneDepth) if err != nil { return nil, err } dir := repoDir - subdir, hasSubdir := input.Options[OptionGitSubdir] + subdir, hasSubdir := ref.Options[OptionGitSubdir] if hasSubdir { dir = filepath.Join(repoDir, subdir) if info, err := os.Stat(dir); err != nil { @@ -138,7 +223,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) { case FormatTar: stripN := 0 - strip, hasStrip := input.Options[OptionArchiveStrip] + strip, hasStrip := ref.Options[OptionArchiveStrip] if hasStrip { parsedStrip, err := strconv.Atoi(strip) if err != nil { @@ -151,7 +236,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) { } gzipped := false - compression, hasCompression := input.Options[OptionArchiveCompression] + compression, hasCompression := ref.Options[OptionArchiveCompression] if hasCompression { if compression == "gzip" { gzipped = true @@ -159,17 +244,17 @@ func (input *InputRef) Retrieve() (*VppInput, error) { return nil, fmt.Errorf("unknown compression: %s", compression) } } else { - if strings.HasSuffix(input.Path, ".gz") || strings.HasSuffix(input.Path, ".tgz") { + if strings.HasSuffix(ref.Path, ".gz") || strings.HasSuffix(ref.Path, ".tgz") { gzipped = true } } - reader, err := getInputPathReader(input.Path) + reader, err := getInputPathReader(ref.Path) if err != nil { return nil, fmt.Errorf("input path: %w", err) } - logrus.Tracef("extracting tarball %s (gzip: %v)", input.Path, gzipped) + logrus.Tracef("extracting tarball %s (gzip: %v)", ref.Path, gzipped) tempDir, err := extractTar(reader, gzipped, stripN) if err != nil { @@ -178,7 +263,7 @@ func (input *InputRef) Retrieve() (*VppInput, error) { dir := tempDir - subdir, hasSubdir := input.Options[OptionArchiveSubdir] + subdir, hasSubdir := ref.Options[OptionArchiveSubdir] if hasSubdir { dir = filepath.Join(tempDir, subdir) if info, err := os.Stat(dir); err != nil { @@ -194,17 +279,13 @@ func (input *InputRef) Retrieve() (*VppInput, error) { return resolveVppInputFromDir(dir) case FormatZip: - return resolveVppInputFromZip(input.Path) + return nil, fmt.Errorf("zip format is not yet supported") default: - return nil, fmt.Errorf("unknown format: %q", input.Format) + return nil, fmt.Errorf("unknown format: %q", ref.Format) } } -func resolveVppInputFromZip(path string) (*VppInput, error) { - return nil, fmt.Errorf("zip format is not yet supported") -} - func getInputPathReader(path string) (io.ReadCloser, error) { var reader io.ReadCloser if path == "-" { @@ -218,77 +299,3 @@ func getInputPathReader(path string) (io.ReadCloser, error) { } return reader, nil } - -func ParseInputRef(input string) (*InputRef, error) { - logrus.Tracef("parsing input string: %q", input) - - path, options := parsePathAndOptions(input) - - var inputFormat InputFormat - format, ok := options[OptionFormat] - if ok { - inputFormat = InputFormat(format) - delete(options, OptionFormat) - } else { - inputFormat = detectFormatFromPath(path) - logrus.Tracef("detected format: %v", inputFormat) - } - - // Use current working dir by default - if inputFormat == FormatDir && path == "" { - path = "." - } - - ref := &InputRef{ - Format: inputFormat, - Path: path, - Options: options, - } - logrus.Tracef("parsed InputRef: %+v", ref) - - return ref, nil - -} - -func parsePathAndOptions(input string) (path string, options map[string]string) { - // Split into path and options - parts := strings.Split(input, "#") - path = parts[0] - options = make(map[string]string) - if len(parts) > 1 { - // Split into key-value pairs - optionsList := strings.Split(parts[1], ",") - for _, option := range optionsList { - // Split each option into key and value - keyValue := strings.SplitN(option, "=", 2) - key := keyValue[0] - value := "" - if len(keyValue) > 1 { - value = keyValue[1] - } - options[key] = value - } - } - return path, options -} - -func detectFormatFromPath(path string) InputFormat { - // By suffix - if strings.HasSuffix(path, ".tar") || strings.HasSuffix(path, ".tar.gz") || strings.HasSuffix(path, ".tgz") { - return FormatTar - } - if strings.HasSuffix(path, ".zip") { - return FormatZip - } - if strings.HasSuffix(path, ".git") { - return FormatGit - } - - // By prefix - if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "ssh://") || strings.HasPrefix(path, "git://") { - return FormatGit - } - - // By default - return FormatDir -} diff --git a/binapigen/vppapi/util.go b/binapigen/vppapi/util.go index 435ea881..83f1c976 100644 --- a/binapigen/vppapi/util.go +++ b/binapigen/vppapi/util.go @@ -30,6 +30,7 @@ import ( const ( VPPVersionEnvVar = "VPP_VERSION" + VPPVersionFile = "VPP_VERSION" VPPDirEnvVar = "VPP_DIR" versionScriptPath = "src/scripts/version" @@ -98,56 +99,57 @@ func makeJsonApiFiles(dir string) error { // // Version resolved here can be overriden by setting VPP_VERSION env var. func ResolveVPPVersion(apidir string) string { + // check if using default dir + if filepath.Clean(apidir) == DefaultDir { + // assuming VPP package is installed + version, err := GetVPPVersionInstalled() + if err != nil { + logrus.Tracef("ERR: failed to resolve VPP version from installed package: %v", err) + } else { + logrus.Tracef("resolved VPP version from installed package: %v", version) + return version + } + } + // check if inside VPP repo repoDir, err := findGitRepoRootDir(apidir) if err != nil { - logrus.Debugf("ERR: failed to check VPP git repo: %v", err) + logrus.Tracef("failed to check VPP git repo: %v", err) } else { - logrus.Debugf("resolved git repo root directory: %v", repoDir) + logrus.Tracef("resolved git repo root directory: %v", repoDir) version, err := GetVPPVersionRepo(repoDir) if err != nil { - logrus.Debugf("ERR: failed to resolve VPP version from version script: %v", err) + logrus.Tracef("ERR: failed to resolve VPP version from version script: %v", err) } else { - logrus.Debugf("resolved VPP version from version script: %v", version) + logrus.Tracef("resolved VPP version from version script: %v", version) return version } // try to read VPP_VERSION file - data, err := os.ReadFile(path.Join(repoDir, "VPP_VERSION")) + data, err := os.ReadFile(path.Join(repoDir, VPPVersionFile)) if err == nil { ver := strings.TrimSpace(string(data)) - logrus.Debugf("VPP version was resolved to %q from VPP_VERSION file", ver) + logrus.Tracef("VPP version was resolved to %q from %s file", ver, VPPVersionFile) return ver } } // try to read VPP_VERSION file - data, err := os.ReadFile(path.Join(apidir, "VPP_VERSION")) + data, err := os.ReadFile(path.Join(apidir, VPPVersionFile)) if err == nil { ver := strings.TrimSpace(string(data)) - logrus.Debugf("VPP version was resolved to %q from VPP_VERSION file", ver) + logrus.Tracef("VPP version was resolved to %q from %s file", ver, VPPVersionFile) 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) + logrus.Tracef("VPP version was manually set to %q via %s env var", ver, VPPVersionEnvVar) return ver } - // assuming VPP package is installed - if filepath.Clean(apidir) == DefaultDir { - version, err := GetVPPVersionInstalled() - if err != nil { - logrus.Debugf("ERR: failed to resolve VPP version from installed package: %v", err) - } else { - logrus.Debugf("resolved VPP version from installed package: %v", version) - return version - } - } - - logrus.Warnf("VPP version could not be resolved, you can set it manually using %s env var", VPPVersionEnvVar) + logrus.Tracef("VPP version could not be resolved, you can set it manually using %s env var", VPPVersionEnvVar) return "" } @@ -177,10 +179,6 @@ func GetVPPVersionRepo(repoDir string) (string, error) { } func findGitRepoRootDir(dir string) (string, error) { - if conf := os.Getenv(VPPDirEnvVar); conf != "" { - logrus.Infof("VPP directory was manually set to %q via %s env var", conf, VPPDirEnvVar) - return conf, nil - } cmd := exec.Command("git", "rev-parse", "--show-toplevel") cmd.Dir = dir out, err := cmd.CombinedOutput() diff --git a/binapigen/vppapi/vppapi.go b/binapigen/vppapi/vppapi.go index cefc12db..cc94acd1 100644 --- a/binapigen/vppapi/vppapi.go +++ b/binapigen/vppapi/vppapi.go @@ -26,50 +26,62 @@ const ( // DefaultDir is default location of API files. DefaultDir = "/usr/share/vpp/api" - // APIFileExtension is a VPP API file extension suffix + // APIFileExtension is a VPP API file extension suffix. APIFileExtension = ".api.json" ) -// FindFiles searches for API files in given directory or in a nested directory -// that is at most one level deeper than dir. This effectively finds all API files -// under core & plugins directories inside API directory. +// FindFiles searches for API files in specified directory or in a subdirectory +// that is at most 1-level deeper than dir. This effectively finds all API files +// under core & plugins directories inside API directory. The returned list of +// files will contain paths relative to dir. func FindFiles(dir string) (files []string, err error) { return FindFilesRecursive(dir, 1) } -// FindFilesRecursive searches for API files under dir or in a nested directory that is not -// nested deeper than deep. +// FindFilesRecursive recursively searches for API files inside specified directory +// or a subdirectory that is not nested more than deep. The returned list of files +// will contain paths relative to dir. func FindFilesRecursive(dir string, deep int) (files []string, err error) { entries, err := os.ReadDir(dir) if err != nil { - return nil, fmt.Errorf("read dir %s failed: %v", dir, err) + return nil, fmt.Errorf("read dir %q error: %v", dir, err) } for _, e := range entries { if e.IsDir() && deep > 0 { - nestedDir := filepath.Join(dir, e.Name()) - if nested, err := FindFilesRecursive(nestedDir, deep-1); err != nil { + nestedFiles, err := FindFilesRecursive(filepath.Join(dir, e.Name()), deep-1) + if err != nil { return nil, err - } else { - files = append(files, nested...) + } + for _, nestedFile := range nestedFiles { + files = append(files, filepath.Join(e.Name(), nestedFile)) } } else if !e.IsDir() && strings.HasSuffix(e.Name(), APIFileExtension) { - files = append(files, filepath.Join(dir, e.Name())) + files = append(files, e.Name()) } } return files, nil } -// Parse parses API files in directory DefaultDir and returns collection of File -// or an error if any error occurs during parsing. -func Parse() ([]File, error) { +// ParseDefault parses API files in the directory DefaultDir, which is a default +// location of the API files for VPP installed on the host system, and returns list +// of File or an error if any occurs. +func ParseDefault() ([]File, error) { + // check if DefaultDir directory exists + if _, err := os.Stat(DefaultDir); os.IsNotExist(err) { + return nil, fmt.Errorf("default API directory %s does not exist", DefaultDir) + } else if err != nil { + return nil, err + } return ParseDir(DefaultDir) } -// ParseDir searches for API files in apiDir, parses the found API files and -// returns collection of File. +// ParseDir searches for API files in apiDir, parses them and returns list of +// File or an error if any occurs during searching or parsing. +// The returned files will have Path field set to a path relative to apiDir. // // API files must have suffix `.api.json` and must be formatted as JSON. func ParseDir(apiDir string) ([]File, error) { + // prepare list of files to parse list, err := FindFiles(apiDir) if err != nil { return nil, err @@ -79,7 +91,7 @@ func ParseDir(apiDir string) ([]File, error) { var files []File for _, f := range list { - file, err := ParseFile(f) + file, err := ParseFile(filepath.Join(apiDir, f)) if err != nil { return nil, err } @@ -93,7 +105,7 @@ func ParseDir(apiDir string) ([]File, error) { } // ParseFile parses API file and returns File or an error if any error occurs -// during parsing. +// during parsing. The retrurned file will have Path field set to apiFile. func ParseFile(apiFile string) (*File, error) { // check API file extension if !strings.HasSuffix(apiFile, APIFileExtension) { @@ -113,7 +125,7 @@ func ParseFile(apiFile string) (*File, error) { file, err := ParseRaw(content) if err != nil { - return nil, fmt.Errorf("parsing API file %q content failed: %w", base, err) + return nil, fmt.Errorf("parsing API file %q data failed: %w", base, err) } file.Name = name file.Path = apiFile @@ -132,7 +144,7 @@ func ParseRaw(content []byte) (file *File, err error) { file, err = parseApiJsonFile(content) if err != nil { - return nil, fmt.Errorf("parseApiJsonFile error: %w", err) + return nil, fmt.Errorf("parse error: %w", err) } return file, nil diff --git a/binapigen/vppapi/vppapi_test.go b/binapigen/vppapi/vppapi_test.go index 6ec9139f..17927d47 100644 --- a/binapigen/vppapi/vppapi_test.go +++ b/binapigen/vppapi/vppapi_test.go @@ -17,11 +17,120 @@ package vppapi import ( "encoding/json" "os" + "path/filepath" + "reflect" "testing" . "github.com/onsi/gomega" ) +// Create folder with files +func createFolderAndFiles(t *testing.T, dir string, files ...string) { + t.Helper() + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + t.Logf("created dir %q", dir) + t.Cleanup(func() { + t.Helper() + if err := os.RemoveAll(dir); err != nil { + t.Fatal(err) + } + t.Logf("removed dir %q", dir) + }) + + for _, file := range files { + filename := filepath.Join(dir, file) + if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil { + t.Fatal(err) + } + f, err := os.Create(filename) + if err != nil { + t.Fatal(err) + } + _ = f.Close() + t.Logf("- file created %q", file) + } +} + +func absPath(path string) string { + p, _ := filepath.Abs(path) + return p +} + +func TestFindFiles(t *testing.T) { + + createFolderAndFiles(t, "testdata/find_files", "A/one.api.json", "A/two.api.json", "B/three.api.json") + + type args struct { + dir string + } + tests := []struct { + name string + cwd string + args args + wantFiles []string + wantErr bool + }{ + { + name: "find files 1", + args: args{dir: "testdata/find_files"}, + wantFiles: []string{"A/one.api.json", "A/two.api.json", "B/three.api.json"}, + wantErr: false, + }, + { + name: "find files 2", + args: args{dir: "./testdata/find_files"}, + wantFiles: []string{"A/one.api.json", "A/two.api.json", "B/three.api.json"}, + wantErr: false, + }, + { + name: "find files 3", + args: args{dir: absPath("./testdata/find_files")}, + wantFiles: []string{"A/one.api.json", "A/two.api.json", "B/three.api.json"}, + wantErr: false, + }, + { + name: "find files 4", + cwd: "testdata", + args: args{dir: "./find_files"}, + wantFiles: []string{"A/one.api.json", "A/two.api.json", "B/three.api.json"}, + wantErr: false, + }, + { + name: "find files 5", + cwd: "testdata/find_files", + args: args{dir: "."}, + wantFiles: []string{"A/one.api.json", "A/two.api.json", "B/three.api.json"}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.cwd != "" { + old := os.Getenv("PWD") + if err := os.Chdir(tt.cwd); err != nil { + t.Fatal(err) + } + t.Logf("curWorkingDir changed to: %v", tt.cwd) + defer func() { + if err := os.Chdir(old); err != nil { + t.Fatal(err) + } + }() + } + gotFiles, err := FindFiles(tt.args.dir) + if (err != nil) != tt.wantErr { + t.Errorf("FindFiles(%s) error = %v, wantErr %v", tt.args.dir, err, tt.wantErr) + return + } + if !reflect.DeepEqual(gotFiles, tt.wantFiles) { + t.Errorf("FindFiles(%s) gotFiles = %v, want %v", tt.args.dir, gotFiles, tt.wantFiles) + } + }) + } +} + func TestGetInputFiles(t *testing.T) { RegisterTestingT(t) @@ -29,7 +138,7 @@ func TestGetInputFiles(t *testing.T) { Expect(err).ShouldNot(HaveOccurred()) Expect(result).To(HaveLen(6)) for _, file := range result { - Expect(file).To(BeAnExistingFile()) + Expect("testdata/" + file).To(BeAnExistingFile()) } } diff --git a/cmd/binapi-generator/main.go b/cmd/binapi-generator/main.go index c9b026d9..8ef7b16a 100644 --- a/cmd/binapi-generator/main.go +++ b/cmd/binapi-generator/main.go @@ -33,7 +33,7 @@ const ( var ( input = pflag.String("input", "", "Input for VPP API (e.g. path to VPP API directory, local VPP repo)") - inputDir = pflag.String("input-dir", "", "DEPRECATED: Input directory containing API files.") + inputDir = pflag.String("input-dir", "", "Input directory containing API files.") theOutputDir = pflag.StringP("output-dir", "o", DefaultOutputDir, "Output directory where code will be generated.") runPlugins = pflag.StringSlice("gen", []string{"rpc"}, "List of generator plugins to run for files.") importPrefix = pflag.String("import-prefix", "", "Prefix imports in the generated go code. \nE.g. other API Files (e.g. api_file.ba.go) will be imported with :\nimport (\n api_file \"/api_file\"\n)") @@ -46,6 +46,7 @@ var ( ) func init() { + pflag.Lookup("input-dir").Deprecated = "The --input-dir is deprecated, use --input instead." pflag.Usage = func() { fmt.Fprintf(os.Stderr, "USAGE\n") fmt.Fprintf(os.Stderr, " Parse API_FILES and generate Go bindings\n") @@ -103,6 +104,12 @@ func main() { theInput = theInputDir } } + if theInput == "" { + if dir := os.Getenv(vppapi.VPPDirEnvVar); dir != "" { + logrus.Infof("VPP input directory was set to %q via %s env var", dir, vppapi.VPPDirEnvVar) + theInput = dir + } + } vppInput, err := vppapi.ResolveVppInput(theInput) if err != nil { diff --git a/cmd/govpp/cmd.go b/cmd/govpp/cmd.go index 37234e82..26f32f1c 100644 --- a/cmd/govpp/cmd.go +++ b/cmd/govpp/cmd.go @@ -16,7 +16,6 @@ package main import ( "github.com/gookit/color" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" "go.fd.io/govpp/internal/version" @@ -35,19 +34,18 @@ func newRootCmd(cli Cli) *cobra.Command { ) cmd := &cobra.Command{ - Use: "govpp [OPTIONS] COMMAND", - Short: "GoVPP CLI tool", - Long: color.Sprintf(logo, version.Short(), version.BuiltBy(), version.BuildTime()), + Use: "govpp [OPTIONS] COMMAND", + Short: "GoVPP CLI tool", + Long: color.Sprintf(logo, version.Short(), version.BuiltBy(), version.BuildTime()) + "\n" + + "GoVPP is an universal CLI tool for any VPP-related development.", Version: version.String(), PersistentPreRunE: func(cmd *cobra.Command, args []string) error { InitOptions(cli, &glob) - logrus.Tracef("global options: %+v", glob) - logrus.Tracef("args: %+v", args) return nil }, - SilenceUsage: true, - SilenceErrors: true, - TraverseChildren: true, + SilenceUsage: true, + SilenceErrors: true, + //TraverseChildren: true, CompletionOptions: cobra.CompletionOptions{ HiddenDefaultCmd: true, }, @@ -67,14 +65,20 @@ func newRootCmd(cli Cli) *cobra.Command { cmd.InitDefaultHelpFlag() cmd.Flags().Lookup("help").Hidden = true + cobra.EnableCommandSorting = false + // Commands cmd.AddCommand( + newCliCommand(cli), newGenerateCmd(cli), - newVppapiCmd(cli), newHttpCmd(cli), - newCliCommand(cli), + newVppapiCmd(cli), ) + forAllCommands(cmd, func(c *cobra.Command) { + c.Flags().SortFlags = false + }) + // Help command cmd.InitDefaultHelpCmd() for _, c := range cmd.Commands() { @@ -82,6 +86,48 @@ func newRootCmd(cli Cli) *cobra.Command { c.Hidden = true } } + // Completion command + cmd.InitDefaultCompletionCmd() + + cmd.SetUsageTemplate(color.Sprint(usageTemplate)) return cmd } + +func forAllCommands(cmd *cobra.Command, f func(c *cobra.Command)) { + for _, c := range cmd.Commands() { + forAllCommands(c, f) + } + f(cmd) +} + +const usageTemplate = `USAGE:{{if .Runnable}} + {{.UseLine}}{{end}}{{if .HasAvailableSubCommands}} + {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}} + +ALIASES: + {{.NameAndAliases}}{{end}}{{if .HasExample}} + +EXAMPLES: +{{- trimRightSpace .Example}}{{end}}{{if .HasAvailableSubCommands}}{{$cmds := .Commands}}{{if eq (len .Groups) 0}} + +COMMANDS:{{range $cmds}}{{if .IsAvailableCommand}} + {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{else}}{{range $group := .Groups}} + +{{.Title}}{{range $cmds}}{{if (and (eq .GroupID $group.ID) .IsAvailableCommand)}} + {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}} + +ADDITIONAL COMMANDS:{{range $cmds}}{{if (and (eq .GroupID "") .IsAvailableCommand)}} + {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} + +OPTIONS: +{{.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}} + +GLOBAL OPTIONS: +{{.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .HasHelpSubCommands}} + +TOPICS:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}} + {{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{if .HasAvailableSubCommands}} + +Use "{{.CommandPath}} [command] --help" for more information about a command.{{end}} +` diff --git a/cmd/govpp/cmd_cli.go b/cmd/govpp/cmd_cli.go index 8e7a1f5e..8563b725 100644 --- a/cmd/govpp/cmd_cli.go +++ b/cmd/govpp/cmd_cli.go @@ -37,16 +37,16 @@ import ( // - try several ways to connect to VPP if not specified const exampleCliCommand = ` - # Execute 'show version' command + # Execute 'show version' command govpp cli show version - # Enter REPL mode to send commands interactively + # Enter REPL mode to send commands interactively govpp cli - # Read CLI command(s) from stdin + # Read CLI command(s) from stdin echo "show errors" | govpp cli - # Execute commands and write output to file + # Execute commands and write output to file govpp cli -o cli.log show version ` @@ -60,7 +60,7 @@ type CliOptions struct { Stdout io.Writer } -func newCliCommand(Cli) *cobra.Command { +func newCliCommand(cli Cli) *cobra.Command { var ( opts = CliOptions{ ApiSocket: socketclient.DefaultSocketName, @@ -74,8 +74,8 @@ func newCliCommand(Cli) *cobra.Command { Example: color.Sprint(exampleCliCommand), DisableFlagsInUseLine: true, RunE: func(cmd *cobra.Command, args []string) error { - opts.Stdin = cmd.InOrStdin() - opts.Stderr = cmd.ErrOrStderr() + opts.Stdin = cli.In() + opts.Stderr = cli.Err() // Setup output if opts.Output != "" { @@ -149,8 +149,10 @@ func newCliCommand(Cli) *cobra.Command { const vppcliPrompt = "vpp# " -func runCliCmd(opts CliOptions, cmds []string) error { - cli, err := connectBinapiVppCLI(opts.ApiSocket) +func runCliCmd(opts CliOptions, cmds []string) (err error) { + var cli VppCli + + cli, err = connectBinapiVppCLI(opts.ApiSocket) if err != nil { return fmt.Errorf("connecting to CLI failed: %w", err) } diff --git a/cmd/govpp/cmd_generate.go b/cmd/govpp/cmd_generate.go index 9eea1a9a..d44ca5ec 100644 --- a/cmd/govpp/cmd_generate.go +++ b/cmd/govpp/cmd_generate.go @@ -44,7 +44,7 @@ func newGenerateCmd(cli Cli) *cobra.Command { Use: "generate [apifile...]", Aliases: []string{"gen"}, Short: "Generate code", - Long: "Generates Go bindings for VPP API", + Long: "Generates 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 b39c6f04..0c41af1e 100644 --- a/cmd/govpp/cmd_http.go +++ b/cmd/govpp/cmd_http.go @@ -69,7 +69,7 @@ func newHttpCmd(Cli) *cobra.Command { } func runHttpCmd(opts HttpCmdOptions) error { - vppInput, err := resolveInput(opts.Input) + vppInput, err := resolveVppInput(opts.Input) if err != nil { return err } diff --git a/cmd/govpp/cmd_vppapi.go b/cmd/govpp/cmd_vppapi.go index b82ea172..7440d03e 100644 --- a/cmd/govpp/cmd_vppapi.go +++ b/cmd/govpp/cmd_vppapi.go @@ -26,32 +26,31 @@ import ( ) const exampleVppapi = ` - # Specify input source of VPP API + # Specify input source forl 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 - # List VPP API contents - govpp vppapi ls [INPUT] - govpp vppapi ls --show-contents + # List VPP API contents + govpp vppapi list [INPUT] + govpp vppapi list --show-contents - # Export VPP API files + # Export VPP API files govpp vppapi export [INPUT] --output vppapi govpp vppapi export [INPUT] --output vppapi.tar.gz - # Lint VPP API definitions + # Lint VPP API definitions govpp vppapi lint [INPUT] - # Compare VPP API schemas + # Compare VPP API schemas govpp vppapi diff [INPUT] --against http://github.com/FDio/vpp.git ` type VppApiCmdOptions struct { - Input string - Paths []string - Format string + Input string + Paths []string } func newVppapiCmd(cli Cli) *cobra.Command { @@ -59,20 +58,22 @@ func newVppapiCmd(cli Cli) *cobra.Command { opts VppApiCmdOptions ) cmd := &cobra.Command{ - Use: "vppapi", - Short: "Manage VPP API", - Long: "Manage VPP API development.", + Use: "vppapi", + Short: "Manage VPP API development", + Long: "Manage the development of VPP API using basic commands to list, browse and export API files,\n" + + "or with more advanced commands to lint API definitions or compare different API versions.", Example: color.Sprint(exampleVppapi), TraverseChildren: true, } - 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.PersistentFlags().StringSliceVar(&opts.Paths, "path", nil, "Limit to specific files or directories.\n"+ + "For example: \"vpe\" or \"core/\".\n"+ + "If specified multiple times, the union is taken.") cmd.AddCommand( newVppApiLsCmd(cli, &opts), - newVppApiExportCmd(cli, &opts), newVppApiDiffCmd(cli, &opts), + newVppApiExportCmd(cli, &opts), newVppApiLintCmd(cli, &opts), ) @@ -80,6 +81,8 @@ func newVppapiCmd(cli Cli) *cobra.Command { } func prepareVppApiFiles(allapifiles []vppapi.File, paths []string, includeImported, sortByName bool) ([]vppapi.File, error) { + logrus.Tracef("preparing %d files", len(allapifiles)) + // remove imported types if !includeImported { binapigen.SortFilesByImports(allapifiles) @@ -96,11 +99,12 @@ func prepareVppApiFiles(allapifiles []vppapi.File, paths []string, includeImport if len(paths) > 0 { apifiles = filterFilesByPaths(allapifiles, paths) if len(apifiles) == 0 { - return nil, fmt.Errorf("no files matching: %q", paths) + return nil, fmt.Errorf("no files matched paths: %q", paths) } - logrus.Tracef("filter (%d paths) matched %d/%d files", len(paths), len(apifiles), len(allapifiles)) + logrus.Tracef("%d/%d files matched paths: %q", len(apifiles), len(allapifiles), paths) } + // sort files if sortByName { binapigen.SortFilesByName(apifiles) } diff --git a/cmd/govpp/cmd_vppapi_diff.go b/cmd/govpp/cmd_vppapi_diff.go index 3e49bef8..e7ec297f 100644 --- a/cmd/govpp/cmd_vppapi_diff.go +++ b/cmd/govpp/cmd_vppapi_diff.go @@ -19,6 +19,7 @@ import ( "io" "github.com/gookit/color" + "github.com/olekukonko/tablewriter" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -30,16 +31,25 @@ import ( // - option to exit with non-zero status on breaking changes const exampleVppApiDiffCommand = ` - # Compare VPP API schemas - govpp vppapi diff . --against https://github.com/FDio/vpp + # Compare VPP API in current directory against master on upstream + govpp vppapi compare --against https://github.com/FDio/vpp.git + + # Compare only specific differences of VPP API schemas + govpp vppapi compare --against https://github.com/FDio/vpp.git --differences=FileVersion,FileCRC + govpp vppapi compare --against https://github.com/FDio/vpp.git --differences=MessageAdded + + # List all types of differences + govpp vppapi lint --list-differences ` type VppApiDiffCmdOptions struct { *VppApiCmdOptions - Against string - Differences []string - CommentDiffs bool + Format string + Against string + Differences []string + CommentDiffs bool + ListDifferences bool } func newVppApiDiffCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { @@ -47,43 +57,55 @@ func newVppApiDiffCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { opts = VppApiDiffCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "diff [INPUT] --against AGAINST [--differences DIFF]...", - Aliases: []string{"cmp", "compare"}, + Use: "compare [INPUT] --against AGAINST [--differences DIFF]... | [--list-differences]", + Aliases: []string{"cmp", "diff", "comp"}, Short: "Compare VPP API schemas", Long: "Compares two VPP API schemas and lists the differences.", Example: color.Sprint(exampleVppApiDiffCommand), Args: cobra.MaximumNArgs(1), + PreRunE: func(cmd *cobra.Command, args []string) error { + if !opts.ListDifferences { + must(cmd.MarkPersistentFlagRequired("against")) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { opts.Input = args[0] } - return runVppApiDiffCmd(cmd.OutOrStdout(), opts) + return runVppApiDiffCmd(cli.Out(), opts) }, } - cmd.PersistentFlags().BoolVar(&opts.CommentDiffs, "comments", false, "Include message comment differences") - cmd.PersistentFlags().StringSliceVar(&opts.Differences, "differences", nil, "List only specific differences") + cmd.PersistentFlags().StringVarP(&opts.Format, "format", "f", "", "Format for the output (json, yaml, go-template..)") cmd.PersistentFlags().StringVar(&opts.Against, "against", "", "The VPP API schema to compare against.") - must(cobra.MarkFlagRequired(cmd.PersistentFlags(), "against")) + cmd.PersistentFlags().BoolVar(&opts.CommentDiffs, "comments", false, "Include message comment differences") + cmd.PersistentFlags().StringSliceVarP(&opts.Differences, "differences", "d", nil, "List only specific differences") + cmd.PersistentFlags().BoolVar(&opts.ListDifferences, "list-differences", false, "List all types of differences") + cmd.MarkFlagsMutuallyExclusive("list-differences", "against") return cmd } -var ( - clrWhite = color.Style{color.White} - clrCyan = color.Style{color.Cyan} - clrDiffFile = color.Style{color.Yellow} -) - func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error { - vppInput, err := resolveInput(opts.Input) + if opts.ListDifferences { + diffs := defaultDifferenceTypes + if opts.Format == "" { + printDiffsAsTable(out, diffs) + } else { + return formatAsTemplate(out, opts.Format, diffs) + } + return nil + } + + vppInput, err := resolveVppInput(opts.Input) if err != nil { return err } vppAgainst, err := vppapi.ResolveVppInput(opts.Against) if err != nil { - return err + return fmt.Errorf("resolving --against failed: %w", err) } logrus.Tracef("VPP against:\n - API dir: %s\n - VPP Version: %s\n - Files: %v", vppAgainst.ApiDirectory, vppAgainst.Schema.Version, len(vppAgainst.Schema.Files)) @@ -92,8 +114,6 @@ func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error { schema1 := vppInput.Schema schema2 := vppAgainst.Schema - logrus.Tracef("comparing schemas:\n\tSCHEMA 1: %+v\n\tSCHEMA 2: %+v\n", schema1, schema2) - diffs := CompareSchemas(&schema1, &schema2) if !opts.CommentDiffs { @@ -122,15 +142,38 @@ func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error { return nil } +func printDiffsAsTable(out io.Writer, diffs []DifferenceType) { + table := tablewriter.NewWriter(out) + table.SetHeader([]string{ + "#", "Difference Type", + }) + table.SetAutoMergeCells(false) + table.SetAutoWrapText(false) + table.SetRowLine(false) + table.SetBorder(false) + for i, d := range diffs { + index := i + 1 + table.Append([]string{ + fmt.Sprint(index), fmt.Sprint(d), + }) + } + table.Render() +} + func printDifferencesSimple(out io.Writer, diffs []Difference) { if len(diffs) == 0 { fmt.Fprintln(out, "No differences found.") return } + var lastFile string fmt.Fprintf(out, "Listing %d differences:\n", len(diffs)) - for _, d := range diffs { - color.Fprintf(out, " - %s\n", d) + for _, diff := range diffs { + if diff.File != lastFile { + color.Fprintf(out, " %s\n", clrDiffFile.Sprint(diff.File)) + } + color.Fprintf(out, " - [%v] %v\n", clrWhite.Sprint(diff.Type), diff.Description) + lastFile = diff.File } } @@ -138,14 +181,14 @@ func filterDiffs(diffs []Difference, differences []string) ([]Difference, error) wantDiffs := map[DifferenceType]bool{} for _, d := range differences { var ok bool - for _, diff := range differenceTypes { + for _, diff := range defaultDifferenceTypes { if string(diff) == d { ok = true break } } if !ok { - return nil, fmt.Errorf("unknown difference type: %s", d) + return nil, fmt.Errorf("unknown difference type: %q", d) } wantDiffs[DifferenceType(d)] = true } diff --git a/cmd/govpp/cmd_vppapi_export.go b/cmd/govpp/cmd_vppapi_export.go index b386bb9c..53c04484 100644 --- a/cmd/govpp/cmd_vppapi_export.go +++ b/cmd/govpp/cmd_vppapi_export.go @@ -23,21 +23,28 @@ import ( "path/filepath" "strings" + "github.com/gookit/color" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "go.fd.io/govpp/binapigen/vppapi" ) -// TODO: -// - add option for exporting flat structure -// - embed VPP version into export somehow +const exampleVppApiExportCmd = ` + # Export VPP API files to directory + govpp vppapi export [INPUT] --output vppapi + govpp vppapi export https://github.com/FDio/vpp --output vppapi + + # Export to archive + govpp vppapi export [INPUT] --output vppapi.tar.gz +` type VppApiExportCmdOptions struct { *VppApiCmdOptions Output string Targz bool + Flat bool // TODO: use this } func newVppApiExportCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { @@ -45,15 +52,16 @@ func newVppApiExportCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { opts = VppApiExportCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - 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), + Use: "export [INPUT] --output OUTPUT [--targz] [--flat]", + Short: "Export VPP API files", + Long: "Export VPP API files from an input location to an output location.", + Example: color.Sprint(exampleVppApiExportCmd), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { opts.Input = args[0] } - // auto-detect .tar.gz + // auto-detect archive from output name if !cmd.Flags().Changed("targz") && strings.HasSuffix(opts.Output, ".tar.gz") { opts.Targz = true } @@ -61,7 +69,8 @@ func newVppApiExportCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { }, } - cmd.PersistentFlags().BoolVar(&opts.Targz, "targz", false, "Export to gzipped tarball") + cmd.PersistentFlags().BoolVar(&opts.Flat, "flat", false, "Export using flat structure") + cmd.PersistentFlags().BoolVar(&opts.Targz, "targz", false, "Export to gzipped tarball archive") cmd.PersistentFlags().StringVarP(&opts.Output, "output", "o", "", "Output directory for the exported files") must(cobra.MarkFlagRequired(cmd.PersistentFlags(), "output")) @@ -69,59 +78,103 @@ func newVppApiExportCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { } func runExportCmd(opts VppApiExportCmdOptions) error { - vppInput, err := resolveInput(opts.Input) + vppInput, err := resolveVppInput(opts.Input) if err != nil { return err } // collect files from input - logrus.Tracef("collecting files for export in API dir: %s", vppInput.ApiDirectory) + logrus.Tracef("preparing export from API dir: %s", vppInput.ApiDirectory) files, err := vppapi.FindFiles(vppInput.ApiDirectory) if err != nil { return err } - logrus.Debugf("exporting %d files", len(files)) + logrus.Debugf("prepared %d API files for export from API dir: %s", len(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) + + tmpApiDir := filepath.Join(temp, "vppapi") + + err = exportVppInputToDir(tmpApiDir, 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) + return fmt.Errorf("exporting files to temp dir failed: %w", err) } - err = exportFilesToTarGz(opts.Output, files2, tmpDir) + + files = append(files, exportVPPVersionFile) + + err = copyFilesToTarGz(opts.Output, files, tmpApiDir) if err != nil { return fmt.Errorf("exporting to gzipped tarball failed: %w", err) } + + logrus.Debugf("exported %d files to archive: %s", len(files), opts.Output) } else { - err = exportFilesToDir(opts.Output, files, vppInput) + err = exportVppInputToDir(opts.Output, files, vppInput) if err != nil { - return fmt.Errorf("exporting failed: %w", err) + return fmt.Errorf("exporting failed to dir failed: %w", err) } + + logrus.Debugf("exported %d files to: %s", len(files), opts.Output) } - logrus.Debugf("exported %d files to %s", len(files), opts.Output) + return nil +} + +const exportVPPVersionFile = "VPP_VERSION" + +func exportVppInputToDir(outputDir string, files []string, vppInput *vppapi.VppInput) error { + logrus.Tracef("exporting %d files into directory: %s", len(files), outputDir) + + apiDir := vppInput.ApiDirectory + + // create the output directory + if err := os.Mkdir(outputDir, 0750); err != nil { + return fmt.Errorf("creating target dir failed: %w", err) + } + + if err := exportFilesToDir(outputDir, files, apiDir); err != nil { + return err + } + + // write VPP version to file + filename := filepath.Join(outputDir, exportVPPVersionFile) + data := []byte(vppInput.Schema.Version) + if err := os.WriteFile(filename, data, 0660); err != nil { + return fmt.Errorf("writing VPP version file failed: %w", err) + } return nil } -func exportFilesToTarGz(outputFile string, files []string, baseDir string) error { - logrus.Tracef("exporting %d files into tarball archive: %s", len(files), outputFile) +func exportFilesToDir(outputDir string, files []string, apiDir string) error { + logrus.Tracef("exporting %d files to output dir: %s", len(files), outputDir) + + // export files to directory + for _, file := range files { + data, err := os.ReadFile(filepath.Join(apiDir, file)) + if err != nil { + return err + } + filename := filepath.Join(outputDir, file) + if err := os.MkdirAll(filepath.Dir(filename), 0750); err != nil { + return err + } + if err := os.WriteFile(filename, data, 0660); err != nil { + return err + } + logrus.Tracef("- exported file %s (%d bytes) to %s", file, len(data), filename) + } + return nil +} + +func copyFilesToTarGz(outputFile string, files []string, baseDir string) error { // create the output file for writing fw, err := os.Create(outputFile) if err != nil { @@ -137,13 +190,15 @@ func exportFilesToTarGz(outputFile string, files []string, baseDir string) error tw := tar.NewWriter(gw) defer tw.Close() + logrus.Tracef("copying %d files into archive: %s", len(files), outputFile) + + // copy files to archive for _, file := range files { // open the file for reading - fr, err := os.Open(file) + fr, err := os.Open(filepath.Join(baseDir, file)) if err != nil { return err } - fi, err := fr.Stat() if err != nil { fr.Close() @@ -160,7 +215,7 @@ func exportFilesToTarGz(outputFile string, files []string, baseDir string) error // update the name to correctly reflect the desired destination when untaring header.Name = strings.TrimPrefix(file, baseDir) - logrus.Tracef("- exporting file: %q to: %s", file, header.Name) + logrus.Tracef("- copying file %s to: %s", file, header.Name) // write the header if err := tw.WriteHeader(header); err != nil { @@ -179,48 +234,3 @@ func exportFilesToTarGz(outputFile string, files []string, baseDir string) error return nil } - -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 - } - - // export files to directory - for _, f := range files { - data, err := os.ReadFile(f) - if err != nil { - return err - } - filerel, err := filepath.Rel(apiDir, f) - if err != nil { - logrus.Debugf("filepath.Rel error: %v", err) - filerel = strings.TrimPrefix(f, apiDir) - } - filename := filepath.Join(outputDir, filerel) - dir := filepath.Dir(filename) - - if err := os.MkdirAll(dir, 0750); err != nil { - return err - } - - if err := os.WriteFile(filename, data, 0660); err != nil { - return err - } - - 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 9afe07b2..2c77d043 100644 --- a/cmd/govpp/cmd_vppapi_lint.go +++ b/cmd/govpp/cmd_vppapi_lint.go @@ -18,17 +18,27 @@ import ( "fmt" "io" + "github.com/gookit/color" "github.com/olekukonko/tablewriter" "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) -// TODO: -// - consider adding categories for linter rules +const exampleVppApiLintCmd = ` + # Lint VPP master using default rules + govpp vppapi lint https://github.com/FDio/vpp + + # Lint using only specific rules + govpp vppapi lint https://github.com/FDio/vpp --rules=MESSAGE_DEPRECATE_OLDER_VERSIONS + + # List all linter rules + govpp vppapi lint --list-rules +` type VppApiLintCmdOptions struct { *VppApiCmdOptions + Format string Rules []string Except []string ExitCode bool @@ -40,20 +50,22 @@ func newVppApiLintCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { opts = VppApiLintCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "lint [INPUT] [--rules RULE]...", - Short: "Lint VPP API", - Long: "Run linter checks for VPP API files", - Args: cobra.MaximumNArgs(1), + Use: "lint [INPUT] [--rules RULE]... [--except RULE]... [--exit-code] | [--list-rules]", + Short: "Lint VPP API definitions", + Long: "Lint VPP API definitions by running linter with rule checks to detect any violations.", + Example: color.Sprint(exampleVppApiLintCmd), + 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) + return runVppApiLintCmd(cli.Out(), 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", "", "Format for the output (json, yaml, go-template..)") + cmd.PersistentFlags().StringSliceVar(&opts.Rules, "rules", nil, "Check only specific rules") + cmd.PersistentFlags().StringSliceVar(&opts.Except, "except", nil, "Exclude specific rules") 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") @@ -62,7 +74,7 @@ func newVppApiLintCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { func runVppApiLintCmd(out io.Writer, opts VppApiLintCmdOptions) error { if opts.ListRules { - rules := LintRules(defaultLintRules...) + rules := ListLintRules(defaultLintRules...) if opts.Format == "" { printLintRulesAsTable(out, rules) } else { @@ -71,51 +83,48 @@ func runVppApiLintCmd(out io.Writer, opts VppApiLintCmdOptions) error { return nil } - vppInput, err := resolveInput(opts.Input) + vppInput, err := resolveVppInput(opts.Input) if err != nil { return err } schema := vppInput.Schema - // filter files - apifiles := filterFilesByPaths(schema.Files, opts.Paths) - if len(apifiles) == 0 { - return fmt.Errorf("filter matched no files") + apifiles, err := prepareVppApiFiles(schema.Files, opts.Paths, false, true) + if err != nil { + return err } - logrus.Debugf("filtered %d/%d files", len(apifiles), len(schema.Files)) + schema.Files = apifiles linter := NewLinter() if len(opts.Rules) > 0 { - logrus.Debugf("setting lint rules to: %v", opts.Rules) + logrus.Debugf("setting rules to: %v", opts.Rules) linter.SetRules(opts.Rules) } if len(opts.Except) > 0 { - logrus.Debugf("disabling lint rules: %v", opts.Except) + logrus.Debugf("disabling rules: %v", opts.Except) linter.Disable(opts.Except...) } - if err := linter.Lint(&schema); err != nil { - if errs, ok := err.(LintIssues); ok { - if opts.Format == "" { - printLintErrorsAsTable(out, errs) - } else { - return formatAsTemplate(out, opts.Format, errs) - } - if opts.ExitCode { - return err - } else { - logrus.Errorln("Linter found:", err) - } - } else { - return fmt.Errorf("linter failure: %w", err) - } + lintIssues, err := linter.Lint(&schema) + if err != nil { + return fmt.Errorf("linter failure: %w", err) + } + + if opts.Format == "" { + printLintErrorsAsTable(out, lintIssues) } else { - if opts.Format == "" { - fmt.Fprintln(out, "No issues found") + if err := formatAsTemplate(out, opts.Format, lintIssues); err != nil { + return err + } + } + + if len(lintIssues) > 0 { + if opts.ExitCode { + return fmt.Errorf("found %d issues", len(lintIssues)) } else { - return formatAsTemplate(out, opts.Format, nil) + logrus.Errorf("found %d issues", len(lintIssues)) } } @@ -140,7 +149,12 @@ func printLintRulesAsTable(out io.Writer, rules []*LintRule) { table.Render() } -func printLintErrorsAsTable(out io.Writer, errs LintIssues) { +func printLintErrorsAsTable(out io.Writer, issues LintIssues) { + if len(issues) == 0 { + fmt.Fprintln(out, "No issues found") + return + } + table := tablewriter.NewWriter(out) table.SetHeader([]string{ "#", "Rule", "Location", "Violation", @@ -149,7 +163,7 @@ func printLintErrorsAsTable(out io.Writer, errs LintIssues) { table.SetAutoWrapText(false) table.SetRowLine(true) table.SetBorder(false) - for i, e := range errs { + for i, e := range issues { index := i + 1 loc := e.File if e.Line > 0 { diff --git a/cmd/govpp/cmd_vppapi_ls.go b/cmd/govpp/cmd_vppapi_ls.go index 38fd2b65..536a497d 100644 --- a/cmd/govpp/cmd_vppapi_ls.go +++ b/cmd/govpp/cmd_vppapi_ls.go @@ -24,6 +24,7 @@ import ( "strconv" "strings" + "github.com/gookit/color" "github.com/olekukonko/tablewriter" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -31,10 +32,29 @@ import ( "go.fd.io/govpp/binapigen/vppapi" ) +const exampleVppApiLsCmd = ` + # List all VPP API files + govpp vppapi ls [INPUT] + + # List only files matching path(s) + govpp vppapi ls [INPUT] --path "core/" + govpp vppapi ls [INPUT] --path "vpe,memclnt" + + # List all contents from files + govpp vppapi ls [INPUT] --show-contents + + # List message definitions + govpp vppapi ls [INPUT] --show-messages + govpp vppapi ls [INPUT] --show-messages --include-fields + + # Print raw VPP API files + govpp vppapi ls [INPUT] --show-raw +` + type VppApiLsCmdOptions struct { *VppApiCmdOptions - Paths []string + Format string IncludeImported bool IncludeFields bool @@ -52,19 +72,21 @@ func newVppApiLsCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { opts = VppApiLsCmdOptions{VppApiCmdOptions: vppapiOpts} ) cmd := &cobra.Command{ - Use: "ls [INPUT] [--path PATH]... [--show-contents | --show-messages | --show-raw | --show-rpc]", - Aliases: []string{"l", "list"}, - Short: "Show VPP API contents", - Long: "Show VPP API files and their contents", + Use: "list [INPUT] [--path PATH]... [--show-contents | --show-messages | --show-raw | --show-rpc]", + Aliases: []string{"l", "ls"}, + Short: "List VPP API contents", + Long: "List VPP API files and their contents", + Example: color.Sprint(exampleVppApiLsCmd), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { opts.Input = args[0] } - return runVppApiLsCmd(cmd.OutOrStdout(), opts) + return runVppApiLsCmd(cli.Out(), opts) }, } + cmd.PersistentFlags().StringVarP(&opts.Format, "format", "f", "", "Format for the output (json, yaml, go-template..)") cmd.PersistentFlags().BoolVar(&opts.IncludeImported, "include-imported", false, "Include imported types") cmd.PersistentFlags().BoolVar(&opts.IncludeFields, "include-fields", false, "Include message fields") cmd.PersistentFlags().BoolVar(&opts.ShowContents, "show-contents", false, "Show contents of VPP API file(s)") @@ -77,22 +99,18 @@ func newVppApiLsCmd(cli Cli, vppapiOpts *VppApiCmdOptions) *cobra.Command { } func runVppApiLsCmd(out io.Writer, opts VppApiLsCmdOptions) error { - vppInput, err := resolveInput(opts.Input) + vppInput, err := resolveVppInput(opts.Input) if err != nil { return err } - allapifiles := vppInput.Schema.Files - - logrus.Debugf("processing %d files", len(allapifiles)) - - apifiles, err := prepareVppApiFiles(allapifiles, opts.Paths, opts.IncludeImported, opts.SortByName) + apifiles, err := prepareVppApiFiles(vppInput.Schema.Files, opts.Paths, opts.IncludeImported, opts.SortByName) if err != nil { return err } // format output - if format := opts.Format; len(format) == 0 { + if opts.Format == "" { if opts.ShowMessages { apimsgs := listVPPAPIMessages(apifiles) showVPPAPIMessages(out, apimsgs, opts.IncludeFields) @@ -104,10 +122,11 @@ func runVppApiLsCmd(out io.Writer, opts VppApiLsCmdOptions) error { rawFiles := getVppApiRawFiles(vppInput.ApiDirectory, apifiles) showVPPAPIRaw(out, rawFiles) } else { - showVPPAPIList(out, apifiles) + files := vppApiFilesToList(apifiles) + showVPPAPIFilesTable(out, files) } } else { - if err := formatAsTemplate(out, format, apifiles); err != nil { + if err := formatAsTemplate(out, opts.Format, apifiles); err != nil { return err } } @@ -151,24 +170,24 @@ func showVPPAPIRaw(out io.Writer, rawFiles []VppApiRawFile) { } } +// VppApiFile holds info about a single VPP API file used for listing files. type VppApiFile struct { - File string + Name string Version string CRC string - Options string + Options []string Path string - NumImports uint - NumMessages uint - NumTypes uint - NumRPCs uint + NumImports int + NumMessages int + NumTypes int + NumRPCs int } -func showVPPAPIList(out io.Writer, apifiles []vppapi.File) { +func showVPPAPIFilesTable(out io.Writer, apifiles []VppApiFile) { table := tablewriter.NewWriter(out) table.SetHeader([]string{ "#", "API", "Version", "CRC", "Path", "Imports", "Messages", "Types", "RPCs", "Options", }) - //table.SetAutoMergeCells(true) table.SetAutoWrapText(false) table.SetRowLine(false) table.SetBorder(false) @@ -179,15 +198,15 @@ func showVPPAPIList(out io.Writer, apifiles []vppapi.File) { for i, apifile := range apifiles { index := i + 1 - typesCount := getFileTypesCount(apifile) - apiVersion := getFileVersion(apifile) - apiCrc := getShortCrc(apifile.CRC) - options := strings.Join(getFileOptionsSlice(apifile), ", ") - apiDirPath := path.Dir(pathOfParentAndFile(apifile.Path)) + typesCount := apifile.NumTypes + apiVersion := apifile.Version + apiCrc := apifile.CRC + options := strings.Join(apifile.Options, ", ") + apiDirPath := path.Dir(apifile.Path) var msgs string - if len(apifile.Messages) > 0 { - msgs = fmt.Sprintf("%3d", len(apifile.Messages)) + if apifile.NumMessages > 0 { + msgs = fmt.Sprintf("%3d", apifile.NumMessages) } else { msgs = fmt.Sprintf("%3s", "-") } @@ -198,14 +217,14 @@ func showVPPAPIList(out io.Writer, apifiles []vppapi.File) { types = fmt.Sprintf("%2s", "-") } var services string - if apifile.Service != nil { - services = fmt.Sprintf("%2d", len(apifile.Service.RPCs)) + if apifile.NumRPCs > 0 { + services = fmt.Sprintf("%2d", apifile.NumRPCs) } else { services = fmt.Sprintf("%2s", "-") } var importCount string - if len(apifile.Imports) > 0 { - importCount = fmt.Sprintf("%2d", len(apifile.Imports)) + if apifile.NumImports > 0 { + importCount = fmt.Sprintf("%2d", apifile.NumImports) } else { importCount = fmt.Sprintf("%2s", "-") } @@ -219,6 +238,28 @@ func showVPPAPIList(out io.Writer, apifiles []vppapi.File) { table.Render() } +func vppApiFilesToList(apifiles []vppapi.File) []VppApiFile { + var list []VppApiFile + for _, apifile := range apifiles { + numRPCs := 0 + if apifile.Service != nil { + numRPCs = len(apifile.Service.RPCs) + } + list = append(list, VppApiFile{ + Name: apifile.Name, + Version: getFileVersion(apifile), + CRC: getShortCrc(apifile.CRC), + Options: getFileOptionsSlice(apifile), + Path: pathOfParentAndFile(apifile.Path), + NumImports: len(apifile.Imports), + NumMessages: len(apifile.Messages), + NumTypes: getFileTypesCount(apifile), + NumRPCs: numRPCs, + }) + } + return list +} + func pathOfParentAndFile(path string) string { // Split the path into components separated by "/" components := strings.Split(path, "/") diff --git a/cmd/govpp/compare.go b/cmd/govpp/compare.go index 0b56bdf5..76b6ca92 100644 --- a/cmd/govpp/compare.go +++ b/cmd/govpp/compare.go @@ -15,6 +15,7 @@ package main import ( + "sort" "strings" "github.com/gookit/color" @@ -27,49 +28,50 @@ import ( type DifferenceType string const ( - VersionDifference DifferenceType = "Version" - TotalFilesDifference DifferenceType = "FilesCount" + VersionDifference DifferenceType = "Version" + TotalFilesDifference DifferenceType = "FilesCount" + FileAddedDifference DifferenceType = "FileAdded" + FileRemovedDifference DifferenceType = "FileRemoved" - FileAddedDifference DifferenceType = "FileAdded" - FileRemovedDifference DifferenceType = "FileRemoved" FileMovedDifference DifferenceType = "FileMoved" FileVersionDifference DifferenceType = "FileVersion" FileCrcDifference DifferenceType = "FileCRC" FileContentsChangedDifference DifferenceType = "FileContentsChanged" + MessageAddedDifference DifferenceType = "MessageAdded" + MessageRemovedDifference DifferenceType = "MessageRemoved" - MessageAddedDifference DifferenceType = "MessageAdded" - MessageRemovedDifference DifferenceType = "MessageRemoved" - MessageCrcDifference DifferenceType = "MessageCRC" - MessageCommentDifference DifferenceType = "MessageComment" - + MessageCrcDifference DifferenceType = "MessageCRC" MsgOptionChangedDifference DifferenceType = "MsgOptionChanged" MsgOptionAddedDifference DifferenceType = "MsgOptionAdded" MsgOptionRemovedDifference DifferenceType = "MsgOptionRemoved" + MessageCommentDifference DifferenceType = "MessageComment" ) -var differenceTypes = []DifferenceType{ +var defaultDifferenceTypes = []DifferenceType{ VersionDifference, TotalFilesDifference, FileAddedDifference, FileRemovedDifference, + FileMovedDifference, FileVersionDifference, FileCrcDifference, FileContentsChangedDifference, MessageAddedDifference, MessageRemovedDifference, + MessageCrcDifference, MsgOptionChangedDifference, MsgOptionAddedDifference, MsgOptionRemovedDifference, } -// Difference represents a difference between two API schemas. +// Difference represents a specific difference found between two schemas. type Difference struct { - Type DifferenceType - File string - Description string - Value1, Value2 any + Type DifferenceType // Type is a type of the difference + File string // File is a file name in which the difference was found in (or nil in case of schema-level differences) + Description string // Description describes the difference + Value1, Value2 any // Value1 & Value2 contain the values that are being compared (or nil in case of add/remove-kind differences) } func (d Difference) String() string { @@ -83,9 +85,31 @@ func (d Difference) String() string { } // CompareSchemas compares two API schemas and returns a list of differences. +// +// Comparison process: +// +// 1. Schema-level properties +// - schema version +// - number of files +// - added/removed files +// +// 2. File-level properties +// - file version/CRC +// - file path +// - file options +// - number of messages/types +// - added/removed messages/types +// +// 3. Object-level properties +// - message CRC +// - message comment +// - added/removed/changed message fields +// - added/removed/changed message options func CompareSchemas(schema1, schema2 *vppapi.Schema) []Difference { var differences []Difference + logrus.Tracef("comparing schemas:\n\tSCHEMA 1: %v (%v files)\n\tSCHEMA 2: %v (%d files)\n", schema1.Version, len(schema1.Files), schema2.Version, len(schema2.Files)) + // compare VPP version if schema1.Version != schema2.Version { differences = append(differences, Difference{ @@ -101,51 +125,84 @@ func CompareSchemas(schema1, schema2 *vppapi.Schema) []Difference { differences = append(differences, Difference{ Type: TotalFilesDifference, Description: color.Sprintf("Total file count %s from %v to %v", - clrWhite.Sprint(numberChangeString(len(schema1.Files), len(schema2.Files))), clrWhite.Sprint(len(schema1.Files)), clrWhite.Sprint(len(schema2.Files))), + clrWhite.Sprint(numberChangeString(len(schema1.Files), len(schema2.Files))), + clrWhite.Sprint(len(schema1.Files)), clrWhite.Sprint(len(schema2.Files))), Value1: len(schema1.Files), Value2: len(schema2.Files), }) } - // compare schema files + fileDiffs := compareSchemaFiles(schema1.Files, schema2.Files) + logrus.Debugf("found %d differences between all schema files", len(fileDiffs)) + + differences = append(differences, fileDiffs...) + + return differences +} + +func compareSchemaFiles(files1 []vppapi.File, files2 []vppapi.File) []Difference { + var differences []Difference + + // prepare files for comparison + var fileList1 []string fileMap1 := make(map[string]vppapi.File) - for _, file := range schema1.Files { + for _, file := range files1 { + fileList1 = append(fileList1, file.Name) fileMap1[file.Name] = file } + var fileList2 []string fileMap2 := make(map[string]vppapi.File) - for _, file := range schema2.Files { + for _, file := range files2 { + fileList2 = append(fileList2, file.Name) fileMap2[file.Name] = file } + sort.Strings(fileList1) + sort.Strings(fileList2) + + var fileCompare []string + // removed files - for fileName, file1 := range fileMap1 { - if file2, ok := fileMap2[fileName]; ok { - fileDiffs := compareFiles(file1, file2) - for _, fileDiff := range fileDiffs { - fileDiff.File = fileName - differences = append(differences, fileDiff) - } + for _, fileName := range fileList1 { + file1 := fileMap1[fileName] + if _, ok := fileMap2[fileName]; ok { + fileCompare = append(fileCompare, fileName) } else { differences = append(differences, Difference{ Type: FileRemovedDifference, - File: fileName, - Description: "File removed", + Description: color.Sprintf("File removed: %s", clrWhite.Sprint(fileName)), Value1: file1, Value2: nil, }) } } // added files - for fileName, file2 := range fileMap2 { + for _, fileName := range fileList2 { + file2 := fileMap2[fileName] if _, ok := fileMap1[fileName]; !ok { differences = append(differences, Difference{ Type: FileAddedDifference, - File: fileName, - Description: "File added", + Description: color.Sprintf("File added: %s", clrWhite.Sprint(fileName)), Value1: nil, Value2: file2, }) } } + sort.Strings(fileCompare) + + // changed files + for _, fileName := range fileCompare { + file1 := fileMap1[fileName] + file2 := fileMap2[fileName] + fileDiffs := compareFiles(file1, file2) + if len(fileDiffs) > 0 { + logrus.Tracef("found %2d differences between files: %s", len(fileDiffs), fileName) + } + for _, fileDiff := range fileDiffs { + fileDiff.File = fileName + differences = append(differences, fileDiff) + } + } + return differences } @@ -185,6 +242,7 @@ func compareFiles(file1, file2 vppapi.File) []Difference { Value2: file2.CRC, }) } + // TODO: compare other file options // Compare number of messages and types if len(file1.Messages) != len(file2.Messages) { @@ -249,6 +307,8 @@ func compareFiles(file1, file2 vppapi.File) []Difference { } } + // TODO: compare added/removed types + return differences } @@ -296,6 +356,8 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference { Type: MsgOptionChangedDifference, Description: color.Sprintf("Message %s changed option %s from %q to %q", clrCyan.Sprint(msg1.Name), clrWhite.Sprint(option), clrWhite.Sprint(val1), clrWhite.Sprint(val2)), + Value1: keyValString(option, val1), + Value2: keyValString(option, val2), }) } } else { @@ -304,19 +366,19 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference { Description: color.Sprintf("Message %s removed option: %s", clrCyan.Sprint(msg1.Name), clrWhite.Sprint(option)), Value1: keyValString(option, val1), - Value2: nil, + Value2: "", }) } } // added options - for option, val := range msg2.Options { + for option, val2 := range msg2.Options { if _, ok := msg1.Options[option]; !ok { differences = append(differences, Difference{ Type: MsgOptionAddedDifference, Description: color.Sprintf("Message %s added option: %s", - clrCyan.Sprint(msg2.Name), clrWhite.Sprint(keyValString(option, val))), - Value1: nil, - Value2: keyValString(option, val), + clrCyan.Sprint(msg2.Name), clrWhite.Sprint(keyValString(option, val2))), + Value1: "", + Value2: keyValString(option, val2), }) } } diff --git a/cmd/govpp/input.go b/cmd/govpp/input.go index 5fa3fc25..ecf716c0 100644 --- a/cmd/govpp/input.go +++ b/cmd/govpp/input.go @@ -19,27 +19,31 @@ import ( "path" "path/filepath" "strings" + "time" "github.com/sirupsen/logrus" "go.fd.io/govpp/binapigen/vppapi" ) -func resolveInput(input string) (*vppapi.VppInput, error) { +func resolveVppInput(input string) (*vppapi.VppInput, error) { if input == "" { - logrus.Tracef("input empty, trying to detect automatically") + logrus.Tracef("VPP input is not set, trying to detect automatically") input = detectVppApiInput() } - logrus.Tracef("resolving VPP input: %q", input) + logrus.Tracef("resolving VPP input: %q\n%s", input, strings.Repeat("-", 100)) + t := time.Now() vppInput, err := vppapi.ResolveVppInput(input) if err != nil { return nil, err } - logrus.Tracef("resolved VPP input:\n - API dir: %s\n - VPP Version: %s\n - Files: %v", - vppInput.ApiDirectory, vppInput.Schema.Version, len(vppInput.Schema.Files)) + tookSec := time.Since(t).Seconds() + + logrus.Tracef("resolved VPP input %q in %.3fs\n%s\n - API dir: %s\n - VPP Version: %s\n - Files: %v", + input, tookSec, strings.Repeat("-", 100), vppInput.ApiDirectory, vppInput.Schema.Version, len(vppInput.Schema.Files)) return vppInput, nil } @@ -80,27 +84,35 @@ func dirExists(dir ...string) bool { 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 { + added := make(map[string]bool) + for _, p := 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 { + if fileMatchesPath(apifile, p) { apifiles = append(apifiles, apifile) found = true added[apifile.Path] = true } } if !found { - logrus.Warnf("path %q did not match any file", arg) + logrus.Debugf("path %q did not match any file", p) } } return apifiles } + +func fileMatchesPath(apifile vppapi.File, arg string) bool { + if apifile.Name == strings.TrimSuffix(arg, ".api") { + return true + } + if apifile.Path == arg { + return true + } + dir, file := path.Split(apifile.Path) + return file == arg || path.Clean(dir) == arg +} diff --git a/cmd/govpp/linter.go b/cmd/govpp/linter.go index 7a5ca3ce..d6a43568 100644 --- a/cmd/govpp/linter.go +++ b/cmd/govpp/linter.go @@ -15,6 +15,7 @@ package main import ( + "errors" "fmt" "regexp" "strconv" @@ -26,42 +27,28 @@ import ( "go.fd.io/govpp/binapigen/vppapi" ) +// TODO: +// - consider adding categories for linter rules + const ( FILE_BASIC = "FILE_BASIC" MESSAGE_DEPRECATE_OLDER_VERSIONS = "MESSAGE_DEPRECATE_OLDER_VERSIONS" MESSAGE_SAME_STATUS = "MESSAGE_SAME_STATUS" + UNUSED_MESSAGE = "UNUSED_MESSAGE" ) var defaultLintRules = []string{ FILE_BASIC, MESSAGE_DEPRECATE_OLDER_VERSIONS, MESSAGE_SAME_STATUS, + UNUSED_MESSAGE, } type LintRule struct { Id string Purpose string - //Categories []string - check func(schema *vppapi.Schema) LintIssues -} - -func LintRules(ids ...string) []*LintRule { - var rules []*LintRule - for _, id := range ids { - rule := GetLintRule(id) - if rule != nil { - rules = append(rules, rule) - } - } - return rules -} -func GetLintRule(id string) *LintRule { - rule, ok := lintRules[id] - if ok { - return &rule - } - return nil + check func(schema *vppapi.Schema) LintIssues } var lintRules = map[string]LintRule{ @@ -77,9 +64,51 @@ var lintRules = map[string]LintRule{ }, MESSAGE_SAME_STATUS: { Id: MESSAGE_SAME_STATUS, - Purpose: "Message request and reply must have the same status", + Purpose: "Messages that are related must have the same status", check: checkFiles(checkFileMessageSameStatus), }, + UNUSED_MESSAGE: { + Id: UNUSED_MESSAGE, + Purpose: "Messages should be used in services", + check: checkFiles(checkFileMessageUsed), + }, +} + +func GetLintRule(id string) *LintRule { + rule, ok := lintRules[id] + if ok { + return &rule + } + return nil +} + +func ListLintRules(ids ...string) []*LintRule { + var rules []*LintRule + for _, id := range ids { + rule := GetLintRule(id) + if rule != nil { + rules = append(rules, rule) + } + } + return rules +} + +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) + } + issues = append(issues, e...) + } + + return issues + } } func checkFileBasic(file *vppapi.File) LintIssues { @@ -164,22 +193,24 @@ func checkFileMessageSameStatus(file *vppapi.File) LintIssues { return issues } -func checkFiles(checkFn func(file *vppapi.File) LintIssues) func(*vppapi.Schema) LintIssues { - return func(schema *vppapi.Schema) LintIssues { - var issues LintIssues +func checkFileMessageUsed(file *vppapi.File) LintIssues { + var issues LintIssues - logrus.Tracef("running checkFiles for %d files", len(schema.Files)) + rpcMsgs := extractFileMessagesToRPC(file) - for _, file := range schema.Files { - e := checkFn(&file) - if e != nil { - logrus.Tracef("checked file: %v (%v)", file.Name, e) - } - issues = append(issues, e...) + for _, message := range file.Messages { + if _, ok := rpcMsgs[message.Name]; !ok { + issues = issues.Append(LintIssue{ + File: file.Path, + Object: map[string]any{ + "Message": message, + }, + Violation: color.Sprintf("message %s is not used by services", color.Cyan.Sprint(message.Name)), + }) } - - return issues } + + return issues } type Linter struct { @@ -188,7 +219,7 @@ type Linter struct { func NewLinter() *Linter { return &Linter{ - rules: LintRules(defaultLintRules...), + rules: ListLintRules(defaultLintRules...), } } @@ -218,18 +249,18 @@ func (l *Linter) Disable(rules ...string) { } } -func (l *Linter) Lint(schema *vppapi.Schema) error { - logrus.Debugf("linter will run %d rule checks for %d files (schema version: %v)", len(l.rules), len(schema.Files), schema.Version) +func (l *Linter) Lint(schema *vppapi.Schema) (LintIssues, error) { + logrus.Debugf("running %d rule checks for %d files (schema version: %v)", len(l.rules), len(schema.Files), schema.Version) var allIssues LintIssues for _, rule := range l.rules { log := logrus.WithField("rule", rule.Id) - log.Debugf("running linter rulecheck (purpose: %v)", rule.Purpose) + log.Tracef("running rule check (purpose: %v)", rule.Purpose) issues := rule.check(schema) if len(issues) > 0 { - log.Tracef("linter rule check failed, found %d issues", len(issues)) + log.Tracef("rule check found %d issues", len(issues)) for _, issue := range issues { if issue.RuleId == "" { @@ -238,18 +269,17 @@ func (l *Linter) Lint(schema *vppapi.Schema) error { allIssues = allIssues.Append(issue) } } else { - log.Tracef("linter rule check passed") + log.Tracef("rule check passed") } } if len(allIssues) > 0 { - logrus.Debugf("found %d issues in %d files", len(allIssues), len(schema.Files)) - return allIssues + logrus.Tracef("found %d issues in %d files", len(allIssues), len(schema.Files)) } else { - logrus.Debugf("no issues found in %d files", len(schema.Files)) + logrus.Tracef("no issues found in %d files", len(schema.Files)) } - return nil + return allIssues, nil } func (l *Linter) SetRules(rules []string) { @@ -270,20 +300,12 @@ func (l *Linter) SetRules(rules []string) { } type LintIssue struct { - RuleId string - - File string - Line int `json:",omitempty"` - - Object any `json:",omitempty"` + RuleId string + File string Violation string -} -func createLintIssue(id string, err error) LintIssue { - return LintIssue{ - RuleId: id, - Violation: err.Error(), - } + Object any `json:",omitempty"` + Line int `json:",omitempty"` } func (l LintIssue) Error() string { @@ -292,26 +314,21 @@ func (l LintIssue) Error() string { type LintIssues []LintIssue -func (le LintIssues) Error() string { - if len(le) == 0 { - return "no issues" - } - return fmt.Sprintf("%d issues", len(le)) -} - func (le LintIssues) Append(errs ...error) LintIssues { var r = le for _, err := range errs { if err == nil { continue } - switch e := err.(type) { - case LintIssue: + var e LintIssue + switch { + case errors.As(err, &e): r = append(r, e) - //case LintIssues: - // r = append(r, e...) default: - r = append(r, createLintIssue("", err)) + r = append(r, LintIssue{ + RuleId: "", + Violation: err.Error(), + }) } } return r @@ -321,6 +338,10 @@ const ( optionStatus = "status" optionStatusInProgress = "in_progress" optionStatusDeprecated = "deprecated" + + noReply = "null" + + noStatus = "n/a" ) func getMessageStatus(message vppapi.Message) string { @@ -333,7 +354,7 @@ func getMessageStatus(message vppapi.Message) string { if status, ok := message.Options[optionStatus]; ok { return status } - return "n/a" + return noStatus } func isMessageDeprecated(message vppapi.Message) bool { @@ -400,13 +421,27 @@ func extractMessageVersions(file *vppapi.File) map[string]map[int]vppapi.Message return messageVersions } -func extractMessagesRPC(file *vppapi.File) map[string]vppapi.RPC { +func extractFileMessagesToRPC(file *vppapi.File) map[string]vppapi.RPC { + if file.Service == nil { + return nil + } messagesRPC := make(map[string]vppapi.RPC) - if file.Service != nil { - for _, rpc := range file.Service.RPCs { - if m := rpc.Request; m != "" { - messagesRPC[m] = rpc - } + for _, rpc := range file.Service.RPCs { + for _, m := range extractRPCMessages(rpc) { + messagesRPC[m] = rpc + } + } + return messagesRPC +} + +func extractMessageRequestsToRPC(file *vppapi.File) map[string]vppapi.RPC { + if file.Service == nil { + return nil + } + messagesRPC := make(map[string]vppapi.RPC) + for _, rpc := range file.Service.RPCs { + if m := rpc.Request; m != "" { + messagesRPC[m] = rpc } } return messagesRPC @@ -417,7 +452,7 @@ func extractRPCMessages(rpc vppapi.RPC) []string { if m := rpc.Request; m != "" { messages = append(messages, m) } - if m := rpc.Reply; m != "" && m != "null" { + if m := rpc.Reply; m != "" && m != noReply { messages = append(messages, m) } if m := rpc.StreamMsg; m != "" { @@ -428,7 +463,7 @@ func extractRPCMessages(rpc vppapi.RPC) []string { } func getRelatedMessages(file *vppapi.File, msg string) []string { - msgsRPC := extractMessagesRPC(file) + msgsRPC := extractMessageRequestsToRPC(file) var related []string if rpc, ok := msgsRPC[msg]; ok { for _, m := range extractRPCMessages(rpc) { diff --git a/cmd/govpp/options.go b/cmd/govpp/options.go index ac227c56..9e141fcd 100644 --- a/cmd/govpp/options.go +++ b/cmd/govpp/options.go @@ -35,8 +35,8 @@ type GlobalOptions struct { func (glob *GlobalOptions) InstallFlags(flags *pflag.FlagSet) { flags.BoolVarP(&glob.Debug, "debug", "D", false, "Enable debug mode") - flags.StringVarP(&glob.LogLevel, "loglevel", "L", "", "Set logging level") - flags.StringVar(&glob.Color, "color", "", "Color mode; auto/always/never") + flags.StringVarP(&glob.LogLevel, "log-level", "L", "", "Set the logging level [trace/debug/info/warn/error]") + flags.StringVar(&glob.Color, "color", "", "Set color mode [auto/always/never]") } func InitOptions(cli Cli, opts *GlobalOptions) { @@ -80,4 +80,6 @@ func InitOptions(cli Cli, opts *GlobalOptions) { } else { logrus.SetLevel(defaultLogLevel) } + + logrus.Tracef("init global options: %+v", opts) } diff --git a/cmd/govpp/util.go b/cmd/govpp/util.go index 09c062f9..be8da34e 100644 --- a/cmd/govpp/util.go +++ b/cmd/govpp/util.go @@ -20,9 +20,16 @@ import ( "sort" "strings" + "github.com/gookit/color" "github.com/sirupsen/logrus" ) +var ( + clrWhite = color.Style{color.White} + clrCyan = color.Style{color.Cyan} + clrDiffFile = color.Style{color.Yellow} +) + const ( codeSuffix = "[0m" codeExpr = `(\\u001b|\033)\[[\d;?]+m`