dnfjson: return err in New{,Base}Solver

The dnfjson.{,Base}Solver will not be very useful without an installed
`osbuild-depsolve-dnf` so error early when none is found.

cmd/build/main.go

@@ -177,7 +177,10 @@ func resolvePipelineCommits(commitSources map[string][]ostree.SourceSpec) (map[s
 }
 
 func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, error) {
-	solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir)
+	solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir)
+	if err != nil {
+		return nil, err
+	}
 	depsolvedSets := make(map[string][]rpmmd.PackageSpec)
 	for name, pkgSet := range packageSets {
 		res, err := solver.Depsolve(pkgSet)

cmd/gen-manifests/main.go

@@ -412,7 +412,10 @@ func mockResolveCommits(commitSources map[string][]ostree.SourceSpec) map[string
 }
 
 func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, error) {
-	solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir)
+	solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir)
+	if err != nil {
+		return nil, err
+	}
 	depsolvedSets := make(map[string][]rpmmd.PackageSpec)
 	for name, pkgSet := range packageSets {
 		res, err := solver.Depsolve(pkgSet)

cmd/osbuild-dnf-json-tests/main_test.go

@@ -31,7 +31,8 @@ func TestCrossArchDepsolve(t *testing.T) {
 
 	// Set up temporary directory for rpm/dnf cache
 	dir := t.TempDir()
-	baseSolver := dnfjson.NewBaseSolver(dir)
+	baseSolver, err := dnfjson.NewBaseSolver(dir)
+	require.Nil(t, err)
 
 	repos, err := rpmmd.LoadRepositories([]string{repoDir}, cs9.Name())
 	require.NoErrorf(t, err, "Failed to LoadRepositories %v", cs9.Name())

cmd/osbuild-playground/playground.go

@@ -18,7 +18,10 @@ import (
 
 func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos map[string][]rpmmd.RepoConfig, state_dir string) {
 
-	solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch.Name(), d.Name(), path.Join(state_dir, "rpmmd"))
+	solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch.Name(), d.Name(), path.Join(state_dir, "rpmmd"))
+	if err != nil {
+		panic(err)
+	}
 
 	// Set cache size to 1 GiB
 	solver.SetMaxCacheSize(1 * common.GiB)

pkg/dnfjson/dnfjson.go

@@ -43,32 +43,37 @@ type BaseSolver struct {
 	resultCache *dnfCache
 }
 
+var depSolveDnfPaths = []string{"/usr/libexec/", "/usr/lib/osbuild/"}
+
 // Find the osbuild-depsolve-dnf script. This checks the default location in
 // /usr/libexec but also /usr/lib in case it's used on a distribution that
 // doesn't use libexec.
-func findDepsolveDnf() string {
-	locations := []string{"/usr/libexec/osbuild-depsolve-dnf", "/usr/lib/osbuild/osbuild-depsolve-dnf"}
-	for _, djPath := range locations {
-		_, err := os.Stat(djPath)
+func findDepsolveDnf() (string, error) {
+	for _, djPath := range depSolveDnfPaths {
+		_, err := os.Stat(filepath.Join(djPath, "osbuild-depsolve-dnf"))
 		if !os.IsNotExist(err) {
-			return djPath
+			return djPath, nil
 		}
 	}
 
 	// if it's not found, return empty string; the run() function will fail if
 	// it's used before setting.
-	return ""
+	return "", fmt.Errorf("cannot find required osbuild-depsolve-dnf in any of %q", depSolveDnfPaths)
 }
 
 // Create a new unconfigured BaseSolver (without platform information). It can
 // be used to create configured Solver instances with the NewWithConfig()
 // method.
-func NewBaseSolver(cacheDir string) *BaseSolver {
+func NewBaseSolver(cacheDir string) (*BaseSolver, error) {
+	depsolveDnf, err := findDepsolveDnf()
+	if err != nil {
+		return nil, err
+	}
 	return &BaseSolver{
 		cache:       newRPMCache(cacheDir, 1024*1024*1024), // 1 GiB
-		dnfJsonCmd:  []string{findDepsolveDnf()},
+		dnfJsonCmd:  []string{depsolveDnf},
 		resultCache: NewDNFCache(60 * time.Second),
-	}
+	}, nil
 }
 
 // SetMaxCacheSize sets the maximum size for the global repository metadata
@@ -129,9 +134,12 @@ type Solver struct {
 }
 
 // Create a new Solver with the given configuration. Initialising a Solver also loads system subscription information.
-func NewSolver(modulePlatformID, releaseVer, arch, distro, cacheDir string) *Solver {
-	s := NewBaseSolver(cacheDir)
-	return s.NewWithConfig(modulePlatformID, releaseVer, arch, distro)
+func NewSolver(modulePlatformID, releaseVer, arch, distro, cacheDir string) (*Solver, error) {
+	s, err := NewBaseSolver(cacheDir)
+	if err != nil {
+		return nil, err
+	}
+	return s.NewWithConfig(modulePlatformID, releaseVer, arch, distro), nil
 }
 
 // GetCacheDir returns a distro specific rpm cache directory

pkg/dnfjson/dnfjson_test.go

@@ -11,15 +11,16 @@ import (
 	"github.com/osbuild/images/internal/mocks/rpmrepo"
 	"github.com/osbuild/images/pkg/rpmmd"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var forceDNF = flag.Bool("force-dnf", false, "force dnf testing, making them fail instead of skip if dnf isn't installed")
 
 func TestDepsolver(t *testing.T) {
 	if !*forceDNF {
 		// dnf tests aren't forced: skip them if the dnf sniff check fails
-		if findDepsolveDnf() == "" {
-			t.Skip("Test needs an installed osbuild-depsolve-dnf")
+		if _, err := findDepsolveDnf(); err != nil {
+			t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err)
 		}
 	}
 
@@ -29,7 +30,8 @@ func TestDepsolver(t *testing.T) {
 	assert := assert.New(t)
 
 	tmpdir := t.TempDir()
-	solver := NewSolver("platform:el9", "9", "x86_64", "rhel9.0", tmpdir)
+	solver, err := NewSolver("platform:el9", "9", "x86_64", "rhel9.0", tmpdir)
+	require.Nil(t, err)
 
 	{ // single depsolve
 		pkgsets := []rpmmd.PackageSet{{Include: []string{"kernel", "vim-minimal", "tmux", "zsh"}, Repositories: []rpmmd.RepoConfig{s.RepoConfig}, InstallWeakDeps: true}} // everything you'll ever need
@@ -57,6 +59,13 @@ func TestDepsolver(t *testing.T) {
 }
 
 func TestMakeDepsolveRequest(t *testing.T) {
+	if !*forceDNF {
+		// dnf tests aren't forced: skip them if the dnf sniff check fails
+		if _, err := findDepsolveDnf(); err != nil {
+			t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err)
+		}
+	}
+
 	baseOS := rpmmd.RepoConfig{
 		Name:     "baseos",
 		BaseURLs: []string{"https://example.org/baseos"},
@@ -402,7 +411,8 @@ func TestMakeDepsolveRequest(t *testing.T) {
 			},
 		},
 	}
-	solver := NewSolver("", "", "", "", "")
+	solver, err := NewSolver("", "", "", "", "")
+	require.Nil(t, err)
 	for idx, tt := range tests {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
 			req, _, err := solver.makeDepsolveRequest(tt.packageSets)
@@ -543,8 +553,8 @@ func expectedResult(repo rpmmd.RepoConfig) []rpmmd.PackageSpec {
 func TestErrorRepoInfo(t *testing.T) {
 	if !*forceDNF {
 		// dnf tests aren't forced: skip them if the dnf sniff check fails
-		if findDepsolveDnf() == "" {
-			t.Skip("Test needs an installed osbuild-depsolve-dnf")
+		if _, err := findDepsolveDnf(); err != nil {
+			t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err)
 		}
 	}
 
@@ -588,7 +598,8 @@ func TestErrorRepoInfo(t *testing.T) {
 		},
 	}
 
-	solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	require.Nil(t, err)
 	for idx, tc := range testCases {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
 			_, err := solver.Depsolve([]rpmmd.PackageSet{
@@ -605,6 +616,13 @@ func TestErrorRepoInfo(t *testing.T) {
 }
 
 func TestRepoConfigHash(t *testing.T) {
+	if !*forceDNF {
+		// dnf tests aren't forced: skip them if the dnf sniff check fails
+		if _, err := findDepsolveDnf(); err != nil {
+			t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err)
+		}
+	}
+
 	repos := []rpmmd.RepoConfig{
 		{
 			Id:        "repoid-1",
@@ -617,7 +635,8 @@ func TestRepoConfigHash(t *testing.T) {
 		},
 	}
 
-	solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	require.Nil(t, err)
 
 	rcs, err := solver.reposFromRPMMD(repos)
 	assert.Nil(t, err)
@@ -629,7 +648,15 @@ func TestRepoConfigHash(t *testing.T) {
 }
 
 func TestRequestHash(t *testing.T) {
-	solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	if !*forceDNF {
+		// dnf tests aren't forced: skip them if the dnf sniff check fails
+		if _, err := findDepsolveDnf(); err != nil {
+			t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err)
+		}
+	}
+
+	solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache")
+	require.Nil(t, err)
 	repos := []rpmmd.RepoConfig{
 		rpmmd.RepoConfig{
 			Name:      "A test repository",
@@ -655,3 +682,13 @@ func TestRepoConfigMarshalAlsmostEmpty(t *testing.T) {
 	// double check here that anything that uses pointers has "omitempty" set
 	assert.Equal(t, string(js), `{"id":"","gpgcheck":false,"check_repogpg":false,"ignoressl":false}`)
 }
+
+func TestFindDepsolveDnfError(t *testing.T) {
+	old := depSolveDnfPaths
+	depSolveDnfPaths = []string{"/rando1", "/rando2"}
+	defer func() { depSolveDnfPaths = old }()
+
+	path, err := findDepsolveDnf()
+	assert.Equal(t, path, "")
+	assert.EqualError(t, err, `cannot find required osbuild-depsolve-dnf in any of ["/rando1" "/rando2"]`)
+}