misc/getfiles: add tests for getFile

This adds some tests for getFile, one of which would have caught
the recent recursion issue and other will hopefully catch future
regressions.

Part-of: https://gitlab.postmarketos.org/postmarketOS/postmarketos-mkinitfs/-/merge_requests/65

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

.gitlab-ci.yml

@@ -6,43 +6,123 @@ image: alpine:edge
 variables:
   GOFLAGS: "-buildvcs=false"
   PACKAGE_REGISTRY_URL: "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/packages/generic/mkinitfs-vendor-${CI_COMMIT_TAG}/${CI_COMMIT_TAG}"
+  CI_TRON_TEMPLATE_PROJECT: &ci-tron-template-project postmarketOS/ci-common
+  CI_TRON_JOB_TEMPLATE_PROJECT_URL: $CI_SERVER_URL/$CI_TRON_TEMPLATE_PROJECT
+  CI_TRON_JOB_TEMPLATE_COMMIT: &ci-tron-template-commit 7c95b5f2d53533e8722abf57c73e558168e811f3
+
+include:
+  - project: *ci-tron-template-project
+    ref: *ci-tron-template-commit
+    file: '/ci-tron/common.yml'
 
 stages:
-  - lint
   - build
+  - hardware tests
   - vendor
   - release
 
-# defaults for "only"
+workflow:
-# We need to run the CI jobs in a "merge request specific context", if CI is
+  rules:
-# running in a merge request. Otherwise the environment variable that holds the
+    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
-# merge request ID is not available. This means, we must set the "only"
+    - if: $CI_COMMIT_BRANCH == 'master'
-# variable accordingly - and if we only do it for one job, all other jobs will
+    - if: '$CI_COMMIT_TAG != null'
-# not get executed. So have the defaults here, and use them in all jobs that
-# should run on both the master branch, and in merge requests.
-# https://docs.gitlab.com/ee/ci/merge_request_pipelines/index.html#excluding-certain-jobs
-.only-default: &only-default
-  only:
-    - master
-    - merge_requests
-    - tags
 
 build:
   stage: build
-  <<: *only-default
+  variables:
+    GOTEST: "gotestsum --junitfile report.xml --format testname -- ./..."
+  parallel:
+    matrix:
+      - TAG: shared
+      - TAG: arm64
+  tags:
+    - $TAG
   before_script:
-    - apk -q add go staticcheck make scdoc
+    - apk -q add go gotestsum staticcheck make scdoc
   script:
     - make test
     - make
+  after_script:
+    - mkdir -p rootfs/usr/sbin
+    - cp mkinitfs rootfs/usr/sbin
   artifacts:
     expire_in: 1 week
+    reports:
+      junit: report.xml
+    paths:
+      - rootfs
+
+.qemu-common:
+  variables:
+    DEVICE_NAME: qemu-$CPU_ARCH
+    KERNEL_VARIANT: lts
+
+.build-ci-tron-qemu:
+  stage: hardware tests
+  extends:
+    - .pmos-ci-tron-build-boot-artifacts
+    - .qemu-common
+  variables:
+    INSTALL_PACKAGES: device-${DEVICE_NAME} device-${DEVICE_NAME}-kernel-${KERNEL_VARIANT} postmarketos-mkinitfs-hook-ci
+
+build-ci-tron-qemu-amd64:
+  extends:
+    - .build-ci-tron-qemu
+  needs:
+    - job: "build"
+      parallel:
+        matrix:
+          - TAG: shared
+  variables:
+    CPU_ARCH: amd64
+
+build-ci-tron-qemu-aarch64:
+  extends:
+    - .build-ci-tron-qemu
+  needs:
+    - job: "build"
+      parallel:
+        matrix:
+          - TAG: arm64
+  variables:
+    CPU_ARCH: aarch64
+
+.test-ci-tron-qemu:
+  stage: hardware tests
+  extends:
+    - .pmos-ci-tron-initramfs-test
+    - .qemu-common
+  dependencies: []
+  variables:
+    CI_TRON_KERNEL__URL: "glartifact://build-ci-tron-qemu-$CPU_ARCH/${CI_TRON__PMB_EXPORT_PATH}/vmlinuz-${KERNEL_VARIANT}"
+    CI_TRON_INITRAMFS__INITRAMFS__URL: "glartifact://build-ci-tron-qemu-$CPU_ARCH/${CI_TRON__PMB_EXPORT_PATH}/initramfs"
+    CI_TRON_KERNEL_CMDLINE__DEVICEINFO: 'console=tty1 console=ttyS0,115200 PMOS_FORCE_PARTITION_RESIZE'
+
+test-ci-tron-qemu-amd64:
+  extends:
+    - .test-ci-tron-qemu
+    - .pmos-ci-tron-runner-qemu-amd64
+  needs:
+    - job: 'build-ci-tron-qemu-amd64'
+      artifacts: false
+  variables:
+    CPU_ARCH: amd64
+
+test-ci-tron-qemu-aarch64:
+  extends:
+    - .test-ci-tron-qemu
+    - .pmos-ci-tron-runner-qemu-aarch64
+  needs:
+    - job: 'build-ci-tron-qemu-aarch64'
+      artifacts: false
+  variables:
+    CPU_ARCH: aarch64
 
 vendor:
   stage: vendor
   image: alpine:latest
