osbuild: add rpmDownloader to GenSources() argument

This commit enables to select what rpm downloader backend to use.
The signature of GenSources() is just expanded to take an additonal
parameter. It would be nice to re-use the `manifest.Inputs` struct
but it is (subtly) different from what GenSource needs :/ I was
also tempated to add a "Options" struct to GenSources instead of
the enum but YAGNI - if we ever need to add another one this will
become a options struct.

pkg/manifest/manifest.go

@@ -41,15 +41,6 @@ const (
 	DISTRO_FEDORA
 )
 
-// RpmDownloader specifies what backend to use for rpm downloads
-// Note that the librepo backend requires a newer osbuild.
-type RpmDownloader uint64
-
-const (
-	RpmDownloaderCurl    = iota
-	RpmDownloaderLibrepo = iota
-)
-
 // Inputs specifies the inputs for manifest instantiating
 type Inputs struct {
 	PackageSets    map[string][]rpmmd.PackageSpec
@@ -58,11 +49,6 @@ type Inputs struct {
 	RpmRepos       map[string][]rpmmd.RepoConfig
 }
 
-// Options contains the (optional) configs for the manifest instantiating
-type Options struct {
-	RpmDownloader RpmDownloader
-}
-
 // An OSBuildManifest is an opaque JSON object, which is a valid input to osbuild
 type OSBuildManifest []byte
 
@@ -171,6 +157,11 @@ func (m Manifest) Serialize(packageSets map[string][]rpmmd.PackageSpec, containe
 	return m.SerializeFull(inputs, nil)
 }
 
+// Options contains the (optional) configs for the manifest instantiating
+type Options struct {
+	RpmDownloader osbuild.RpmDownloader
+}
+
 // TODO: rename to Serialize() onces callers are fixed
 func (m Manifest) SerializeFull(inputs *Inputs, opts *Options) (OSBuildManifest, error) {
 	if opts == nil {
@@ -195,7 +186,7 @@ func (m Manifest) SerializeFull(inputs *Inputs, opts *Options) (OSBuildManifest,
 		pipeline.serializeEnd()
 	}
 
-	sources, err := osbuild.GenSources(packages, commits, inline, containers, inputs.RpmRepos)
+	sources, err := osbuild.GenSources(packages, commits, inline, containers, inputs.RpmRepos, opts.RpmDownloader)
 	if err != nil {
 		return nil, err
 	}

pkg/osbuild/source.go

@@ -3,7 +3,7 @@ package osbuild
 import (
 	"encoding/json"
 	"errors"
-	"os"
+	"fmt"
 
 	"github.com/osbuild/images/pkg/container"
 	"github.com/osbuild/images/pkg/ostree"
@@ -79,18 +79,32 @@ func addPackagesLibrepo(sources Sources, packages []rpmmd.PackageSpec, rpmRepos
 	return nil
 }
 
-func GenSources(packages []rpmmd.PackageSpec, ostreeCommits []ostree.CommitSpec, inlineData []string, containers []container.Spec, rpmRepos map[string][]rpmmd.RepoConfig) (Sources, error) {
+// RpmDownloader specifies what backend to use for rpm downloads
+// Note that the librepo backend requires a newer osbuild.
+type RpmDownloader uint64
+
+const (
+	RpmDownloaderCurl    = iota
+	RpmDownloaderLibrepo = iota
+)
+
+func GenSources(packages []rpmmd.PackageSpec, ostreeCommits []ostree.CommitSpec, inlineData []string, containers []container.Spec, rpmRepos map[string][]rpmmd.RepoConfig, rpmDownloader RpmDownloader) (Sources, error) {
+	// The signature of this functionis already relatively long,
+	// if we need to add more options, refactor into "struct
+	// Inputs" (rpm,ostree,etc) and "struct Options"
+	// (rpmDownloader)
 	sources := Sources{}
 
 	// collect rpm package sources
 	if len(packages) > 0 {
-		// XXX: hack, we have no good way to pass options to GenSource
-		// right now
 		var err error
-		if s := os.Getenv("OSBUILD_USE_LIBREPO"); s == "1" {
-			err = addPackagesLibrepo(sources, packages, rpmRepos)
-		} else {
+		switch rpmDownloader {
+		case RpmDownloaderCurl:
 			err = addPackagesCurl(sources, packages)
+		case RpmDownloaderLibrepo:
+			err = addPackagesLibrepo(sources, packages, rpmRepos)
+		default:
+			err = fmt.Errorf("unknown rpm downloader %v", rpmDownloader)
 		}
 		if err != nil {
 			return nil, err

pkg/osbuild/source_test.go

@@ -119,7 +119,7 @@ func TestSource_UnmarshalJSON(t *testing.T) {
 }
 
 func TestGenSourcesTrivial(t *testing.T) {
-	sources, err := GenSources(nil, nil, nil, nil, nil)
+	sources, err := GenSources(nil, nil, nil, nil, nil, 0)
 	assert.NoError(t, err)
 
 	jsonOutput, err := json.MarshalIndent(sources, "", "  ")
@@ -135,7 +135,7 @@ func TestGenSourcesContainerStorage(t *testing.T) {
 			LocalStorage: true,
 		},
 	}
-	sources, err := GenSources(nil, nil, nil, containers, nil)
+	sources, err := GenSources(nil, nil, nil, containers, nil, 0)
 	assert.NoError(t, err)
 
 	jsonOutput, err := json.MarshalIndent(sources, "", "  ")
@@ -159,7 +159,7 @@ func TestGenSourcesSkopeo(t *testing.T) {
 			ImageID: imageID,
 		},
 	}
-	sources, err := GenSources(nil, nil, nil, containers, nil)
+	sources, err := GenSources(nil, nil, nil, containers, nil, 0)
 	assert.NoError(t, err)
 
 	jsonOutput, err := json.MarshalIndent(sources, "", "  ")
@@ -190,7 +190,7 @@ func TestGenSourcesWithSkopeoIndex(t *testing.T) {
 			ImageID:    imageID,
 		},
 	}
-	sources, err := GenSources(nil, nil, nil, containers, nil)
+	sources, err := GenSources(nil, nil, nil, containers, nil, 0)
 	assert.NoError(t, err)
 
 	jsonOutput, err := json.MarshalIndent(sources, "", "  ")