From d28e7cbea3e1bc3fbba890506c20560998bd5ca0 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:19:36 +0000 Subject: [PATCH 1/7] More u-root build tests Signed-off-by: Chris Koch --- uroot/initramfs/files.go | 30 +++- uroot/initramfs/test/ramfs.go | 15 ++ uroot/uroot.go | 32 ++-- uroot/uroot_test.go | 265 +++++++++++++++++++++++++++++++--- 4 files changed, 303 insertions(+), 39 deletions(-) diff --git a/uroot/initramfs/files.go b/uroot/initramfs/files.go index da8de05..7aecfe2 100644 --- a/uroot/initramfs/files.go +++ b/uroot/initramfs/files.go @@ -73,7 +73,7 @@ func (af *Files) addFile(src string, dest string, follow bool) error { // a record or file, we want to include its children anyway. sInfo, err := os.Lstat(src) if err != nil { - return fmt.Errorf("adding %q to archive failed because Lstat failed: %v", src, err) + return fmt.Errorf("adding %q to archive failed because Lstat failed: %w", src, err) } // Recursively add children. @@ -92,7 +92,11 @@ func (af *Files) addFile(src string, dest string, follow bool) error { } if record, ok := af.Records[dest]; ok { - return fmt.Errorf("record for %q already exists in archive: %v", dest, record) + return &os.PathError{ + Op: "add to archive", + Path: dest, + Err: fmt.Errorf("%w: is %v", os.ErrExist, record), + } } if srcFile, ok := af.Files[dest]; ok { @@ -100,7 +104,11 @@ func (af *Files) addFile(src string, dest string, follow bool) error { if src == srcFile { return nil } - return fmt.Errorf("record for %q already exists in archive (is %q)", dest, src) + return &os.PathError{ + Op: "add to archive", + Path: dest, + Err: fmt.Errorf("%w: backed by %q", os.ErrExist, src), + } } af.Files[dest] = src @@ -137,13 +145,21 @@ func (af *Files) AddRecord(r cpio.Record) error { } if src, ok := af.Files[r.Name]; ok { - return fmt.Errorf("record for %q already exists in archive: file %q", r.Name, src) + return &os.PathError{ + Op: "add to archive", + Path: r.Name, + Err: fmt.Errorf("%w: backed by %q", os.ErrExist, src), + } } - if rr, ok := af.Records[r.Name]; ok { - if rr.Info == r.Info { + if record, ok := af.Records[r.Name]; ok { + if record.Info == r.Info { return nil } - return fmt.Errorf("record for %q already exists in archive: %v", r.Name, rr) + return &os.PathError{ + Op: "add to archive", + Path: r.Name, + Err: fmt.Errorf("%w: is %v", os.ErrExist, record), + } } af.Records[r.Name] = r diff --git a/uroot/initramfs/test/ramfs.go b/uroot/initramfs/test/ramfs.go index a998df5..0869716 100644 --- a/uroot/initramfs/test/ramfs.go +++ b/uroot/initramfs/test/ramfs.go @@ -43,6 +43,21 @@ func (hf HasFile) Validate(a *cpio.Archive) error { return fmt.Errorf("archive does not contain %s, but should", hf.Path) } +type HasDir struct { + Path string +} + +func (h HasDir) Validate(a *cpio.Archive) error { + r, ok := a.Get(h.Path) + if !ok { + return fmt.Errorf("archive does not contain %s, but should", h.Path) + } + if r.Mode&cpio.S_IFDIR == 0 { + return fmt.Errorf("file %v should be directory, but isn't", h.Path) + } + return nil +} + type HasContent struct { Path string Content string diff --git a/uroot/uroot.go b/uroot/uroot.go index 6f0768c..e772243 100644 --- a/uroot/uroot.go +++ b/uroot/uroot.go @@ -10,6 +10,7 @@ package uroot import ( "debug/elf" + "errors" "fmt" "os" "path" @@ -233,7 +234,7 @@ type Opts struct { // CreateInitramfs creates an initramfs built to opts' specifications. func CreateInitramfs(logger ulog.Logger, opts Opts) error { if _, err := os.Stat(opts.TempDir); os.IsNotExist(err) { - return fmt.Errorf("temp dir %q must exist: %v", opts.TempDir, err) + return fmt.Errorf("temp dir %q must exist: %w", opts.TempDir, err) } if opts.OutputFile == nil { return fmt.Errorf("must give output file") @@ -254,7 +255,7 @@ func CreateInitramfs(logger ulog.Logger, opts Opts) error { for index, cmds := range opts.Commands { paths, err := findpkg.ResolveGlobs(logger, env, lookupEnv, cmds.Packages) if err != nil { - return err + return fmt.Errorf("%w: %w", errResolvePackage, err) } opts.Commands[index].Packages = paths } @@ -294,21 +295,21 @@ func CreateInitramfs(logger ulog.Logger, opts Opts) error { return err } if err := opts.addSymlinkTo(logger, archive, opts.UinitCmd, "bin/uinit"); err != nil { - return fmt.Errorf("%v: specify -uinitcmd=\"\" to ignore this error and build without a uinit", err) + return fmt.Errorf("%w: %w", err, errUinitSymlink) } if len(opts.UinitArgs) > 0 { if err := archive.AddRecord(cpio.StaticFile("etc/uinit.flags", fileflag.ArgvToFile(opts.UinitArgs), 0o444)); err != nil { - return fmt.Errorf("%v: could not add uinit arguments from UinitArgs (-uinitcmd) to initramfs", err) + return fmt.Errorf("%w: %w", err, errUinitArgs) } } if err := opts.addSymlinkTo(logger, archive, opts.InitCmd, "init"); err != nil { - return fmt.Errorf("%v: specify -initcmd=\"\" to ignore this error and build without an init (or, did you specify a list, and are you missing github.com/u-root/u-root/cmds/core/init?)", err) + return fmt.Errorf("%w: %w", err, errInitSymlink) } if err := opts.addSymlinkTo(logger, archive, opts.DefaultShell, "bin/sh"); err != nil { - return fmt.Errorf("%v: specify -defaultsh=\"\" to ignore this error and build without a shell", err) + return fmt.Errorf("%w: %w", err, errDefaultshSymlink) } if err := opts.addSymlinkTo(logger, archive, opts.DefaultShell, "bin/defaultsh"); err != nil { - return fmt.Errorf("%v: specify -defaultsh=\"\" to ignore this error and build without a shell", err) + return fmt.Errorf("%w: %w", err, errDefaultshSymlink) } // Finally, write the archive. @@ -318,6 +319,15 @@ func CreateInitramfs(logger ulog.Logger, opts Opts) error { return nil } +var ( + errResolvePackage = errors.New("failed to resolve package paths") + errInitSymlink = errors.New("specify -initcmd=\"\" to ignore this error and build without an init (or, did you specify a list, and are you missing github.com/u-root/u-root/cmds/core/init?)") + errUinitSymlink = errors.New("specify -uinitcmd=\"\" to ignore this error and build without a uinit") + errDefaultshSymlink = errors.New("specify -defaultsh=\"\" to ignore this error and build without a shell") + errSymlink = errors.New("could not create symlink") + errUinitArgs = errors.New("could not add uinit arguments") +) + func (o *Opts) addSymlinkTo(logger ulog.Logger, archive *initramfs.Opts, command string, source string) error { if len(command) == 0 { return nil @@ -326,7 +336,7 @@ func (o *Opts) addSymlinkTo(logger ulog.Logger, archive *initramfs.Opts, command target, err := resolveCommandOrPath(command, o.Commands) if err != nil { if o.Commands != nil { - return fmt.Errorf("could not create symlink from %q to %q: %v", source, command, err) + return fmt.Errorf("%w from %q to %q: %w", errSymlink, source, command, err) } logger.Printf("Could not create symlink from %q to %q: %v", source, command, err) return nil @@ -343,7 +353,7 @@ func (o *Opts) addSymlinkTo(logger ulog.Logger, archive *initramfs.Opts, command } if err := archive.AddRecord(cpio.Symlink(source, relTarget)); err != nil { - return fmt.Errorf("failed to add symlink %s -> %s to initramfs: %v", source, relTarget, err) + return fmt.Errorf("failed to add symlink %s -> %s to initramfs: %w", source, relTarget, err) } return nil } @@ -405,10 +415,10 @@ func ParseExtraFiles(logger ulog.Logger, archive *initramfs.Files, extraFiles [] } src, err := filepath.Abs(src) if err != nil { - return fmt.Errorf("couldn't find absolute path for %q: %v", src, err) + return fmt.Errorf("couldn't find absolute path for %q: %w", src, err) } if err := archive.AddFile(src, dst); err != nil { - return fmt.Errorf("couldn't add %q to archive: %v", file, err) + return fmt.Errorf("couldn't add %q to archive: %w", file, err) } if lddDeps { diff --git a/uroot/uroot_test.go b/uroot/uroot_test.go index e099763..f5a568b 100644 --- a/uroot/uroot_test.go +++ b/uroot/uroot_test.go @@ -5,6 +5,7 @@ package uroot import ( + "errors" "fmt" "os" "path/filepath" @@ -35,28 +36,33 @@ func TestCreateInitramfs(t *testing.T) { } tmp777 := filepath.Join(dir, "tmp777") - if err := os.MkdirAll(tmp777, 0o777); err != nil { - t.Error(err) - } + _ = os.MkdirAll(tmp777, 0o777) + tmp400 := filepath.Join(dir, "tmp400") + _ = os.MkdirAll(tmp400, 0o400) + + somefile := filepath.Join(dir, "somefile") + somefile2 := filepath.Join(dir, "somefile2") + _ = os.WriteFile(somefile, []byte("foobar"), 0o777) + _ = os.WriteFile(somefile2, []byte("spongebob"), 0o777) + + cwd, _ := os.Getwd() l := ulogtest.Logger{TB: t} for i, tt := range []struct { name string opts Opts - want string + errs []error validators []itest.ArchiveValidator }{ { - name: "BB archive with ls and init", + name: "BB archive", opts: Opts{ - Env: golang.Default(golang.DisableCGO()), - TempDir: dir, - ExtraFiles: nil, - UseExistingInit: false, - InitCmd: "init", - DefaultShell: "ls", - UrootSource: urootpath, + Env: golang.Default(golang.DisableCGO()), + TempDir: dir, + InitCmd: "init", + DefaultShell: "ls", + UrootSource: urootpath, Commands: []Commands{ { Builder: builder.BusyBox, @@ -67,7 +73,6 @@ func TestCreateInitramfs(t *testing.T) { }, }, }, - want: "", validators: []itest.ArchiveValidator{ itest.HasFile{Path: "bbin/bb"}, itest.HasRecord{R: cpio.Symlink("bbin/init", "bb")}, @@ -82,7 +87,7 @@ func TestCreateInitramfs(t *testing.T) { InitCmd: "init", DefaultShell: "", }, - want: "temp dir \"\" must exist: stat : no such file or directory", + errs: []error{os.ErrNotExist}, validators: []itest.ArchiveValidator{ itest.IsEmpty{}, }, @@ -92,11 +97,187 @@ func TestCreateInitramfs(t *testing.T) { opts: Opts{ TempDir: dir, }, - want: "", validators: []itest.ArchiveValidator{ itest.MissingFile{Path: "bbin/bb"}, }, }, + { + name: "files", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + somefile + ":etc/somefile", + somefile2 + ":etc/somefile2", + somefile, + // Empty is ignored. + "", + "uroot_test.go", + filepath.Join(cwd, "uroot_test.go"), + // Parent directory is created. + somefile + ":somedir/somefile", + }, + }, + validators: []itest.ArchiveValidator{ + itest.MissingFile{Path: "bbin/bb"}, + itest.HasContent{Path: "etc/somefile", Content: "foobar"}, + itest.HasContent{Path: somefile, Content: "foobar"}, + itest.HasContent{Path: "etc/somefile2", Content: "spongebob"}, + // TODO: This behavior is weird. + itest.HasFile{Path: "uroot_test.go"}, + itest.HasFile{Path: filepath.Join(cwd, "uroot_test.go")}, + itest.HasDir{Path: "somedir"}, + itest.HasContent{Path: "somedir/somefile", Content: "foobar"}, + }, + }, + { + name: "files conflict", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + somefile + ":etc/somefile", + somefile2 + ":etc/somefile", + }, + }, + errs: []error{os.ErrExist}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file does not exist", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + filepath.Join(dir, "doesnotexist") + ":etc/somefile", + }, + }, + errs: []error{os.ErrNotExist}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + /* TODO: case is broken. + { + name: "files invalid syntax 1", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + ":etc/somefile", + }, + }, + //errs: []error{os.ErrExist}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + */ + /* TODO: case is broken. + { + name: "files invalid syntax 2", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + somefile + ":", + }, + }, + //errs: []error{os.ErrExist}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + */ + // TODO: files are directories. + { + name: "file conflicts with init", + opts: Opts{ + TempDir: dir, + InitCmd: "/bin/systemd", + ExtraFiles: []string{ + somefile + ":init", + }, + }, + errs: []error{os.ErrExist, errInitSymlink}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file conflicts with uinit flags", + opts: Opts{ + TempDir: dir, + UinitArgs: []string{"-foo", "-bar"}, + ExtraFiles: []string{ + somefile + ":etc/uinit.flags", + }, + }, + errs: []error{os.ErrExist, errUinitArgs}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file conflicts with uinit", + opts: Opts{ + TempDir: dir, + UinitCmd: "/bin/systemd", + ExtraFiles: []string{ + somefile + ":bin/uinit", + }, + }, + errs: []error{os.ErrExist, errUinitSymlink}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file conflicts with sh", + opts: Opts{ + TempDir: dir, + DefaultShell: "/bin/systemd", + ExtraFiles: []string{ + somefile + ":bin/sh", + }, + }, + errs: []error{os.ErrExist, errDefaultshSymlink}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file conflicts with defaultsh", + opts: Opts{ + TempDir: dir, + DefaultShell: "/bin/systemd", + ExtraFiles: []string{ + somefile + ":bin/defaultsh", + }, + }, + errs: []error{os.ErrExist, errDefaultshSymlink}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "file does not conflict if default files not specified", + opts: Opts{ + TempDir: dir, + // No DefaultShell, Init, or UinitCmd. + ExtraFiles: []string{ + somefile + ":bin/defaultsh", + somefile + ":bin/sh", + somefile + ":bin/uinit", + somefile + ":etc/uinit.flags", + somefile + ":init", + }, + }, + validators: []itest.ArchiveValidator{ + itest.HasContent{Path: "bin/defaultsh", Content: "foobar"}, + itest.HasContent{Path: "bin/sh", Content: "foobar"}, + itest.HasContent{Path: "bin/uinit", Content: "foobar"}, + itest.HasContent{Path: "etc/uinit.flags", Content: "foobar"}, + itest.HasContent{Path: "init", Content: "foobar"}, + }, + }, { name: "init specified, but not in commands", opts: Opts{ @@ -114,18 +295,30 @@ func TestCreateInitramfs(t *testing.T) { }, }, }, - want: "could not create symlink from \"init\" to \"foobar\": command or path \"foobar\" not included in u-root build: specify -initcmd=\"\" to ignore this error and build without an init (or, did you specify a list, and are you missing github.com/u-root/u-root/cmds/core/init?)", + errs: []error{errSymlink, errInitSymlink}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + /* TODO: case broken. + { + name: "init not resolvable", + opts: Opts{ + TempDir: dir, + InitCmd: "init", + }, + errs: []error{errSymlink, errInitSymlink}, validators: []itest.ArchiveValidator{ itest.IsEmpty{}, }, }, + */ { name: "init symlinked to absolute path", opts: Opts{ TempDir: dir, InitCmd: "/bin/systemd", }, - want: "", validators: []itest.ArchiveValidator{ itest.HasRecord{R: cpio.Symlink("init", "bin/systemd")}, }, @@ -157,7 +350,6 @@ func TestCreateInitramfs(t *testing.T) { }, }, }, - want: "", validators: []itest.ArchiveValidator{ itest.HasRecord{R: cpio.Symlink("init", "bbin/init")}, @@ -173,13 +365,44 @@ func TestCreateInitramfs(t *testing.T) { itest.HasFile{Path: "bin/dd"}, }, }, + { + name: "glob fail", + opts: Opts{ + Env: golang.Default(golang.DisableCGO()), + TempDir: dir, + UrootSource: urootpath, + Commands: BinaryCmds("github.com/u-root/u-root/cmds/notexist/*"), + }, + errs: []error{errResolvePackage}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, + { + name: "tmp not writable", + opts: Opts{ + Env: golang.Default(golang.DisableCGO()), + TempDir: tmp400, + UrootSource: urootpath, + Commands: BinaryCmds("github.com/u-root/u-root/cmds/core/..."), + }, + errs: []error{os.ErrPermission}, + validators: []itest.ArchiveValidator{ + itest.IsEmpty{}, + }, + }, } { t.Run(fmt.Sprintf("Test %d [%s]", i, tt.name), func(t *testing.T) { archive := inMemArchive{cpio.InMemArchive()} tt.opts.OutputFile = archive - // Compare error type or error string. - if err := CreateInitramfs(l, tt.opts); (err != nil && err.Error() != tt.want) || (len(tt.want) > 0 && err == nil) { - t.Errorf("CreateInitramfs(%v) = %v, want %v", tt.opts, err, tt.want) + err := CreateInitramfs(l, tt.opts) + for _, want := range tt.errs { + if !errors.Is(err, want) { + t.Errorf("CreateInitramfs = %v, want %v", err, want) + } + } + if err != nil && len(tt.errs) == 0 { + t.Errorf("CreateInitramfs = %v, want %v", err, nil) } for _, v := range tt.validators { From 81b547e501ed25d5727fb40dd2c58a015c6bbf0f Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:22:03 +0000 Subject: [PATCH 2/7] uroot: rename BusyBox Busybox Signed-off-by: Chris Koch --- uroot/builder/builder.go | 4 ++-- uroot/uroot.go | 16 ++++++++-------- uroot/uroot_test.go | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/uroot/builder/builder.go b/uroot/builder/builder.go index 0de02c7..1c6f6b8 100644 --- a/uroot/builder/builder.go +++ b/uroot/builder/builder.go @@ -13,8 +13,8 @@ import ( ) var ( - // BusyBox is a shared GBBBuilder instance. - BusyBox = GBBBuilder{} + // Busybox is a shared GBBBuilder instance. + Busybox = GBBBuilder{} // Binary is a shared BinaryBuilder instance. Binary = BinaryBuilder{} ) diff --git a/uroot/uroot.go b/uroot/uroot.go index e772243..3e7f6bc 100644 --- a/uroot/uroot.go +++ b/uroot/uroot.go @@ -136,7 +136,7 @@ type Opts struct { // // []Commands{ // Commands{ - // Builder: builder.BusyBox, + // Builder: builder.Busybox, // Packages: []string{ // "github.com/u-root/u-root/cmds/ls", // "github.com/u-root/u-root/cmds/ip", @@ -465,17 +465,17 @@ func (o *Opts) AddCommands(c ...Commands) { o.Commands = append(o.Commands, c...) } -// AddBusyBoxCommands adds Go commands to the busybox build. -func (o *Opts) AddBusyBoxCommands(pkgs ...string) { +// AddBusyboxCommands adds Go commands to the busybox build. +func (o *Opts) AddBusyboxCommands(pkgs ...string) { for i, cmds := range o.Commands { - if cmds.Builder == builder.BusyBox { + if cmds.Builder == builder.Busybox { o.Commands[i].Packages = append(o.Commands[i].Packages, pkgs...) return } } // Not found? Add first busybox. - o.AddCommands(BusyBoxCmds(pkgs...)...) + o.AddCommands(BusyboxCmds(pkgs...)...) } // BinaryCmds returns a list of Commands with cmds built as a busybox. @@ -491,14 +491,14 @@ func BinaryCmds(cmds ...string) []Commands { } } -// BusyBoxCmds returns a list of Commands with cmds built as a busybox. -func BusyBoxCmds(cmds ...string) []Commands { +// BusyboxCmds returns a list of Commands with cmds built as a busybox. +func BusyboxCmds(cmds ...string) []Commands { if len(cmds) == 0 { return nil } return []Commands{ { - Builder: builder.BusyBox, + Builder: builder.Busybox, Packages: cmds, }, } diff --git a/uroot/uroot_test.go b/uroot/uroot_test.go index f5a568b..953defd 100644 --- a/uroot/uroot_test.go +++ b/uroot/uroot_test.go @@ -65,7 +65,7 @@ func TestCreateInitramfs(t *testing.T) { UrootSource: urootpath, Commands: []Commands{ { - Builder: builder.BusyBox, + Builder: builder.Busybox, Packages: []string{ "github.com/u-root/u-root/cmds/core/init", "github.com/u-root/u-root/cmds/core/ls", @@ -335,7 +335,7 @@ func TestCreateInitramfs(t *testing.T) { UrootSource: urootpath, Commands: []Commands{ { - Builder: builder.BusyBox, + Builder: builder.Busybox, Packages: []string{ "github.com/u-root/u-root/cmds/core/init", "github.com/u-root/u-root/cmds/core/ls", From 6c4c37a33b64ac186410333d9569165c36dadc43 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:24:05 +0000 Subject: [PATCH 3/7] uroot: remove UrootSource from testing Signed-off-by: Chris Koch --- uroot/uroot_test.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/uroot/uroot_test.go b/uroot/uroot_test.go index 953defd..d5e9a90 100644 --- a/uroot/uroot_test.go +++ b/uroot/uroot_test.go @@ -30,11 +30,6 @@ func TestCreateInitramfs(t *testing.T) { dir := t.TempDir() syscall.Umask(0) - urootpath, err := filepath.Abs("../../") - if err != nil { - t.Fatalf("failure to set up test: %v", err) - } - tmp777 := filepath.Join(dir, "tmp777") _ = os.MkdirAll(tmp777, 0o777) tmp400 := filepath.Join(dir, "tmp400") @@ -62,7 +57,6 @@ func TestCreateInitramfs(t *testing.T) { TempDir: dir, InitCmd: "init", DefaultShell: "ls", - UrootSource: urootpath, Commands: []Commands{ { Builder: builder.Busybox, @@ -285,7 +279,6 @@ func TestCreateInitramfs(t *testing.T) { TempDir: dir, DefaultShell: "zoocar", InitCmd: "foobar", - UrootSource: urootpath, Commands: []Commands{ { Builder: builder.Binary, @@ -332,7 +325,6 @@ func TestCreateInitramfs(t *testing.T) { UseExistingInit: false, InitCmd: "init", DefaultShell: "ls", - UrootSource: urootpath, Commands: []Commands{ { Builder: builder.Busybox, @@ -368,10 +360,9 @@ func TestCreateInitramfs(t *testing.T) { { name: "glob fail", opts: Opts{ - Env: golang.Default(golang.DisableCGO()), - TempDir: dir, - UrootSource: urootpath, - Commands: BinaryCmds("github.com/u-root/u-root/cmds/notexist/*"), + Env: golang.Default(golang.DisableCGO()), + TempDir: dir, + Commands: BinaryCmds("github.com/u-root/u-root/cmds/notexist/*"), }, errs: []error{errResolvePackage}, validators: []itest.ArchiveValidator{ @@ -381,10 +372,9 @@ func TestCreateInitramfs(t *testing.T) { { name: "tmp not writable", opts: Opts{ - Env: golang.Default(golang.DisableCGO()), - TempDir: tmp400, - UrootSource: urootpath, - Commands: BinaryCmds("github.com/u-root/u-root/cmds/core/..."), + Env: golang.Default(golang.DisableCGO()), + TempDir: tmp400, + Commands: BinaryCmds("github.com/u-root/u-root/cmds/core/..."), }, errs: []error{os.ErrPermission}, validators: []itest.ArchiveValidator{ From ba05c90c4d2d59c7cb1f73ad446e508bc13fb5db Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:30:07 +0000 Subject: [PATCH 4/7] initramfs test: convert to errors.Is Signed-off-by: Chris Koch --- uroot/initramfs/files.go | 5 +- uroot/initramfs/files_test.go | 86 +++++++++++++++-------------------- 2 files changed, 41 insertions(+), 50 deletions(-) diff --git a/uroot/initramfs/files.go b/uroot/initramfs/files.go index 7aecfe2..dfe1ad0 100644 --- a/uroot/initramfs/files.go +++ b/uroot/initramfs/files.go @@ -5,6 +5,7 @@ package initramfs import ( + "errors" "fmt" "log" "os" @@ -137,11 +138,13 @@ func (af *Files) AddFileNoFollow(src string, dest string) error { return af.addFile(src, dest, false) } +var errAbsoluteName = errors.New("record name must not be absolute") + // AddRecord adds a cpio.Record into the archive at `r.Name`. func (af *Files) AddRecord(r cpio.Record) error { r.Name = path.Clean(r.Name) if filepath.IsAbs(r.Name) { - return fmt.Errorf("record name %q must not be absolute", r.Name) + return fmt.Errorf("%w: %q", errAbsoluteName, r.Name) } if src, ok := af.Files[r.Name]; ok { diff --git a/uroot/initramfs/files_test.go b/uroot/initramfs/files_test.go index 66c5160..29d0db6 100644 --- a/uroot/initramfs/files_test.go +++ b/uroot/initramfs/files_test.go @@ -5,12 +5,12 @@ package initramfs import ( + "errors" "fmt" "io" "os" "path/filepath" "reflect" - "strings" "testing" "github.com/u-root/mkuimage/cpio" @@ -34,12 +34,12 @@ func TestFilesAddFileNoFollow(t *testing.T) { } for i, tt := range []struct { - name string - af *Files - src string - dest string - result *Files - errContains string + name string + af *Files + src string + dest string + result *Files + err error }{ { name: "just add a file", @@ -70,11 +70,8 @@ func TestFilesAddFileNoFollow(t *testing.T) { } { t.Run(fmt.Sprintf("Test %02d: %s", i, tt.name), func(t *testing.T) { err := tt.af.AddFileNoFollow(tt.src, tt.dest) - if err != nil && !strings.Contains(err.Error(), tt.errContains) { - t.Errorf("Error is %v, does not contain %v", err, tt.errContains) - } - if err == nil && len(tt.errContains) > 0 { - t.Errorf("Got no error, want %v", tt.errContains) + if !errors.Is(err, tt.err) { + t.Errorf("AddFileNoFollow = %v, want %v", err, tt.err) } if tt.result != nil && !reflect.DeepEqual(tt.af, tt.result) { @@ -97,25 +94,22 @@ func TestFilesAddFile(t *testing.T) { symlinkToDir3 := filepath.Join(dir3, "fooSymDir/") fooDir := filepath.Join(dir3, "fooDir") - //nolint:errcheck - { - os.Create(filepath.Join(dir, "foo")) - os.Create(filepath.Join(dir, "foo2")) - os.Symlink(filepath.Join(dir, "foo2"), filepath.Join(dir2, "foo3")) + _ = os.WriteFile(filepath.Join(dir, "foo"), nil, 0o777) + _ = os.WriteFile(filepath.Join(dir, "foo2"), nil, 0o777) + _ = os.Symlink(filepath.Join(dir, "foo2"), filepath.Join(dir2, "foo3")) - os.Mkdir(fooDir, os.ModePerm) - os.Symlink(fooDir, symlinkToDir3) - os.Create(filepath.Join(fooDir, "foo")) - os.Create(filepath.Join(fooDir, "bar")) - } + _ = os.Mkdir(fooDir, os.ModePerm) + _ = os.Symlink(fooDir, symlinkToDir3) + _ = os.WriteFile(filepath.Join(fooDir, "foo"), nil, 0o777) + _ = os.WriteFile(filepath.Join(fooDir, "bar"), nil, 0o777) for i, tt := range []struct { - name string - af *Files - src string - dest string - result *Files - errContains string + name string + af *Files + src string + dest string + result *Files + err error }{ { name: "just add a file", @@ -171,7 +165,7 @@ func TestFilesAddFile(t *testing.T) { "bar/foo": "/some/other/place", }, }, - errContains: "already exists in archive", + err: os.ErrExist, }, { name: "add a file that exists in Records", @@ -187,7 +181,7 @@ func TestFilesAddFile(t *testing.T) { "bar/foo": cpio.Symlink("bar/foo", "/some/other/place"), }, }, - errContains: "already exists in archive", + err: os.ErrExist, }, { name: "add a file that already exists in Files, but is the same one", @@ -261,18 +255,15 @@ func TestFilesAddFile(t *testing.T) { "bar/foo/foo2": "/some/place/real/zed", }, }, - src: dir, - dest: "bar/foo", - errContains: "already exists in archive", + src: dir, + dest: "bar/foo", + err: os.ErrExist, }, } { t.Run(fmt.Sprintf("Test %02d: %s", i, tt.name), func(t *testing.T) { err := tt.af.AddFile(tt.src, tt.dest) - if err != nil && !strings.Contains(err.Error(), tt.errContains) { - t.Errorf("Error is %v, does not contain %v", err, tt.errContains) - } - if err == nil && len(tt.errContains) > 0 { - t.Errorf("Got no error, want %v", tt.errContains) + if !errors.Is(err, tt.err) { + t.Errorf("AddFile = %v, want %v", err, tt.err) } if tt.result != nil && !reflect.DeepEqual(tt.af, tt.result) { @@ -287,8 +278,8 @@ func TestFilesAddRecord(t *testing.T) { af *Files record cpio.Record - result *Files - errContains string + result *Files + err error }{ { af: NewFiles(), @@ -312,7 +303,7 @@ func TestFilesAddRecord(t *testing.T) { "bar/foo": "/some/other/place", }, }, - errContains: "already exists in archive", + err: os.ErrExist, }, { af: &Files{ @@ -326,7 +317,7 @@ func TestFilesAddRecord(t *testing.T) { "bar/foo": cpio.Symlink("bar/foo", "/some/other/place"), }, }, - errContains: "already exists in archive", + err: os.ErrExist, }, { af: &Files{ @@ -342,17 +333,14 @@ func TestFilesAddRecord(t *testing.T) { }, }, { - record: cpio.Symlink("/bar/foo", ""), - errContains: "must not be absolute", + record: cpio.Symlink("/bar/foo", ""), + err: errAbsoluteName, }, } { t.Run(fmt.Sprintf("Test %02d", i), func(t *testing.T) { err := tt.af.AddRecord(tt.record) - if err != nil && !strings.Contains(err.Error(), tt.errContains) { - t.Errorf("Error is %v, does not contain %v", err, tt.errContains) - } - if err == nil && len(tt.errContains) > 0 { - t.Errorf("Got no error, want %v", tt.errContains) + if !errors.Is(err, tt.err) { + t.Errorf("AddRecord = %v, want %v", err, tt.err) } if !reflect.DeepEqual(tt.af, tt.result) { From cc9180a69166e7cbb5137af3d112097f35886483 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:44:56 +0000 Subject: [PATCH 5/7] files: fix invalid syntax cases Signed-off-by: Chris Koch --- uroot/uroot.go | 7 ++++++- uroot/uroot_test.go | 8 ++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/uroot/uroot.go b/uroot/uroot.go index 3e7f6bc..03dcbf5 100644 --- a/uroot/uroot.go +++ b/uroot/uroot.go @@ -395,7 +395,12 @@ func ParseExtraFiles(logger ulog.Logger, archive *initramfs.Files, extraFiles [] var src, dst string parts := strings.SplitN(file, ":", 2) if len(parts) == 2 { - // treat the entry with the new src:dst syntax + if len(parts[0]) == 0 { + return fmt.Errorf("%w: invalid extra files %q", os.ErrInvalid, file) + } + if len(parts[1]) == 0 { + return fmt.Errorf("%w: invalid extra files %q", os.ErrInvalid, file) + } src = filepath.Clean(parts[0]) dst = filepath.Clean(parts[1]) } else { diff --git a/uroot/uroot_test.go b/uroot/uroot_test.go index d5e9a90..d74bba6 100644 --- a/uroot/uroot_test.go +++ b/uroot/uroot_test.go @@ -150,7 +150,6 @@ func TestCreateInitramfs(t *testing.T) { itest.IsEmpty{}, }, }, - /* TODO: case is broken. { name: "files invalid syntax 1", opts: Opts{ @@ -159,13 +158,11 @@ func TestCreateInitramfs(t *testing.T) { ":etc/somefile", }, }, - //errs: []error{os.ErrExist}, + errs: []error{os.ErrInvalid}, validators: []itest.ArchiveValidator{ itest.IsEmpty{}, }, }, - */ - /* TODO: case is broken. { name: "files invalid syntax 2", opts: Opts{ @@ -174,12 +171,11 @@ func TestCreateInitramfs(t *testing.T) { somefile + ":", }, }, - //errs: []error{os.ErrExist}, + errs: []error{os.ErrInvalid}, validators: []itest.ArchiveValidator{ itest.IsEmpty{}, }, }, - */ // TODO: files are directories. { name: "file conflicts with init", From 1621b62b9ac585a0c335f365d1f09b2721ec4b79 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:47:34 +0000 Subject: [PATCH 6/7] uroot: test adding directory Signed-off-by: Chris Koch --- uroot/uroot_test.go | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/uroot/uroot_test.go b/uroot/uroot_test.go index d74bba6..1b222f5 100644 --- a/uroot/uroot_test.go +++ b/uroot/uroot_test.go @@ -35,8 +35,10 @@ func TestCreateInitramfs(t *testing.T) { tmp400 := filepath.Join(dir, "tmp400") _ = os.MkdirAll(tmp400, 0o400) - somefile := filepath.Join(dir, "somefile") - somefile2 := filepath.Join(dir, "somefile2") + somedir := filepath.Join(dir, "dir") + _ = os.MkdirAll(somedir, 0o777) + somefile := filepath.Join(dir, "dir", "somefile") + somefile2 := filepath.Join(dir, "dir", "somefile2") _ = os.WriteFile(somefile, []byte("foobar"), 0o777) _ = os.WriteFile(somefile2, []byte("spongebob"), 0o777) @@ -176,7 +178,39 @@ func TestCreateInitramfs(t *testing.T) { itest.IsEmpty{}, }, }, - // TODO: files are directories. + { + name: "files are directories", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + somedir + ":etc/foo/bar", + }, + }, + validators: []itest.ArchiveValidator{ + itest.HasDir{Path: "etc"}, + itest.HasDir{Path: "etc/foo"}, + itest.HasDir{Path: "etc/foo/bar"}, + itest.HasContent{Path: "etc/foo/bar/somefile", Content: "foobar"}, + itest.HasContent{Path: "etc/foo/bar/somefile2", Content: "spongebob"}, + }, + }, + { + name: "files are directories SkipLDD", + opts: Opts{ + TempDir: dir, + ExtraFiles: []string{ + somedir + ":etc/foo/bar", + }, + SkipLDD: true, + }, + validators: []itest.ArchiveValidator{ + itest.HasDir{Path: "etc"}, + itest.HasDir{Path: "etc/foo"}, + itest.HasDir{Path: "etc/foo/bar"}, + itest.HasContent{Path: "etc/foo/bar/somefile", Content: "foobar"}, + itest.HasContent{Path: "etc/foo/bar/somefile2", Content: "spongebob"}, + }, + }, { name: "file conflicts with init", opts: Opts{ From 8906270223c51fe70df082c42234c9c5ad23fa15 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Thu, 8 Feb 2024 04:54:30 +0000 Subject: [PATCH 7/7] Remove pkg_test Signed-off-by: Chris Koch --- uroot/initramfs/test/pkg_test.go | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 uroot/initramfs/test/pkg_test.go diff --git a/uroot/initramfs/test/pkg_test.go b/uroot/initramfs/test/pkg_test.go deleted file mode 100644 index e67b495..0000000 --- a/uroot/initramfs/test/pkg_test.go +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright 2021 the u-root Authors. All rights reserved -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package test - -import "testing" - -func TestTODO(t *testing.T) { - // TODO: Write a unit test. -}