From 3e55f38c91e6bb1106c11609f3b483b21cf90202 Mon Sep 17 00:00:00 2001 From: Tigah <88289044+TBX3D@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:54:05 -0700 Subject: [PATCH] fix(modules): search system data dirs for built-in modules (#338) packaged installs put the binary in /usr/bin and the modules under /usr/share/sif/modules, so the loader found zero built-in modules with no error. add the freedesktop data dirs ($XDG_DATA_DIRS, default /usr/local/share and /usr/share) as fallback search paths after the existing executable-relative and working-dir locations, first-existing wins. the new IsDir check also makes resolution stricter: a plain file named modules no longer counts as the directory. --- docs/installation.md | 4 ++- internal/modules/loader.go | 59 +++++++++++++++++++++++++++------ internal/modules/loader_test.go | 57 +++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index 06d8965..3fe99d9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -87,7 +87,9 @@ make sif looks for modules in these locations: -- **built-in**: `modules/` directory next to the sif binary +- **built-in**: `modules/` directory next to the sif binary, then `modules/` in the + working directory, then the system data dirs (`$XDG_DATA_DIRS`, default + `/usr/local/share/sif/modules` and `/usr/share/sif/modules`) for packaged installs - **user modules**: `~/.config/sif/modules/` (linux/macos) or `%LOCALAPPDATA%\sif\modules\` (windows) user modules override built-in modules with the same id. diff --git a/internal/modules/loader.go b/internal/modules/loader.go index 24e5352..36ae0a7 100644 --- a/internal/modules/loader.go +++ b/internal/modules/loader.go @@ -50,17 +50,7 @@ func NewLoader() (*Loader, error) { return nil, fmt.Errorf("get home dir: %w", err) } - // Find built-in modules relative to executable - execPath, err := os.Executable() - if err != nil { - execPath = "." - } - builtinDir := filepath.Join(filepath.Dir(execPath), "modules") - - // Also check current working directory for development - if _, err := os.Stat(builtinDir); os.IsNotExist(err) { - builtinDir = "modules" - } + builtinDir := resolveBuiltinDir() // User modules directory based on OS var userDir string @@ -78,6 +68,53 @@ func NewLoader() (*Loader, error) { }, nil } +// resolveBuiltinDir picks the built-in modules directory: the first existing +// candidate, or the working-directory default when none are present (LoadAll +// then logs "no built-in modules found" as before). +func resolveBuiltinDir() string { + if dir := firstExistingDir(builtinDirCandidates()); dir != "" { + return dir + } + return "modules" +} + +// builtinDirCandidates lists the directories to probe for built-in modules, +// most specific first: next to the executable, the working directory (for +// development), then the freedesktop system data dirs so packaged installs +// (modules under /usr/share/sif) are found too. +func builtinDirCandidates() []string { + candidates := make([]string, 0, 4) + + if execPath, err := os.Executable(); err == nil { + candidates = append(candidates, filepath.Join(filepath.Dir(execPath), "modules")) + } + candidates = append(candidates, "modules") + + for _, dir := range dataDirs() { + candidates = append(candidates, filepath.Join(dir, "sif", "modules")) + } + + return candidates +} + +// dataDirs returns the freedesktop base data directories, honoring +// $XDG_DATA_DIRS and falling back to the spec default when it is unset. +func dataDirs() []string { + if env := os.Getenv("XDG_DATA_DIRS"); env != "" { + return filepath.SplitList(env) + } + return []string{"/usr/local/share", "/usr/share"} +} + +func firstExistingDir(candidates []string) string { + for _, dir := range candidates { + if info, err := os.Stat(dir); err == nil && info.IsDir() { + return dir + } + } + return "" +} + // LoadAll discovers and loads all modules from both built-in // and user directories. func (l *Loader) LoadAll() error { diff --git a/internal/modules/loader_test.go b/internal/modules/loader_test.go index 1356a36..b1d08df 100644 --- a/internal/modules/loader_test.go +++ b/internal/modules/loader_test.go @@ -267,3 +267,60 @@ func TestLoaderScriptStubNoop(t *testing.T) { t.Errorf("loadScript stub returned error: %v", err) } } + +func TestDataDirs(t *testing.T) { + t.Setenv("XDG_DATA_DIRS", "/opt/a"+string(os.PathListSeparator)+"/opt/b") + if got := dataDirs(); len(got) != 2 || got[0] != "/opt/a" || got[1] != "/opt/b" { + t.Errorf("dataDirs() with XDG set = %v, want [/opt/a /opt/b]", got) + } + + t.Setenv("XDG_DATA_DIRS", "") + if got := dataDirs(); len(got) != 2 || got[0] != "/usr/local/share" || got[1] != "/usr/share" { + t.Errorf("dataDirs() default = %v, want [/usr/local/share /usr/share]", got) + } +} + +func TestFirstExistingDir(t *testing.T) { + tmp := t.TempDir() + realDir := filepath.Join(tmp, "real") + if err := os.Mkdir(realDir, 0o755); err != nil { + t.Fatal(err) + } + file := writeModule(t, tmp, "afile", "x") + + if got := firstExistingDir([]string{filepath.Join(tmp, "missing"), file, realDir}); got != realDir { + t.Errorf("firstExistingDir = %q, want %q (a file must be skipped)", got, realDir) + } + if got := firstExistingDir([]string{filepath.Join(tmp, "nope")}); got != "" { + t.Errorf("firstExistingDir with no match = %q, want empty", got) + } +} + +// TestBuiltinDirCandidatesIncludesDataDirs is the FHS regression: packaged +// installs keep modules under /sif/modules, so that path must be a +// candidate, ordered after the executable-relative and working-dir paths. +func TestBuiltinDirCandidatesIncludesDataDirs(t *testing.T) { + t.Setenv("XDG_DATA_DIRS", "/usr/share") + candidates := builtinDirCandidates() + + want := "/usr/share/sif/modules" + if candidates[len(candidates)-1] != want { + t.Errorf("candidates %v missing trailing data-dir path %q", candidates, want) + } +} + +// TestResolveBuiltinDirFindsPackagedModules is the resolveBuiltinDir-level +// counterpart to TestBuiltinDirCandidatesIncludesDataDirs above. +func TestResolveBuiltinDirFindsPackagedModules(t *testing.T) { + tmp := t.TempDir() + pkg := filepath.Join(tmp, "sif", "modules") + if err := os.MkdirAll(pkg, 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("XDG_DATA_DIRS", tmp) + t.Chdir(t.TempDir()) // scratch cwd so the "modules" candidate does not exist + + if got := resolveBuiltinDir(); got != pkg { + t.Errorf("resolveBuiltinDir = %q, want packaged %q", got, pkg) + } +}