From 213160656e29d4b43b6123192af9718a402a82f4 Mon Sep 17 00:00:00 2001
From: Gordon Bleux <33967640+UiP9AV6Y@users.noreply.github.com>
Date: Sat, 16 Mar 2024 10:54:33 +0100
Subject: [PATCH] tools: implement version info parser based around `rpmspec`

RPM spec files contain only a subset of the supported version
information. nontheless, this could provide useful for packaging.

the parser relies on `rpmspec` to expand any macros in the spec file
in order to receive "clean" information.
---
 tools/parser/git/parser.go                    |  71 ++-------
 tools/parser/parser.go                        |   7 +
 tools/parser/rpmspec/parser.go                | 113 ++++++++++++++
 tools/parser/rpmspec/parser_test.go           | 143 ++++++++++++++++++
 .../rpmspec/testdata/broken/broken.spec       |  10 ++
 .../parser/rpmspec/testdata/macro/macro.spec  |  14 ++
 .../rpmspec/testdata/minimal/minimal.spec     |  10 ++
 .../rpmspec/testdata/multiple/multiple.spec   |  10 ++
 .../testdata/multiple/xxx_multiple.spec       |  10 ++
 tools/parser/rpmspec/testdata/nospec/.gitkeep |   0
 tools/parser/rpmspec/testdata/rpmspec-mock.sh |  50 ++++++
 tools/util/run_cmd.go                         |  54 +++++++
 tools/util/run_cmd_test.go                    |  71 +++++++++
 13 files changed, 508 insertions(+), 55 deletions(-)
 create mode 100644 tools/parser/rpmspec/parser.go
 create mode 100644 tools/parser/rpmspec/parser_test.go
 create mode 100644 tools/parser/rpmspec/testdata/broken/broken.spec
 create mode 100644 tools/parser/rpmspec/testdata/macro/macro.spec
 create mode 100644 tools/parser/rpmspec/testdata/minimal/minimal.spec
 create mode 100644 tools/parser/rpmspec/testdata/multiple/multiple.spec
 create mode 100644 tools/parser/rpmspec/testdata/multiple/xxx_multiple.spec
 create mode 100644 tools/parser/rpmspec/testdata/nospec/.gitkeep
 create mode 100755 tools/parser/rpmspec/testdata/rpmspec-mock.sh
 create mode 100644 tools/util/run_cmd.go
 create mode 100644 tools/util/run_cmd_test.go

