deviceinfo: replace implementation with mvdan/sh (MR 52)

This library has a convenient "source file" method designed for sourcing
shell envs and returning values set in them. deviceinfo's syntax every
where else seems to be "whatever sh can 'source'", so using this library
seems a lot nicer than trying to implement a parser/interpreter here
(and almost certainly missing corner cases, functionality, etc)

[ci:skip-build]: already built successfully in CI

pkgs/deviceinfo/deviceinfo.go

@@ -4,14 +4,14 @@
 package deviceinfo
 
 import (
-	"bufio"
+	"context"
 	"fmt"
-	"io"
-	"log"
-	"os"
 	"reflect"
+	"strconv"
 	"strings"
+	"time"
 
+	"github.com/mvdan/sh/shell"
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/misc"
 )
 
@@ -20,6 +20,7 @@ type DeviceInfo struct {
 	InitfsExtraCompression string
 	UbootBoardname         string
 	GenerateSystemdBoot    string
+	FormatVersion          string
 }
 
 // Reads the relevant entries from "file" into DeviceInfo struct
@@ -32,13 +33,7 @@ func (d *DeviceInfo) ReadDeviceinfo(file string) error {
 		return fmt.Errorf("unexpected error getting status for %q: %s", file, err)
 	}
 
-	fd, err := os.Open(file)
-	if err != nil {
-		return err
-	}
-	defer fd.Close()
-
-	if err := d.unmarshal(fd); err != nil {
+	if err := d.unmarshal(file); err != nil {
 		return err
 	}
 
@@ -46,53 +41,44 @@ func (d *DeviceInfo) ReadDeviceinfo(file string) error {
 }
 
 // Unmarshals a deviceinfo into a DeviceInfo struct
-func (d *DeviceInfo) unmarshal(r io.Reader) error {
-	s := bufio.NewScanner(r)
-	for s.Scan() {
-		line := s.Text()
-		if strings.HasPrefix(line, "#") {
-			continue
-		}
-
-		// line isn't setting anything, so just ignore it
-		if !strings.Contains(line, "=") {
-			continue
-		}
-
-		// sometimes line has a comment at the end after setting an option
-		line = strings.SplitN(line, "#", 2)[0]
-		line = strings.TrimSpace(line)
-
-		// must support having '=' in the value (e.g. kernel cmdline)
-		parts := strings.SplitN(line, "=", 2)
-		if len(parts) != 2 {
-			return fmt.Errorf("error parsing deviceinfo line, invalid format: %s", line)
-		}
-
-		name, val := parts[0], parts[1]
-		val = strings.ReplaceAll(val, "\"", "")
-
-		if name == "deviceinfo_format_version" && val != "0" {
-			return fmt.Errorf("deviceinfo format version %q is not supported", val)
-		}
-
-		fieldName := nameToField(name)
-
-		if fieldName == "" {
-			return fmt.Errorf("error parsing deviceinfo line, invalid format: %s", line)
-		}
+func (d *DeviceInfo) unmarshal(file string) error {
+	ctx, cancelCtx := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second))
+	defer cancelCtx()
+	vars, err := shell.SourceFile(ctx, file)
+	if err != nil {
+		return fmt.Errorf("parsing deviceinfo %q failed: %w", file, err)
+	}
 
+	for k, v := range vars {
+		fieldName := nameToField(k)
 		field := reflect.ValueOf(d).Elem().FieldByName(fieldName)
 		if !field.IsValid() {
 			// an option that meets the deviceinfo "specification", but isn't
 			// one we care about in this module
 			continue
 		}
-		field.SetString(val)
+		switch field.Interface().(type) {
+		case string:
+			field.SetString(v.String())
+		case bool:
+			if v, err := strconv.ParseBool(v.String()); err != nil {
+				return fmt.Errorf("deviceinfo %q has unsupported type for field %q, expected 'bool'", file, k)
+			} else {
+				field.SetBool(v)
+			}
+		case int:
+			if v, err := strconv.ParseInt(v.String(), 10, 32); err != nil {
+				return fmt.Errorf("deviceinfo %q has unsupported type for field %q, expected 'int'", file, k)
+			} else {
+				field.SetInt(v)
+			}
+		default:
+			return fmt.Errorf("deviceinfo %q has unsupported type for field %q", file, k)
+		}
 	}
-	if err := s.Err(); err != nil {
-		log.Print("unable to parse deviceinfo: ", err)
-		return err
+
+	if d.FormatVersion != "0" {
+		return fmt.Errorf("deviceinfo %q has an unsupported format version %q", file, d.FormatVersion)
 	}
 
 	return nil
@@ -125,8 +111,10 @@ func (d DeviceInfo) String() string {
 			%s: %v
 			%s: %v
 			%s: %v
+			%s: %v
 	}`,
 		"deviceinfo_format_version", d.FormatVersion,
+		"deviceinfo_", d.FormatVersion,
 		"deviceinfo_initfs_compression", d.InitfsCompression,
 		"deviceinfo_initfs_extra_compression", d.InitfsCompression,
 		"deviceinfo_ubootBoardname", d.UbootBoardname,

pkgs/deviceinfo/deviceinfo_test.go

@@ -4,8 +4,6 @@
 package deviceinfo
 
 import (
-	"fmt"
-	"reflect"
 	"strings"
 	"testing"
 )
@@ -59,37 +57,24 @@ func TestUnmarshal(t *testing.T) {
 	tables := []struct {
 		// field is just used for reflection within the test, so it must be a
 		// valid DeviceInfo field
-		field    string
-		in       string
-		expected string
+		file     string
+		expected DeviceInfo
 	}{
-		{"InitfsCompression", "deviceinfo_initfs_compression=\"gzip:-9\"\n", "gzip:-9"},
-		// line with multiple '='
-		{"InitfsCompression", "deviceinfo_initfs_compression=zstd:--foo=1 -T0 --bar=bazz", "zstd:--foo=1 -T0 --bar=bazz"},
-		// empty option
-		{"InitfsCompression", "deviceinfo_initfs_compression=\"\"\n", ""},
-		// line with comment at the end
-		{"", "# this is a comment!\n", ""},
-		// empty lines are fine
-		{"", "", ""},
-		// line with whitepace characters only
-		{"", " \t \n\r", ""},
+		{"./test_resources/deviceinfo-unmarshal-1", DeviceInfo{
+			FormatVersion:          "0",
+			UbootBoardname:         "foobar-bazz",
+			InitfsCompression:      "zstd:--foo=1 -T0 --bar=bazz",
+			InitfsExtraCompression: "",
+		},
+		},
 	}
 	var d DeviceInfo
 	for _, table := range tables {
-		testName := fmt.Sprintf("unmarshal::'%s':", strings.ReplaceAll(table.in, "\n", "\\n"))
-		if err := d.unmarshal(strings.NewReader(table.in)); err != nil {
-			t.Errorf("%s received an unexpected err: ", err)
+		if err := d.unmarshal(table.file); err != nil {
+			t.Error(err)
 		}
-
-		// Check against expected value
-		field := reflect.ValueOf(&d).Elem().FieldByName(table.field)
-		out := ""
-		if table.field != "" {
-			out = field.String()
-		}
-		if out != table.expected {
-			t.Errorf("%s expected: %q, got: %q", testName, table.expected, out)
+		if d != table.expected {
+			t.Errorf("expected: %s, got: %s", table.expected, d)
 		}
 	}
 

pkgs/deviceinfo/test_resources/deviceinfo-first

@@ -1,2 +1,3 @@
+deviceinfo_format_version="0"
 deviceinfo_initfs_compression="gz -9"
 deviceinfo_mesa_driver="panfrost"

pkgs/deviceinfo/test_resources/deviceinfo-msm

@@ -1 +1,2 @@
-deviceinfo_mesa_driver="msm"
+deviceinfo_format_version="0"
+deviceinfo_mesa_driver="msm"

pkgs/deviceinfo/test_resources/deviceinfo-unmarshal-1

@@ -0,0 +1,6 @@
+deviceinfo_format_version="0"
+deviceinfo_uboot_boardname="foobar-bazz"
+# line with multiple =
+deviceinfo_initfs_compression="zstd:--foo=1 -T0 --bar=bazz"
+# empty option
+deviceinfo_initfs_extra_compression=""