Skip to content

Commit 3489355

Browse files
committed
spec: reject subpackage strip control
Keep automatic dependency discovery package-scoped, but reject strip control where targets cannot safely apply it independently. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent 22a186d commit 3489355

4 files changed

Lines changed: 98 additions & 15 deletions

File tree

artifacts.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,12 @@ type Artifacts struct {
5151
// Groups is a list of groups to add to the system when the package is installed.
5252
Groups []AddGroupConfig `yaml:"groups,omitempty" json:"groups,omitempty"`
5353

54-
// DisableStrip is used to disable stripping of artifacts.
54+
// DisableStrip disables stripping of root package artifacts.
55+
// It is not supported in subpackages.
5556
DisableStrip bool `yaml:"disable_strip,omitempty" json:"disable_strip,omitempty"`
5657

57-
// DisableAutoRequires is used to disable automatic dependency discovery for
58-
// the produced package.
59-
//
60-
// Some tooling, such as `rpmbuild`, will look at all artifacts and
61-
// automatically inject missing dependencies into the package metadata.
62-
// For instance, if you include a `.sh` script, rpmbuild with automatically
63-
// add `bash` as a dependency for the package.
64-
// It also does this for libraries being linked against.
65-
//
66-
// This is useful if you want to have more control over the dependencies
67-
// that are included in the package.
68-
// However, you must be careful to manually include all dependencies that are required.
58+
// DisableAutoRequires disables automatic dependency discovery for package
59+
// artifacts. Required dependencies must be declared explicitly when it is enabled.
6960
DisableAutoRequires bool `yaml:"disable_auto_requires,omitempty" json:"disable_auto_requires,omitempty"`
7061
}
7162

docs/spec.schema.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,13 @@
307307
"type": [
308308
"boolean"
309309
],
310-
"description": "DisableAutoRequires is used to disable automatic dependency discovery for\nthe produced package.\n\nSome tooling, such as `rpmbuild`, will look at all artifacts and\nautomatically inject missing dependencies into the package metadata.\nFor instance, if you include a `.sh` script, rpmbuild with automatically\nadd `bash` as a dependency for the package.\nIt also does this for libraries being linked against.\n\nThis is useful if you want to have more control over the dependencies\nthat are included in the package.\nHowever, you must be careful to manually include all dependencies that are required."
310+
"description": "DisableAutoRequires disables automatic dependency discovery for package\nartifacts. Required dependencies must be declared explicitly when it is enabled."
311311
},
312312
"disable_strip": {
313313
"type": [
314314
"boolean"
315315
],
316-
"description": "DisableStrip is used to disable stripping of artifacts."
316+
"description": "DisableStrip disables stripping of root package artifacts.\nIt is not supported in subpackages."
317317
},
318318
"docs": {
319319
"additionalProperties": {

subpackage.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ func (s *SubPackage) validate() error {
132132
}
133133

134134
if s.Artifacts != nil {
135+
if s.Artifacts.DisableStrip {
136+
errs = append(errs, fmt.Errorf("artifacts: disable_strip is only valid for root package artifacts"))
137+
}
135138
if err := s.Artifacts.validate(); err != nil {
136139
errs = append(errs, errors.Wrap(err, "artifacts"))
137140
}

subpackage_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,95 @@ func TestSubPackageMetadataIsStatic(t *testing.T) {
163163
}
164164
}
165165

166+
func TestSubPackageArtifactControls(t *testing.T) {
167+
t.Parallel()
168+
169+
testCases := []struct {
170+
name string
171+
artifacts *Artifacts
172+
errSubstr string
173+
}{
174+
{
175+
name: "A supplemental package with disable_strip enabled is rejected",
176+
artifacts: &Artifacts{
177+
DisableStrip: true,
178+
},
179+
errSubstr: "artifacts: disable_strip is only valid for root package artifacts",
180+
},
181+
{
182+
name: "A supplemental package with disable_auto_requires enabled is valid",
183+
artifacts: &Artifacts{
184+
DisableAutoRequires: true,
185+
},
186+
},
187+
{
188+
name: "A supplemental package with root-only artifact controls unset is valid",
189+
artifacts: &Artifacts{},
190+
},
191+
}
192+
193+
for _, tc := range testCases {
194+
t.Run(tc.name, func(t *testing.T) {
195+
t.Parallel()
196+
197+
pkg := SubPackage{
198+
Description: "Tools package",
199+
Artifacts: tc.artifacts,
200+
}
201+
202+
err := pkg.validate()
203+
204+
if tc.errSubstr == "" {
205+
assert.NilError(t, err)
206+
return
207+
}
208+
assert.ErrorContains(t, err, tc.errSubstr)
209+
})
210+
}
211+
}
212+
213+
func TestRootPackageArtifactControls(t *testing.T) {
214+
t.Parallel()
215+
216+
testCases := []struct {
217+
name string
218+
artifacts Artifacts
219+
}{
220+
{
221+
name: "A root package with disable_strip enabled is valid",
222+
artifacts: Artifacts{
223+
DisableStrip: true,
224+
},
225+
},
226+
{
227+
name: "A root package with disable_auto_requires enabled is valid",
228+
artifacts: Artifacts{
229+
DisableAutoRequires: true,
230+
},
231+
},
232+
}
233+
234+
for _, tc := range testCases {
235+
t.Run(tc.name, func(t *testing.T) {
236+
t.Parallel()
237+
238+
spec := Spec{
239+
Name: "tools",
240+
Description: "Tools package",
241+
Website: "https://example.com",
242+
Version: "1.0.0",
243+
Revision: "1",
244+
License: "MIT",
245+
Artifacts: tc.artifacts,
246+
}
247+
248+
err := spec.Validate()
249+
250+
assert.NilError(t, err)
251+
})
252+
}
253+
}
254+
166255
func TestSubPackageBuildArgumentSubstitution(t *testing.T) {
167256
t.Parallel()
168257

0 commit comments

Comments
 (0)