diff --git a/tools/parser/git/parser.go b/tools/parser/git/parser.go
index 85eab13..f405cd9 100644
--- a/tools/parser/git/parser.go
+++ b/tools/parser/git/parser.go
@@ -1,14 +1,13 @@
 package git
 
 import (
-	"errors"
 	"fmt"
-	"io"
 	"io/fs"
 	"os/exec"
 	"strings"
 
 	"github.com/UiP9AV6Y/buildinfo"
+	"github.com/UiP9AV6Y/buildinfo/tools/util"
 )
 
 const (
@@ -43,12 +42,15 @@ func TryParse(cmd, path string) (*Git, error) {
 		return nil, ErrNoRepository
 	}
 
-	o, e, err := run(cmd, path, "rev-parse", "--show-toplevel")
-	if strings.Contains(e, errParse) {
-		// ignore the error type, as long as the error output
-		// contains hints about the failure cause
-		return nil, ErrNoRepository
-	} else if err != nil {
+	argv := []string{"-C", path, "rev-parse", "--show-toplevel"}
+	o, err := util.RunCmd(realCmd, argv)
+	if err != nil {
+		if strings.Contains(err.Error(), errParse) {
+			// ignore the error type, as long as the error output
+			// contains hints about the failure cause
+			return nil, ErrNoRepository
+		}
+
 		return nil, err
 	}
 
@@ -92,14 +94,14 @@ func (g *Git) Equal(o *Git) bool {
 func (g *Git) ParseVersionInfo() (*buildinfo.VersionInfo, error) {
 	result := buildinfo.NewVersionInfo()
 
-	branch, err := g.run("rev-parse", "--abbrev-ref", "HEAD")
+	branch, err := g.git("rev-parse", "--abbrev-ref", "HEAD")
 	if err != nil {
 		return nil, fmt.Errorf("Unable to determine current git branch: %w", err)
 	} else if branch != "" {
 		result.Branch = branch
 	}
 
-	revision, err := g.run("rev-parse", "HEAD")
+	revision, err := g.git("rev-parse", "HEAD")
 	if err != nil {
 		return nil, fmt.Errorf("Unable to determine git HEAD revision: %w", err)
 	} else if revision != "" {
@@ -107,7 +109,7 @@ func (g *Git) ParseVersionInfo() (*buildinfo.VersionInfo, error) {
 	}
 
 	// ignore error in case the project has no tags
-	version, _ := g.run("describe", "--tags", "--abbrev=0")
+	version, _ := g.git("describe", "--tags", "--abbrev=0")
 	if version != "" {
 		result.Version = strings.TrimPrefix(version, "v")
 	}
@@ -115,49 +117,8 @@ func (g *Git) ParseVersionInfo() (*buildinfo.VersionInfo, error) {
 	return result, nil
 }
 
-func (g *Git) run(arg ...string) (string, error) {
-	o, e, err := run(g.cmd, g.root, arg...)
-	if err == nil {
-		return o, nil
-	} else if e != "" {
-		return "", errors.New(e)
-	}
-
-	return "", err
-}
-
-func run(cmd, cwd string, arg ...string) (string, string, error) {
-	argv := append([]string{"-C", cwd}, arg...)
-	git := exec.Command(cmd, argv...)
-	if git.Err != nil {
-		return "", "", git.Err
-	}
-
-	stderr, err := git.StderrPipe()
-	if err != nil {
-		return "", "", err
-	}
-
-	stdout, err := git.StdoutPipe()
-	if err != nil {
-		return "", "", err
-	}
-
-	if err := git.Start(); err != nil {
-		return "", "", err
-	}
-
-	e, err := io.ReadAll(stderr)
-	if err != nil {
-		return "", "", err
-	}
-
-	o, _ := io.ReadAll(stdout)
-	if err != nil {
-		return "", "", err
-	}
+func (g *Git) git(arg ...string) (string, error) {
+	argv := append([]string{"-C", g.root}, arg...)
 
-	return strings.Trim(string(o), " \n\r"),
-		strings.Trim(string(e), " \n\r"),
-		git.Wait()
+	return util.RunCmd(g.cmd, argv)
 }
diff --git a/tools/parser/parser.go b/tools/parser/parser.go
index 7f1a1e2..3026c85 100644
--- a/tools/parser/parser.go
+++ b/tools/parser/parser.go
@@ -12,6 +12,7 @@ import (
 	"github.com/UiP9AV6Y/buildinfo/tools/parser/git"
 	"github.com/UiP9AV6Y/buildinfo/tools/parser/mock"
 	"github.com/UiP9AV6Y/buildinfo/tools/parser/os"
+	"github.com/UiP9AV6Y/buildinfo/tools/parser/rpmspec"
 )
 
 const (
@@ -50,6 +51,12 @@ func ParseVersionParser(dir string) (VersionParser, error) {
 		return nil, err
 	}
 
+	if r, err := rpmspec.TrySystemParse(base); err == nil {
+		return r, nil
+	} else if !errors.Is(err, rpmspec.ErrNoSpec) {
+		return nil, err
+	}
+
 	if g, err := git.TrySystemParse(base); err == nil {
 		return g, nil
 	} else if !errors.Is(err, git.ErrNoRepository) {
diff --git a/tools/parser/rpmspec/parser.go b/tools/parser/rpmspec/parser.go
new file mode 100644
index 0000000..7818e1b
--- /dev/null
+++ b/tools/parser/rpmspec/parser.go
@@ -0,0 +1,113 @@
+package rpmspec
+
+import (
+	"fmt"
+	"io/fs"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/UiP9AV6Y/buildinfo"
+	"github.com/UiP9AV6Y/buildinfo/tools/util"
+)
+
+const (
+	// spec field to render/extract for the version
+	versionMacro = "%{version}"
+	// spec field to render/extract for the revision
+	revisionMacro = "%{release}"
+	systemCommand = "rpmspec"
+)
+
+var (
+	// Error when no RPM spec file or `rpmspec` command was found
+	ErrNoSpec = fs.ErrNotExist
+)
+
+// parser.VersionParser implementation rendering and parsing a RPM spec file
+type RPMSpec struct {
+	cmd  string
+	file string
+}
+
+// TrySystemParse calls TryParse using the rpmspec command found in the PATH
+func TrySystemParse(path string) (*RPMSpec, error) {
+	return TryParse(systemCommand, path)
+}
+
+// TryParse attempts to parse the given directory for a RPM spec file.
+// If no file was found or the given command was not found
+// ErrNoRepository is returned. All other errors are a result of file
+// access problems.
+func TryParse(cmd, path string) (*RPMSpec, error) {
+	realCmd, err := exec.LookPath(cmd)
+	if err != nil {
+		// unable to parse spec file without `rpmspec`
+		return nil, ErrNoSpec
+	}
+
+	pattern := filepath.Join(path, "*.spec")
+	haystack, err := filepath.Glob(pattern)
+	if err != nil {
+		return nil, err
+	} else if haystack == nil || len(haystack) == 0 {
+		return nil, ErrNoSpec
+	}
+
+	return New(realCmd, haystack[0]), nil
+}
+
+// NewSystem creates a new parser.Parser instance using the provided
+// RPM spec file. the rpmspec executable is invoked as-is,
+// relying on its presence in one of the PATH directories.
+func NewSystem(file string) *RPMSpec {
+	return New(systemCommand, file)
+}
+
+// New creates a new parser.Parser instance using the provided
+// RPM spec file. the rpmspec executable is invoked using
+// the provided path.
+func New(cmd, file string) *RPMSpec {
+	result := &RPMSpec{
+		cmd:  cmd,
+		file: file,
+	}
+
+	return result
+}
+
+// String implements the fmt.Stringer interface
+func (s *RPMSpec) String() string {
+	return fmt.Sprintf("(cmd=%s, spec=%s)", s.cmd, s.file)
+}
+
+// Equal compares the fields of this instance to the given one
+func (s *RPMSpec) Equal(o *RPMSpec) bool {
+	if o == nil {
+		return s == nil
+	}
+
+	return s.cmd == o.cmd && s.file == o.file
+}
+
+// ParseVersionInfo implements the parser.VersionParser interface
+func (s *RPMSpec) ParseVersionInfo() (*buildinfo.VersionInfo, error) {
+	result := buildinfo.NewVersionInfo()
+
+	if version, err := s.rpmspecQuery(versionMacro); err != nil {
+		return nil, err
+	} else if version != "" {
+		result.Version = version
+	}
+
+	if revision, err := s.rpmspecQuery(revisionMacro); err != nil {
+		return nil, err
+	} else if revision != "" {
+		result.Revision = revision
+	}
+
+	return result, nil
+}
+
+func (s *RPMSpec) rpmspecQuery(query string) (string, error) {
+	return util.RunCmd(s.cmd, []string{"--query", "--queryformat", query, s.file})
+}
diff --git a/tools/parser/rpmspec/parser_test.go b/tools/parser/rpmspec/parser_test.go
new file mode 100644
index 0000000..04a9575
--- /dev/null
+++ b/tools/parser/rpmspec/parser_test.go
@@ -0,0 +1,143 @@
+package rpmspec
+
+import (
+	"os"
+	"testing"
+
+	"gotest.tools/v3/assert"
+
+	"github.com/UiP9AV6Y/buildinfo"
+)
+
+func mockRPMSPECBin() (string, error) {
+	wd, err := os.Getwd()
+	if err != nil {
+		return "", err
+	}
+	mockPath := wd + "/testdata"
+
+	os.Setenv("PATH", mockPath)
+
+	return mockPath + "/rpmspec-mock.sh", nil
+}
+
+func TestTryParse(t *testing.T) {
+	type testCase struct {
+		haveCmd, havePath string
+		wantError         bool
+		want              *RPMSpec
+	}
+
+	rpmspecBin, err := mockRPMSPECBin()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := map[string]testCase{
+		"not in PATH": {
+			haveCmd:   "rpmspec-notexists",
+			wantError: true,
+		},
+		"no spec file": {
+			haveCmd:   "rpmspec-mock.sh",
+			havePath:  "testdata/nospec",
+			wantError: true,
+		},
+		"broken spec file": {
+			haveCmd:  "rpmspec-mock.sh",
+			havePath: "testdata/broken",
+			want:     New(rpmspecBin, "testdata/broken/broken.spec"),
+		},
+		"macro spec file": {
+			haveCmd:  "rpmspec-mock.sh",
+			havePath: "testdata/macro",
+			want:     New(rpmspecBin, "testdata/macro/macro.spec"),
+		},
+		"minimal spec file": {
+			haveCmd:  "rpmspec-mock.sh",
+			havePath: "testdata/minimal",
+			want:     New(rpmspecBin, "testdata/minimal/minimal.spec"),
+		},
+		"multiple spec files": {
+			haveCmd:  "rpmspec-mock.sh",
+			havePath: "testdata/multiple",
+			want:     New(rpmspecBin, "testdata/multiple/multiple.spec"),
+		},
+		"relative bin": {
+			haveCmd:  "rpmspec-mock.sh",
+			havePath: "testdata/minimal",
+			want:     New(rpmspecBin, "testdata/minimal/minimal.spec"),
+		},
+		"absolute bin": {
+			haveCmd:  rpmspecBin,
+			havePath: "testdata/minimal",
+			want:     New(rpmspecBin, "testdata/minimal/minimal.spec"),
+		},
+	}
+
+	for ctx, tc := range testCases {
+		t.Run(ctx, func(t *testing.T) {
+			got, err := TryParse(tc.haveCmd, tc.havePath)
+
+			if tc.wantError {
+				assert.Assert(t, err != nil)
+			} else {
+				assert.Assert(t, err)
+				assert.Assert(t, tc.want.Equal(got), "want=%s; got=%s", tc.want, got)
+			}
+		})
+	}
+}
+
+func TestParseVersionInfo(t *testing.T) {
+	type testCase struct {
+		have      *RPMSpec
+		wantError bool
+		want      *buildinfo.VersionInfo
+	}
+
+	rpmspecBin, err := mockRPMSPECBin()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := map[string]testCase{
+		"broken": {
+			have:      New(rpmspecBin, "testdata/broken/broken.spec"),
+			wantError: true,
+		},
+		"nospec": {
+			have:      New(rpmspecBin, "testdata/nospec/nospec.spec"),
+			wantError: true,
+		},
+		"macro": {
+			have: New(rpmspecBin, "testdata/macro/macro.spec"),
+			want: &buildinfo.VersionInfo{
+				Version:  "1.2.3~19701230gitd5a3191",
+				Revision: "1.rhel",
+				Branch:   "trunk",
+			},
+		},
+		"minimal": {
+			have: New(rpmspecBin, "testdata/minimal/minimal.spec"),
+			want: &buildinfo.VersionInfo{
+				Version:  "1.0",
+				Revision: "1",
+				Branch:   "trunk",
+			},
+		},
+	}
+
+	for ctx, tc := range testCases {
+		t.Run(ctx, func(t *testing.T) {
+			got, err := tc.have.ParseVersionInfo()
+
+			if tc.wantError {
+				assert.Assert(t, err != nil)
+			} else {
+				assert.Assert(t, err)
+				assert.Assert(t, tc.want.Equal(got), "want=%s; got=%s", tc.want, got)
+			}
+		})
+	}
+}
diff --git a/tools/parser/rpmspec/testdata/broken/broken.spec b/tools/parser/rpmspec/testdata/broken/broken.spec
new file mode 100644
index 0000000..3a2818e
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/broken/broken.spec
@@ -0,0 +1,10 @@
+Name:           broken
+Summary:        Malformed spec file
+License:        none
+Version:        1.0
+
+%description
+Spec file which is missing the Release field, which
+is a violation of the specification
+
+%files
diff --git a/tools/parser/rpmspec/testdata/macro/macro.spec b/tools/parser/rpmspec/testdata/macro/macro.spec
new file mode 100644
index 0000000..dcfbc26
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/macro/macro.spec
@@ -0,0 +1,14 @@
+%global commit d5a31912eb9f69ea1c8fed59811089ff7c4ccebf
+%global shortcommit %(echo %{commit} | cut -c1-7)
+%global commitdate 19701230
+
+Name:           macro
+Release:        1%{?dist}
+Summary:        Macro test case for parsing
+License:        none
+Version:        1.2.3~%{commitdate}git%{shortcommit}
+
+%description
+RPM spec file with macros for testing the version parser
+
+%files
diff --git a/tools/parser/rpmspec/testdata/minimal/minimal.spec b/tools/parser/rpmspec/testdata/minimal/minimal.spec
new file mode 100644
index 0000000..8b67f83
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/minimal/minimal.spec
@@ -0,0 +1,10 @@
+Name:           minimal
+Release:        1
+Summary:        Minimal test case for parsing
+License:        none
+Version:        1.0
+
+%description
+Smallest possible RPM spec file for testing the version parser
+
+%files
diff --git a/tools/parser/rpmspec/testdata/multiple/multiple.spec b/tools/parser/rpmspec/testdata/multiple/multiple.spec
new file mode 100644
index 0000000..f0533b5
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/multiple/multiple.spec
@@ -0,0 +1,10 @@
+Name:           first
+Release:        1
+Summary:        Minimal test case for parsing
+License:        none
+Version:        1.0
+
+%description
+Smallest possible RPM spec file for testing the version parser
+
+%files
diff --git a/tools/parser/rpmspec/testdata/multiple/xxx_multiple.spec b/tools/parser/rpmspec/testdata/multiple/xxx_multiple.spec
new file mode 100644
index 0000000..942b974
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/multiple/xxx_multiple.spec
@@ -0,0 +1,10 @@
+Name:           second
+Release:        2
+Summary:        Ignored test case for parsing
+License:        none
+Version:        2.0
+
+%description
+RPM spec file which should not be processed
+
+%files
diff --git a/tools/parser/rpmspec/testdata/nospec/.gitkeep b/tools/parser/rpmspec/testdata/nospec/.gitkeep
new file mode 100644
index 0000000..e69de29
diff --git a/tools/parser/rpmspec/testdata/rpmspec-mock.sh b/tools/parser/rpmspec/testdata/rpmspec-mock.sh
new file mode 100755
index 0000000..786a67a
--- /dev/null
+++ b/tools/parser/rpmspec/testdata/rpmspec-mock.sh
@@ -0,0 +1,50 @@
+#!/bin/sh -eu
+
+mock_broken_spec() {
+  echo "error: Release field must be present in package: (main package)" >&2
+  echo "error: query of specfile $1 failed, can't parse" >&2
+  exit 1
+}
+
+mock_macro_spec() {
+  case "$1" in
+  "%{version}") echo "1.2.3~19701230gitd5a3191" ;;
+  "%{release}") echo "1.rhel" ;;
+  *) echo "Unsupported spec query '$1'" >&2 ; exit 1 ;;
+  esac
+}
+
+mock_minimal_spec() {
+  case "$1" in
+  "%{version}") echo "1.0" ;;
+  "%{release}") echo "1" ;;
+  *) echo "Unsupported spec query '$1'" >&2 ; exit 1 ;;
+  esac
+}
+
+if test $# -lt 4; then
+  echo "Not enough arguments" >&2
+  exit 1
+fi
+
+if ! test -s "$4"; then
+  echo "No such file: $4" >&2
+  exit 1
+fi
+
+SPEC_QUERY="$3"
+SPEC_FILE="$4"
+MOCK_STRATEGY=$(/bin/basename -s .spec "$SPEC_FILE")
+
+case "$MOCK_STRATEGY" in
+  broken) mock_broken_spec "$SPEC_FILE" ;;
+  macro) mock_macro_spec "$SPEC_QUERY" ;;
+  minimal) mock_minimal_spec "$SPEC_QUERY" ;;
+  multiple) mock_minimal_spec "$SPEC_QUERY" ;;
+  *)
+    echo "Invalid mock strategy $MOCK_STRATEGY" >&2
+    exit 1
+    ;;
+esac
+
+:
diff --git a/tools/util/run_cmd.go b/tools/util/run_cmd.go
new file mode 100644
index 0000000..543a5ef
--- /dev/null
+++ b/tools/util/run_cmd.go
@@ -0,0 +1,54 @@
+package util
+
+import (
+	"errors"
+	"io"
+	"os/exec"
+	"strings"
+)
+
+// RunCmd is a wrapper around [exec.Command] and returns
+// its standard output. Any error is the result of an
+// execution failure or an error instance comprisef of
+// the error output, should no standard output be produced.
+func RunCmd(cmd string, argv []string) (string, error) {
+	run := exec.Command(cmd, argv...)
+	if run.Err != nil {
+		return "", run.Err
+	}
+
+	stderr, err := run.StderrPipe()
+	if err != nil {
+		return "", err
+	}
+
+	stdout, err := run.StdoutPipe()
+	if err != nil {
+		return "", err
+	}
+
+	if err := run.Start(); err != nil {
+		return "", err
+	}
+
+	e, err := io.ReadAll(stderr)
+	if err != nil {
+		return "", err
+	}
+
+	o, _ := io.ReadAll(stdout)
+	if err != nil {
+		return "", err
+	}
+
+	outData := strings.Trim(string(o), " \n\r")
+	errData := strings.Trim(string(e), " \n\r")
+	err = run.Wait()
+	if errData != "" && (err != nil || outData == "") {
+		return "", errors.New(errData)
+	} else if err != nil {
+		return "", err
+	}
+
+	return outData, nil
+}
diff --git a/tools/util/run_cmd_test.go b/tools/util/run_cmd_test.go
new file mode 100644
index 0000000..3bdad46
--- /dev/null
+++ b/tools/util/run_cmd_test.go
@@ -0,0 +1,71 @@
+package util
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestRunCmd(t *testing.T) {
+	type testCase struct {
+		haveCmd   string
+		haveArgv  []string
+		wantError string
+		want      string
+	}
+
+	testCases := map[string]testCase{
+		"nothing success": {
+			haveCmd:  "/bin/true",
+			haveArgv: []string{},
+		},
+		"nothing error": {
+			haveCmd:   "/bin/false",
+			haveArgv:  []string{},
+			wantError: "exit status 1",
+		},
+		"stdout": {
+			haveCmd:  "echo",
+			haveArgv: []string{"hello", "world"},
+			want:     "hello world",
+		},
+		"stderr": {
+			haveCmd:   "/bin/sh",
+			haveArgv:  []string{"-c", "echo error: foo >&2; false"},
+			wantError: "error: foo",
+		},
+		"both": {
+			haveCmd:   "/bin/sh",
+			haveArgv:  []string{"-c", "echo transaction complete; echo error: foo >&2; false"},
+			wantError: "error: foo",
+		},
+		"ignore output": {
+			haveCmd:   "/bin/sh",
+			haveArgv:  []string{"-c", "echo test; false"},
+			wantError: "exit status 1",
+		},
+		"ignore error": {
+			haveCmd:  "/bin/sh",
+			haveArgv: []string{"-c", "echo transaction complete; echo error: foo >&2"},
+			want:     "transaction complete",
+		},
+		"no executable": {
+			haveCmd:   "/opt/fail-inc/bin/foo-bar",
+			haveArgv:  []string{},
+			wantError: "fork/exec /opt/fail-inc/bin/foo-bar: no such file or directory",
+		},
+	}
+
+	for ctx, tc := range testCases {
+		t.Run(ctx, func(t *testing.T) {
+			got, err := RunCmd(tc.haveCmd, tc.haveArgv)
+
+			if tc.wantError != "" {
+				assert.Error(t, err, tc.wantError)
+			} else {
+				assert.Assert(t, err)
+				assert.Assert(t, tc.want == got, "want=%s; got=%s", tc.want, got)
+			}
+		})
+	}
+}