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 <[email protected]> * Update Signed-off-by: Ondrej Fabry <[email protected]> --------- Signed-off-by: Ondrej Fabry <[email protected]>
FDio · Aug 31, 2023 · bc51fdc · bc51fdc
1 parent fb23b14
commit bc51fdc
Show file tree

Hide file tree

Showing 21 changed files with 945 additions and 523 deletions.
diff --git 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,34 +49,36 @@ 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"
         run: |
           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/[email protected]
         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
@@ -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