Skip to content

Commit 22acb5d

Browse files
cpuguy83cpuguy83
committed
spec: simplify subpackage metadata validation
Keep subpackage metadata literal, validate package rules through the exported spec boundary, and remove unused package helpers. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent 40ec727 commit 22acb5d

4 files changed

Lines changed: 148 additions & 463 deletions

File tree

‎docs/spec.schema.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2230,14 +2230,14 @@
22302230
"type": [
22312231
"string"
22322232
],
2233-
"description": "Description is the package description. This is required — both RPM and\nDebian require a description/summary for every subpackage.\nBuild arguments are not supported in this field."
2233+
"description": "Description is the package description. This is required — both RPM and\nDebian require a description/summary for every subpackage.\nBuild argument references are not substituted in this field."
22342234
},
22352235
"name": {
22362236
"type": [
22372237
"string",
22382238
"null"
22392239
],
2240-
"description": "Name overrides the default package name.\nBy default, the package name is \"\u003cparent\u003e-\u003ckey\u003e\" where \u003ckey\u003e is the map\nkey under which this SubPackage is defined. Set this to use a fully custom\npackage name instead. Build arguments are not supported in this field."
2240+
"description": "Name overrides the default package name.\nBy default, the package name is \"\u003cparent\u003e-\u003ckey\u003e\" where \u003ckey\u003e is the map\nkey under which this SubPackage is defined. Set this to use a fully custom\npackage name instead. Build argument references are not substituted in this field."
22412241
},
22422242
"provides": {
22432243
"$ref": "#/$defs/PackageDependencyList",

‎subpackage.go‎

Lines changed: 38 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,49 @@ import (
44
goerrors "errors"
55
"fmt"
66
"iter"
7-
"slices"
8-
"unicode"
97

108
"github.com/moby/buildkit/frontend/dockerfile/shell"
119
"github.com/pkg/errors"
1210
)
1311

12+
// SubPackage defines a supplemental package produced from the same build
13+
// output as the primary package. Each supplemental package has its own
14+
// artifact selection, runtime dependencies, and package metadata.
15+
//
16+
// Supplemental packages share the primary package's build steps, sources,
17+
// version, revision, license, vendor, and website. They cannot override
18+
// these fields.
19+
type SubPackage struct {
20+
// Name overrides the default package name.
21+
// By default, the package name is "<parent>-<key>" where <key> is the map
22+
// key under which this SubPackage is defined. Set this to use a fully custom
23+
// package name instead. Build argument references are not substituted in this field.
24+
Name string `yaml:"name,omitempty" json:"name,omitempty"`
25+
26+
// Description is the package description. This is required — both RPM and
27+
// Debian require a description/summary for every subpackage.
28+
// Build argument references are not substituted in this field.
29+
Description string `yaml:"description" json:"description" jsonschema:"required"`
30+
31+
// Artifacts specifies which build outputs go into this supplemental package.
32+
// This is self-contained — no artifacts are inherited from the primary package.
33+
Artifacts *Artifacts `yaml:"artifacts,omitempty" json:"artifacts,omitempty"`
34+
35+
// Dependencies specifies runtime dependencies for this supplemental package.
36+
// Only runtime and recommends dependencies are allowed; build dependencies
37+
// are shared with the primary package.
38+
Dependencies *SubPackageDependencies `yaml:"dependencies,omitempty" json:"dependencies,omitempty"`
39+
40+
// Conflicts is the list of packages that conflict with this supplemental package.
41+
Conflicts PackageDependencyList `yaml:"conflicts,omitempty" json:"conflicts,omitempty"`
42+
43+
// Provides is the list of things this supplemental package provides.
44+
Provides PackageDependencyList `yaml:"provides,omitempty" json:"provides,omitempty"`
45+
46+
// Replaces is the list of packages that this supplemental package replaces/obsoletes.
47+
Replaces PackageDependencyList `yaml:"replaces,omitempty" json:"replaces,omitempty"`
48+
}
49+
1450
// SubPackageDependencies contains only the dependency fields valid for
1551
// supplemental packages. Build dependencies are shared with the primary
1652
// package and cannot be overridden.
@@ -69,44 +105,6 @@ func (d *SubPackageDependencies) processBuildArgs(lex *shell.Lex, args map[strin
69105
return goerrors.Join(errs...)
70106
}
71107

72-
// SubPackage defines a supplemental package produced from the same build
73-
// output as the primary package. Each supplemental package has its own
74-
// artifact selection, runtime dependencies, and package metadata.
75-
//
76-
// Supplemental packages share the primary package's build steps, sources,
77-
// version, revision, license, vendor, and website. They cannot override
78-
// these fields.
79-
type SubPackage struct {
80-
// Name overrides the default package name.
81-
// By default, the package name is "<parent>-<key>" where <key> is the map
82-
// key under which this SubPackage is defined. Set this to use a fully custom
83-
// package name instead. Build arguments are not supported in this field.
84-
Name string `yaml:"name,omitempty" json:"name,omitempty"`
85-
86-
// Description is the package description. This is required — both RPM and
87-
// Debian require a description/summary for every subpackage.
88-
// Build arguments are not supported in this field.
89-
Description string `yaml:"description" json:"description" jsonschema:"required"`
90-
91-
// Artifacts specifies which build outputs go into this supplemental package.
92-
// This is self-contained — no artifacts are inherited from the primary package.
93-
Artifacts *Artifacts `yaml:"artifacts,omitempty" json:"artifacts,omitempty"`
94-
95-
// Dependencies specifies runtime dependencies for this supplemental package.
96-
// Only runtime and recommends dependencies are allowed; build dependencies
97-
// are shared with the primary package.
98-
Dependencies *SubPackageDependencies `yaml:"dependencies,omitempty" json:"dependencies,omitempty"`
99-
100-
// Conflicts is the list of packages that conflict with this supplemental package.
101-
Conflicts PackageDependencyList `yaml:"conflicts,omitempty" json:"conflicts,omitempty"`
102-
103-
// Provides is the list of things this supplemental package provides.
104-
Provides PackageDependencyList `yaml:"provides,omitempty" json:"provides,omitempty"`
105-
106-
// Replaces is the list of packages that this supplemental package replaces/obsoletes.
107-
Replaces PackageDependencyList `yaml:"replaces,omitempty" json:"replaces,omitempty"`
108-
}
109-
110108
// GetSubPackagesForTarget returns the supplemental packages defined for the
111109
// given target in map-key order. A missing target or a target without
112110
// supplemental packages yields no values.
@@ -131,14 +129,6 @@ func (s *SubPackage) validate() error {
131129
errs = append(errs, fmt.Errorf("description is required"))
132130
}
133131

134-
if err := validateNoBuildArgReferences(s.Name); err != nil {
135-
errs = append(errs, errors.Wrap(err, "name"))
136-
}
137-
138-
if err := validateNoBuildArgReferences(s.Description); err != nil {
139-
errs = append(errs, errors.Wrap(err, "description"))
140-
}
141-
142132
if s.Artifacts != nil {
143133
if s.Artifacts.DisableStrip {
144134
errs = append(errs, fmt.Errorf("artifacts: disable_strip is only valid for root package artifacts"))
@@ -154,14 +144,6 @@ func (s *SubPackage) validate() error {
154144
func (s *SubPackage) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error {
155145
var errs []error
156146

157-
if err := validateNoBuildArgReferences(s.Name); err != nil {
158-
errs = append(errs, errors.Wrap(err, "name"))
159-
}
160-
161-
if err := validateNoBuildArgReferences(s.Description); err != nil {
162-
errs = append(errs, errors.Wrap(err, "description"))
163-
}
164-
165147
if err := s.Dependencies.processBuildArgs(lex, args, allowArg); err != nil {
166148
errs = append(errs, errors.Wrap(err, "dependencies"))
167149
}
@@ -202,40 +184,6 @@ func (s *SubPackage) processBuildArgs(lex *shell.Lex, args map[string]string, al
202184
return goerrors.Join(errs...)
203185
}
204186

205-
func validateNoBuildArgReferences(value string) error {
206-
lex := shell.NewLex('\\')
207-
lex.SkipProcessQuotes = true
208-
209-
result, _ := lex.ProcessWordWithMatches(value, envGetterMap(nil))
210-
refs := make([]string, 0, len(result.Unmatched))
211-
for ref := range result.Unmatched {
212-
if isBuildArgName(ref) {
213-
refs = append(refs, ref)
214-
}
215-
}
216-
if len(refs) == 0 {
217-
return nil
218-
}
219-
220-
slices.Sort(refs)
221-
return fmt.Errorf("build arguments are not supported (found %q)", refs)
222-
}
223-
224-
func isBuildArgName(value string) bool {
225-
for i, r := range value {
226-
if i == 0 && r != '_' && !unicode.IsLetter(r) {
227-
return false
228-
}
229-
if i > 0 && r != '_' && !unicode.IsLetter(r) && !unicode.IsDigit(r) {
230-
return false
231-
}
232-
}
233-
return value != ""
234-
}
235-
236-
func (s *SubPackage) fillDefaults() {
237-
}
238-
239187
// validateSubPackageNames checks that no two supplemental packages in the same
240188
// target resolve to the same name, and that no supplemental package name
241189
// conflicts with the primary package name.
@@ -262,13 +210,3 @@ func validateSubPackageNames(specName, targetName string, packages map[string]Su
262210

263211
return goerrors.Join(errs...)
264212
}
265-
266-
// GetAllPackageNames returns the resolved names of all packages (primary +
267-
// supplemental) for the given target. The primary package name is always first.
268-
func (s *Spec) GetAllPackageNames(targetKey string) []string {
269-
names := []string{s.Name}
270-
for key, pkg := range GetSubPackagesForTarget(s, targetKey) {
271-
names = append(names, pkg.ResolvedName(s.Name, key))
272-
}
273-
return names
274-
}

0 commit comments

Comments
 (0)