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"
-	if exists, err := misc.Exists(deviceinfoFile); !exists {
-		log.Printf("NOTE: %q not found, this file is required by mkinitfs.\n", deviceinfoFile)
-		return
-	} else if err != nil {
-		retCode = 1
-		log.Printf("received unexpected error when getting status for %q: %s", deviceinfoFile, err)
-	}
-
-	devinfo, err := deviceinfo.ReadDeviceinfo(deviceinfoFile)
-	if err != nil {
-		log.Println(err)
+	var devinfo deviceinfo.DeviceInfo
+	deverr_usr := devinfo.ReadDeviceinfo("/usr/share/deviceinfo/deviceinfo")
+	deverr_etc := devinfo.ReadDeviceinfo("/etc/deviceinfo")
+	if deverr_etc != nil && deverr_usr != nil {
+		log.Println("Error reading deviceinfo")
+		log.Println("\t/usr/share/deviceinfo/deviceinfo:", deverr_usr)
+		log.Println("\t/etc/deviceinfo:", deverr_etc)
 		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,26 +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, err := archive.New(format, level)
-	if err != nil {
-		return err
-	}
-
-	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

@@ -24,12 +24,22 @@ is purely to generate the archive(s). mkinitfs does call *boot-deploy* after
 creating the archive(s), in order to install/deploy them and any other relevant
 boot-related items onto the system.
 
+Design goals of this project are:
+
+	- Support as many distros as possible
+	- Simplify configuration, while still giving multiple opportunities to set or override defaults
+	- Execute an external app to do any boot install/setup finalization
+		- One such app is here: https://gitlab.com/postmarketOS/boot-deploy
+		- But implementation can be anything, see the section on *BOOT-DEPLOY*
+		for more info
+
 # DEVICEINFO
 
 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
@@ -117,6 +127,30 @@ create/manage. mkinitfs reads configuration from */usr/share/mkinitfs* first, an
 	directories to create within the initramfs. There is no *-extra* variant,
 	since directories are of negligible size.
 
+# BOOT-DEPLOY
+
+After generating archives, mkinitfs will execute *boot-deploy*, using *$PATH* to
+search for the app. The following commandline options are passed to it:
+
+	*-i* <initramfs filename>
+
+		Currently this is hardcoded to be "initramfs"
+
+	*-k* <kernel filename>
+
+	*-d* <work directory>
+
+		Path to the directory containing the build artifacts from mkinitfs.
+
+	*-o* <destination directory>
+
+		Path to the directory that boot-deploy should use as its root when
+		installing files.
+
+	*initramfs-extra*
+
+		This string is the filename of the initramfs-extra archive.
+
 # AUTHORS
 
 *Clayton Craft* <clayton@craftyguy.net>

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"
 )
@@ -51,7 +53,7 @@ type Archive struct {
 	items           archiveItems
 }
 
-func New(format CompressFormat, level CompressLevel) (*Archive, error) {
+func New(format CompressFormat, level CompressLevel) *Archive {
 	buf := new(bytes.Buffer)
 	archive := &Archive{
 		cpioWriter:      cpio.NewWriter(buf),
@@ -60,7 +62,7 @@ func New(format CompressFormat, level CompressLevel) (*Archive, error) {
 		compress_level:  level,
 	}
 
-	return archive, nil
+	return archive
 }
 
 type archiveItem struct {
@@ -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:
@@ -184,10 +187,11 @@ func (archive *Archive) Write(path string, mode os.FileMode) error {
 	return nil
 }
 
-// Adds the given items in the map to the archive. The map format is {source path:dest path}.
-// Internally this just calls AddItem on each key,value pair in the map.
-func (archive *Archive) AddItems(f filelist.FileLister) error {
-	list, err := f.List()
+// AddItems adds the given items in the map to the archive. The map format is
+// {source path:dest path}. Internally this just calls AddItem on each
+// key,value pair in the map.
+func (archive *Archive) AddItems(flister filelist.FileLister) error {
+	list, err := flister.List()
 	if err != nil {
 		return err
 	}
@@ -199,6 +203,38 @@ func (archive *Archive) AddItems(f 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 {
 
@@ -315,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/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
 }

internal/filelist/modules/modules.go

@@ -70,7 +70,6 @@ func (m *Modules) List() (*filelist.FileList, error) {
 	log.Printf("- Searching for kernel modules from %s", m.modulesListPath)
 	fileInfo, err := os.ReadDir(m.modulesListPath)
 	if err != nil {
-		log.Println("-- Unable to find dir, skipping...")
 		return files, nil
 	}
 	for _, file := range fileInfo {

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) {
-	var deviceinfo DeviceInfo
+// Reads the relevant entries from "file" into DeviceInfo struct
+// 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 {
-		return deviceinfo, err
+	if err := d.unmarshal(fd); err != nil {
+		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"