-  only:
+  rules:
-    - tags
+    - if: '$CI_COMMIT_TAG != null'
   before_script:
     - apk -q add curl go make
   script:
@@ -54,8 +134,8 @@ vendor:
 release:
   stage: release
   image: registry.gitlab.com/gitlab-org/release-cli:latest
-  only:
+  rules:
-    - tags
+    - if: '$CI_COMMIT_TAG != null'
   script:
     - |
       release-cli create --name "Release $CI_COMMIT_TAG" --tag-name $CI_COMMIT_TAG \

Makefile

@@ -12,7 +12,13 @@ GO?=go
 GOFLAGS?=
 LDFLAGS+=-s -w -X main.Version=$(VERSION)
 RM?=rm -f
-GOTEST=go test -count=1 -race
+GOTESTOPTS?=-count=1 -race
+GOTEST?=go test ./...
+DISABLE_GOGC?=
+
+ifeq ($(DISABLE_GOGC),1)
+	LDFLAGS+=-X main.DisableGC=true
+endif
 
 GOSRC!=find * -name '*.go'
 GOSRC+=go.mod go.sum
@@ -42,7 +48,7 @@ test:
 	fi
 	@staticcheck ./...
 
-	@$(GOTEST) ./...
+	$(GOTEST) $(GOTESTOPTS)
 
 clean:
 	$(RM) mkinitfs $(DOCS)

cmd/mkinitfs/main.go

@@ -9,6 +9,8 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"runtime/debug"
+	"strings"
 	"time"
 
 	"gitlab.com/postmarketOS/postmarketos-mkinitfs/internal/archive"
@@ -26,8 +28,14 @@ import (
 
 // set at build time
 var Version string
+var DisableGC string
 
 func main() {
+	// To allow working around silly GC-related issues, like https://gitlab.com/qemu-project/qemu/-/issues/2560
+	if strings.ToLower(DisableGC) == "true" {
+		debug.SetGCPercent(-1)
+	}
+
 	retCode := 0
 	defer func() { os.Exit(retCode) }()
 
@@ -106,30 +114,6 @@ func main() {
 		modules.New("/usr/share/mkinitfs/modules"),
 		modules.New("/etc/mkinitfs/modules"),
 	})
-	if err := initramfsAr.AddItems(initfs); err != nil {
-		log.Println(err)
-		log.Println("failed to generate: ", "initramfs")
-		retCode = 1
-		return
-	}
-	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)
-	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"),
@@ -138,19 +122,58 @@ func main() {
 		modules.New("/usr/share/mkinitfs/modules-extra"),
 		modules.New("/etc/mkinitfs/modules-extra"),
 	})
