dnfjson: wrap Depsolve() return values by DepsolveResult struct

Since the number of the `Solver.Depsolve()` return values is slowly getting out of hands, introduce a `DepsolveResult` struct containing all of the return values. `Depsolve()` now returns a pointer to `DepsolveResult`. Signed-off-by: Tomáš Hozza <[email protected]>
osbuild · Sep 19, 2024 · 5b447eb · 5b447eb
1 parent bd8893d
commit 5b447eb
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 23 deletions.
diff --git a/cmd/build/main.go b/cmd/build/main.go
@@ -134,12 +134,12 @@ func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d dist
 	depsolvedSets := make(map[string][]rpmmd.PackageSpec)
 	repoSets := make(map[string][]rpmmd.RepoConfig)
 	for name, pkgSet := range packageSets {
-		pkgs, repos, _, err := solver.Depsolve(pkgSet, sbom.StandardTypeNone)
+		res, err := solver.Depsolve(pkgSet, sbom.StandardTypeNone)
 		if err != nil {
 			return nil, nil, err
 		}
-		depsolvedSets[name] = pkgs
-		repoSets[name] = repos
+		depsolvedSets[name] = res.Packages
+		repoSets[name] = res.Repos
 	}
 	return depsolvedSets, repoSets, nil
 }

diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go
@@ -358,12 +358,12 @@ func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d dist
 	depsolvedSets := make(map[string][]rpmmd.PackageSpec)
 	repoSets := make(map[string][]rpmmd.RepoConfig)
 	for name, pkgSet := range packageSets {
-		packages, repos, _, err := solver.Depsolve(pkgSet, sbom.StandardTypeNone)
+		res, err := solver.Depsolve(pkgSet, sbom.StandardTypeNone)
 		if err != nil {
 			return nil, nil, err
 		}
-		depsolvedSets[name] = packages
-		repoSets[name] = repos
+		depsolvedSets[name] = res.Packages
+		repoSets[name] = res.Repos
 	}
 	return depsolvedSets, repoSets, nil
 }

diff --git a/cmd/osbuild-playground/playground.go b/cmd/osbuild-playground/playground.go
@@ -37,11 +37,11 @@ func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos
 
 	packageSpecs := make(map[string][]rpmmd.PackageSpec)
 	for name, chain := range manifest.GetPackageSetChains() {
-		packages, _, _, err := solver.Depsolve(chain, sbom.StandardTypeNone)
+		res, err := solver.Depsolve(chain, sbom.StandardTypeNone)
 		if err != nil {
 			panic(fmt.Sprintf("failed to depsolve for pipeline %s: %s\n", name, err.Error()))
 		}
-		packageSpecs[name] = packages
+		packageSpecs[name] = res.Packages
 	}
 
 	if err := solver.CleanCache(); err != nil {

diff --git a/pkg/dnfjson/dnfjson.go b/pkg/dnfjson/dnfjson.go
@@ -156,6 +156,13 @@ type Solver struct {
 	subscriptions *rhsm.Subscriptions
 }
 
+// DepsolveResult contains the results of a depsolve operation.
+type DepsolveResult struct {
+	Packages []rpmmd.PackageSpec
+	Repos    []rpmmd.RepoConfig
+	SBOM     *sbom.Document
+}
+
 // 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)
@@ -194,10 +201,10 @@ func (s *Solver) SetProxy(proxy string) error {
 // their associated repositories.  Each package set is depsolved as a separate
 // transactions in a chain.  It returns a list of all packages (with solved
 // dependencies) that will be installed into the system.
-func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet, sbomType sbom.StandardType) ([]rpmmd.PackageSpec, []rpmmd.RepoConfig, *sbom.Document, error) {
+func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet, sbomType sbom.StandardType) (*DepsolveResult, error) {
 	req, rhsmMap, err := s.makeDepsolveRequest(pkgSets, sbomType)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("makeDepsolveRequest failed: %w", err)
+		return nil, fmt.Errorf("makeDepsolveRequest failed: %w", err)
 	}
 
 	// get non-exclusive read lock
@@ -206,7 +213,7 @@ func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet, sbomType sbom.StandardType
 
 	output, err := run(s.dnfJsonCmd, req)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("running osbuild-depsolve-dnf failed:\n%w", err)
+		return nil, fmt.Errorf("running osbuild-depsolve-dnf failed:\n%w", err)
 	}
 	// touch repos to now
 	now := time.Now().Local()
