filelist: trim whitespace from lines read from files (MR 55)

Fixes issues with leading/trailing whitespaces really messing with mkinitfs

doc/mkinitfs.1.scd

@@ -146,7 +146,7 @@ create/manage. mkinitfs reads configuration from */usr/share/mkinitfs* first, an
 ## /usr/share/mkinitfs/modules, /etc/mkinitfs/modules
 ## /usr/share/mkinitfs/modules-extra, /etc/mkinitfs/modules-extra
 
-	Files with the *.modules* extention in these directories are lists of
+	Files with the *.modules* extension in these directories are lists of
 	kernel modules to include in the initramfs. Individual modules and
 	directories can be listed in the files here. Globbing is also supported.
 

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
 )

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=

internal/archive/archive.go

@@ -237,7 +237,10 @@ func (archive *Archive) AddItemsExclude(flister filelist.FileLister, exclude fil
 
 // Adds the given file or directory at "source" to the archive at "dest"
 func (archive *Archive) AddItem(source string, dest string) error {
-
+	if osutil.HasMergedUsr() {
+		source = osutil.MergeUsr(source)
+		dest = osutil.MergeUsr(dest)
+	}
 	sourceStat, err := os.Lstat(source)
 	if err != nil {
 		e, ok := err.(*os.PathError)
@@ -248,6 +251,12 @@ func (archive *Archive) AddItem(source string, dest string) error {
 		return fmt.Errorf("AddItem: failed to get stat for %q: %w", source, err)
 	}
 
+	// A symlink to a directory doesn't have the os.ModeDir bit set, so we need
+	// to check if it's a symlink first
+	if sourceStat.Mode()&os.ModeSymlink != 0 {
+		return archive.addSymlink(source, dest)
+	}
+
 	if sourceStat.Mode()&os.ModeDir != 0 {
 		return archive.addDir(dest)
 	}
@@ -255,6 +264,45 @@ func (archive *Archive) AddItem(source string, dest string) error {
 	return archive.addFile(source, dest)
 }
 
+func (archive *Archive) addSymlink(source string, dest string) error {
+	target, err := os.Readlink(source)
+	if err != nil {
+		log.Print("addSymlink: failed to get symlink target for: ", source)
+		return err
+	}
+
+	// Make sure we pick up the symlink target too
+	targetAbs := target
+	if filepath.Dir(target) == "." {
+		// relative symlink, make it absolute so we can add the target to the archive
+		targetAbs = filepath.Join(filepath.Dir(source), target)
+	}
+
+	if !filepath.IsAbs(targetAbs) {
+		targetAbs, err = osutil.RelativeSymlinkTargetToDir(targetAbs, filepath.Dir(source))
+		if err != nil {
+			return err
+		}
+	}
+
+	archive.AddItem(targetAbs, targetAbs)
+
+	// Now add the symlink itself
+	destFilename := strings.TrimPrefix(dest, "/")
+
+	archive.items.add(archiveItem{
+		sourcePath: source,
+		header: &cpio.Header{
+			Name:     destFilename,
+			Linkname: target,
+			Mode:     0644 | cpio.ModeSymlink,
+			Size:     int64(len(target)),
+		},
+	})
+
+	return nil
+}
+
 func (archive *Archive) addFile(source string, dest string) error {
 	if err := archive.addDir(filepath.Dir(dest)); err != nil {
 		return err
@@ -266,42 +314,6 @@ func (archive *Archive) addFile(source string, dest string) error {
 		return err
 	}
 
-	// Symlink: write symlink to archive then set 'file' to link target
-	if sourceStat.Mode()&os.ModeSymlink != 0 {
-		// log.Printf("File %q is a symlink", file)
-		target, err := os.Readlink(source)
-		if err != nil {
-			log.Print("addFile: failed to get symlink target: ", source)
-			return err
-		}
-
-		destFilename := strings.TrimPrefix(dest, "/")
-
-		archive.items.add(archiveItem{
-			sourcePath: source,
-			header: &cpio.Header{
-				Name:     destFilename,
-				Linkname: target,
-				Mode:     0644 | cpio.ModeSymlink,
-				Size:     int64(len(target)),
-				// Checksum: 1,
-			},
-		})
-
-		if filepath.Dir(target) == "." {
-			target = filepath.Join(filepath.Dir(source), target)
-		}
-		// make sure target is an absolute path
-		if !filepath.IsAbs(target) {
-			target, err = osutil.RelativeSymlinkTargetToDir(target, filepath.Dir(source))
-			if err != nil {
-				return err
-			}
-		}
-		err = archive.addFile(target, target)
-		return err
-	}
-
 	destFilename := strings.TrimPrefix(dest, "/")
 
 	archive.items.add(archiveItem{
@@ -404,6 +416,13 @@ func (archive *Archive) writeCompressed(path string, mode os.FileMode) (err erro
 }
 
 func (archive *Archive) writeCpio() error {
+	// Just in case
+	if osutil.HasMergedUsr() {
+		archive.addSymlink("/bin", "/bin")
+		archive.addSymlink("/sbin", "/sbin")
+		archive.addSymlink("/lib", "/lib")
+		archive.addSymlink("/usr/sbin", "/usr/sbin")
+	}
 	// having a transient function for actually adding files to the archive
 	// allows the deferred fd.close to run after every copy and prevent having
 	// tons of open file handles until the copying is all done
@@ -418,19 +437,19 @@ func (archive *Archive) writeCpio() error {
 			if header.Mode.IsRegular() {
 				fd, err := os.Open(source)
 				if err != nil {
-					return fmt.Errorf("archive.writeCpio: uname to open file %q, %w", source, err)
+					return fmt.Errorf("archive.writeCpio: Unable to open file %q, %w", source, err)
 				}
 				defer fd.Close()
 				if _, err := io.Copy(archive.cpioWriter, fd); err != nil {
-					return fmt.Errorf("archive.writeCpio: unable to write out archive: %w", err)
+					return fmt.Errorf("archive.writeCpio: Couldn't process %q: %w", source, err)
 				}
 			} else if header.Linkname != "" {
 				// the contents of a symlink is just need the link name
 				if _, err := archive.cpioWriter.Write([]byte(header.Linkname)); err != nil {
-					return fmt.Errorf("archive.writeCpio: unable to write out symlink: %w", err)
+					return fmt.Errorf("archive.writeCpio: unable to write out symlink: %q -> %q: %w", source, header.Linkname, err)
 				}
 			} else {
-				return fmt.Errorf("archive.writeCpio: unknown type for file: %s", source)
+				return fmt.Errorf("archive.writeCpio: unknown type for file: %q: %d", source, header.Mode)
 			}
 		}
 

internal/filelist/hookdirs/hookdirs.go

@@ -44,7 +44,7 @@ func (h *HookDirs) List() (*filelist.FileList, error) {
 
 		s := bufio.NewScanner(f)
 		for s.Scan() {
-			dir := s.Text()
+			dir := strings.TrimSpace(s.Text())
 			if len(dir) == 0 || strings.HasPrefix(dir, "#") {
 				continue
 			}

internal/filelist/hookfiles/hookfiles.go

@@ -11,6 +11,7 @@ import (
 
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/filelist"
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/misc"
+	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/osutil"
 )
 
 type HookFiles struct {
@@ -58,12 +59,15 @@ func slurpFiles(fd io.Reader) (*filelist.FileList, error) {
 
 	s := bufio.NewScanner(fd)
 	for s.Scan() {
-		line := s.Text()
+		line := strings.TrimSpace(s.Text())
 		if len(line) == 0 || strings.HasPrefix(line, "#") {
 			continue
 		}
 
 		src, dest, has_dest := strings.Cut(line, ":")
+		if osutil.HasMergedUsr() {
+			src = osutil.MergeUsr(src)
+		}
 
 		fFiles, err := misc.GetFiles([]string{src}, true)
 		if err != nil {

internal/filelist/modules/modules.go

@@ -33,8 +33,14 @@ func (m *Modules) List() (*filelist.FileList, error) {
 	}
 
 	files := filelist.NewFileList()
+	libDir := "/usr/lib/modules"
+	if exists, err := misc.Exists(libDir); !exists {
+		libDir = "/lib/modules"
+	} else if err != nil {
+		return nil, fmt.Errorf("received unexpected error when getting status for %q: %w", libDir, err)
+	}
 
-	modDir := filepath.Join("/lib/modules", kernVer)
+	modDir := filepath.Join(libDir, kernVer)
 	if exists, err := misc.Exists(modDir); !exists {
 		// dir /lib/modules/<kernel> if kernel built without module support, so just print a message
 		log.Printf("-- kernel module directory not found: %q, not including modules", modDir)
@@ -77,7 +83,7 @@ func slurpModules(fd io.Reader, modDir string) (*filelist.FileList, error) {
 	files := filelist.NewFileList()
 	s := bufio.NewScanner(fd)
 	for s.Scan() {
-		line := s.Text()
+		line := strings.TrimSpace(s.Text())
 		if len(line) == 0 || strings.HasPrefix(line, "#") {
 			continue
 		}
@@ -97,8 +103,8 @@ func slurpModules(fd io.Reader, modDir string) (*filelist.FileList, error) {
 			}
 		} else if dir == "" {
 			// item is a module name
-			if modFilelist, err := getModule(s.Text(), modDir); err != nil {
-				return nil, fmt.Errorf("unable to get module file %q: %w", s.Text(), err)
+			if modFilelist, err := getModule(line, modDir); err != nil {
+				return nil, fmt.Errorf("unable to get module file %q: %w", line, err)
 			} else {
 				for _, file := range modFilelist {
 					files.Add(file, file)
@@ -182,7 +188,7 @@ func getModuleDeps(modName string, modulesDep io.Reader) ([]string, error) {
 
 	s := bufio.NewScanner(modulesDep)
 	for s.Scan() {
-		line := s.Text()
+		line := strings.TrimSpace(s.Text())
 		if len(line) == 0 || strings.HasPrefix(line, "#") {
 			continue
 		}

internal/filelist/modules/modules_test.go

@@ -18,6 +18,7 @@ func TestStripExts(t *testing.T) {
 		{"another_file", "another_file"},
 		{"a.b.c.d.e.f.g.h.i", "a"},
 		{"virtio_blk.ko", "virtio_blk"},
+		{"virtio_blk.ko ", "virtio_blk"},
 	}
 	for _, table := range tables {
 		out := stripExts(table.in)

internal/misc/getfiles.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
-	"log"
 
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/osutil"
 )
@@ -24,51 +22,39 @@ func GetFiles(list []string, required bool) (files []string, err error) {
 	return
 }
 
-// This function doesn't handle globs, use getFile() instead.
-func getFileNormalized(file string, required bool) (files []string, err error) {
-	fileInfo, err := os.Stat(file)
-
-	// Trying some fallbacks...
+func getFile(file string, required bool) (files []string, err error) {
+	// Expand glob expression
+	expanded, err := filepath.Glob(file)
 	if err != nil {
-		type triedResult struct {
-			file string
-			err error
-		}
-
-		triedFiles := make([]triedResult, 0, 1)
-
-		// Temporary fallback until alpine/pmOS usr-merge happened
-		// If a path starts with /bin or /sbin, also try /usr equivalent before giving up
-		if strings.HasPrefix(file, "/bin/") || strings.HasPrefix(file, "/sbin/") {
-			fileUsr := filepath.Join("/usr", file)
-			_, err := os.Stat(fileUsr);
-			if err == nil {
-				log.Printf("getFile: failed to find %q, but found it in %q. Please adjust the path.", file, fileUsr)
-				return getFileNormalized(fileUsr, required)
+		return
+	}
+	if len(expanded) > 0 && expanded[0] != file {
+		for _, path := range expanded {
+			if globFiles, err := getFile(path, required); err != nil {
+				return files, err
 			} else {
-				triedFiles = append(triedFiles, triedResult{fileUsr, err})
+				files = append(files, globFiles...)
 			}
 		}
+		return RemoveDuplicates(files), nil
+	}
 
-		{
-			// Check if there is a Zstd-compressed version of the file
-			fileZstd := file + ".zst" // .zst is the extension used by linux-firmware
-			_, err := os.Stat(fileZstd);
-			if err == nil {
-				return getFileNormalized(fileZstd, required)
-			} else {
-				triedFiles = append(triedFiles, triedResult{fileZstd, err})
-			}
-		}
-		
-		// Failed to find anything
-		if required {
-			failStrings := make([]string, 0, 2)
-			for _, result := range triedFiles {
-				failStrings = append(failStrings, fmt.Sprintf("\n - also tried %q: %v", result.file, result.err))
-			}
-			return files, fmt.Errorf("getFile: failed to stat file %q: %v%q", file, err, strings.Join(failStrings, ""))
+	fileInfo, err := os.Stat(file)
+	if err != nil {
+		// Check if there is a Zstd-compressed version of the file
+		fileZstd := file + ".zst" // .zst is the extension used by linux-firmware
+		fileInfoZstd, errZstd := os.Stat(fileZstd)
+
+		if errZstd == nil {
+			file = fileZstd
+			fileInfo = fileInfoZstd
+			// Unset nil so we don't retain the error from the os.Stat call for the uncompressed version.
+			err = nil
 		} else {
+			if required {
+				return files, fmt.Errorf("getFile: failed to stat file %q: %w (also tried %q: %w)", file, err, fileZstd, errZstd)
+			}
+
 			return files, nil
 		}
 	}
@@ -109,26 +95,6 @@ func getFileNormalized(file string, required bool) (files []string, err error) {
 	return
 }
 
-func getFile(file string, required bool) (files []string, err error) {
-	// Expand glob expression
-	expanded, err := filepath.Glob(file)
-	if err != nil {
-		return
-	}
-	if len(expanded) > 0 && expanded[0] != file {
-		for _, path := range expanded {
-			if globFiles, err := getFile(path, required); err != nil {
-				return files, err
-			} else {
-				files = append(files, globFiles...)
-			}
-		}
-		return RemoveDuplicates(files), nil
-	}
-
-	return getFileNormalized(file, required)
-}
-
 func getDeps(file string, parents map[string]struct{}) (files []string, err error) {
 
 	if _, found := parents[file]; found {
@@ -154,6 +120,7 @@ func getDeps(file string, parents map[string]struct{}) (files []string, err erro
 		"/usr/lib",
 		"/lib",
 		"/usr/lib/expect*",
+		"/usr/lib/systemd",
 	}
 
 	for _, lib := range libs {

internal/osutil/osutil.go

@@ -10,6 +10,44 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+// Try to guess whether the system has merged dirs under /usr
+func HasMergedUsr() bool {
+	for _, dir := range []string{"/bin", "/lib"} {
+		stat, err := os.Lstat(dir)
+		if err != nil {
+			// TODO: probably because the dir doesn't exist... so
+			// should we assume that it's because the system has some weird
+			// implementation of "merge /usr"?
+			return true
+		} else if stat.Mode()&os.ModeSymlink == 0 {
+			// Not a symlink, so must not be merged /usr
+			return false
+		}
+	}
+	return true
+}
+
+// Converts given path to one supported by a merged /usr config.
+// E.g., /bin/foo becomes /usr/bin/foo, /lib/bar becomes /usr/lib/bar
+// See: https://www.freedesktop.org/wiki/Software/systemd/TheCaseForTheUsrMerge
+func MergeUsr(file string) string {
+
+	// Prepend /usr to supported paths
+	for _, prefix := range []string{"/bin", "/sbin", "/lib", "/lib64"} {
+		if strings.HasPrefix(file, prefix) {
+			file = filepath.Join("/usr", file)
+			break
+		}
+	}
+
+	// Convert /usr/sbin --> /usr/bin
+	if part, found := strings.CutPrefix(file, "/usr/sbin"); found {
+		file = filepath.Join("/usr/bin/", part)
+	}
+
+	return file
+}
+
 // Converts a relative symlink target path (e.g. ../../lib/foo.so), that is
 // absolute path
 func RelativeSymlinkTargetToDir(symPath string, dir string) (string, error) {

internal/osutil/osutil_test.go

@@ -0,0 +1,49 @@
+// Copyright 2024 Clayton Craft <clayton@craftyguy.net>
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package osutil
+
+import (
+	"testing"
+)
+
+func TestMergeUsr(t *testing.T) {
+	subtests := []struct {
+		in       string
+		expected string
+	}{
+		{
+			in:       "/bin/foo",
+			expected: "/usr/bin/foo",
+		},
+		{
+			in:       "/sbin/foo",
+			expected: "/usr/bin/foo",
+		},
+		{
+			in:       "/usr/sbin/foo",
+			expected: "/usr/bin/foo",
+		},
+		{
+			in:       "/usr/bin/foo",
+			expected: "/usr/bin/foo",
+		},
+		{
+			in:       "/lib/foo.so",
+			expected: "/usr/lib/foo.so",
+		},
+		{
+			in:       "/lib64/foo.so",
+			expected: "/usr/lib64/foo.so",
+		},
+	}
+
+	for _, st := range subtests {
+		t.Run(st.in, func(t *testing.T) {
+			out := MergeUsr(st.in)
+			if out != st.expected {
+				t.Fatalf("expected: %q, got: %q\n", st.expected, out)
+			}
+		})
+	}
+}

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
@@ -116,3 +102,23 @@ func nameToField(name string) string {
 
 	return field
 }
+
+func (d DeviceInfo) String() string {
+	return fmt.Sprintf(`{
+			%s: %v
+			%s: %v
+			%s: %v
+			%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,
+		"deviceinfo_generateSystemdBoot", d.GenerateSystemdBoot,
+		"deviceinfo_formatVersion", d.FormatVersion,
+	)
+}

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=""