deviceinfo: add tests for ReadDeviceinfo (MR 37)

Relates https://gitlab.com/postmarketOS/pmaports/-/issues/1836

The error checking for the detection of both files is not ideal, but there
are no obvious better solutions. For now, we want to avoid requiring one
by default, since it allows this change to be forward compatible so not
all MRs related to /usr/share have to go in at once. I followed the same
pattern as in boot-deploy!29 although there we check for "deviceinfo_arch"
to make sure that at least one of those files is complete.

The other alternative would be to add an Arch field to DeviceInfo struct,
and use it to check that at least one of the files was complete, and
ignore any errors if it exists. Since this is not ideal either, keep the
double test, and let's take care of fixes once one of them is compulsory.

cmd/mkinitfs/main.go

@@ -49,18 +49,13 @@ func main() {
 
 	log.Default().SetFlags(log.Lmicroseconds)
 
-	deviceinfoFile := "/etc/deviceinfo"
+	var devinfo deviceinfo.DeviceInfo
-	if exists, err := misc.Exists(deviceinfoFile); !exists {
+	deverr_usr := devinfo.ReadDeviceinfo("/usr/share/deviceinfo/deviceinfo")
-		log.Printf("NOTE: %q not found, this file is required by mkinitfs.\n", deviceinfoFile)
+	deverr_etc := devinfo.ReadDeviceinfo("/etc/deviceinfo")
-		return
+	if deverr_etc != nil && deverr_usr != nil {
-	} else if err != nil {
+		log.Println("Error reading deviceinfo")
-		retCode = 1
+		log.Println("\t/usr/share/deviceinfo/deviceinfo:", deverr_usr)
-		log.Printf("received unexpected error when getting status for %q: %s", deviceinfoFile, err)
+		log.Println("\t/etc/deviceinfo:", deverr_etc)
-	}
-
-	devinfo, err := deviceinfo.ReadDeviceinfo(deviceinfoFile)
-	if err != nil {
-		log.Println(err)
 		retCode = 1
 		return
 	}
@@ -93,9 +88,17 @@ func main() {
 	log.Print("Generating for kernel version: ", kernVer)
 	log.Print("Output directory: ", *outDir)
 
+	//
+	// initramfs
+	//
 	// deviceinfo.InitfsCompression needs a little more post-processing
 	compressionFormat, compressionLevel := archive.ExtractFormatLevel(devinfo.InitfsCompression)
-	if err := generateArchive("initramfs", compressionFormat, compressionLevel, workDir, []filelist.FileLister{
+	log.Printf("== Generating %s ==\n", "initramfs")
+	log.Printf("- Using compression format %s with level %q\n", compressionFormat, compressionLevel)
+
+	start := time.Now()
+	initramfsAr := archive.New(compressionFormat, compressionLevel)
+	initfs := initramfs.New([]filelist.FileLister{
 		hookdirs.New("/usr/share/mkinitfs/dirs"),
 		hookdirs.New("/etc/mkinitfs/dirs"),
 		hookfiles.New("/usr/share/mkinitfs/files"),
@@ -104,16 +107,27 @@ func main() {
 		hookscripts.New("/etc/mkinitfs/hooks", "/hooks"),
 		modules.New(strings.Fields(devinfo.ModulesInitfs), "/usr/share/mkinitfs/modules"),
 		modules.New([]string{}, "/etc/mkinitfs/modules"),
-	}); err != nil {
+	})
+	initramfsAr.AddItems(initfs)
+	if err := initramfsAr.Write(filepath.Join(workDir, "initramfs"), os.FileMode(0644)); err != nil {
 		log.Println(err)
 		log.Println("failed to generate: ", "initramfs")
 		retCode = 1
 		return
 	}
+	misc.TimeFunc(start, "initramfs")
 
+	//
+	// initramfs-extra
+	//
 	// deviceinfo.InitfsExtraCompression needs a little more post-processing
 	compressionFormat, compressionLevel = archive.ExtractFormatLevel(devinfo.InitfsExtraCompression)
-	if err := generateArchive("initramfs-extra", compressionFormat, compressionLevel, workDir, []filelist.FileLister{
+	log.Printf("== Generating %s ==\n", "initramfs-extra")
+	log.Printf("- Using compression format %s with level %q\n", compressionFormat, compressionLevel)
+
+	start = time.Now()
+	initramfsExtraAr := archive.New(compressionFormat, compressionLevel)
+	initfsExtra := initramfs.New([]filelist.FileLister{
 		hookfiles.New("/usr/share/mkinitfs/files-extra"),
 		hookfiles.New("/etc/mkinitfs/files-extra"),
 		hookscripts.New("/usr/share/mkinitfs/hooks-extra", "/hooks-extra"),
@@ -121,12 +135,15 @@ func main() {
 		modules.New([]string{}, "/usr/share/mkinitfs/modules-extra"),
 		modules.New([]string{}, "/etc/mkinitfs/modules-extra"),
 		osksdl.New(devinfo.MesaDriver),
-	}); err != nil {
+	})
+	initramfsExtraAr.AddItemsExclude(initfsExtra, initfs)
+	if err := initramfsExtraAr.Write(filepath.Join(workDir, "initramfs-extra"), os.FileMode(0644)); err != nil {
 		log.Println(err)
 		log.Println("failed to generate: ", "initramfs-extra")
 		retCode = 1
 		return
 	}
+	misc.TimeFunc(start, "initramfs-extra")
 
 	// Final processing of initramfs / kernel is done by boot-deploy
 	if !disableBootDeploy {
@@ -146,23 +163,3 @@ func bootDeploy(workDir, outDir, ubootBoardname string) error {
 	bd := bootdeploy.New(workDir, outDir, ubootBoardname)
 	return bd.Run()
 }
-
-func generateArchive(name string, format archive.CompressFormat, level archive.CompressLevel, path string, features []filelist.FileLister) error {
-	log.Printf("== Generating %s ==\n", name)
-	log.Printf("- Using compression format %s with level %q\n", format, level)
-
-	defer misc.TimeFunc(time.Now(), name)
-	a := archive.New(format, level)
-
-	fs := initramfs.New(features)
-	if err := a.AddItems(fs); err != nil {
-		return err
-	}
-
-	log.Println("- Writing and verifying archive: ", name)
-	if err := a.Write(filepath.Join(path, name), os.FileMode(0644)); err != nil {
-		return err
-	}
-
-	return nil
-}

doc/mkinitfs.1.scd

@@ -38,7 +38,8 @@ Design goals of this project are:
 The canonical deviceinfo "specification" is at
 https://wiki.postmarketos.org/wiki/Deviceinfo_reference
 
-mkinitfs reads deviceinfo values from */etc/deviceinfo*. The following variables
+mkinitfs reads deviceinfo values from */usr/share/deviceinfo/deviceinfo* and
+*/etc/deviceinfo*, in that order. The following variables
 are *required* by mkinitfs:
 
 	- deviceinfo_initfs_compression
@@ -63,7 +64,7 @@ it are for constructing the initramfs archive.
 
 Configuration under */usr/share/mkinitfs* is intended to be managed by
 distributions, while configuration under */etc/mkinitfs* is for users to
-create/manage. mkinitfs reads configuration from */usr/share/mkinitfs* first, and then from */etc/mkinitfs*. 
+create/manage. mkinitfs reads configuration from */usr/share/mkinitfs* first, and then from */etc/mkinitfs*.
 
 ## /usr/share/mkinitfs/files, /etc/mkinitfs/files
 ## /usr/share/mkinitfs/files-extra, /etc/mkinitfs/files-extra
@@ -136,7 +137,7 @@ search for the app. The following commandline options are passed to it:
 		Currently this is hardcoded to be "initramfs"
 
 	*-k* <kernel filename>
-	
+
 	*-d* <work directory>
 
 		Path to the directory containing the build artifacts from mkinitfs.

go.mod

@@ -5,6 +5,7 @@ go 1.20
 require (
 	github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e
 	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
 )

go.sum

@@ -2,6 +2,8 @@ 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/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/sys v0.0.0-20210630005230-0f9fa26af87c h1:F1jZWGFhYfh0Ci55sIpILtKKK8p3i2/krTr0H1rg74I=

internal/archive/archive.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/cavaliercoder/go-cpio"
 	"github.com/klauspost/compress/zstd"
+	"github.com/pierrec/lz4/v4"
 	"github.com/ulikunitz/xz"
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/filelist"
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/osutil"
@@ -28,6 +29,7 @@ type CompressFormat string
 const (
 	FormatGzip CompressFormat = "gzip"
 	FormatLzma CompressFormat = "lzma"
+	FormatLz4  CompressFormat = "lz4"
 	FormatZstd CompressFormat = "zstd"
 	FormatNone CompressFormat = "none"
 )
@@ -104,6 +106,7 @@ func ExtractFormatLevel(s string) (format CompressFormat, level CompressLevel) {
 	case FormatLzma:
 		log.Println("Format lzma doesn't support a compression level, using default settings")
 		level = LevelDefault
+	case FormatLz4:
 	case FormatNone:
 	case FormatZstd:
 	default:
@@ -200,6 +203,38 @@ func (archive *Archive) AddItems(flister filelist.FileLister) error {
 	return nil
 }
 
+// AddItemsExclude is like AddItems, but takes a second FileLister that lists
+// items that should not be added to the archive from the first FileLister
+func (archive *Archive) AddItemsExclude(flister filelist.FileLister, exclude filelist.FileLister) error {
+	list, err := flister.List()
+	if err != nil {
+		return err
+	}
+
+	excludeList, err := exclude.List()
+	if err != nil {
+		return err
+	}
+
+	for i := range list.IterItems() {
+		dest, found := excludeList.Get(i.Source)
+
+		if found {
+			if i.Dest != dest {
+				found = false
+			}
+		}
+
+		if !found {
+			if err := archive.AddItem(i.Source, i.Dest); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 // Adds the given file or directory at "source" to the archive at "dest"
 func (archive *Archive) AddItem(source string, dest string) error {
 
@@ -316,6 +351,23 @@ func (archive *Archive) writeCompressed(path string, mode os.FileMode) (err erro
 		if err != nil {
 			return err
 		}
+	case FormatLz4:
+		// The default compression for the lz4 library is Fast, and
+		// they don't define a Default level otherwise
+		level := lz4.Fast
+		switch archive.compress_level {
+		case LevelBest:
+			level = lz4.Level9
+		case LevelFast:
+			level = lz4.Fast
+		}
+
+		var writer = lz4.NewWriter(fd)
+		err = writer.Apply(lz4.LegacyOption(true), lz4.CompressionLevelOption(level))
+		if err != nil {
+			return err
+		}
+		compressor = writer
 	case FormatNone:
 		compressor = fd
 	case FormatZstd:

internal/archive/archive_test.go

@@ -249,6 +249,12 @@ func TestExtractFormatLevel(t *testing.T) {
 			expectedFormat: FormatLzma,
 			expectedLevel:  LevelDefault,
 		},
+		{
+			name:           "lz4, fast",
+			in:             "lz4:fast",
+			expectedFormat: FormatLz4,
+			expectedLevel:  LevelFast,
+		},
 		{
 			name:           "none",
 			in:             "none",

internal/filelist/filelist.go

@@ -1,12 +1,6 @@
 package filelist
 
-import (
+import "sync"
-	"fmt"
-	"path/filepath"
-	"sync"
-
-	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/misc"
-)
 
 type FileLister interface {
 	List() (*FileList, error)
@@ -51,26 +45,6 @@ func (f *FileList) Import(src *FileList) {
 	}
 }
 
-func (f *FileList) AddGlobbed(src string, dest string) error {
-	fFiles, err := misc.GetFiles([]string{src}, true)
-	if err != nil {
-		return fmt.Errorf("unable to add %q: %w", src, err)
-	}
-	// loop over all returned files from GetFile
-	for _, file := range fFiles {
-		if len(fFiles) > 1 {
-			// Glob with arbitrary subdirectories, so we need to
-			// remove the src path and prepend the dest path
-			f.Add(file, filepath.Join(dest, file[len(src):]))
-		} else {
-			// dest path specified, and only 1 file
-			f.Add(file, dest)
-		}
-	}
-
-	return nil
-}
-
 // iterate through the list and and send each one as a new File over the
 // returned channel
 func (f *FileList) IterItems() <-chan File {

internal/filelist/hookfiles/hookfiles.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/filelist"
+	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/misc"
 )
 
 type HookFiles struct {
@@ -59,13 +60,22 @@ func slurpFiles(fd io.Reader) (*filelist.FileList, error) {
 	for s.Scan() {
 		src, dest, has_dest := strings.Cut(s.Text(), ":")
 
-		if !has_dest {
+		fFiles, err := misc.GetFiles([]string{src}, true)
-			dest = src
-		}
-
-		err := files.AddGlobbed(src, dest)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("unable to add %q: %w", src, err)
+		}
+		// loop over all returned files from GetFile
+		for _, file := range fFiles {
+			if !has_dest {
+				files.Add(file, file)
+			} else if len(fFiles) > 1 {
+				// Don't support specifying dest if src was a glob
+				// NOTE: this could support this later...
+				files.Add(file, file)
+			} else {
+				// dest path specified, and only 1 file
+				files.Add(file, dest)
+			}
 		}
 	}
 

internal/filelist/hookscripts/hookscripts.go

@@ -35,13 +35,8 @@ func (h *HookScripts) List() (*filelist.FileList, error) {
 	}
 	for _, file := range fileInfo {
 		path := filepath.Join(h.scriptsDir, file.Name())
-		if file.IsDir() {
+		log.Printf("-- Including script: %s\n", path)
-			log.Printf("-- Including dir %s\n", path)
+		files.Add(path, filepath.Join(h.destPath, file.Name()))
-			files.AddGlobbed(filepath.Join(path, "*"), filepath.Join(h.destPath, file.Name()))
-		} else {
-			log.Printf("-- Including script: %s\n", path)
-			files.Add(path, filepath.Join(h.destPath, file.Name()))
-		}
 	}
 	return files, nil
 }

internal/filelist/initramfs/initramfs.go

@@ -9,6 +9,7 @@ import (
 // combining the output from them.
 type Initramfs struct {
 	features []filelist.FileLister
+	files    *filelist.FileList
 }
 
 // New returns a new Initramfs that generate a list of files based on the given
@@ -20,15 +21,18 @@ func New(features []filelist.FileLister) *Initramfs {
 }
 
 func (i *Initramfs) List() (*filelist.FileList, error) {
-	files := filelist.NewFileList()
+	if i.files != nil {
+		return i.files, nil
+	}
+	i.files = filelist.NewFileList()
 
 	for _, f := range i.features {
 		list, err := f.List()
 		if err != nil {
 			return nil, err
 		}
-		files.Import(list)
+		i.files.Import(list)
 	}
 
-	return files, nil
+	return i.files, nil
 }

pkgs/deviceinfo/deviceinfo.go

@@ -11,6 +11,8 @@ import (
 	"os"
 	"reflect"
 	"strings"
+
+	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/misc"
 )
 
 type DeviceInfo struct {
@@ -21,24 +23,31 @@ type DeviceInfo struct {
 	UbootBoardname         string
 }
 
-func ReadDeviceinfo(file string) (DeviceInfo, error) {
+// Reads the relevant entries from "file" into DeviceInfo struct
-	var deviceinfo DeviceInfo
+// Any already-set entries will be overwriten if they are present
+// in "file"
+func (d *DeviceInfo) ReadDeviceinfo(file string) error {
+	if exists, err := misc.Exists(file); !exists {
+		return fmt.Errorf("%q not found, required by mkinitfs", file)
+	} else if err != nil {
+		return fmt.Errorf("unexpected error getting status for %q: %s", file, err)
+	}
 
 	fd, err := os.Open(file)
 	if err != nil {
-		return deviceinfo, err
+		return err
 	}
 	defer fd.Close()
 
-	if err := unmarshal(fd, &deviceinfo); err != nil {
+	if err := d.unmarshal(fd); err != nil {
-		return deviceinfo, err
+		return err
 	}
 
-	return deviceinfo, nil
+	return nil
 }
 
 // Unmarshals a deviceinfo into a DeviceInfo struct
-func unmarshal(r io.Reader, devinfo *DeviceInfo) error {
+func (d *DeviceInfo) unmarshal(r io.Reader) error {
 	s := bufio.NewScanner(r)
 	for s.Scan() {
 		line := s.Text()
@@ -74,7 +83,7 @@ func unmarshal(r io.Reader, devinfo *DeviceInfo) error {
 			return fmt.Errorf("error parsing deviceinfo line, invalid format: %s", line)
 		}
 
-		field := reflect.ValueOf(devinfo).Elem().FieldByName(fieldName)
+		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

pkgs/deviceinfo/deviceinfo_test.go

@@ -10,6 +10,32 @@ import (
 	"testing"
 )
 
+// Test ReadDeviceinfo and the logic of reading from multiple files
+func TestReadDeviceinfo(t *testing.T) {
+	modules_expected := "panfrost foo bar bazz"
+	mesa_expected := "msm"
+
+	var devinfo DeviceInfo
+	err := devinfo.ReadDeviceinfo("./test_resources/deviceinfo-missing")
+	if !strings.Contains(err.Error(), "required by mkinitfs") {
+		t.Errorf("received an unexpected err: %s", err)
+	}
+	err = devinfo.ReadDeviceinfo("./test_resources/deviceinfo-first")
+	if err != nil {
+		t.Errorf("received an unexpected err: %s", err)
+	}
+	err = devinfo.ReadDeviceinfo("./test_resources/deviceinfo-msm")
+	if err != nil {
+		t.Errorf("received an unexpected err: %s", err)
+	}
+	if devinfo.ModulesInitfs != modules_expected {
+		t.Errorf("expected %q, got: %q", modules_expected, devinfo.ModulesInitfs)
+	}
+	if devinfo.MesaDriver != mesa_expected {
+		t.Errorf("expected %q, got: %q", mesa_expected, devinfo.MesaDriver)
+	}
+}
+
 // Test conversion of name to DeviceInfo struct field format
 func TestNameToField(t *testing.T) {
 	tables := []struct {
@@ -58,7 +84,7 @@ func TestUnmarshal(t *testing.T) {
 	var d DeviceInfo
 	for _, table := range tables {
 		testName := fmt.Sprintf("unmarshal::'%s':", strings.ReplaceAll(table.in, "\n", "\\n"))
-		if err := unmarshal(strings.NewReader(table.in), &d); err != nil {
+		if err := d.unmarshal(strings.NewReader(table.in)); err != nil {
 			t.Errorf("%s received an unexpected err: ", err)
 		}
 

pkgs/deviceinfo/test_resources/deviceinfo-first

@@ -0,0 +1,2 @@
+deviceinfo_modules_initfs="panfrost foo bar bazz"
+deviceinfo_mesa_driver="panfrost"

pkgs/deviceinfo/test_resources/deviceinfo-msm

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