From d70d2738250f2a56a30b3d6f060f934999759141 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 17 Oct 2025 16:11:14 +0200 Subject: [PATCH 1/9] feat: format v3 Format v3 uses a map instead of a list for `essential` and deprecates the field `v3-essential` which is only used during the transition from v2 to v3. --- cmd/chisel/cmd_info.go | 1 + cmd/chisel/cmd_info_test.go | 2 +- internal/setup/setup.go | 6 +- internal/setup/setup_test.go | 355 ++++++++++++++++++++++++++++++----- internal/setup/yaml.go | 202 +++++++++++++------- 5 files changed, 458 insertions(+), 108 deletions(-) diff --git a/cmd/chisel/cmd_info.go b/cmd/chisel/cmd_info.go index fa5485c6..ceabfca8 100644 --- a/cmd/chisel/cmd_info.go +++ b/cmd/chisel/cmd_info.go @@ -123,6 +123,7 @@ func selectPackageSlices(release *setup.Release, queries []string) (packages []* } else { releasePkg := release.Packages[pkgName] pkg = &setup.Package{ + Format: release.Format, Name: releasePkg.Name, Archive: releasePkg.Archive, Slices: make(map[string]*setup.Slice), diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index b3fd6eb0..d2dc1408 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -138,7 +138,7 @@ var infoTests = []infoTest{{ }} var infoRelease = map[string]string{ - "chisel.yaml": string(testutil.DefaultChiselYaml), + "chisel.yaml": testutil.DefaultChiselYaml, "slices/mypkg1.yaml": ` package: mypkg1 essential: diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 46b5ca94..12431114 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -22,6 +22,8 @@ type Release struct { Packages map[string]*Package Archives map[string]*Archive Maintenance *Maintenance + // Format is used for marshaling, unmarshalling and error checking. + Format string } type Maintenance struct { @@ -53,6 +55,8 @@ type Package struct { Path string Archive string Slices map[string]*Slice + // Format is used for marshaling, unmarshalling and error checking. + Format string } // Slice holds the details about a package slice. @@ -447,7 +451,7 @@ func readSlices(release *Release, baseDir, dirName string) error { return fmt.Errorf("cannot read slice definition file: %v", err) } - pkg, err := parsePackage(baseDir, pkgName, stripBase(baseDir, pkgPath), data) + pkg, err := parsePackage(release.Format, baseDir, pkgName, stripBase(baseDir, pkgPath), data) if err != nil { return err } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index ac956ce1..a145f97a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -78,6 +78,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -90,6 +91,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -124,6 +126,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -136,8 +139,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -188,6 +192,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -200,8 +205,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -451,6 +457,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -463,8 +470,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -668,6 +676,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -680,8 +689,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -710,6 +720,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -722,8 +733,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -753,6 +765,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -765,8 +778,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -815,6 +829,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "foo": { Name: "foo", @@ -837,6 +852,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -1030,6 +1046,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1042,8 +1059,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1094,6 +1112,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "foo": { Name: "foo", @@ -1116,6 +1135,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -1216,6 +1236,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1228,8 +1249,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "jq": { - Name: "jq", - Path: "slices/mydir/jq.yaml", + Format: "v1", + Name: "jq", + Path: "slices/mydir/jq.yaml", Slices: map[string]*setup.Slice{ "bins": { Package: "jq", @@ -1284,6 +1306,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1296,8 +1319,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1356,6 +1380,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1368,8 +1393,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1390,8 +1416,9 @@ var setupTests = []setupTest{{ }, }, "myotherpkg": { - Name: "myotherpkg", - Path: "slices/mydir/myotherpkg.yaml", + Format: "v1", + Name: "myotherpkg", + Path: "slices/mydir/myotherpkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "myotherpkg", @@ -1538,6 +1565,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1550,8 +1578,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1590,6 +1619,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1602,8 +1632,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1684,6 +1715,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1696,8 +1728,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1709,8 +1742,9 @@ var setupTests = []setupTest{{ }, }, "mypkg2": { - Name: "mypkg2", - Path: "slices/mydir/mypkg2.yaml", + Format: "v1", + Name: "mypkg2", + Path: "slices/mydir/mypkg2.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg2", @@ -1835,6 +1869,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "default": { Name: "default", @@ -1866,6 +1901,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -1938,6 +1974,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -1991,6 +2028,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2035,6 +2073,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "default": { Name: "default", @@ -2057,6 +2096,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2128,6 +2168,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -2151,6 +2192,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2264,6 +2306,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -2276,8 +2319,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg1": { - Name: "mypkg1", - Path: "slices/mydir/mypkg1.yaml", + Format: "v1", + Name: "mypkg1", + Path: "slices/mydir/mypkg1.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg1", @@ -2297,8 +2341,9 @@ var setupTests = []setupTest{{ }, }, "mypkg2": { - Name: "mypkg2", - Path: "slices/mydir/mypkg2.yaml", + Format: "v1", + Name: "mypkg2", + Path: "slices/mydir/mypkg2.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg2", @@ -2311,8 +2356,9 @@ var setupTests = []setupTest{{ }, }, "mypkg3": { - Name: "mypkg3", - Path: "slices/mydir/mypkg3.yaml", + Format: "v1", + Name: "mypkg3", + Path: "slices/mydir/mypkg3.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg3", @@ -2697,6 +2743,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -2710,6 +2757,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2746,6 +2794,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -2759,6 +2808,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2893,6 +2943,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -2947,6 +2998,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3014,6 +3066,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -3068,6 +3121,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3135,6 +3189,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -3189,6 +3244,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3256,6 +3312,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -3310,6 +3367,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3375,6 +3433,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -3429,6 +3488,7 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { + Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3459,6 +3519,7 @@ var setupTests = []setupTest{{ `, }, release: &setup.Release{ + Format: "v1", Archives: map[string]*setup.Archive{ "ubuntu": { Name: "ubuntu", @@ -3471,8 +3532,9 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Format: "v1", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -3548,6 +3610,62 @@ var setupTests = []setupTest{{ `, }, relerror: `package "mypkg" repeats mypkg_myslice2 in essential fields`, +}, { + summary: "Format v3 expects a map in 'essential' (pkg)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + essential: + - mypkg_myslice2 + slices: + myslice1: + myslice2: + `, + }, + relerror: `cannot parse package "mypkg" slice definitions: (.|\n)*`, +}, { + summary: "Format v3 expects a map in 'essential' (slice)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice1: + essential: + - mypkg_myslice2 + myslice2: + `, + }, + relerror: `cannot parse package "mypkg" slice definitions: (.|\n)*`, +}, { + summary: "In format v3 'v3-essential' is not supported (pkg)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + v3-essential: + mypkg_myslice2: + slices: + myslice1: + myslice2: + `, + }, + relerror: `package "mypkg": v3-essential is deprecated since format v3`, +}, { + summary: "In format v3 'v3-essential' is not supported (slice)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice1: + v3-essential: + mypkg_myslice2: + myslice2: + `, + }, + relerror: `slice mypkg_myslice1: v3-essential is deprecated since format v3`, }} func (s *S) TestParseRelease(c *C) { @@ -3560,7 +3678,7 @@ func (s *S) TestParseRelease(c *C) { m := make(map[string]string) for k, v := range t.input { if !strings.Contains(v, "v2-archives:") && strings.Contains(v, "format: v1") { - v = strings.Replace(v, "archives:", "v2-archives:", -1) + v = strings.ReplaceAll(v, "archives:", "v2-archives:") } m[k] = v } @@ -3572,19 +3690,70 @@ func (s *S) TestParseRelease(c *C) { // Run tests for "v2" format. v2FormatTests := make([]setupTest, 0, len(setupTests)) for _, t := range setupTests { + t.summary += " (v2)" m := make(map[string]string) + skip := false for k, v := range t.input { - if strings.Contains(v, "format: v1") && - !strings.Contains(v, "v2-archives:") && - !strings.Contains(v, "default: true") { - v = strings.Replace(v, "format: v1", "format: v2", -1) + if strings.Contains(v, "format: v2") || + strings.Contains(v, "format: v3") || + strings.Contains(v, "v2-archives:") || + strings.Contains(v, "default: true") { + skip = true + break } + v = strings.ReplaceAll(v, "format: v1", "format: v2") m[k] = v } + if skip { + // Test was not affected, no need to re-run. + continue + } t.input = m + if t.release != nil { + t.release.Format = "v2" + for _, pkg := range t.release.Packages { + pkg.Format = "v2" + } + } v2FormatTests = append(v2FormatTests, t) } runParseReleaseTests(c, v2FormatTests) + + // Run tests for "v3" format. + v3FormatTests := make([]setupTest, 0, len(setupTests)) + for _, t := range setupTests { + t.summary += " (v3)" + m := make(map[string]string) + skip := false + for k, v := range t.input { + if strings.Contains(v, "format: v2") || + strings.Contains(v, "format: v3") || + strings.Contains(v, "v2-archives:") || + strings.Contains(v, "default: true") { + skip = true + break + } + v, skip = oldEssentialToV3(c, testutil.Reindent(v)) + if skip { + break + } + v = strings.ReplaceAll(v, "format: v1", "format: v3") + m[k] = v + } + if skip { + // Test was not affected, or it is not meaningful, no need to re-run. + continue + } + t.input = m + if t.release != nil { + t.release.Format = "v3" + for _, pkg := range t.release.Packages { + pkg.Format = "v3" + } + } + v3FormatTests = append(v3FormatTests, t) + } + runParseReleaseTests(c, v3FormatTests) } func runParseReleaseTests(c *C, tests []setupTest) { @@ -3789,6 +3958,39 @@ func (s *S) TestPackageYAMLFormat(c *C) { /dir/prefer: {} `, }, + }, { + summary: "Format v3", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mypkg.yaml": ` + package: mypkg + archive: ubuntu + essential: + mypkg_three: {arch: i386} + slices: + one: + essential: + mypkg_two: {arch: [amd64, aarch64]} + two: + three: + `, + }, + expected: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mypkg.yaml": ` + package: mypkg + archive: ubuntu + slices: + one: + essential: + mypkg_three: {arch: i386} + mypkg_two: {arch: [amd64, aarch64]} + three: {} + two: + essential: + mypkg_three: {arch: i386} + `, + }, }} for _, test := range tests { @@ -3850,3 +4052,70 @@ func (s *S) TestYAMLPathGenerate(c *C) { c.Assert(result, Equals, test.result) } } + +// oldEssentialToV3 converts the essentials in v1 and v2, both 'essential', and +// 'v3-essential' to the shape expected by the v3 format. +// skip is set to true when an accurate translation of the test is not +// possible, for example having duplicates in the list. +func oldEssentialToV3(c *C, input []byte) (out string, skip bool) { + var raw map[string]any + err := yaml.Unmarshal(input, &raw) + c.Assert(err, IsNil) + + if slices, ok := raw["slices"].(map[string]any); ok { + for _, rawSlice := range slices { + if slice, ok := rawSlice.(map[string]any); ok { + newEssential := make(map[string]any) + if oldEssential, ok := slice["essential"].([]any); ok { + for _, value := range oldEssential { + s := value.(string) + if _, ok := newEssential[s]; ok { + // Duplicated entries are impossible in v3. + return "", true + } + newEssential[s] = nil + } + } + if oldEssential, ok := slice["v3-essential"].(map[string]any); ok { + for key, value := range oldEssential { + if _, ok := newEssential[key]; ok { + return "", true + return "", true + } + newEssential[key] = value + } + delete(slice, "v3-essential") + } + slice["essential"] = newEssential + } + } + } + + newEssential := make(map[string]any) + if oldEssential, ok := raw["essential"].([]any); ok { + for _, item := range oldEssential { + s := item.(string) + if _, ok := newEssential[s]; ok { + // Duplicated entries are impossible in v3. + return "", true + } + newEssential[s] = nil + } + } + if oldEssential, ok := raw["v3-essential"].(map[string]any); ok { + for key, value := range oldEssential { + if _, ok := newEssential[key]; ok { + // Duplicated entries are impossible in v3. + return "", true + } + newEssential[key] = value + } + delete(raw, "v3-essential") + } + raw["essential"] = newEssential + + bs, err := yaml.Marshal(raw) + c.Assert(err, IsNil) + // Maintenance dates get marshaled as T00:00:00Z by default. + return strings.ReplaceAll(string(bs), "T00:00:00Z", ""), false +} diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index b7fc65b4..f704a219 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -19,7 +19,35 @@ import ( ) func (p *Package) MarshalYAML() (any, error) { - return packageToYAML(p) + if p.Format == "v1" || p.Format == "v2" { + oldPkg := &v1v2YAMLPackage{ + Name: p.Name, + Archive: p.Archive, + Slices: make(map[string]v1v2YAMLSlice, len(p.Slices)), + } + for name, slice := range p.Slices { + yamlSlice, err := v1v2SliceToYAML(slice) + if err != nil { + return nil, err + } + oldPkg.Slices[name] = *yamlSlice + } + return oldPkg, nil + } + + pkg := &yamlPackage{ + Name: p.Name, + Archive: p.Archive, + Slices: make(map[string]yamlSlice, len(p.Slices)), + } + for name, slice := range p.Slices { + yamlSlice, err := sliceToYAML(slice) + if err != nil { + return nil, err + } + pkg.Slices[name] = *yamlSlice + } + return pkg, nil } var _ yaml.Marshaler = (*Package)(nil) @@ -52,13 +80,14 @@ type yamlArchive struct { } type yamlPackage struct { - Name string `yaml:"package"` - Archive string `yaml:"archive,omitempty"` - Essential []string `yaml:"essential,omitempty"` - Slices map[string]yamlSlice `yaml:"slices,omitempty"` - // "v3-essential" is used for backwards porting of arch-specific essential - // to releases that use "v1" or "v2". When using older versions of Chisel - // the field will be ignored and `essential` is used as a fallback. + Name string `yaml:"package"` + Archive string `yaml:"archive,omitempty"` + Slices map[string]yamlSlice `yaml:"slices,omitempty"` + // For backwards-compatibility reasons with v1 and v2, essential needs + // custom logic to be parsed. See [parsePackage]. + Essential map[string]*yamlEssential `yaml:"essential,omitempty"` + // It is only used to present an error message to the user if the field + // is used in v3. V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` } @@ -146,12 +175,13 @@ func (ym yamlMode) MarshalYAML() (any, error) { var _ yaml.Marshaler = yamlMode(0) type yamlSlice struct { - Essential []string `yaml:"essential,omitempty"` - Contents map[string]*yamlPath `yaml:"contents,omitempty"` - Mutate string `yaml:"mutate,omitempty"` - // "v3-essential" is used for backwards porting of arch-specific essential - // to releases that use "v1" or "v2". When using older versions of Chisel - // the field will be ignored and `essential` is used as a fallback. + Contents map[string]*yamlPath `yaml:"contents,omitempty"` + Mutate string `yaml:"mutate,omitempty"` + // For backwards-compatibility reasons with v1 and v2, essential needs + // custom logic to be parsed. See [parsePackage]. + Essential map[string]*yamlEssential `yaml:"essential,omitempty"` + // It is only used to present an error message to the user if the field + // is used in v3. V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` } @@ -193,13 +223,14 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { if err != nil { return nil, fmt.Errorf("%s: cannot parse release definition: %v", fileName, err) } - if yamlVar.Format != "v1" && yamlVar.Format != "v2" { + if yamlVar.Format != "v1" && yamlVar.Format != "v2" && yamlVar.Format != "v3" { return nil, fmt.Errorf("%s: unknown format %q", fileName, yamlVar.Format) } + release.Format = yamlVar.Format + if yamlVar.Format != "v1" && len(yamlVar.V2Archives) > 0 { return nil, fmt.Errorf("%s: v2-archives is deprecated since format v2", fileName) } - if len(yamlVar.Archives)+len(yamlVar.V2Archives) == 0 { return nil, fmt.Errorf("%s: no archives defined", fileName) } @@ -373,35 +404,74 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { return release, err } -func parsePackage(baseDir, pkgName, pkgPath string, data []byte) (*Package, error) { +func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Package, error) { pkg := Package{ Name: pkgName, Path: pkgPath, Slices: make(map[string]*Slice), + Format: format, } yamlPkg := yamlPackage{} dec := yaml.NewDecoder(bytes.NewBuffer(data)) dec.KnownFields(false) - err := dec.Decode(&yamlPkg) - if err != nil { - return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) + if format == "v1" || format == "v2" { + oldPkg := &v1v2YAMLPackage{} + err := dec.Decode(&oldPkg) + if err != nil { + return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) + } + if oldPkg.V3Essential == nil { + oldPkg.V3Essential = map[string]*yamlEssential{} + } + for _, refName := range oldPkg.Essential { + if _, ok := oldPkg.V3Essential[refName]; ok { + // This check is only needed because the list format can contain + // duplicates. It should be removed when format "v2" is deprecated. + return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) + } + oldPkg.V3Essential[refName] = &yamlEssential{} + } + yamlPkg.Archive = oldPkg.Archive + yamlPkg.Name = oldPkg.Name + yamlPkg.Slices = map[string]yamlSlice{} + for sliceName, oldSlice := range oldPkg.Slices { + if oldSlice.V3Essential == nil { + oldSlice.V3Essential = map[string]*yamlEssential{} + } + for _, refName := range oldSlice.Essential { + if _, ok := oldSlice.V3Essential[refName]; ok { + // This check is only needed because the list format can contain + // duplicates. It should be removed when format "v2" is deprecated. + return nil, fmt.Errorf("slice %s repeats %s in essential fields", SliceKey{pkgName, sliceName}, refName) + } + oldSlice.V3Essential[refName] = &yamlEssential{} + } + yamlPkg.Slices[sliceName] = yamlSlice{ + Contents: oldSlice.Contents, + Mutate: oldSlice.Mutate, + Essential: oldSlice.V3Essential, + } + } + yamlPkg.Essential = oldPkg.V3Essential + } else { + err := dec.Decode(&yamlPkg) + if err != nil { + return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) + } + if yamlPkg.V3Essential != nil { + return nil, fmt.Errorf("package %q: v3-essential is deprecated since format v3", pkgName) + } + for sliceName, slice := range yamlPkg.Slices { + if len(slice.V3Essential) > 0 { + return nil, fmt.Errorf("slice %s: v3-essential is deprecated since format v3", SliceKey{pkgName, sliceName}) + } + } } + if yamlPkg.Name != pkg.Name { return nil, fmt.Errorf("%s: filename and 'package' field (%q) disagree", pkgPath, yamlPkg.Name) } - if yamlPkg.V3Essential == nil { - yamlPkg.V3Essential = map[string]*yamlEssential{} - } - for _, refName := range yamlPkg.Essential { - if _, ok := yamlPkg.V3Essential[refName]; ok { - // This check is only needed because the list format can contain - // duplicates. It should be removed when format "v2" is deprecated. - return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) - } - yamlPkg.V3Essential[refName] = &yamlEssential{} - } - pkg.Archive = yamlPkg.Archive zeroPath := yamlPath{} for sliceName, yamlSlice := range yamlPkg.Slices { @@ -417,18 +487,7 @@ func parsePackage(baseDir, pkgName, pkgPath string, data []byte) (*Package, erro }, } - if yamlSlice.V3Essential == nil { - yamlSlice.V3Essential = map[string]*yamlEssential{} - } - for _, refName := range yamlSlice.Essential { - if _, ok := yamlSlice.V3Essential[refName]; ok { - // This check is only needed because the list format can contain - // duplicates. It should be removed when format "v2" is deprecated. - return nil, fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) - } - yamlSlice.V3Essential[refName] = &yamlEssential{} - } - for refName, essentialInfo := range yamlPkg.V3Essential { + for refName, essentialInfo := range yamlPkg.Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) @@ -449,7 +508,7 @@ func parsePackage(baseDir, pkgName, pkgPath string, data []byte) (*Package, erro } slice.Essential[sliceKey] = EssentialInfo{Arch: archList} } - for refName, essentialInfo := range yamlSlice.V3Essential { + for refName, essentialInfo := range yamlSlice.Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) @@ -581,7 +640,7 @@ func parsePackage(baseDir, pkgName, pkgPath string, data []byte) (*Package, erro pkg.Slices[sliceName] = slice } - return &pkg, err + return &pkg, nil } // validateGeneratePath validates that the path follows the following format: @@ -628,41 +687,58 @@ func pathInfoToYAML(pi *PathInfo) (*yamlPath, error) { return path, nil } -// sliceToYAML converts a Slice object to a yamlSlice object. -func sliceToYAML(s *Slice) (*yamlSlice, error) { - slice := &yamlSlice{ +func v1v2SliceToYAML(s *Slice) (*v1v2YAMLSlice, error) { + oldSlice := &v1v2YAMLSlice{ Contents: make(map[string]*yamlPath, len(s.Contents)), Mutate: s.Scripts.Mutate, - V3Essential: make(map[string]*yamlEssential, len(s.Essential)), + V3Essential: map[string]*yamlEssential{}, } for key, info := range s.Essential { - slice.V3Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} + oldSlice.V3Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} } for path, info := range s.Contents { yamlPath, err := pathInfoToYAML(&info) if err != nil { return nil, err } - slice.Contents[path] = yamlPath + oldSlice.Contents[path] = yamlPath } - return slice, nil + return oldSlice, nil } -// packageToYAML converts a Package object to a yamlPackage object. -func packageToYAML(p *Package) (*yamlPackage, error) { - pkg := &yamlPackage{ - Name: p.Name, - Archive: p.Archive, - Slices: make(map[string]yamlSlice, len(p.Slices)), +// sliceToYAML converts a Slice object to a yamlSlice object. +func sliceToYAML(s *Slice) (*yamlSlice, error) { + slice := &yamlSlice{ + Contents: make(map[string]*yamlPath, len(s.Contents)), + Mutate: s.Scripts.Mutate, + Essential: make(map[string]*yamlEssential, len(s.Essential)), } - for name, slice := range p.Slices { - yamlSlice, err := sliceToYAML(slice) + for key, info := range s.Essential { + slice.Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} + } + for path, info := range s.Contents { + yamlPath, err := pathInfoToYAML(&info) if err != nil { return nil, err } - pkg.Slices[name] = *yamlSlice + slice.Contents[path] = yamlPath } - return pkg, nil + return slice, nil +} + +type v1v2YAMLSlice struct { + Essential []string `yaml:"essential,omitempty"` + Contents map[string]*yamlPath `yaml:"contents,omitempty"` + Mutate string `yaml:"mutate,omitempty"` + V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` +} + +type v1v2YAMLPackage struct { + Name string `yaml:"package"` + Archive string `yaml:"archive,omitempty"` + Slices map[string]v1v2YAMLSlice `yaml:"slices,omitempty"` + Essential []string `yaml:"essential,omitempty"` + V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` } type yamlMaintenance struct { From e8b236b60eea72dcc5afd062ea2187aaae5f54b8 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Fri, 17 Oct 2025 16:15:11 +0200 Subject: [PATCH 2/9] typo --- internal/setup/setup_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index a145f97a..43e56242 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -4080,7 +4080,6 @@ func oldEssentialToV3(c *C, input []byte) (out string, skip bool) { for key, value := range oldEssential { if _, ok := newEssential[key]; ok { return "", true - return "", true } newEssential[key] = value } From e6d26fbe40ed2b4bc7808005abb05ed664b75fa0 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 11:20:58 +0100 Subject: [PATCH 3/9] checkpoint --- cmd/chisel/cmd_info_test.go | 4 +- internal/setup/yaml.go | 195 ++++++++++++++++-------------------- 2 files changed, 88 insertions(+), 111 deletions(-) diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index d2dc1408..93ce0e25 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -52,7 +52,7 @@ var infoTests = []infoTest{{ contents: /dir/file: {} myslice2: - v3-essential: + essential: mypkg1_myslice1: {} mypkg2_myslice: {arch: amd64} `, @@ -67,7 +67,7 @@ var infoTests = []infoTest{{ contents: /dir/file: {} myslice2: - v3-essential: + essential: mypkg1_myslice1: {} mypkg2_myslice: {arch: amd64} --- diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index f704a219..b3513046 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -19,22 +19,6 @@ import ( ) func (p *Package) MarshalYAML() (any, error) { - if p.Format == "v1" || p.Format == "v2" { - oldPkg := &v1v2YAMLPackage{ - Name: p.Name, - Archive: p.Archive, - Slices: make(map[string]v1v2YAMLSlice, len(p.Slices)), - } - for name, slice := range p.Slices { - yamlSlice, err := v1v2SliceToYAML(slice) - if err != nil { - return nil, err - } - oldPkg.Slices[name] = *yamlSlice - } - return oldPkg, nil - } - pkg := &yamlPackage{ Name: p.Name, Archive: p.Archive, @@ -79,13 +63,38 @@ type yamlArchive struct { PubKeys []string `yaml:"public-keys"` } +type essentialListMap struct { + Essential map[string]*yamlEssential + list bool +} + +func (es *essentialListMap) UnmarshalYAML(value *yaml.Node) error { + type essentialList struct { + Essential []string `yaml:"essential,omitempty"` + } + type essentialMap struct { + Essential map[string]*yamlEssential `yaml:"essential,omitempty"` + } + essential := essentialMap{} + err := value.Decode(&essential) + if err != nil { + list := essentialList{} + value.Decode(&essential) + for _, slice := range list.Essential { + essential.Essential[slice] = &yamlEssential{} + } + } + es.Essential = essential.Essential + return nil +} + type yamlPackage struct { Name string `yaml:"package"` Archive string `yaml:"archive,omitempty"` Slices map[string]yamlSlice `yaml:"slices,omitempty"` // For backwards-compatibility reasons with v1 and v2, essential needs // custom logic to be parsed. See [parsePackage]. - Essential map[string]*yamlEssential `yaml:"essential,omitempty"` + Essential essentialListMap `yaml:"essential,omitempty"` // It is only used to present an error message to the user if the field // is used in v3. V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` @@ -177,12 +186,12 @@ var _ yaml.Marshaler = yamlMode(0) type yamlSlice struct { Contents map[string]*yamlPath `yaml:"contents,omitempty"` Mutate string `yaml:"mutate,omitempty"` - // For backwards-compatibility reasons with v1 and v2, essential needs - // custom logic to be parsed. See [parsePackage]. - Essential map[string]*yamlEssential `yaml:"essential,omitempty"` // It is only used to present an error message to the user if the field // is used in v3. V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` + // For backwards-compatibility reasons with v1 and v2, essential needs + // custom logic to be parsed. See [parsePackage]. + Essential essentialListMap `yaml:"essential,omitempty"` } type yamlPubKey struct { @@ -415,56 +424,32 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa yamlPkg := yamlPackage{} dec := yaml.NewDecoder(bytes.NewBuffer(data)) dec.KnownFields(false) - if format == "v1" || format == "v2" { - oldPkg := &v1v2YAMLPackage{} - err := dec.Decode(&oldPkg) - if err != nil { - return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) - } - if oldPkg.V3Essential == nil { - oldPkg.V3Essential = map[string]*yamlEssential{} - } - for _, refName := range oldPkg.Essential { - if _, ok := oldPkg.V3Essential[refName]; ok { - // This check is only needed because the list format can contain - // duplicates. It should be removed when format "v2" is deprecated. - return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) - } - oldPkg.V3Essential[refName] = &yamlEssential{} - } - yamlPkg.Archive = oldPkg.Archive - yamlPkg.Name = oldPkg.Name - yamlPkg.Slices = map[string]yamlSlice{} - for sliceName, oldSlice := range oldPkg.Slices { - if oldSlice.V3Essential == nil { - oldSlice.V3Essential = map[string]*yamlEssential{} - } - for _, refName := range oldSlice.Essential { - if _, ok := oldSlice.V3Essential[refName]; ok { - // This check is only needed because the list format can contain - // duplicates. It should be removed when format "v2" is deprecated. - return nil, fmt.Errorf("slice %s repeats %s in essential fields", SliceKey{pkgName, sliceName}, refName) - } - oldSlice.V3Essential[refName] = &yamlEssential{} - } - yamlPkg.Slices[sliceName] = yamlSlice{ - Contents: oldSlice.Contents, - Mutate: oldSlice.Mutate, - Essential: oldSlice.V3Essential, - } - } - yamlPkg.Essential = oldPkg.V3Essential - } else { - err := dec.Decode(&yamlPkg) - if err != nil { - return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) - } - if yamlPkg.V3Essential != nil { + // if _, ok := oldPkg.V3Essential[refName]; ok { + // // This check is only needed because the list format can contain + // // duplicates. It should be removed when format "v2" is deprecated. + // return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) + // } + // for _, refName := range oldSlice.Essential { + // if _, ok := oldSlice.V3Essential[refName]; ok { + // // This check is only needed because the list format can contain + // // duplicates. It should be removed when format "v2" is deprecated. + // return nil, fmt.Errorf("slice %s repeats %s in essential fields", SliceKey{pkgName, sliceName}, refName) + // } + // oldSlice.V3Essential[refName] = &yamlEssential{} + // } + err := dec.Decode(&yamlPkg) + if err != nil { + return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) + } + if yamlPkg.V3Essential != nil { + if format != "v1" && format != "v2" { return nil, fmt.Errorf("package %q: v3-essential is deprecated since format v3", pkgName) } - for sliceName, slice := range yamlPkg.Slices { - if len(slice.V3Essential) > 0 { - return nil, fmt.Errorf("slice %s: v3-essential is deprecated since format v3", SliceKey{pkgName, sliceName}) + } + for _, yamlSlice := range yamlPkg.Slices { + if yamlSlice.V3Essential != nil { + if format != "v1" && format != "v2" { + return nil, fmt.Errorf("package %q: v3-essential is deprecated since format v3", pkgName) } } } @@ -487,7 +472,8 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa }, } - for refName, essentialInfo := range yamlPkg.Essential { + // TODO + for refName, essentialInfo := range yamlPkg.Essential.Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) @@ -508,7 +494,28 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa } slice.Essential[sliceKey] = EssentialInfo{Arch: archList} } - for refName, essentialInfo := range yamlSlice.Essential { + // TODO + for refName, essentialInfo := range yamlSlice.Essential.Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + return nil, fmt.Errorf("cannot add slice to itself as essential %q in %s", refName, pkgPath) + } + if _, ok := slice.Essential[sliceKey]; ok { + return nil, fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } + for refName, essentialInfo := range yamlSlice.V3Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) @@ -687,34 +694,19 @@ func pathInfoToYAML(pi *PathInfo) (*yamlPath, error) { return path, nil } -func v1v2SliceToYAML(s *Slice) (*v1v2YAMLSlice, error) { - oldSlice := &v1v2YAMLSlice{ - Contents: make(map[string]*yamlPath, len(s.Contents)), - Mutate: s.Scripts.Mutate, - V3Essential: map[string]*yamlEssential{}, - } - for key, info := range s.Essential { - oldSlice.V3Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} - } - for path, info := range s.Contents { - yamlPath, err := pathInfoToYAML(&info) - if err != nil { - return nil, err - } - oldSlice.Contents[path] = yamlPath - } - return oldSlice, nil -} - // sliceToYAML converts a Slice object to a yamlSlice object. func sliceToYAML(s *Slice) (*yamlSlice, error) { slice := &yamlSlice{ - Contents: make(map[string]*yamlPath, len(s.Contents)), - Mutate: s.Scripts.Mutate, - Essential: make(map[string]*yamlEssential, len(s.Essential)), - } + Contents: make(map[string]*yamlPath, len(s.Contents)), + Mutate: s.Scripts.Mutate, + // TODO + Essential: essentialListMap{ + Essential: make(map[string]*yamlEssential, len(s.Essential)), + }, + } + // TODO for key, info := range s.Essential { - slice.Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} + slice.Essential.Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} } for path, info := range s.Contents { yamlPath, err := pathInfoToYAML(&info) @@ -726,21 +718,6 @@ func sliceToYAML(s *Slice) (*yamlSlice, error) { return slice, nil } -type v1v2YAMLSlice struct { - Essential []string `yaml:"essential,omitempty"` - Contents map[string]*yamlPath `yaml:"contents,omitempty"` - Mutate string `yaml:"mutate,omitempty"` - V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` -} - -type v1v2YAMLPackage struct { - Name string `yaml:"package"` - Archive string `yaml:"archive,omitempty"` - Slices map[string]v1v2YAMLSlice `yaml:"slices,omitempty"` - Essential []string `yaml:"essential,omitempty"` - V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` -} - type yamlMaintenance struct { Standard string `yaml:"standard"` Expanded string `yaml:"expanded"` From 9401f01910ad6a50b5678bbd1d3be08da8612b50 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:14:22 +0100 Subject: [PATCH 4/9] working --- cmd/chisel/cmd_info_test.go | 2 +- internal/setup/setup_test.go | 16 +++---- internal/setup/yaml.go | 93 +++++++++++++++++++++++------------- 3 files changed, 70 insertions(+), 41 deletions(-) diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index 93ce0e25..86a04c96 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -88,7 +88,7 @@ var infoTests = []infoTest{{ contents: /dir/file: {} myslice2: - v3-essential: + essential: mypkg1_myslice1: {} mypkg2_myslice: {arch: amd64} `, diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 43e56242..8b4b0373 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -1490,7 +1490,7 @@ var setupTests = []setupTest{{ slice2: `, }, - relerror: `slice mypkg_slice1 repeats mypkg_slice2 in essential fields`, + relerror: `cannot parse package "mypkg" slice definitions: cannot decode essential: repeats mypkg_slice2`, }, { summary: "Duplicated package essentials", input: map[string]string{ @@ -1504,7 +1504,7 @@ var setupTests = []setupTest{{ slice2: `, }, - relerror: `package "mypkg" repeats mypkg_slice1 in essential fields`, + relerror: `cannot parse package "mypkg" slice definitions: cannot decode essential: repeats mypkg_slice1`, }, { summary: "Bad slice reference in slice essential", input: map[string]string{ @@ -3623,7 +3623,7 @@ var setupTests = []setupTest{{ myslice2: `, }, - relerror: `cannot parse package "mypkg" slice definitions: (.|\n)*`, + relerror: `cannot parse package "mypkg": essential expects a map`, }, { summary: "Format v3 expects a map in 'essential' (slice)", input: map[string]string{ @@ -3637,7 +3637,7 @@ var setupTests = []setupTest{{ myslice2: `, }, - relerror: `cannot parse package "mypkg" slice definitions: (.|\n)*`, + relerror: `cannot parse slice mypkg_myslice1: essential expects a map`, }, { summary: "In format v3 'v3-essential' is not supported (pkg)", input: map[string]string{ @@ -3651,7 +3651,7 @@ var setupTests = []setupTest{{ myslice2: `, }, - relerror: `package "mypkg": v3-essential is deprecated since format v3`, + relerror: `cannot parse package "mypkg": v3-essential is deprecated since format v3`, }, { summary: "In format v3 'v3-essential' is not supported (slice)", input: map[string]string{ @@ -3665,7 +3665,7 @@ var setupTests = []setupTest{{ myslice2: `, }, - relerror: `slice mypkg_myslice1: v3-essential is deprecated since format v3`, + relerror: `cannot parse slice mypkg_myslice1: v3-essential is deprecated since format v3`, }} func (s *S) TestParseRelease(c *C) { @@ -3925,13 +3925,13 @@ func (s *S) TestPackageYAMLFormat(c *C) { myslice1: contents: /dir/file1: {} - v3-essential: + essential: mypkg_myslice2: {arch: i386} mypkg_myslice3: {} myslice2: contents: /dir/file2: {} - v3-essential: + essential: mypkg_myslice3: {} myslice3: contents: diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index b3513046..35c42138 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -69,35 +69,42 @@ type essentialListMap struct { } func (es *essentialListMap) UnmarshalYAML(value *yaml.Node) error { - type essentialList struct { - Essential []string `yaml:"essential,omitempty"` - } - type essentialMap struct { - Essential map[string]*yamlEssential `yaml:"essential,omitempty"` - } - essential := essentialMap{} - err := value.Decode(&essential) + m := map[string]*yamlEssential{} + es.list = false + err := value.Decode(&m) if err != nil { - list := essentialList{} - value.Decode(&essential) - for _, slice := range list.Essential { - essential.Essential[slice] = &yamlEssential{} + l := []string{} + err = value.Decode(&l) + if err != nil { + return errors.New("cannot decode essential") + } + es.list = true + for _, sliceName := range l { + if _, ok := m[sliceName]; ok { + return fmt.Errorf("cannot decode essential: repeats %s", sliceName) + } + m[sliceName] = &yamlEssential{} } } - es.Essential = essential.Essential + es.Essential = m return nil } +func (es essentialListMap) MarshalYAML() (any, error) { + return es.Essential, nil +} + type yamlPackage struct { Name string `yaml:"package"` Archive string `yaml:"archive,omitempty"` Slices map[string]yamlSlice `yaml:"slices,omitempty"` + // "v3-essential" is used for backwards porting of arch-specific essential + // to releases that use "v1" or "v2". When using older versions of Chisel + // the field will be ignored and `essential` is used as a fallback. + V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` // For backwards-compatibility reasons with v1 and v2, essential needs - // custom logic to be parsed. See [parsePackage]. + // custom logic to be parsed. See [essentialListMap]. Essential essentialListMap `yaml:"essential,omitempty"` - // It is only used to present an error message to the user if the field - // is used in v3. - V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` } type yamlPath struct { @@ -186,11 +193,12 @@ var _ yaml.Marshaler = yamlMode(0) type yamlSlice struct { Contents map[string]*yamlPath `yaml:"contents,omitempty"` Mutate string `yaml:"mutate,omitempty"` - // It is only used to present an error message to the user if the field - // is used in v3. + // "v3-essential" is used for backwards porting of arch-specific essential + // to releases that use "v1" or "v2". When using older versions of Chisel + // the field will be ignored and `essential` is used as a fallback. V3Essential map[string]*yamlEssential `yaml:"v3-essential,omitempty"` // For backwards-compatibility reasons with v1 and v2, essential needs - // custom logic to be parsed. See [parsePackage]. + // custom logic to be parsed. See [essentialListMap]. Essential essentialListMap `yaml:"essential,omitempty"` } @@ -441,15 +449,19 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa if err != nil { return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) } - if yamlPkg.V3Essential != nil { - if format != "v1" && format != "v2" { - return nil, fmt.Errorf("package %q: v3-essential is deprecated since format v3", pkgName) + if format != "v1" && format != "v2" { + if yamlPkg.V3Essential != nil { + return nil, fmt.Errorf("cannot parse package %q: v3-essential is deprecated since format v3", pkgName) } - } - for _, yamlSlice := range yamlPkg.Slices { - if yamlSlice.V3Essential != nil { - if format != "v1" && format != "v2" { - return nil, fmt.Errorf("package %q: v3-essential is deprecated since format v3", pkgName) + if yamlPkg.Essential.list { + return nil, fmt.Errorf("cannot parse package %q: essential expects a map", pkgName) + } + for sliceName, yamlSlice := range yamlPkg.Slices { + if yamlSlice.V3Essential != nil { + return nil, fmt.Errorf("cannot parse slice %s: v3-essential is deprecated since format v3", SliceKey{pkgName, sliceName}) + } + if yamlSlice.Essential.list { + return nil, fmt.Errorf("cannot parse slice %s: essential expects a map", SliceKey{pkgName, sliceName}) } } } @@ -472,7 +484,6 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa }, } - // TODO for refName, essentialInfo := range yamlPkg.Essential.Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { @@ -494,7 +505,27 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa } slice.Essential[sliceKey] = EssentialInfo{Arch: archList} } - // TODO + for refName, essentialInfo := range yamlPkg.V3Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + // Do not add the slice to its own essentials list. + continue + } + if _, ok := slice.Essential[sliceKey]; ok { + return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } for refName, essentialInfo := range yamlSlice.Essential.Essential { sliceKey, err := ParseSliceKey(refName) if err != nil { @@ -699,12 +730,10 @@ func sliceToYAML(s *Slice) (*yamlSlice, error) { slice := &yamlSlice{ Contents: make(map[string]*yamlPath, len(s.Contents)), Mutate: s.Scripts.Mutate, - // TODO Essential: essentialListMap{ Essential: make(map[string]*yamlEssential, len(s.Essential)), }, } - // TODO for key, info := range s.Essential { slice.Essential.Essential[key.String()] = &yamlEssential{Arch: yamlArch{info.Arch}} } From 821300ccb85f97aaa29c9ad5d0735828102ef130 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:30:02 +0100 Subject: [PATCH 5/9] refactor --- cmd/chisel/cmd_info.go | 1 - internal/setup/setup.go | 4 +- internal/setup/setup_test.go | 117 +++++++------------ internal/setup/yaml.go | 216 +++++++++++++++++------------------ 4 files changed, 147 insertions(+), 191 deletions(-) diff --git a/cmd/chisel/cmd_info.go b/cmd/chisel/cmd_info.go index ceabfca8..fa5485c6 100644 --- a/cmd/chisel/cmd_info.go +++ b/cmd/chisel/cmd_info.go @@ -123,7 +123,6 @@ func selectPackageSlices(release *setup.Release, queries []string) (packages []* } else { releasePkg := release.Packages[pkgName] pkg = &setup.Package{ - Format: release.Format, Name: releasePkg.Name, Archive: releasePkg.Archive, Slices: make(map[string]*setup.Slice), diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 12431114..634ac0cc 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -22,7 +22,7 @@ type Release struct { Packages map[string]*Package Archives map[string]*Archive Maintenance *Maintenance - // Format is used for marshaling, unmarshalling and error checking. + // Format is used for parsing and error checking. Format string } @@ -55,8 +55,6 @@ type Package struct { Path string Archive string Slices map[string]*Slice - // Format is used for marshaling, unmarshalling and error checking. - Format string } // Slice holds the details about a package slice. diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 8b4b0373..0afa382a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -91,7 +91,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -139,9 +138,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -205,9 +203,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -470,9 +467,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -689,9 +685,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -733,9 +728,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -778,9 +772,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -852,7 +845,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -1059,9 +1051,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1135,7 +1126,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -1249,9 +1239,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "jq": { - Format: "v1", - Name: "jq", - Path: "slices/mydir/jq.yaml", + Name: "jq", + Path: "slices/mydir/jq.yaml", Slices: map[string]*setup.Slice{ "bins": { Package: "jq", @@ -1319,9 +1308,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1393,9 +1381,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "mypkg", @@ -1416,9 +1403,8 @@ var setupTests = []setupTest{{ }, }, "myotherpkg": { - Format: "v1", - Name: "myotherpkg", - Path: "slices/mydir/myotherpkg.yaml", + Name: "myotherpkg", + Path: "slices/mydir/myotherpkg.yaml", Slices: map[string]*setup.Slice{ "slice1": { Package: "myotherpkg", @@ -1461,7 +1447,7 @@ var setupTests = []setupTest{{ - mypkg_slice1 `, }, - relerror: `cannot add slice to itself as essential "mypkg_slice1" in slices/mydir/mypkg.yaml`, + relerror: `package "mypkg": cannot add slice to itself as essential "mypkg_slice1"`, }, { summary: "Package essentials clashes with slice essentials", input: map[string]string{ @@ -1578,9 +1564,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1632,9 +1617,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1728,9 +1712,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg", @@ -1742,9 +1725,8 @@ var setupTests = []setupTest{{ }, }, "mypkg2": { - Format: "v1", - Name: "mypkg2", - Path: "slices/mydir/mypkg2.yaml", + Name: "mypkg2", + Path: "slices/mydir/mypkg2.yaml", Slices: map[string]*setup.Slice{ "myslice": { Package: "mypkg2", @@ -1901,7 +1883,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2028,7 +2009,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2096,7 +2076,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2192,7 +2171,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2319,9 +2297,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg1": { - Format: "v1", - Name: "mypkg1", - Path: "slices/mydir/mypkg1.yaml", + Name: "mypkg1", + Path: "slices/mydir/mypkg1.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg1", @@ -2341,9 +2318,8 @@ var setupTests = []setupTest{{ }, }, "mypkg2": { - Format: "v1", - Name: "mypkg2", - Path: "slices/mydir/mypkg2.yaml", + Name: "mypkg2", + Path: "slices/mydir/mypkg2.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg2", @@ -2356,9 +2332,8 @@ var setupTests = []setupTest{{ }, }, "mypkg3": { - Format: "v1", - Name: "mypkg3", - Path: "slices/mydir/mypkg3.yaml", + Name: "mypkg3", + Path: "slices/mydir/mypkg3.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg3", @@ -2757,7 +2732,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2808,7 +2782,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -2998,7 +2971,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3121,7 +3093,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3244,7 +3215,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3367,7 +3337,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3488,7 +3457,6 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", Name: "mypkg", Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{}, @@ -3532,9 +3500,8 @@ var setupTests = []setupTest{{ }, Packages: map[string]*setup.Package{ "mypkg": { - Format: "v1", - Name: "mypkg", - Path: "slices/mydir/mypkg.yaml", + Name: "mypkg", + Path: "slices/mydir/mypkg.yaml", Slices: map[string]*setup.Slice{ "myslice1": { Package: "mypkg", @@ -3711,9 +3678,6 @@ func (s *S) TestParseRelease(c *C) { t.input = m if t.release != nil { t.release.Format = "v2" - for _, pkg := range t.release.Packages { - pkg.Format = "v2" - } } v2FormatTests = append(v2FormatTests, t) } @@ -3747,9 +3711,6 @@ func (s *S) TestParseRelease(c *C) { t.input = m if t.release != nil { t.release.Format = "v3" - for _, pkg := range t.release.Packages { - pkg.Format = "v3" - } } v3FormatTests = append(v3FormatTests, t) } diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index 35c42138..a0e7cfe7 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -19,19 +19,7 @@ import ( ) func (p *Package) MarshalYAML() (any, error) { - pkg := &yamlPackage{ - Name: p.Name, - Archive: p.Archive, - Slices: make(map[string]yamlSlice, len(p.Slices)), - } - for name, slice := range p.Slices { - yamlSlice, err := sliceToYAML(slice) - if err != nil { - return nil, err - } - pkg.Slices[name] = *yamlSlice - } - return pkg, nil + return packageToYAML(p) } var _ yaml.Marshaler = (*Package)(nil) @@ -426,25 +414,11 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa Name: pkgName, Path: pkgPath, Slices: make(map[string]*Slice), - Format: format, } yamlPkg := yamlPackage{} dec := yaml.NewDecoder(bytes.NewBuffer(data)) dec.KnownFields(false) - // if _, ok := oldPkg.V3Essential[refName]; ok { - // // This check is only needed because the list format can contain - // // duplicates. It should be removed when format "v2" is deprecated. - // return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) - // } - // for _, refName := range oldSlice.Essential { - // if _, ok := oldSlice.V3Essential[refName]; ok { - // // This check is only needed because the list format can contain - // // duplicates. It should be removed when format "v2" is deprecated. - // return nil, fmt.Errorf("slice %s repeats %s in essential fields", SliceKey{pkgName, sliceName}, refName) - // } - // oldSlice.V3Essential[refName] = &yamlEssential{} - // } err := dec.Decode(&yamlPkg) if err != nil { return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) @@ -483,88 +457,9 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa Mutate: yamlSlice.Mutate, }, } - - for refName, essentialInfo := range yamlPkg.Essential.Essential { - sliceKey, err := ParseSliceKey(refName) - if err != nil { - return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) - } - if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { - // Do not add the slice to its own essentials list. - continue - } - if _, ok := slice.Essential[sliceKey]; ok { - return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) - } - if slice.Essential == nil { - slice.Essential = map[SliceKey]EssentialInfo{} - } - var archList []string - if essentialInfo != nil { - archList = essentialInfo.Arch.List - } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} - } - for refName, essentialInfo := range yamlPkg.V3Essential { - sliceKey, err := ParseSliceKey(refName) - if err != nil { - return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) - } - if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { - // Do not add the slice to its own essentials list. - continue - } - if _, ok := slice.Essential[sliceKey]; ok { - return nil, fmt.Errorf("package %q repeats %s in essential fields", pkgName, refName) - } - if slice.Essential == nil { - slice.Essential = map[SliceKey]EssentialInfo{} - } - var archList []string - if essentialInfo != nil { - archList = essentialInfo.Arch.List - } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} - } - for refName, essentialInfo := range yamlSlice.Essential.Essential { - sliceKey, err := ParseSliceKey(refName) - if err != nil { - return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) - } - if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { - return nil, fmt.Errorf("cannot add slice to itself as essential %q in %s", refName, pkgPath) - } - if _, ok := slice.Essential[sliceKey]; ok { - return nil, fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) - } - if slice.Essential == nil { - slice.Essential = map[SliceKey]EssentialInfo{} - } - var archList []string - if essentialInfo != nil { - archList = essentialInfo.Arch.List - } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} - } - for refName, essentialInfo := range yamlSlice.V3Essential { - sliceKey, err := ParseSliceKey(refName) - if err != nil { - return nil, fmt.Errorf("package %q has invalid essential slice reference: %q", pkgName, refName) - } - if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { - return nil, fmt.Errorf("cannot add slice to itself as essential %q in %s", refName, pkgPath) - } - if _, ok := slice.Essential[sliceKey]; ok { - return nil, fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) - } - if slice.Essential == nil { - slice.Essential = map[SliceKey]EssentialInfo{} - } - var archList []string - if essentialInfo != nil { - archList = essentialInfo.Arch.List - } - slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + err := parseSliceEssential(&yamlPkg, &yamlSlice, slice) + if err != nil { + return nil, err } if len(yamlSlice.Contents) > 0 { @@ -747,6 +642,23 @@ func sliceToYAML(s *Slice) (*yamlSlice, error) { return slice, nil } +// packageToYAML converts a Package object to a yamlPackage object. +func packageToYAML(p *Package) (*yamlPackage, error) { + pkg := &yamlPackage{ + Name: p.Name, + Archive: p.Archive, + Slices: make(map[string]yamlSlice, len(p.Slices)), + } + for name, slice := range p.Slices { + yamlSlice, err := sliceToYAML(slice) + if err != nil { + return nil, err + } + pkg.Slices[name] = *yamlSlice + } + return pkg, nil +} + type yamlMaintenance struct { Standard string `yaml:"standard"` Expanded string `yaml:"expanded"` @@ -834,3 +746,89 @@ var defaultMaintenance = map[string]Maintenance{ EndOfLife: time.Date(2026, time.January, 15, 0, 0, 0, 0, time.UTC), }, } + +func parseSliceEssential(yamlPkg *yamlPackage, yamlSlice *yamlSlice, slice *Slice) error { + for refName, essentialInfo := range yamlPkg.Essential.Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return fmt.Errorf("package %q has invalid essential slice reference: %q", yamlPkg.Name, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + // Do not add the slice to its own essentials list. + continue + } + if _, ok := slice.Essential[sliceKey]; ok { + return fmt.Errorf("package %q repeats %s in essential fields", yamlPkg.Name, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } + for refName, essentialInfo := range yamlPkg.V3Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return fmt.Errorf("package %q has invalid essential slice reference: %q", yamlPkg.Name, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + // Do not add the slice to its own essentials list. + continue + } + if _, ok := slice.Essential[sliceKey]; ok { + return fmt.Errorf("package %q repeats %s in essential fields", yamlPkg.Name, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } + for refName, essentialInfo := range yamlSlice.Essential.Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return fmt.Errorf("package %q has invalid essential slice reference: %q", yamlPkg.Name, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + return fmt.Errorf("package %q: cannot add slice to itself as essential %q", yamlPkg.Name, refName) + } + if _, ok := slice.Essential[sliceKey]; ok { + return fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } + for refName, essentialInfo := range yamlSlice.V3Essential { + sliceKey, err := ParseSliceKey(refName) + if err != nil { + return fmt.Errorf("package %q has invalid essential slice reference: %q", yamlPkg.Name, refName) + } + if sliceKey.Package == slice.Package && sliceKey.Slice == slice.Name { + return fmt.Errorf("package %q: cannot add slice to itself as essential %q", yamlPkg.Name, refName) + } + if _, ok := slice.Essential[sliceKey]; ok { + return fmt.Errorf("slice %s repeats %s in essential fields", slice, refName) + } + if slice.Essential == nil { + slice.Essential = map[SliceKey]EssentialInfo{} + } + var archList []string + if essentialInfo != nil { + archList = essentialInfo.Arch.List + } + slice.Essential[sliceKey] = EssentialInfo{Arch: archList} + } + return nil +} From a4fb047f1006a9f04a2278424fcf0ac69c2e9441 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:42:11 +0100 Subject: [PATCH 6/9] more robust checking --- internal/setup/yaml.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index a0e7cfe7..852e0107 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -82,6 +82,9 @@ func (es essentialListMap) MarshalYAML() (any, error) { return es.Essential, nil } +var _ yaml.Marshaler = essentialListMap{} +var _ yaml.Unmarshaler = (*essentialListMap)(nil) + type yamlPackage struct { Name string `yaml:"package"` Archive string `yaml:"archive,omitempty"` @@ -423,18 +426,27 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa if err != nil { return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) } - if format != "v1" && format != "v2" { + if format == "v1" || format == "v2" { + if len(yamlPkg.Essential.Essential) > 0 && !yamlPkg.Essential.list { + return nil, fmt.Errorf("cannot parse package %q: essential expects a list", pkgName) + } + for sliceName, yamlSlice := range yamlPkg.Slices { + if len(yamlSlice.Essential.Essential) > 0 && !yamlSlice.Essential.list { + return nil, fmt.Errorf("cannot parse slice %s: essential expects a list", SliceKey{pkgName, sliceName}) + } + } + } else { if yamlPkg.V3Essential != nil { return nil, fmt.Errorf("cannot parse package %q: v3-essential is deprecated since format v3", pkgName) } - if yamlPkg.Essential.list { + if len(yamlPkg.Essential.Essential) > 0 && yamlPkg.Essential.list { return nil, fmt.Errorf("cannot parse package %q: essential expects a map", pkgName) } for sliceName, yamlSlice := range yamlPkg.Slices { if yamlSlice.V3Essential != nil { return nil, fmt.Errorf("cannot parse slice %s: v3-essential is deprecated since format v3", SliceKey{pkgName, sliceName}) } - if yamlSlice.Essential.list { + if len(yamlSlice.Essential.Essential) > 0 && yamlSlice.Essential.list { return nil, fmt.Errorf("cannot parse slice %s: essential expects a map", SliceKey{pkgName, sliceName}) } } From 6e00778129ca287d3538a7601c5f5be23719f865 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:47:19 +0100 Subject: [PATCH 7/9] more tests --- internal/setup/setup_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 0afa382a..887f820a 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3633,6 +3633,34 @@ var setupTests = []setupTest{{ `, }, relerror: `cannot parse slice mypkg_myslice1: v3-essential is deprecated since format v3`, +}, { + summary: "Format v1/v2 expect a list in 'essential' (pkg)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v2"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + essential: + mypkg_myslice2: {} + slices: + myslice1: + myslice2: + `, + }, + relerror: `cannot parse package "mypkg": essential expects a list`, +}, { + summary: "Format v1/v2 expect a list in 'essential' (slice)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v2"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice1: + essential: + mypkg_myslice2: {} + myslice2: + `, + }, + relerror: `cannot parse slice mypkg_myslice1: essential expects a list`, }} func (s *S) TestParseRelease(c *C) { From 42910ea5c1b2e15716a042ec9b3ae5bd03dcd0c0 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:49:14 +0100 Subject: [PATCH 8/9] comment --- internal/setup/yaml.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index 852e0107..254c95b7 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -53,7 +53,10 @@ type yamlArchive struct { type essentialListMap struct { Essential map[string]*yamlEssential - list bool + // list is set to true when the marshaler found a list and false if it + // found a map. The former is only valid in format "v1" and "v2" while the + // latter is valid from "v3" onwards. + list bool } func (es *essentialListMap) UnmarshalYAML(value *yaml.Node) error { From 8246bb682403cdb41de8b893984e953deb15aba1 Mon Sep 17 00:00:00 2001 From: Alberto Carretero Date: Tue, 13 Jan 2026 12:50:40 +0100 Subject: [PATCH 9/9] move --- internal/setup/yaml.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index 254c95b7..958b2814 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -429,6 +429,10 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa if err != nil { return nil, fmt.Errorf("cannot parse package %q slice definitions: %v", pkgName, err) } + if yamlPkg.Name != pkg.Name { + return nil, fmt.Errorf("%s: filename and 'package' field (%q) disagree", pkgPath, yamlPkg.Name) + } + if format == "v1" || format == "v2" { if len(yamlPkg.Essential.Essential) > 0 && !yamlPkg.Essential.list { return nil, fmt.Errorf("cannot parse package %q: essential expects a list", pkgName) @@ -455,9 +459,6 @@ func parsePackage(format, baseDir, pkgName, pkgPath string, data []byte) (*Packa } } - if yamlPkg.Name != pkg.Name { - return nil, fmt.Errorf("%s: filename and 'package' field (%q) disagree", pkgPath, yamlPkg.Name) - } pkg.Archive = yamlPkg.Archive zeroPath := yamlPath{} for sliceName, yamlSlice := range yamlPkg.Slices {