@@ -220,7 +227,7 @@ func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet, sbomType sbom.StandardType
 	dec := json.NewDecoder(bytes.NewReader(output))
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&result); err != nil {
-		return nil, nil, nil, fmt.Errorf("decoding depsolve result failed: %w", err)
+		return nil, fmt.Errorf("decoding depsolve result failed: %w", err)
 	}
 
 	packages, repos := result.toRPMMD(rhsmMap)
@@ -229,11 +236,15 @@ func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet, sbomType sbom.StandardType
 	if sbomType != sbom.StandardTypeNone {
 		sbomDoc, err = sbom.NewDocument(sbomType, result.SBOM)
 		if err != nil {
-			return nil, nil, nil, fmt.Errorf("creating SBOM document failed: %w", err)
+			return nil, fmt.Errorf("creating SBOM document failed: %w", err)
 		}
 	}
 
-	return packages, repos, sbomDoc, nil
+	return &DepsolveResult{
+		Packages: packages,
+		Repos:    repos,
+		SBOM:     sbomDoc,
+	}, nil
 }
 
 // FetchMetadata returns the list of all the available packages in repos and

diff --git a/pkg/dnfjson/dnfjson_test.go b/pkg/dnfjson/dnfjson_test.go
@@ -124,22 +124,23 @@ func TestDepsolver(t *testing.T) {
 			}
 
 			solver.SetRootDir(tc.rootDir)
-			deps, _, sbomDoc, err := solver.Depsolve(pkgsets, tc.sbomType)
+			res, err := solver.Depsolve(pkgsets, tc.sbomType)
 			if tc.err {
 				assert.Error(err)
 				assert.Contains(err.Error(), tc.expMsg)
 				return
 			} else {
 				assert.Nil(err)
+				assert.NotNil(res)
 			}
 
-			assert.Equal(expectedResult(s.RepoConfig), deps)
+			assert.Equal(expectedResult(s.RepoConfig), res.Packages)
 
 			if tc.sbomType != sbom.StandardTypeNone {
-				assert.NotNil(sbomDoc)
-				assert.Equal(sbom.StandardTypeSpdx, sbomDoc.DocType)
+				assert.NotNil(res.SBOM)
+				assert.Equal(sbom.StandardTypeSpdx, res.SBOM.DocType)
 			} else {
-				assert.Nil(sbomDoc)
+				assert.Nil(res.SBOM)
 			}
 		})
 	}
@@ -779,7 +780,7 @@ func TestErrorRepoInfo(t *testing.T) {
 	solver := NewSolver("platform:f38", "38", "x86_64", "fedora-38", "/tmp/cache")
 	for idx, tc := range testCases {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
-			_, _, _, err := solver.Depsolve([]rpmmd.PackageSet{
+			_, err := solver.Depsolve([]rpmmd.PackageSet{
 				{
 					Include:      []string{"osbuild"},
 					Exclude:      nil,
@@ -868,8 +869,9 @@ echo '{"solver": "zypper"}'
 
 	solver := NewSolver("platform:f38", "38", "x86_64", "fedora-38", "/tmp/cache")
 	solver.dnfJsonCmd = []string{fakeSolverPath}
-	pkgSpec, repoCfg, _, err := solver.Depsolve(nil, sbom.StandardTypeNone)
+	res, err := solver.Depsolve(nil, sbom.StandardTypeNone)
 	assert.NoError(t, err)
+	assert.NotNil(t, res)
 
 	// prerequisite check, i.e. ensure our fake was called in the right way
 	stdin, err := os.ReadFile(fakeSolverPath + ".stdin")
@@ -878,6 +880,6 @@ echo '{"solver": "zypper"}'
 
 	// adding the "solver" did not cause any issues
 	assert.NoError(t, err)
-	assert.Equal(t, 0, len(pkgSpec))
-	assert.Equal(t, 0, len(repoCfg))
+	assert.Equal(t, 0, len(res.Packages))
+	assert.Equal(t, 0, len(res.Repos))
 }