-	if err := initramfsExtraAr.AddItemsExclude(initfsExtra, initfs); err != nil {
+
+	if err := initramfsAr.AddItems(initfs); err != nil {
 		log.Println(err)
-		log.Println("failed to generate: ", "initramfs-extra")
+		log.Println("failed to generate: ", "initramfs")
 		retCode = 1
 		return
 	}
-	if err := initramfsExtraAr.Write(filepath.Join(workDir, "initramfs-extra"), os.FileMode(0644)); err != nil {
+
+	// Include initramfs-extra files in the initramfs if not making a separate
+	// archive
+	if !devinfo.CreateInitfsExtra {
+		if err := initramfsAr.AddItems(initfsExtra); err != nil {
+			log.Println(err)
+			log.Println("failed to generate: ", "initramfs")
+			retCode = 1
+			return
+		}
+	}
+
+	if err := initramfsAr.Write(filepath.Join(workDir, "initramfs"), os.FileMode(0644)); err != nil {
 		log.Println(err)
-		log.Println("failed to generate: ", "initramfs-extra")
+		log.Println("failed to generate: ", "initramfs")
 		retCode = 1
 		return
 	}
-	misc.TimeFunc(start, "initramfs-extra")
+	misc.TimeFunc(start, "initramfs")
+
+	if devinfo.CreateInitfsExtra {
+		//
+		// initramfs-extra
+		//
+		// deviceinfo.InitfsExtraCompression needs a little more post-processing
+		compressionFormat, compressionLevel = archive.ExtractFormatLevel(devinfo.InitfsExtraCompression)
+		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)
+		if err := initramfsExtraAr.AddItemsExclude(initfsExtra, initfs); err != nil {
+			log.Println(err)
+			log.Println("failed to generate: ", "initramfs-extra")
+			retCode = 1
+			return
+		}
+		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 {

doc/mkinitfs.1.scd

@@ -42,6 +42,7 @@ mkinitfs reads deviceinfo values from */usr/share/deviceinfo/deviceinfo* and
 */etc/deviceinfo*, in that order. The following variables
 are *required* by mkinitfs:
 
+	- deviceinfo_create_initfs_extra
 	- deviceinfo_generate_systemd_boot
 	- deviceinfo_initfs_compression
 	- deviceinfo_initfs_extra_compression
@@ -146,7 +147,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.
 

internal/bootdeploy/bootdeploy.go

@@ -78,12 +78,17 @@ func (b *BootDeploy) Run() error {
 	}
 
 	// boot-deploy -i initramfs -k vmlinuz-postmarketos-rockchip -d /tmp/cpio -o /tmp/foo initramfs-extra
-	cmd := exec.Command("boot-deploy",
+	args := []string{
 		"-i", "initramfs",
 		"-k", kernFilename,
 		"-d", b.inDir,
 		"-o", b.outDir,
-		"initramfs-extra")
+	}
+
+	if b.devinfo.CreateInitfsExtra {
+		args = append(args, "initramfs-extra")
+	}
+	cmd := exec.Command("boot-deploy", args...)
 
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr

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

@@ -59,7 +59,7 @@ 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
 		}

internal/filelist/modules/modules.go

@@ -83,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
 		}
@@ -103,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 {
+			if modFilelist, err := getModule(line, modDir); err != nil {
-				return nil, fmt.Errorf("unable to get module file %q: %w", s.Text(), err)
+				return nil, fmt.Errorf("unable to get module file %q: %w", line, err)
 			} else {
 				for _, file := range modFilelist {
 					files.Add(file, file)
@@ -188,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

@@ -3,6 +3,7 @@ package misc
 import (
 	"debug/elf"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 
@@ -39,6 +40,24 @@ func getFile(file string, required bool) (files []string, err error) {
 		return RemoveDuplicates(files), nil
 	}
 
+	// If the file is a symlink we need to do this to prevent an infinite recursion
+	// loop:
+	// Symlinks need special handling to prevent infinite recursion:
+	// 1) add the symlink to the list of files
+	// 2) set file to dereferenced target
+	// 4) continue this function to either walk it if the target is a dir or add the
+	// target to the list of files
+	if s, err := os.Lstat(file); err != nil {
+		return files, err
+	} else if s.Mode()&fs.ModeSymlink != 0 {
+		files = append(files, file)
+		if target, err := filepath.EvalSymlinks(file); err != nil {
+			return files, err
+		} else {
+			file = target
+		}
+	}
+
 	fileInfo, err := os.Stat(file)
 	if err != nil {
 		// Check if there is a Zstd-compressed version of the file

internal/misc/getfiles_test.go

@@ -0,0 +1,149 @@
+// Copyright 2025 Clayton Craft <clayton@craftyguy.net>
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package misc
+
+import (
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"testing"
+	"time"
+)
+
+func TestGetFile(t *testing.T) {
+	subtests := []struct {
+		name     string
+		setup    func(tmpDir string) (inputPath string, expectedFiles []string, err error)
+		required bool
+	}{
+		{
+			name: "symlink to directory - no infinite recursion",
+			setup: func(tmpDir string) (string, []string, error) {
+				// Create target directory with files
+				targetDir := filepath.Join(tmpDir, "target")
+				if err := os.MkdirAll(targetDir, 0755); err != nil {
+					return "", nil, err
+				}
+
+				testFile1 := filepath.Join(targetDir, "file1.txt")
+				testFile2 := filepath.Join(targetDir, "file2.txt")
+				if err := os.WriteFile(testFile1, []byte("content1"), 0644); err != nil {
+					return "", nil, err
+				}
+				if err := os.WriteFile(testFile2, []byte("content2"), 0644); err != nil {
+					return "", nil, err
+				}
+
+				// Create symlink pointing to target directory
+				symlinkPath := filepath.Join(tmpDir, "symlink")
+				if err := os.Symlink(targetDir, symlinkPath); err != nil {
+					return "", nil, err
+				}
+
+				expected := []string{symlinkPath, testFile1, testFile2}
+				return symlinkPath, expected, nil
+			},
+			required: true,
+		},
+		{
+			name: "symlink to file - returns both symlink and target",
+			setup: func(tmpDir string) (string, []string, error) {
+				// Create target file
+				targetFile := filepath.Join(tmpDir, "target.txt")
+				if err := os.WriteFile(targetFile, []byte("content"), 0644); err != nil {
+					return "", nil, err
+				}
+
+				// Create symlink pointing to target file
+				symlinkPath := filepath.Join(tmpDir, "symlink.txt")
+				if err := os.Symlink(targetFile, symlinkPath); err != nil {
+					return "", nil, err
+				}
+
+				expected := []string{symlinkPath, targetFile}
+				return symlinkPath, expected, nil
+			},
+			required: true,
+		},
+		{
+			name: "regular file",
+			setup: func(tmpDir string) (string, []string, error) {
+				regularFile := filepath.Join(tmpDir, "regular.txt")
+				if err := os.WriteFile(regularFile, []byte("content"), 0644); err != nil {
+					return "", nil, err
+				}
+
+				expected := []string{regularFile}
+				return regularFile, expected, nil
+			},
+			required: true,
+		},
+		{
+			name: "regular directory",
+			setup: func(tmpDir string) (string, []string, error) {
+				// Create directory with files
+				dirPath := filepath.Join(tmpDir, "testdir")
+				if err := os.MkdirAll(dirPath, 0755); err != nil {
+					return "", nil, err
+				}
+
+				file1 := filepath.Join(dirPath, "file1.txt")
+				file2 := filepath.Join(dirPath, "subdir", "file2.txt")
+
+				if err := os.WriteFile(file1, []byte("content1"), 0644); err != nil {
+					return "", nil, err
+				}
+				if err := os.MkdirAll(filepath.Dir(file2), 0755); err != nil {
+					return "", nil, err
+				}
+				if err := os.WriteFile(file2, []byte("content2"), 0644); err != nil {
+					return "", nil, err
+				}
+
+				expected := []string{file1, file2}
+				return dirPath, expected, nil
+			},
+			required: true,
+		},
+	}
+
+	for _, st := range subtests {
+		t.Run(st.name, func(t *testing.T) {
+			tmpDir := t.TempDir()
+
+			inputPath, expectedFiles, err := st.setup(tmpDir)
+			if err != nil {
+				t.Fatalf("setup failed: %v", err)
+			}
+
+			// Add timeout protection for infinite recursion test
+			done := make(chan struct{})
+			var files []string
+			var getFileErr error
+
+			go func() {
+				defer close(done)
+				files, getFileErr = getFile(inputPath, st.required)
+			}()
+
+			select {
+			case <-done:
+				if getFileErr != nil {
+					t.Fatalf("getFile failed: %v", getFileErr)
+				}
+			case <-time.After(5 * time.Second):
+				t.Fatal("getFile appears to be in infinite recursion (timeout)")
+			}
+
+			// Sort for comparison
+			sort.Strings(expectedFiles)
+			sort.Strings(files)
+
+			if !reflect.DeepEqual(expectedFiles, files) {
+				t.Fatalf("expected: %q, got: %q", expectedFiles, files)
+			}
+		})
+	}
+}

internal/osutil/osutil.go

@@ -40,11 +40,6 @@ func MergeUsr(file string) string {
 		}
 	}
 
-	// Convert /usr/sbin --> /usr/bin
-	if part, found := strings.CutPrefix(file, "/usr/sbin"); found {
-		file = filepath.Join("/usr/bin/", part)
-	}
-
 	return file
 }
 

internal/osutil/osutil_test.go

@@ -18,11 +18,11 @@ func TestMergeUsr(t *testing.T) {
 		},
 		{
 			in:       "/sbin/foo",
-			expected: "/usr/bin/foo",
+			expected: "/usr/sbin/foo",
 		},
 		{
 			in:       "/usr/sbin/foo",
-			expected: "/usr/bin/foo",
+			expected: "/usr/sbin/foo",
 		},
 		{
 			in:       "/usr/bin/foo",

pkgs/deviceinfo/deviceinfo.go

@@ -21,6 +21,7 @@ type DeviceInfo struct {
 	UbootBoardname         string
 	GenerateSystemdBoot    string
 	FormatVersion          string
+	CreateInitfsExtra      bool
 }
 
 // Reads the relevant entries from "file" into DeviceInfo struct
@@ -112,6 +113,7 @@ func (d DeviceInfo) String() string {
 			%s: %v
 			%s: %v
 			%s: %v
+			%s: %v
 	}`,
 		"deviceinfo_format_version", d.FormatVersion,
 		"deviceinfo_", d.FormatVersion,
@@ -120,5 +122,6 @@ func (d DeviceInfo) String() string {
 		"deviceinfo_ubootBoardname", d.UbootBoardname,
 		"deviceinfo_generateSystemdBoot", d.GenerateSystemdBoot,
 		"deviceinfo_formatVersion", d.FormatVersion,
+		"deviceinfo_createInitfsExtra", d.CreateInitfsExtra,
 	)
 }

pkgs/deviceinfo/deviceinfo_test.go

@@ -42,6 +42,7 @@ func TestNameToField(t *testing.T) {
 		{"modules_initfs", "ModulesInitfs"},
 		{"deviceinfo_initfs_compression___", "InitfsCompression"},
 		{"deviceinfo_initfs_extra_compression", "InitfsExtraCompression"},
+		{"deviceinfo_create_initfs_extra", "CreateInitfsExtra"},
 	}
 
 	for _, table := range tables {
@@ -65,6 +66,7 @@ func TestUnmarshal(t *testing.T) {
 			UbootBoardname:         "foobar-bazz",
 			InitfsCompression:      "zstd:--foo=1 -T0 --bar=bazz",
 			InitfsExtraCompression: "",
+			CreateInitfsExtra:      true,
 		},
 		},
 	}

pkgs/deviceinfo/test_resources/deviceinfo-unmarshal-1

@@ -4,3 +4,4 @@ deviceinfo_uboot_boardname="foobar-bazz"
 deviceinfo_initfs_compression="zstd:--foo=1 -T0 --bar=bazz"
 # empty option
 deviceinfo_initfs_extra_compression=""
+deviceinfo_create_initfs_extra="true" # in-line comment that should be ignored