Skip to content

Commit

Permalink
manifest: extract tomlPkgsFor() helper and fix ordering
Browse files Browse the repository at this point in the history
The current code will install python3-toml on fedora and also
on rhel11 (once it comes out) because that is the default for
unhandled distros.

This commit makes the various toml libs for each distro more
explicit, see also osbuild/osbuild#1851
  • Loading branch information
mvo5 committed Sep 19, 2024
1 parent b78165e commit 23bba03
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 17 deletions.
38 changes: 21 additions & 17 deletions pkg/manifest/os.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ type OS struct {
// OSTreeParent source spec (optional). If nil the new commit (if
// applicable) will have no parent
OSTreeParent *ostree.SourceSpec

// Enabling Bootupd runs bootupctl generate-update-metadata in the tree to
// transform /usr/lib/ostree-boot into a bootupd-compatible update
// payload. Only works with ostree-based images.
Expand Down Expand Up @@ -289,6 +288,25 @@ func (p *OS) getContainerSources() []container.SourceSpec {
return p.OSCustomizations.Containers
}

func tomlPkgsFor(distro Distro) []string {
switch distro {
case DISTRO_EL7:
// nothing needs toml in rhel7
panic("no support for toml on rhel7")
case DISTRO_EL8:
// deprecated, needed for backwards compatibility (EL8 manifests)
return []string{"python3-pytoml"}
case DISTRO_EL9:
// older unmaintained lib, needed for backwards compatibility
return []string{"python3-toml"}
default:
// No extra package needed for reading, on rhel10 and
// fedora as stdlib has "tomlib" but we need tomli-w
// for writing
return []string{"python3-tomli-w"}
}
}

func (p *OS) getBuildPackages(distro Distro) []string {
packages := p.platform.GetBuildPackages()
if p.PartitionTable != nil {
Expand All @@ -315,14 +333,7 @@ func (p *OS) getBuildPackages(distro Distro) []string {

if len(p.OSCustomizations.Containers) > 0 {
if p.OSCustomizations.ContainersStorage != nil {
switch distro {
case DISTRO_EL8:
packages = append(packages, "python3-pytoml")
case DISTRO_EL10:
// no extra package needed, stdlib has "tomli"
default:
packages = append(packages, "python3-toml")
}
packages = append(packages, tomlPkgsFor(distro)...)
}
packages = append(packages, "skopeo")
}
Expand All @@ -332,14 +343,7 @@ func (p *OS) getBuildPackages(distro Distro) []string {
}

if p.BootcConfig != nil {
switch distro {
case DISTRO_EL8:
packages = append(packages, "python3-pytoml")
case DISTRO_EL10:
// no extra package needed, stdlib has "tomli"
default:
packages = append(packages, "python3-toml")
}
packages = append(packages, tomlPkgsFor(distro)...)
}

return packages
Expand Down
43 changes: 43 additions & 0 deletions pkg/manifest/os_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import (
"fmt"
"testing"

"github.com/osbuild/images/internal/common"
"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/customizations/bootc"
"github.com/osbuild/images/pkg/customizations/subscription"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/platform"
Expand Down Expand Up @@ -181,3 +184,43 @@ func TestBootupdStage(t *testing.T) {
st := findStage("org.osbuild.bootupd.gen-metadata", pipeline.Stages)
require.NotNil(t, st)
}

func TestTomlLibUsedNoneByDefault(t *testing.T) {
os := NewTestOS()
buildPkgs := os.getBuildPackages(DISTRO_FEDORA)
for _, pkg := range []string{"python3-pytoml", "python3-toml", "python3-tomli-w"} {
assert.NotContains(t, buildPkgs, pkg)
}
}

func TestTomlLibUsedForContainer(t *testing.T) {
os := NewTestOS()
os.OSCustomizations.Containers = []container.SourceSpec{
{Source: "some-source"},
}
os.OSCustomizations.ContainersStorage = common.ToPtr("foo")

testTomlPkgsFor(t, os)
}

func TestTomlLibUsedForBootcConfig(t *testing.T) {
os := NewTestOS()
os.BootcConfig = &bootc.Config{Filename: "something"}

testTomlPkgsFor(t, os)
}

func testTomlPkgsFor(t *testing.T, os *OS) {
for _, tc := range []struct {
distro Distro
expectedTomlPkg string
}{
{DISTRO_EL8, "python3-pytoml"},
{DISTRO_EL9, "python3-toml"},
{DISTRO_EL10, "python3-tomli-w"},
{DISTRO_FEDORA, "python3-tomli-w"},
} {
buildPkgs := os.getBuildPackages(tc.distro)
assert.Contains(t, buildPkgs, tc.expectedTomlPkg)
}
}

0 comments on commit 23bba03

Please sign in to comment.