From 1334fdfa26a21e5d248cee4f6bcae669ed9ef61d Mon Sep 17 00:00:00 2001 From: Clayton Craft Date: Mon, 18 Mar 2024 16:41:23 -0700 Subject: [PATCH] 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 --- go.mod | 10 ++- go.sum | 12 +++ pkgs/deviceinfo/deviceinfo.go | 88 ++++++++----------- pkgs/deviceinfo/deviceinfo_test.go | 41 +++------ .../test_resources/deviceinfo-first | 1 + pkgs/deviceinfo/test_resources/deviceinfo-msm | 3 +- .../test_resources/deviceinfo-unmarshal-1 | 6 ++ 7 files changed, 81 insertions(+), 80 deletions(-) create mode 100644 pkgs/deviceinfo/test_resources/deviceinfo-unmarshal-1 diff --git a/go.mod b/go.mod index 4bebb32..801c6e1 100644 --- a/go.mod +++ b/go.mod @@ -7,5 +7,13 @@ require ( github.com/klauspost/compress v1.15.12 github.com/pierrec/lz4/v4 v4.1.17 github.com/ulikunitz/xz v0.5.10 - golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c + golang.org/x/sys v0.18.0 +) + +require ( + github.com/mvdan/sh v2.6.4+incompatible // indirect + golang.org/x/crypto v0.21.0 // indirect + golang.org/x/sync v0.6.0 // indirect + golang.org/x/term v0.18.0 // indirect + mvdan.cc/sh v2.6.4+incompatible // indirect ) diff --git a/go.sum b/go.sum index 3160900..07697cf 100644 --- a/go.sum +++ b/go.sum @@ -2,9 +2,21 @@ github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e h1:hHg27A0RS github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e/go.mod h1:oDpT4efm8tSYHXV5tHSdRvBet/b/QzxZ+XyyPehvm3A= github.com/klauspost/compress v1.15.12 h1:YClS/PImqYbn+UILDnqxQCZ3RehC9N318SU3kElDUEM= github.com/klauspost/compress v1.15.12/go.mod h1:QPwzmACJjUTFsnSHH934V6woptycfrDDJnH7hvFVbGM= +github.com/mvdan/sh v2.6.4+incompatible h1:D4oEWW0J8cL7zeQkrXw76IAYXF0mJfDaBwjgzmKb6zs= +github.com/mvdan/sh v2.6.4+incompatible/go.mod h1:kipHzrJQZEDCMTNRVRAlMMFjqHEYrthfIlFkJSrmDZE= github.com/pierrec/lz4/v4 v4.1.17 h1:kV4Ip+/hUBC+8T6+2EgburRtkE9ef4nbY3f4dFhGjMc= github.com/pierrec/lz4/v4 v4.1.17/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4= github.com/ulikunitz/xz v0.5.10 h1:t92gobL9l3HE202wg3rlk19F6X+JOxl9BBrCCMYEYd8= github.com/ulikunitz/xz v0.5.10/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= +golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= +golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= +golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= +golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c h1:F1jZWGFhYfh0Ci55sIpILtKKK8p3i2/krTr0H1rg74I= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= +golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= +mvdan.cc/sh v2.6.4+incompatible h1:eD6tDeh0pw+/TOTI1BBEryZ02rD2nMcFsgcvde7jffM= +mvdan.cc/sh v2.6.4+incompatible/go.mod h1:IeeQbZq+x2SUGBensq/jge5lLQbS3XT2ktyp3wrt4x8= diff --git a/pkgs/deviceinfo/deviceinfo.go b/pkgs/deviceinfo/deviceinfo.go index 8b8e891..d537971 100644 --- a/pkgs/deviceinfo/deviceinfo.go +++ b/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, diff --git a/pkgs/deviceinfo/deviceinfo_test.go b/pkgs/deviceinfo/deviceinfo_test.go index 34c816d..41b2e47 100644 --- a/pkgs/deviceinfo/deviceinfo_test.go +++ b/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) } } diff --git a/pkgs/deviceinfo/test_resources/deviceinfo-first b/pkgs/deviceinfo/test_resources/deviceinfo-first index c269c43..2aa20ba 100644 --- a/pkgs/deviceinfo/test_resources/deviceinfo-first +++ b/pkgs/deviceinfo/test_resources/deviceinfo-first @@ -1,2 +1,3 @@ +deviceinfo_format_version="0" deviceinfo_initfs_compression="gz -9" deviceinfo_mesa_driver="panfrost" diff --git a/pkgs/deviceinfo/test_resources/deviceinfo-msm b/pkgs/deviceinfo/test_resources/deviceinfo-msm index e73e3a9..dea1b70 100644 --- a/pkgs/deviceinfo/test_resources/deviceinfo-msm +++ b/pkgs/deviceinfo/test_resources/deviceinfo-msm @@ -1 +1,2 @@ -deviceinfo_mesa_driver="msm" \ No newline at end of file +deviceinfo_format_version="0" +deviceinfo_mesa_driver="msm" diff --git a/pkgs/deviceinfo/test_resources/deviceinfo-unmarshal-1 b/pkgs/deviceinfo/test_resources/deviceinfo-unmarshal-1 new file mode 100644 index 0000000..6dfcdc0 --- /dev/null +++ b/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=""