From c092783a70598edba0ea910ebfaf7e865242ff4b Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 9 May 2022 17:41:13 +0200 Subject: [PATCH] simplify package set chain handling Move package set chain collation to the distro package and add repositories to the package sets while returning the package sets from their source, i.e., the ImageType.PackageSets() method. This also removes the concept of "base repositories". There are no longer repositories that are added implicitly to all package sets but instead each package set needs to specify *all* the repositories it will be depsolved against. This paves the way for the requirement we have for building RHEL 7 images with a RHEL 8 build root. The build root package set has to be depsolved against RHEL 8 repositories without any "base repos" included. This is now possible since package sets and repositories are explicitly associated from the start and there is no implicit global repository set. The change requires adding a list of PackageSet names to the core rpmmd.RepoConfig. In the cloud API, repositories that are limited to specific package sets already contain the correct package set names and these are now copied to the internal RepoConfig when converting types in genRepoConfig(). The user-specified repositories are only associated with the payload package sets like before. --- cmd/osbuild-dnf-json-tests/main_test.go | 27 +---- cmd/osbuild-package-sets/main.go | 2 +- cmd/osbuild-pipeline/main.go | 22 +--- cmd/osbuild-store-dump/main.go | 4 +- cmd/osbuild-worker/jobimpl-depsolve.go | 23 +--- internal/cloudapi/v2/handler.go | 70 ++++++------ internal/cloudapi/v2/server.go | 15 +-- internal/cloudapi/v2/v2_internal_test.go | 59 ++-------- internal/distro/distro.go | 54 ++++++++- internal/distro/distro_test.go | 7 -- .../distro_test_common/distro_test_common.go | 71 +++++------- internal/distro/fedora/distro.go | 4 +- internal/distro/fedora/distro_test.go | 5 +- internal/distro/rhel8/distro.go | 20 ++-- internal/distro/rhel8/distro_test.go | 12 +- internal/distro/rhel84/distro.go | 20 ++-- internal/distro/rhel84/distro_test.go | 12 +- internal/distro/rhel84/distro_v2.go | 26 +++-- internal/distro/rhel85/distro.go | 4 +- internal/distro/rhel85/distro_test.go | 5 +- internal/distro/rhel86/distro.go | 4 +- internal/distro/rhel86/distro_test.go | 5 +- internal/distro/rhel90/distro.go | 4 +- internal/distro/rhel90/distro_test.go | 5 +- internal/distro/rhel90beta/distro.go | 4 +- internal/distro/rhel90beta/distro_test.go | 5 +- internal/distro/test_distro/distro.go | 18 +-- internal/dnfjson/dnfjson.go | 21 +--- internal/dnfjson/dnfjson_test.go | 106 ++++++++---------- internal/kojiapi/server.go | 17 ++- internal/rpmmd/repository.go | 1 + internal/weldr/api.go | 51 +++------ internal/worker/json.go | 21 ++-- 33 files changed, 323 insertions(+), 401 deletions(-) diff --git a/cmd/osbuild-dnf-json-tests/main_test.go b/cmd/osbuild-dnf-json-tests/main_test.go index ea81a02e9..6a094aac7 100644 --- a/cmd/osbuild-dnf-json-tests/main_test.go +++ b/cmd/osbuild-dnf-json-tests/main_test.go @@ -81,12 +81,12 @@ func TestCrossArchDepsolve(t *testing.T) { imgType, err := arch.GetImageType(imgTypeStr) require.NoError(t, err) - packages := imgType.PackageSets(blueprint.Blueprint{}) + packages := imgType.PackageSets(blueprint.Blueprint{}, repos[archStr]) - _, err = solver.Depsolve([]rpmmd.PackageSet{packages["build"]}, repos[archStr]) + _, err = solver.Depsolve(packages["build"]) assert.NoError(t, err) - _, err = solver.Depsolve([]rpmmd.PackageSet{packages["packages"]}, repos[archStr]) + _, err = solver.Depsolve(packages["packages"]) assert.NoError(t, err) }) } @@ -125,11 +125,11 @@ func TestDepsolvePackageSets(t *testing.T) { qcow2Image, err := x86Arch.GetImageType(qcow2ImageTypeName) require.Nilf(t, err, "failed to get %q image type of %q/%q distro/arch", qcow2ImageTypeName, distroStruct.Name(), distro.X86_64ArchName) - imagePkgSets := qcow2Image.PackageSets(blueprint.Blueprint{Packages: []blueprint.Package{{Name: "bind"}}}) + imagePkgSets := qcow2Image.PackageSets(blueprint.Blueprint{Packages: []blueprint.Package{{Name: "bind"}}}, x86Repos) imagePkgSetChains := qcow2Image.PackageSetsChains() require.NotEmptyf(t, imagePkgSetChains, "the %q image has no package set chains defined", qcow2ImageTypeName) - expectedPackageSpecsSetNames := func(pkgSets map[string]rpmmd.PackageSet, pkgSetChains map[string][]string) []string { + expectedPackageSpecsSetNames := func(pkgSets map[string][]rpmmd.PackageSet, pkgSetChains map[string][]string) []string { expectedPkgSpecsSetNames := make([]string, 0, len(pkgSets)) chainPkgSets := make(map[string]struct{}, len(pkgSets)) for name, pkgSetChain := range pkgSetChains { @@ -148,29 +148,14 @@ func TestDepsolvePackageSets(t *testing.T) { }(imagePkgSets, imagePkgSetChains) gotPackageSpecsSets := make(map[string]*dnfjson.DepsolveResult, len(imagePkgSets)) - // first depsolve package sets that are part of a chain - for specName, setNames := range imagePkgSetChains { - pkgSets := make([]rpmmd.PackageSet, len(setNames)) - for idx, pkgSetName := range setNames { - pkgSets[idx] = imagePkgSets[pkgSetName] - delete(imagePkgSets, pkgSetName) // will be depsolved here: remove from map - } - res, err := solver.Depsolve(pkgSets, x86Repos) - if err != nil { - require.Nil(t, err) - } - gotPackageSpecsSets[specName] = res - } - // depsolve the rest of the package sets for name, pkgSet := range imagePkgSets { - res, err := solver.Depsolve([]rpmmd.PackageSet{pkgSet}, x86Repos) + res, err := solver.Depsolve(pkgSet) if err != nil { require.Nil(t, err) } gotPackageSpecsSets[name] = res } - require.Nil(t, err) require.EqualValues(t, len(expectedPackageSpecsSetNames), len(gotPackageSpecsSets)) for _, name := range expectedPackageSpecsSetNames { _, ok := gotPackageSpecsSets[name] diff --git a/cmd/osbuild-package-sets/main.go b/cmd/osbuild-package-sets/main.go index 8a4b61032..87f5218d0 100644 --- a/cmd/osbuild-package-sets/main.go +++ b/cmd/osbuild-package-sets/main.go @@ -41,6 +41,6 @@ func main() { encoder := json.NewEncoder(os.Stdout) encoder.SetIndent("", " ") - pkgset := image.PackageSets(blueprint.Blueprint{}) + pkgset := image.PackageSets(blueprint.Blueprint{}, nil) _ = encoder.Encode(pkgset) } diff --git a/cmd/osbuild-pipeline/main.go b/cmd/osbuild-pipeline/main.go index cdddbf8a1..4fb374fa7 100644 --- a/cmd/osbuild-pipeline/main.go +++ b/cmd/osbuild-pipeline/main.go @@ -140,28 +140,14 @@ func main() { panic("os.UserHomeDir(): " + err.Error()) } - packageSets := imageType.PackageSets(composeRequest.Blueprint) - solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch.Name(), path.Join(home, ".cache/osbuild-composer/rpmmd")) solver.SetDNFJSONPath(findDnfJsonBin()) - depsolvedSets := make(map[string][]rpmmd.PackageSpec) - // first depsolve package sets that are part of a chain - for specName, setNames := range imageType.PackageSetsChains() { - pkgSets := make([]rpmmd.PackageSet, len(setNames)) - for idx, pkgSetName := range setNames { - pkgSets[idx] = packageSets[pkgSetName] - delete(packageSets, pkgSetName) // will be depsolved here: remove from map - } - res, err := solver.Depsolve(pkgSets, repos) - if err != nil { - panic("Could not depsolve: " + err.Error()) - } - depsolvedSets[specName] = res.Dependencies - } - // depsolve the rest of the package sets + packageSets := imageType.PackageSets(composeRequest.Blueprint, repos) + depsolvedSets := make(map[string][]rpmmd.PackageSpec) + for name, pkgSet := range packageSets { - res, err := solver.Depsolve([]rpmmd.PackageSet{pkgSet}, repos) + res, err := solver.Depsolve(pkgSet) if err != nil { panic("Could not depsolve: " + err.Error()) } diff --git a/cmd/osbuild-store-dump/main.go b/cmd/osbuild-store-dump/main.go index 244f9617a..1a227d317 100644 --- a/cmd/osbuild-store-dump/main.go +++ b/cmd/osbuild-store-dump/main.go @@ -19,11 +19,11 @@ import ( ) func getManifest(bp blueprint.Blueprint, t distro.ImageType, a distro.Arch, d distro.Distro, cacheDir string, repos []rpmmd.RepoConfig) (distro.Manifest, []rpmmd.PackageSpec) { - packageSets := t.PackageSets(bp) + packageSets := t.PackageSets(bp, repos) pkgSpecSets := make(map[string][]rpmmd.PackageSpec) solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), a.Name(), cacheDir) for name, packages := range packageSets { - res, err := solver.Depsolve([]rpmmd.PackageSet{packages}, repos) + res, err := solver.Depsolve(packages) if err != nil { panic(err) } diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go index 7d382ea8a..8404f991e 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve.go +++ b/cmd/osbuild-worker/jobimpl-depsolve.go @@ -19,29 +19,12 @@ type DepsolveJobImpl struct { // in repos are used for all package sets, whereas the repositories in // packageSetsRepos are only used for the package set with the same name // (matching map keys). -func (impl *DepsolveJobImpl) depsolve(packageSetsChains map[string][]string, packageSets map[string]rpmmd.PackageSet, repos []rpmmd.RepoConfig, packageSetsRepos map[string][]rpmmd.RepoConfig, modulePlatformID, arch, releasever string) (map[string][]rpmmd.PackageSpec, error) { +func (impl *DepsolveJobImpl) depsolve(packageSets map[string][]rpmmd.PackageSet, modulePlatformID, arch, releasever string) (map[string][]rpmmd.PackageSpec, error) { solver := impl.Solver.NewWithConfig(modulePlatformID, releasever, arch) depsolvedSets := make(map[string][]rpmmd.PackageSpec) - // first depsolve package sets that are part of a chain - for specName, setNames := range packageSetsChains { - pkgSets := make([]rpmmd.PackageSet, len(setNames)) - for idx, pkgSetName := range setNames { - pkgSets[idx] = packageSets[pkgSetName] - pkgSets[idx].Repositories = packageSetsRepos[pkgSetName] - delete(packageSets, pkgSetName) // will be depsolved here: remove from map - } - res, err := solver.Depsolve(pkgSets, repos) - if err != nil { - return nil, err - } - depsolvedSets[specName] = res.Dependencies - } - - // depsolve the rest of the package sets for name, pkgSet := range packageSets { - pkgSet.Repositories = packageSetsRepos[name] - res, err := solver.Depsolve([]rpmmd.PackageSet{pkgSet}, repos) + res, err := solver.Depsolve(pkgSet) if err != nil { return nil, err } @@ -59,7 +42,7 @@ func (impl *DepsolveJobImpl) Run(job worker.Job) error { } var result worker.DepsolveJobResult - result.PackageSpecs, err = impl.depsolve(args.PackageSetsChains, args.PackageSets, args.Repos, args.PackageSetsRepos, args.ModulePlatformID, args.Arch, args.Releasever) + result.PackageSpecs, err = impl.depsolve(args.PackageSets, args.ModulePlatformID, args.Arch, args.Releasever) if err != nil { switch e := err.(type) { case *dnfjson.Error: diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index 777c4a42a..a636c0f0c 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -107,12 +107,11 @@ func splitExtension(filename string) string { } type imageRequest struct { - imageType distro.ImageType - arch distro.Arch - repositories []rpmmd.RepoConfig - packageSetsRepositories map[string][]rpmmd.RepoConfig - imageOptions distro.ImageOptions - target *target.Target + imageType distro.ImageType + arch distro.Arch + repositories []rpmmd.RepoConfig + imageOptions distro.ImageOptions + target *target.Target } func (h *apiHandlers) PostCompose(ctx echo.Context) error { @@ -238,7 +237,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { return HTTPError(ErrorUnsupportedImageType) } - repos, pkgSetsRepos, err := collectRepos(ir.Repositories, payloadRepositories, imageType.PayloadPackageSets()) + repos, err := convertRepos(ir.Repositories, payloadRepositories, imageType.PayloadPackageSets()) if err != nil { return err } @@ -426,12 +425,11 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } irs = append(irs, imageRequest{ - imageType: imageType, - arch: arch, - repositories: repos, - imageOptions: imageOptions, - packageSetsRepositories: pkgSetsRepos, - target: irTarget, + imageType: imageType, + arch: arch, + repositories: repos, + imageOptions: imageOptions, + target: irTarget, }) } @@ -972,38 +970,32 @@ func (h *apiHandlers) GetComposeManifests(ctx echo.Context, id string) error { return ctx.JSON(http.StatusOK, resp) } -func collectRepos(irRepos, payloadRepositories []Repository, payloadPackageSets []string) ([]rpmmd.RepoConfig, map[string][]rpmmd.RepoConfig, error) { - // build package set repository map and base repository set based on repository package sets - // No package_sets -> base set - mainRepoConfigs := make([]rpmmd.RepoConfig, 0, len(irRepos)) - pkgSetsRepoConfigs := make(map[string][]rpmmd.RepoConfig, len(irRepos)) +// Converts repositories in the request to the internal rpmmd.RepoConfig representation +func convertRepos(irRepos, payloadRepositories []Repository, payloadPackageSets []string) ([]rpmmd.RepoConfig, error) { + repos := make([]rpmmd.RepoConfig, 0, len(irRepos)+len(payloadRepositories)) - for _, irRepo := range irRepos { - repo, err := genRepoConfig(irRepo) + for idx := range irRepos { + r, err := genRepoConfig(irRepos[idx]) if err != nil { - return nil, nil, err - } - if pkgSets := irRepo.PackageSets; pkgSets != nil && len(*irRepo.PackageSets) > 0 { - for _, pkgSet := range *pkgSets { - pkgSetsRepoConfigs[pkgSet] = append(pkgSetsRepoConfigs[pkgSet], *repo) - } - } else { - mainRepoConfigs = append(mainRepoConfigs, *repo) + return nil, err } + repos = append(repos, *r) } - // add user custom repos to all payload package sets - for _, plRepo := range payloadRepositories { - for _, payloadName := range payloadPackageSets { - repo, err := genRepoConfig(plRepo) - if err != nil { - return nil, nil, err - } - pkgSetsRepoConfigs[payloadName] = append(pkgSetsRepoConfigs[payloadName], *repo) + for idx := range payloadRepositories { + // the PackageSets (package_sets) field for these repositories is + // ignored (see openapi.v2.yml description for payload_repositories) + // and we replace any value in it with the names of the payload package + // sets + r, err := genRepoConfig(payloadRepositories[idx]) + if err != nil { + return nil, err } + r.PackageSets = payloadPackageSets + repos = append(repos, *r) } - return mainRepoConfigs, pkgSetsRepoConfigs, nil + return repos, nil } func genRepoConfig(repo Repository) (*rpmmd.RepoConfig, error) { @@ -1035,5 +1027,9 @@ func genRepoConfig(repo Repository) (*rpmmd.RepoConfig, error) { return nil, HTTPError(ErrorNoGPGKey) } + if repo.PackageSets != nil { + repoConfig.PackageSets = *repo.PackageSets + } + return repoConfig, nil } diff --git a/internal/cloudapi/v2/server.go b/internal/cloudapi/v2/server.go index 11f98d244..657b7ce50 100644 --- a/internal/cloudapi/v2/server.go +++ b/internal/cloudapi/v2/server.go @@ -132,13 +132,10 @@ func (s *Server) enqueueCompose(distribution distro.Distro, bp blueprint.Bluepri ir := irs[0] depsolveJobID, err := s.workers.EnqueueDepsolve(&worker.DepsolveJob{ - PackageSetsChains: ir.imageType.PackageSetsChains(), - PackageSets: ir.imageType.PackageSets(bp), - Repos: ir.repositories, - ModulePlatformID: distribution.ModulePlatformID(), - Arch: ir.arch.Name(), - Releasever: distribution.Releasever(), - PackageSetsRepos: ir.packageSetsRepositories, + PackageSets: ir.imageType.PackageSets(bp, ir.repositories), + ModulePlatformID: distribution.ModulePlatformID(), + Arch: ir.arch.Name(), + Releasever: distribution.Releasever(), }, channel) if err != nil { return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) @@ -188,12 +185,10 @@ func (s *Server) enqueueKojiCompose(taskID uint64, server, name, version, releas var buildIDs []uuid.UUID for _, ir := range irs { depsolveJobID, err := s.workers.EnqueueDepsolve(&worker.DepsolveJob{ - PackageSets: ir.imageType.PackageSets(bp), - Repos: ir.repositories, + PackageSets: ir.imageType.PackageSets(bp, ir.repositories), ModulePlatformID: distribution.ModulePlatformID(), Arch: ir.arch.Name(), Releasever: distribution.Releasever(), - PackageSetsRepos: ir.packageSetsRepositories, }, channel) if err != nil { return id, HTTPErrorWithInternal(ErrorEnqueueingJob, err) diff --git a/internal/cloudapi/v2/v2_internal_test.go b/internal/cloudapi/v2/v2_internal_test.go index 2bb22afa9..23a3b9341 100644 --- a/internal/cloudapi/v2/v2_internal_test.go +++ b/internal/cloudapi/v2/v2_internal_test.go @@ -51,7 +51,7 @@ func TestCollectRepos(t *testing.T) { Baseurl: common.StringToPtr("http://example.com/appstream"), // empty field -> all package sets }, { - Baseurl: common.StringToPtr("http://example.com/baseos-rhel7"), // only build + Baseurl: common.StringToPtr("http://example.com/baseos-rhel7"), // build only PackageSets: &[]string{"build"}, }, { @@ -59,63 +59,28 @@ func TestCollectRepos(t *testing.T) { PackageSets: &[]string{"build", "archive"}, }, { - Baseurl: common.StringToPtr("http://example.com/custom-os-stuff"), // blueprint -> merge with custom + Baseurl: common.StringToPtr("http://example.com/custom-os-stuff"), // blueprint only PackageSets: &[]string{"blueprint"}, }, } - expectedPkgSetRepos := map[string][]rpmmd.RepoConfig{ - "build": { - { - BaseURL: "http://example.com/baseos-rhel7", - }, - { - BaseURL: "http://example.com/extra-tools", - }, - }, - "archive": { - { - BaseURL: "http://example.com/extra-tools", - }, - }, - "blueprint": { - { - BaseURL: "http://example.com/custom-os-stuff", - }, - { - BaseURL: "http://example.com/repoone", - }, - { - BaseURL: "http://example.com/repotwo", - }, - }, + expectedRepos := []rpmmd.RepoConfig{ + {BaseURL: "http://example.com/baseos", PackageSets: nil}, + {BaseURL: "http://example.com/appstream", PackageSets: nil}, + {BaseURL: "http://example.com/baseos-rhel7", PackageSets: []string{"build"}}, + {BaseURL: "http://example.com/extra-tools", PackageSets: []string{"build", "archive"}}, + {BaseURL: "http://example.com/custom-os-stuff", PackageSets: []string{"blueprint"}}, + {BaseURL: "http://example.com/repoone", PackageSets: []string{"blueprint"}}, + {BaseURL: "http://example.com/repotwo", PackageSets: []string{"blueprint"}}, } payloadPkgSets := []string{"blueprint"} - baseRepos, pkgSetRepos, err := collectRepos(irRepos, customRepos, payloadPkgSets) + repos, err := convertRepos(irRepos, customRepos, payloadPkgSets) // check lengths assert.NoError(err) - assert.Len(baseRepos, 2) - assert.Len(pkgSetRepos, 3) - - // check tags in package set repo map - for _, tag := range []string{"blueprint", "build", "archive"} { - assert.Contains(pkgSetRepos, tag) - } - assert.NotContains(pkgSetRepos, "should-be-ignored") - - // check URLs - baseRepoURLs := make([]string, len(baseRepos)) - for idx, baseRepo := range baseRepos { - baseRepoURLs[idx] = baseRepo.BaseURL - } - for _, url := range []string{"http://example.com/baseos", "http://example.com/appstream"} { - assert.Contains(baseRepoURLs, url) - } - - assert.Equal(pkgSetRepos, expectedPkgSetRepos) + assert.Equal(repos, expectedRepos) } func TestRepoConfigConversion(t *testing.T) { diff --git a/internal/distro/distro.go b/internal/distro/distro.go index 24598193b..69a438c86 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -105,7 +105,7 @@ type ImageType interface { // Returns the sets of packages to include and exclude when building the image. // Indexed by a string label. How each set is labeled and used depends on the // image type. - PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet + PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet // Returns the names of the pipelines that set up the build environment (buildroot). BuildPipelines() []string @@ -275,3 +275,55 @@ func ExportsFallback() []string { func PayloadPackageSets() []string { return []string{} } + +func MakePackageSetChains(t ImageType, packageSets map[string]rpmmd.PackageSet, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { + allSetNames := make([]string, len(packageSets)) + idx := 0 + for setName := range packageSets { + allSetNames[idx] = setName + idx++ + } + + // map repository PackageSets to the list of repo configs + packageSetsRepos := make(map[string][]rpmmd.RepoConfig) + for idx := range repos { + repo := repos[idx] + if len(repo.PackageSets) == 0 { + // repos that don't specify package sets get used everywhere + repo.PackageSets = allSetNames + } + for _, name := range repo.PackageSets { + psRepos := packageSetsRepos[name] + psRepos = append(psRepos, repo) + packageSetsRepos[name] = psRepos + } + } + + chainedSets := make(map[string][]rpmmd.PackageSet) + addedSets := make(map[string]bool) + // first collect package sets that are part of a chain + for specName, setNames := range t.PackageSetsChains() { + pkgSets := make([]rpmmd.PackageSet, len(setNames)) + + // add package-set-specific repositories to each set if one is defined + for idx, pkgSetName := range setNames { + pkgSets[idx] = packageSets[pkgSetName] + pkgSets[idx].Repositories = packageSetsRepos[pkgSetName] + addedSets[pkgSetName] = true + } + chainedSets[specName] = pkgSets + } + + // add the rest of the package sets + for name, pkgSet := range packageSets { + if addedSets[name] { + // already added + continue + } + pkgSet.Repositories = packageSetsRepos[name] + chainedSets[name] = []rpmmd.PackageSet{pkgSet} + addedSets[name] = true // NOTE: not really necessary but good book-keeping in case this function gets expanded + } + + return chainedSets +} diff --git a/internal/distro/distro_test.go b/internal/distro/distro_test.go index c619ab6fe..be547ad58 100644 --- a/internal/distro/distro_test.go +++ b/internal/distro/distro_test.go @@ -99,10 +99,3 @@ func TestDistro_Version(t *testing.T) { require.Error(err, "Invalid manifest did not return an error") } } - -func TestDistro_ImageType_PackageSetsChains(t *testing.T) { - dr := distroregistry.NewDefault() - for _, distroName := range dr.List() { - distro_test_common.TestImageType_PackageSetsChains(t, dr.GetDistro(distroName)) - } -} diff --git a/internal/distro/distro_test_common/distro_test_common.go b/internal/distro/distro_test_common/distro_test_common.go index 90de43d6f..516f857f1 100644 --- a/internal/distro/distro_test_common/distro_test_common.go +++ b/internal/distro/distro_test_common/distro_test_common.go @@ -138,26 +138,28 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, regis } func getImageTypePkgSpecSets(imageType distro.ImageType, bp blueprint.Blueprint, repos []rpmmd.RepoConfig, cacheDir, dnfJsonPath string) map[string][]rpmmd.PackageSpec { - imgPackageSets := imageType.PackageSets(bp) + imgPackageSets := imageType.PackageSets(bp, repos) solver := dnfjson.NewSolver(imageType.Arch().Distro().ModulePlatformID(), imageType.Arch().Distro().Releasever(), imageType.Arch().Name(), cacheDir) - imgPackageSpecSets := make(map[string][]rpmmd.PackageSpec) + depsolvedSets := make(map[string][]rpmmd.PackageSpec) for name, packages := range imgPackageSets { - res, err := solver.Depsolve([]rpmmd.PackageSet{packages}, repos) + res, err := solver.Depsolve(packages) if err != nil { panic("Could not depsolve: " + err.Error()) } - imgPackageSpecSets[name] = res.Dependencies + depsolvedSets[name] = res.Dependencies } - return imgPackageSpecSets + return depsolvedSets } func isOSTree(imgType distro.ImageType) bool { - packageSets := imgType.PackageSets(blueprint.Blueprint{}) - for _, pkg := range packageSets["build-packages"].Include { - if pkg == "rpm-ostree" { - return true + packageSets := imgType.PackageSets(blueprint.Blueprint{}, nil) + for _, set := range packageSets["build-packages"] { + for _, pkg := range set.Include { + if pkg == "rpm-ostree" { + return true + } } } return false @@ -167,14 +169,26 @@ var knownKernels = []string{"kernel", "kernel-debug", "kernel-rt"} // Returns the number of known kernels in the package list func kernelCount(imgType distro.ImageType) int { - sets := imgType.PackageSets(blueprint.Blueprint{}) - pkgs := sets["packages"].Append(sets["blueprint"]).Include + sets := imgType.PackageSets(blueprint.Blueprint{}, nil) n := 0 - for _, pkg := range pkgs { - for _, kernel := range knownKernels { - if kernel == pkg { - n++ + for _, pset := range sets["packages"] { + for _, pkg := range pset.Include { + for _, kernel := range knownKernels { + if kernel == pkg { + n++ + } } + + } + } + for _, bset := range sets["blueprint"] { + for _, pkg := range bset.Include { + for _, kernel := range knownKernels { + if kernel == pkg { + n++ + } + } + } } return n @@ -231,32 +245,9 @@ func GetTestingPackageSpecSets(packageName, arch string, pkgSetNames []string) m // defined by the provided ImageType, which is useful for unit testing. func GetTestingImagePackageSpecSets(packageName string, i distro.ImageType) map[string][]rpmmd.PackageSpec { arch := i.Arch().Name() - imagePackageSets := make([]string, 0, len(i.PackageSets(blueprint.Blueprint{}))) - for pkgSetName := range i.PackageSets(blueprint.Blueprint{}) { + imagePackageSets := make([]string, 0, len(i.PackageSets(blueprint.Blueprint{}, nil))) + for pkgSetName := range i.PackageSets(blueprint.Blueprint{}, nil) { imagePackageSets = append(imagePackageSets, pkgSetName) } return GetTestingPackageSpecSets(packageName, arch, imagePackageSets) } - -// Ensure that all package sets defined in the package set chains are defined for the image type -func TestImageType_PackageSetsChains(t *testing.T, d distro.Distro) { - distroName := d.Name() - for _, archName := range d.ListArches() { - arch, err := d.GetArch(archName) - require.Nil(t, err) - for _, imageTypeName := range arch.ListImageTypes() { - t.Run(fmt.Sprintf("%s/%s/%s", distroName, archName, imageTypeName), func(t *testing.T) { - imageType, err := arch.GetImageType(imageTypeName) - require.Nil(t, err) - - imagePkgSets := imageType.PackageSets(blueprint.Blueprint{}) - for _, pkgSetsChain := range imageType.PackageSetsChains() { - for _, packageSetName := range pkgSetsChain { - _, ok := imagePkgSets[packageSetName] - assert.Truef(t, ok, "package set %q defined in a package set chain is not present in the image package sets", packageSetName) - } - } - }) - } - } -} diff --git a/internal/distro/fedora/distro.go b/internal/distro/fedora/distro.go index 4e561fc65..5a50a72f0 100644 --- a/internal/distro/fedora/distro.go +++ b/internal/distro/fedora/distro.go @@ -548,7 +548,7 @@ func (t *imageType) getPackages(name string) rpmmd.PackageSet { return getter(t) } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch mergedSets := make(map[string]rpmmd.PackageSet) @@ -604,7 +604,7 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.Package // add bp kernel to main OS package set to avoid duplicate kernels mergedSets[osPkgsKey] = mergedSets[osPkgsKey].Append(rpmmd.PackageSet{Include: []string{kernel}}) - return mergedSets + return distro.MakePackageSetChains(t, mergedSets, repos) } diff --git a/internal/distro/fedora/distro_test.go b/internal/distro/fedora/distro_test.go index c9d2cd754..9f3ff3383 100644 --- a/internal/distro/fedora/distro_test.go +++ b/internal/distro/fedora/distro_test.go @@ -231,9 +231,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) diff --git a/internal/distro/rhel8/distro.go b/internal/distro/rhel8/distro.go index 3ef926576..89cfc6f1c 100644 --- a/internal/distro/rhel8/distro.go +++ b/internal/distro/rhel8/distro.go @@ -219,16 +219,18 @@ func (t *imageType) BuildPackages() []string { return packages } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { includePackages, excludePackages := t.Packages(bp) - return map[string]rpmmd.PackageSet{ - "packages": { - Include: includePackages, - Exclude: excludePackages, - }, - "build-packages": { - Include: t.BuildPackages(), - }, + return map[string][]rpmmd.PackageSet{ + "packages": {{ + Include: includePackages, + Exclude: excludePackages, + Repositories: repos, + }}, + "build-packages": {{ + Include: t.BuildPackages(), + Repositories: repos, + }}, } } diff --git a/internal/distro/rhel8/distro_test.go b/internal/distro/rhel8/distro_test.go index a110969be..f49282ee7 100644 --- a/internal/distro/rhel8/distro_test.go +++ b/internal/distro/rhel8/distro_test.go @@ -127,9 +127,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build-packages"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build-packages"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } } @@ -342,16 +343,17 @@ func TestImageType_BasePackages(t *testing.T) { for _, pkgMap := range pkgMaps { imgType, err := arch.GetImageType(pkgMap.name) assert.NoError(t, err) - packages := imgType.PackageSets(blueprint.Blueprint{})["packages"] + packages := imgType.PackageSets(blueprint.Blueprint{}, nil)["packages"] assert.NotNil(t, packages) + assert.Len(t, packages, 1) assert.Equalf( t, append(pkgMap.basePackages, pkgMap.bootloaderPackages...), - packages.Include, + packages[0].Include, "image type: %s", pkgMap.name, ) - assert.Equalf(t, pkgMap.excludedPackages, packages.Exclude, "image type: %s", pkgMap.name) + assert.Equalf(t, pkgMap.excludedPackages, packages[0].Exclude, "image type: %s", pkgMap.name) } } diff --git a/internal/distro/rhel84/distro.go b/internal/distro/rhel84/distro.go index de193d6b2..a9fb45fb1 100644 --- a/internal/distro/rhel84/distro.go +++ b/internal/distro/rhel84/distro.go @@ -258,16 +258,18 @@ func (t *imageType) BuildPackages() []string { return packages } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { includePackages, excludePackages := t.Packages(bp) - return map[string]rpmmd.PackageSet{ - "packages": { - Include: includePackages, - Exclude: excludePackages, - }, - "build-packages": { - Include: t.BuildPackages(), - }, + return map[string][]rpmmd.PackageSet{ + "packages": {{ + Include: includePackages, + Exclude: excludePackages, + Repositories: repos, + }}, + "build-packages": {{ + Include: t.BuildPackages(), + Repositories: repos, + }}, } } diff --git a/internal/distro/rhel84/distro_test.go b/internal/distro/rhel84/distro_test.go index afe9da809..f01016345 100644 --- a/internal/distro/rhel84/distro_test.go +++ b/internal/distro/rhel84/distro_test.go @@ -165,9 +165,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build-packages"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build-packages"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) @@ -407,8 +408,9 @@ func TestImageType_BasePackages(t *testing.T) { for _, pkgMap := range pkgMaps { imgType, err := arch.GetImageType(pkgMap.name) assert.NoError(t, err) - packages := imgType.PackageSets(blueprint.Blueprint{})["packages"] + packages := imgType.PackageSets(blueprint.Blueprint{}, nil)["packages"] assert.NotNil(t, packages) + assert.Len(t, packages, 1) expectedPackages := append(pkgMap.basePackages, pkgMap.bootloaderPackages...) if dist.name == "rhel" { expectedPackages = append(expectedPackages, pkgMap.rhelOnlyBasePackages...) @@ -416,11 +418,11 @@ func TestImageType_BasePackages(t *testing.T) { assert.ElementsMatchf( t, expectedPackages, - packages.Include, + packages[0].Include, "image type: %s", pkgMap.name, ) - assert.Equalf(t, pkgMap.excludedPackages, packages.Exclude, "image type: %s", pkgMap.name) + assert.Equalf(t, pkgMap.excludedPackages, packages[0].Exclude, "image type: %s", pkgMap.name) } }) } diff --git a/internal/distro/rhel84/distro_v2.go b/internal/distro/rhel84/distro_v2.go index d6d7f86f7..81ce1d1d9 100644 --- a/internal/distro/rhel84/distro_v2.go +++ b/internal/distro/rhel84/distro_v2.go @@ -113,21 +113,27 @@ func (t *imageTypeS2) BuildPackages() []string { return buildPackages } -func (t *imageTypeS2) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { - sets := map[string]rpmmd.PackageSet{ - "build-packages": { - Include: t.BuildPackages(), - }, +func (t *imageTypeS2) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { + sets := map[string][]rpmmd.PackageSet{ + "build-packages": {{ + Include: t.BuildPackages(), + Repositories: repos, + }}, } - for name, pkgSet := range t.packageSets { + for name := range t.packageSets { if name == "packages" { // treat base packages separately to combine with blueprint - packages := new(rpmmd.PackageSet) - packages.Include, packages.Exclude = t.Packages(bp) - sets[name] = *packages + include, exclude := t.Packages(bp) + sets[name] = []rpmmd.PackageSet{{ + Include: include, + Exclude: exclude, + Repositories: repos, + }} continue } - sets[name] = pkgSet + pkgSet := t.packageSets[name] + pkgSet.Repositories = repos + sets[name] = []rpmmd.PackageSet{pkgSet} } return sets } diff --git a/internal/distro/rhel85/distro.go b/internal/distro/rhel85/distro.go index 562972eb5..b5ca034c2 100644 --- a/internal/distro/rhel85/distro.go +++ b/internal/distro/rhel85/distro.go @@ -240,7 +240,7 @@ func (t *imageType) getPackages(name string) rpmmd.PackageSet { return getter(t) } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch mergedSets := make(map[string]rpmmd.PackageSet) @@ -293,8 +293,8 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.Package // add bp kernel to main OS package set to avoid duplicate kernels mergedSets[osPkgsKey] = mergedSets[osPkgsKey].Append(rpmmd.PackageSet{Include: []string{kernel}}) - return mergedSets + return distro.MakePackageSetChains(t, mergedSets, repos) } func (t *imageType) BuildPipelines() []string { diff --git a/internal/distro/rhel85/distro_test.go b/internal/distro/rhel85/distro_test.go index b67b5a812..8896f6ed1 100644 --- a/internal/distro/rhel85/distro_test.go +++ b/internal/distro/rhel85/distro_test.go @@ -253,9 +253,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) diff --git a/internal/distro/rhel86/distro.go b/internal/distro/rhel86/distro.go index f3b60c555..89c9dd62b 100644 --- a/internal/distro/rhel86/distro.go +++ b/internal/distro/rhel86/distro.go @@ -304,7 +304,7 @@ func (t *imageType) getPackages(name string) rpmmd.PackageSet { return getter(t) } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch mergedSets := make(map[string]rpmmd.PackageSet) @@ -357,8 +357,8 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.Package // add bp kernel to main OS package set to avoid duplicate kernels mergedSets[osPkgsKey] = mergedSets[osPkgsKey].Append(rpmmd.PackageSet{Include: []string{kernel}}) - return mergedSets + return distro.MakePackageSetChains(t, mergedSets, repos) } func (t *imageType) BuildPipelines() []string { diff --git a/internal/distro/rhel86/distro_test.go b/internal/distro/rhel86/distro_test.go index 2ad5b5584..80550b110 100644 --- a/internal/distro/rhel86/distro_test.go +++ b/internal/distro/rhel86/distro_test.go @@ -269,9 +269,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) diff --git a/internal/distro/rhel90/distro.go b/internal/distro/rhel90/distro.go index 46a26e6da..46421f5f0 100644 --- a/internal/distro/rhel90/distro.go +++ b/internal/distro/rhel90/distro.go @@ -304,7 +304,7 @@ func (t *imageType) getPackages(name string) rpmmd.PackageSet { return getter(t) } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch mergedSets := make(map[string]rpmmd.PackageSet) @@ -357,8 +357,8 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.Package // add bp kernel to main OS package set to avoid duplicate kernels mergedSets[osPkgsKey] = mergedSets[osPkgsKey].Append(rpmmd.PackageSet{Include: []string{kernel}}) - return mergedSets + return distro.MakePackageSetChains(t, mergedSets, repos) } func (t *imageType) BuildPipelines() []string { diff --git a/internal/distro/rhel90/distro_test.go b/internal/distro/rhel90/distro_test.go index 0ab8bed33..0aacfa4ba 100644 --- a/internal/distro/rhel90/distro_test.go +++ b/internal/distro/rhel90/distro_test.go @@ -261,9 +261,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) diff --git a/internal/distro/rhel90beta/distro.go b/internal/distro/rhel90beta/distro.go index 3e7a28538..4d9e4d5b8 100644 --- a/internal/distro/rhel90beta/distro.go +++ b/internal/distro/rhel90beta/distro.go @@ -240,7 +240,7 @@ func (t *imageType) Size(size uint64) uint64 { return size } -func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { +func (t *imageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { // merge package sets that appear in the image type with the package sets // of the same name from the distro and arch mergedSets := make(map[string]rpmmd.PackageSet) @@ -307,8 +307,8 @@ func (t *imageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.Package // add bp kernel to main OS package set to avoid duplicate kernels mergedSets[osPkgsKey] = mergedSets[osPkgsKey].Append(rpmmd.PackageSet{Include: []string{kernel}}) - return mergedSets + return distro.MakePackageSetChains(t, mergedSets, repos) } func (t *imageType) BuildPipelines() []string { diff --git a/internal/distro/rhel90beta/distro_test.go b/internal/distro/rhel90beta/distro_test.go index f4bc7767c..05cfe1f87 100644 --- a/internal/distro/rhel90beta/distro_test.go +++ b/internal/distro/rhel90beta/distro_test.go @@ -245,9 +245,10 @@ func TestImageType_BuildPackages(t *testing.T) { if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) { continue } - buildPkgs := itStruct.PackageSets(blueprint.Blueprint{})["build"] + buildPkgs := itStruct.PackageSets(blueprint.Blueprint{}, nil)["build"] assert.NotNil(t, buildPkgs) - assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs.Include) + assert.Len(t, buildPkgs, 1) + assert.ElementsMatch(t, buildPackages[archLabel], buildPkgs[0].Include) } } }) diff --git a/internal/distro/test_distro/distro.go b/internal/distro/test_distro/distro.go index 45fb89803..84f7b4ef3 100644 --- a/internal/distro/test_distro/distro.go +++ b/internal/distro/test_distro/distro.go @@ -185,25 +185,29 @@ func (t *TestImageType) PartitionType() string { return "" } -func (t *TestImageType) PackageSets(bp blueprint.Blueprint) map[string]rpmmd.PackageSet { - return map[string]rpmmd.PackageSet{ - buildPkgsKey: { +func (t *TestImageType) PackageSets(bp blueprint.Blueprint, repos []rpmmd.RepoConfig) map[string][]rpmmd.PackageSet { + return map[string][]rpmmd.PackageSet{ + buildPkgsKey: {{ Include: []string{ "dep-package1", "dep-package2", "dep-package3", }, + Repositories: repos, }, - blueprintPkgsKey: { - Include: bp.GetPackages(), }, - osPkgsKey: { + blueprintPkgsKey: {{ + Include: bp.GetPackages(), + Repositories: repos, + }}, + osPkgsKey: {{ Include: []string{ "dep-package1", "dep-package2", "dep-package3", }, - }, + Repositories: repos, + }}, } } diff --git a/internal/dnfjson/dnfjson.go b/internal/dnfjson/dnfjson.go index c225cc0fb..a2afe6d4e 100644 --- a/internal/dnfjson/dnfjson.go +++ b/internal/dnfjson/dnfjson.go @@ -87,16 +87,16 @@ func NewSolver(modulePlatformID string, releaseVer string, arch string, cacheDir } // Depsolve the given packages with explicit excludes using the given configuration and repos -func Depsolve(pkgSets []rpmmd.PackageSet, repos []rpmmd.RepoConfig, modulePlatformID string, releaseVer string, arch string, cacheDir string) (*DepsolveResult, error) { - return NewSolver(modulePlatformID, releaseVer, arch, cacheDir).Depsolve(pkgSets, repos) +func Depsolve(pkgSets []rpmmd.PackageSet, modulePlatformID string, releaseVer string, arch string, cacheDir string) (*DepsolveResult, error) { + return NewSolver(modulePlatformID, releaseVer, arch, cacheDir).Depsolve(pkgSets) } // Depsolve the list of required package sets with explicit excludes using -// the given repositories. Each package set is depsolved as a separate +// 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, baseRepos []rpmmd.RepoConfig) (*DepsolveResult, error) { - req, repoMap, err := s.makeDepsolveRequest(pkgSets, baseRepos) +func (s *Solver) Depsolve(pkgSets []rpmmd.PackageSet) (*DepsolveResult, error) { + req, repoMap, err := s.makeDepsolveRequest(pkgSets) if err != nil { return nil, err } @@ -205,7 +205,7 @@ type repoConfig struct { // NOTE: Due to implementation limitations of DNF and dnf-json, each package set // in the chain must use all of the repositories used by its predecessor. // An error is returned if this requirement is not met. -func (s *Solver) makeDepsolveRequest(pkgSets []rpmmd.PackageSet, baseRepos []rpmmd.RepoConfig) (*Request, map[string]rpmmd.RepoConfig, error) { +func (s *Solver) makeDepsolveRequest(pkgSets []rpmmd.PackageSet) (*Request, map[string]rpmmd.RepoConfig, error) { // dedupe repository configurations but maintain order // the order in which repositories are added to the request affects the @@ -213,14 +213,6 @@ func (s *Solver) makeDepsolveRequest(pkgSets []rpmmd.PackageSet, baseRepos []rpm repos := make([]rpmmd.RepoConfig, 0) rpmRepoMap := make(map[string]rpmmd.RepoConfig) - // These repo IDs will be used for all transactions in the chain - baseRepoIDs := make([]string, len(baseRepos)) - for idx, repo := range baseRepos { - id := repo.Hash() - rpmRepoMap[id] = repo - baseRepoIDs[idx] = id - repos = append(repos, repo) - } for _, ps := range pkgSets { for _, repo := range ps.Repositories { id := repo.Hash() @@ -236,7 +228,6 @@ func (s *Solver) makeDepsolveRequest(pkgSets []rpmmd.PackageSet, baseRepos []rpm transactions[dsIdx] = transactionArgs{ PackageSpecs: pkgSet.Include, ExcludeSpecs: pkgSet.Exclude, - RepoIDs: baseRepoIDs, // due to its capacity, the slice will be copied when appended to } for _, jobRepo := range pkgSet.Repositories { diff --git a/internal/dnfjson/dnfjson_test.go b/internal/dnfjson/dnfjson_test.go index b8f821204..3f6e26af4 100644 --- a/internal/dnfjson/dnfjson_test.go +++ b/internal/dnfjson/dnfjson_test.go @@ -42,9 +42,9 @@ func TestDepsolver(t *testing.T) { solver.SetDNFJSONPath("../../dnf-json") { // single depsolve - pkgsets := []rpmmd.PackageSet{{Include: []string{"kernel", "vim-minimal", "tmux", "zsh"}}} // everything you'll ever need + pkgsets := []rpmmd.PackageSet{{Include: []string{"kernel", "vim-minimal", "tmux", "zsh"}, Repositories: []rpmmd.RepoConfig{s.RepoConfig}}} // everything you'll ever need - deps, err := solver.Depsolve(pkgsets, []rpmmd.RepoConfig{s.RepoConfig}) + deps, err := solver.Depsolve(pkgsets) if err != nil { t.Fatal(err) } @@ -54,24 +54,10 @@ func TestDepsolver(t *testing.T) { { // chain depsolve of the same packages in order should produce the same result (at least in this case) pkgsets := []rpmmd.PackageSet{ - {Include: []string{"kernel"}}, - {Include: []string{"vim-minimal", "tmux", "zsh"}}, + {Include: []string{"kernel"}, Repositories: []rpmmd.RepoConfig{s.RepoConfig}}, + {Include: []string{"vim-minimal", "tmux", "zsh"}, Repositories: []rpmmd.RepoConfig{s.RepoConfig}}, } - deps, err := solver.Depsolve(pkgsets, []rpmmd.RepoConfig{s.RepoConfig}) - if err != nil { - t.Fatal(err) - } - exp := expectedResult(s.RepoConfig) - assert.Equal(deps, exp) - } - - { // adding the same repository to the psRepos for the second depsolve should have no effect - pkgsets := []rpmmd.PackageSet{ - {Include: []string{"kernel"}}, - {Include: []string{"vim-minimal", "tmux", "zsh"}}, - } - pkgsets[1].Repositories = []rpmmd.RepoConfig{s.RepoConfig} - deps, err := solver.Depsolve(pkgsets, []rpmmd.RepoConfig{s.RepoConfig}) + deps, err := solver.Depsolve(pkgsets) if err != nil { t.Fatal(err) } @@ -99,12 +85,10 @@ func TestMakeDepsolveRequest(t *testing.T) { BaseURL: "https://example.org/user-repo-2", } tests := []struct { - packageSets []rpmmd.PackageSet - repos []rpmmd.RepoConfig - packageSetsRepos [][]rpmmd.RepoConfig - args []transactionArgs - wantRepos []repoConfig - err bool + packageSets []rpmmd.PackageSet + args []transactionArgs + wantRepos []repoConfig + err bool }{ // single transaction { @@ -112,12 +96,12 @@ func TestMakeDepsolveRequest(t *testing.T) { { Include: []string{"pkg1"}, Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{ + baseOS, + appstream, + }, }, }, - repos: []rpmmd.RepoConfig{ - baseOS, - appstream, - }, args: []transactionArgs{ { PackageSpecs: []string{"pkg1"}, @@ -142,15 +126,15 @@ func TestMakeDepsolveRequest(t *testing.T) { { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { Include: []string{"pkg3"}, - Repositories: []rpmmd.RepoConfig{userRepo}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, args: []transactionArgs{ { PackageSpecs: []string{"pkg1"}, @@ -184,14 +168,15 @@ func TestMakeDepsolveRequest(t *testing.T) { { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { - Include: []string{"pkg3"}, + Include: []string{"pkg3"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, args: []transactionArgs{ { PackageSpecs: []string{"pkg1"}, @@ -220,19 +205,19 @@ func TestMakeDepsolveRequest(t *testing.T) { { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { Include: []string{"pkg3"}, - Repositories: []rpmmd.RepoConfig{userRepo}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo}, }, { Include: []string{"pkg4"}, - Repositories: []rpmmd.RepoConfig{userRepo}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, args: []transactionArgs{ { PackageSpecs: []string{"pkg1"}, @@ -271,19 +256,19 @@ func TestMakeDepsolveRequest(t *testing.T) { { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { Include: []string{"pkg3"}, - Repositories: []rpmmd.RepoConfig{userRepo}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo}, }, { Include: []string{"pkg4"}, - Repositories: []rpmmd.RepoConfig{userRepo, userRepo2}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo, userRepo2}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, args: []transactionArgs{ { PackageSpecs: []string{"pkg1"}, @@ -326,44 +311,45 @@ func TestMakeDepsolveRequest(t *testing.T) { { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { Include: []string{"pkg3"}, - Repositories: []rpmmd.RepoConfig{userRepo}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo}, }, { Include: []string{"pkg4"}, - Repositories: []rpmmd.RepoConfig{userRepo2}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo2}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, - err: true, + err: true, }, // Error: 3 transactions but last one doesn't specify user repos in 2nd { packageSets: []rpmmd.PackageSet{ { - Include: []string{"pkg1"}, - Exclude: []string{"pkg2"}, + Include: []string{"pkg1"}, + Exclude: []string{"pkg2"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, { Include: []string{"pkg3"}, - Repositories: []rpmmd.RepoConfig{userRepo, userRepo2}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream, userRepo, userRepo2}, }, { - Include: []string{"pkg4"}, + Include: []string{"pkg4"}, + Repositories: []rpmmd.RepoConfig{baseOS, appstream}, }, }, - repos: []rpmmd.RepoConfig{baseOS, appstream}, - err: true, + err: true, }, } solver := NewSolver("", "", "", "") for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - req, _, err := solver.makeDepsolveRequest(tt.packageSets, tt.repos) + req, _, err := solver.makeDepsolveRequest(tt.packageSets) if tt.err { assert.NotNilf(t, err, "expected an error, but got 'nil' instead") assert.Nilf(t, req, "got non-nill request, but expected an error") diff --git a/internal/kojiapi/server.go b/internal/kojiapi/server.go index 3762b58d5..2513a5fd4 100644 --- a/internal/kojiapi/server.go +++ b/internal/kojiapi/server.go @@ -124,21 +124,18 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { } solver := h.server.solver.NewWithConfig(d.ModulePlatformID(), d.Releasever(), arch.Name()) - packageSets := imageType.PackageSets(*bp) - packageSpecSets := make(map[string][]rpmmd.PackageSpec) - for specName, setNames := range imageType.PackageSetsChains() { - chain := make([]rpmmd.PackageSet, len(setNames)) - for idx, pkgSetName := range setNames { - chain[idx] = packageSets[pkgSetName] - } - res, err := solver.Depsolve(chain, repositories) + packageSets := imageType.PackageSets(*bp, repositories) + depsolvedSets := make(map[string][]rpmmd.PackageSpec, len(packageSets)) + + for name, pkgSet := range packageSets { + res, err := solver.Depsolve(pkgSet) if err != nil { return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Failed to depsolve base packages for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err)) } - packageSpecSets[specName] = res.Dependencies + depsolvedSets[name] = res.Dependencies } - manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, repositories, packageSpecSets, manifestSeed) + manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, repositories, depsolvedSets, manifestSeed) if err != nil { return echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("Failed to get manifest for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err)) } diff --git a/internal/rpmmd/repository.go b/internal/rpmmd/repository.go index 4cf737bc9..6091cf329 100644 --- a/internal/rpmmd/repository.go +++ b/internal/rpmmd/repository.go @@ -38,6 +38,7 @@ type RepoConfig struct { MetadataExpire string RHSM bool ImageTypeTags []string + PackageSets []string } // Hash calculates an ID string that uniquely represents a repository diff --git a/internal/weldr/api.go b/internal/weldr/api.go index c426a6465..352d6403f 100644 --- a/internal/weldr/api.go +++ b/internal/weldr/api.go @@ -1261,7 +1261,7 @@ func (api *API) modulesInfoHandler(writer http.ResponseWriter, request *http.Req solver := api.solver.NewWithConfig(d.ModulePlatformID(), d.Releasever(), api.archName) for i := range packageInfos { pkgName := packageInfos[i].Name - solved, err := solver.Depsolve([]rpmmd.PackageSet{{Include: []string{pkgName}}}, repos) + solved, err := solver.Depsolve([]rpmmd.PackageSet{{Include: []string{pkgName}, Repositories: repos}}) if err != nil { errors := responseError{ ID: errorId, @@ -1339,7 +1339,7 @@ func (api *API) projectsDepsolveHandler(writer http.ResponseWriter, request *htt } solver := api.solver.NewWithConfig(d.ModulePlatformID(), d.Releasever(), api.archName) - deps, err := solver.Depsolve([]rpmmd.PackageSet{{Include: names}}, repos) + deps, err := solver.Depsolve([]rpmmd.PackageSet{{Include: names, Repositories: repos}}) if err != nil { errors := responseError{ ID: "ProjectsError", @@ -2132,42 +2132,19 @@ func (api *API) depsolveBlueprintForImageType(bp blueprint.Blueprint, imageType return nil, fmt.Errorf("Blueprint distro %s does not match imageType distro %s", bp.Distro, imageType.Arch().Distro().Name()) } - imageTypeRepos, payloadRepos, err := api.allRepositoriesByImageType(imageType) + imageTypeRepos, err := api.allRepositoriesByImageType(imageType) if err != nil { return nil, err } - packageSetsRepos := make(map[string][]rpmmd.RepoConfig, len(imageType.PayloadPackageSets())) - for _, name := range imageType.PayloadPackageSets() { - packageSetsRepos[name] = payloadRepos - } - platformID := imageType.Arch().Distro().ModulePlatformID() releasever := imageType.Arch().Distro().Releasever() solver := api.solver.NewWithConfig(platformID, releasever, api.archName) - packageSets := imageType.PackageSets(bp) - depsolvedSets := make(map[string][]rpmmd.PackageSpec) - // first depsolve package sets that are part of a chain - for specName, setNames := range imageType.PackageSetsChains() { - pkgSets := make([]rpmmd.PackageSet, len(setNames)) + packageSets := imageType.PackageSets(bp, imageTypeRepos) + depsolvedSets := make(map[string][]rpmmd.PackageSpec, len(packageSets)) - // add package-set-specific repositories to each set if one is defined - for idx, pkgSetName := range setNames { - pkgSets[idx] = packageSets[pkgSetName] - pkgSets[idx].Repositories = packageSetsRepos[pkgSetName] // will be nil if it doesn't exist - delete(packageSets, pkgSetName) // will be depsolved here: remove from map - } - res, err := solver.Depsolve(pkgSets, imageTypeRepos) - if err != nil { - return nil, err - } - depsolvedSets[specName] = res.Dependencies - } - - // depsolve the rest of the package sets for name, pkgSet := range packageSets { - pkgSet.Repositories = packageSetsRepos[name] - res, err := solver.Depsolve([]rpmmd.PackageSet{pkgSet}, imageTypeRepos) + res, err := solver.Depsolve(pkgSet) if err != nil { return nil, err } @@ -2319,7 +2296,7 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request } seed := bigSeed.Int64() - imageRepos, payloadRepos, err := api.allRepositoriesByImageType(imageType) + imageRepos, err := api.allRepositoriesByImageType(imageType) // this should not happen if the api.depsolveBlueprintForImageType() call above worked if err != nil { errors := responseError{ @@ -2329,7 +2306,6 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request statusResponseError(writer, http.StatusInternalServerError, errors) return } - imageRepos = append(imageRepos, payloadRepos...) manifest, err := imageType.Manifest(bp.Customizations, distro.ImageOptions{ @@ -3166,15 +3142,20 @@ func (api *API) payloadRepositories(distroName string) []rpmmd.RepoConfig { // The difference from allRepositories() is that this method may return additional repositories, // which are needed to build the specific image type. The allRepositories() can't do this, because // it is used in places where image types are not considered. -func (api *API) allRepositoriesByImageType(imageType distro.ImageType) ([]rpmmd.RepoConfig, []rpmmd.RepoConfig, error) { +func (api *API) allRepositoriesByImageType(imageType distro.ImageType) ([]rpmmd.RepoConfig, error) { repos, err := api.repoRegistry.ReposByImageType(imageType) if err != nil { - return nil, nil, err + return nil, err } payloadRepos := api.payloadRepositories(imageType.Arch().Distro().Name()) + // tag payload repositories with the payload package set names and add them to list of repos + for _, pr := range payloadRepos { + pr.ImageTypeTags = imageType.PayloadPackageSets() + repos = append(repos, pr) + } - return repos, payloadRepos, nil + return repos, nil } // Returns all configured repositories (base + sources) as rpmmd.RepoConfig @@ -3206,7 +3187,7 @@ func (api *API) depsolveBlueprint(bp blueprint.Blueprint) ([]rpmmd.PackageSpec, } solver := api.solver.NewWithConfig(d.ModulePlatformID(), d.Releasever(), api.archName) - solved, err := solver.Depsolve([]rpmmd.PackageSet{{Include: bp.GetPackages()}}, repos) + solved, err := solver.Depsolve([]rpmmd.PackageSet{{Include: bp.GetPackages(), Repositories: repos}}) if err != nil { return nil, err } diff --git a/internal/worker/json.go b/internal/worker/json.go index 293d21c75..66cdce1b9 100644 --- a/internal/worker/json.go +++ b/internal/worker/json.go @@ -105,19 +105,16 @@ func (pn *PipelineNames) All() []string { return append(pn.Build, pn.Payload...) } -// DepsolveJob defines the parameters of one or more depsolve jobs. Each -// package set is meant to be depsolved separately. The repositories defined -// in Repos are used for all package sets, whereas the repositories defined in -// PackageSetsRepos are only used by the package sets that share the same name -// (map key). +// DepsolveJob defines the parameters of one or more depsolve jobs. Each named +// list of package sets defines a separate job. Lists with multiple package +// sets are depsolved in a chain, combining the results of sequential depsolves +// into a single PackageSpec list in the result. Each PackageSet defines the +// repositories it will be depsolved against. type DepsolveJob struct { - PackageSetsChains map[string][]string `json:"package_sets_chains"` - PackageSets map[string]rpmmd.PackageSet `json:"package_sets"` - Repos []rpmmd.RepoConfig `json:"repos"` - ModulePlatformID string `json:"module_platform_id"` - Arch string `json:"arch"` - Releasever string `json:"releasever"` - PackageSetsRepos map[string][]rpmmd.RepoConfig `json:"package_sets_repositories,omitempty"` + PackageSets map[string][]rpmmd.PackageSet `json:"package_sets"` + ModulePlatformID string `json:"module_platform_id"` + Arch string `json:"arch"` + Releasever string `json:"releasever"` } type ErrorType string