From 57b0fa2d31a53741e659d3ee3b5f1e99093e2869 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 4 Jul 2024 00:56:00 +0000 Subject: [PATCH 1/2] Add test for meta packages While working on some rpm spec template changes I broke meta packages but didn't know until I happen to run a windows test that is internally creating a meta package and it was panicing. This adds an explicit test for meta packages so we don't break this accidentally. Signed-off-by: Brian Goff --- test/azlinux_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/azlinux_test.go b/test/azlinux_test.go index 5b630616b..b1e78d651 100644 --- a/test/azlinux_test.go +++ b/test/azlinux_test.go @@ -995,6 +995,42 @@ Environment="KUBELET_KUBECONFIG_ARGS=--bootstrap-kubeconfig=/etc/kubernetes/boot solveT(ctx, t, client, sr) }) }) + + t.Run("meta package", func(t *testing.T) { + // Ensure that packages that just install other packages give the expected output + + t.Parallel() + ctx := startTestSpan(baseCtx, t) + + spec := &dalec.Spec{ + Name: "some-meta-thing", + Version: "0.0.1", + Revision: "1", + Description: "meta test", + License: "MIT", + Dependencies: &dalec.PackageDependencies{ + Runtime: map[string][]string{ + "curl": {}, + }, + }, + } + + testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) { + req := newSolveRequest(withBuildTarget(testConfig.BuildTarget), withSpec(ctx, t, spec)) + res := solveT(ctx, t, client, req) + ref, err := res.SingleRef() + if err != nil { + t.Fatal(err) + } + + _, err = ref.StatFile(ctx, gwclient.StatRequest{ + Path: "/usr/bin/curl", + }) + if err != nil { + t.Fatal(err) + } + }) + }) } func validatePathAndPermissions(ctx context.Context, ref gwclient.Reference, path string, expected os.FileMode) error { From 132d4f90c696d03c0034d6195c30df6359915f4c Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 4 Jul 2024 00:28:55 +0000 Subject: [PATCH 2/2] rpm: Don't emit optional fields when empty This also cleans up some of the rpm spec template layout for easier reading. Signed-off-by: Brian Goff --- docs/spec.schema.json | 4 +- frontend/rpm/rpmbuild.go | 4 +- frontend/rpm/template.go | 74 +++++++++++++++++++--------- frontend/rpm/template_test.go | 92 ++++++++++++++++++++++++++++++++--- spec.go | 4 +- test/azlinux_test.go | 1 + 6 files changed, 142 insertions(+), 37 deletions(-) diff --git a/docs/spec.schema.json b/docs/spec.schema.json index 93d9a63e2..d281c3643 100644 --- a/docs/spec.schema.json +++ b/docs/spec.schema.json @@ -933,9 +933,7 @@ "website", "version", "revision", - "license", - "vendor", - "packager" + "license" ], "description": "Spec is the specification for a package build." }, diff --git a/frontend/rpm/rpmbuild.go b/frontend/rpm/rpmbuild.go index 3ad15b98d..e46e8f9d5 100644 --- a/frontend/rpm/rpmbuild.go +++ b/frontend/rpm/rpmbuild.go @@ -56,8 +56,8 @@ func ValidateSpec(spec *dalec.Spec) (out error) { if spec.Description == "" { out = errors.Join(out, fmt.Errorf("%w: description", errMissingRequiredField)) } - if spec.Website == "" { - out = errors.Join(out, fmt.Errorf("%w: website", errMissingRequiredField)) + if spec.License == "" { + out = errors.Join(out, fmt.Errorf("%w: license", errMissingRequiredField)) } return out } diff --git a/frontend/rpm/template.go b/frontend/rpm/template.go index bfe05cee4..c8a9c93f8 100644 --- a/frontend/rpm/template.go +++ b/frontend/rpm/template.go @@ -16,38 +16,48 @@ import ( const gomodsName = "__gomods" -var specTmpl = template.Must(template.New("spec").Parse(strings.TrimSpace(` -Summary: {{.Description}} +var specTmpl = template.Must(template.New("spec").Funcs(tmplFuncs).Parse(strings.TrimSpace(` Name: {{.Name}} Version: {{.Version}} Release: {{.Release}}%{?dist} -License: {{.License}} -URL: {{.Website}} -Vendor: {{.Vendor}} -Packager: {{.Packager}} -{{- if .NoArch}} +License: {{ .License }} +Summary: {{ .Description }} +{{ optionalField "URL" .Website -}} +{{ optionalField "Vendor" .Vendor -}} +{{ optionalField "Packager" .Packager -}} +{{ if .NoArch }} BuildArch: noarch -{{- end}} - - -{{ .Sources }} -{{ .Conflicts }} -{{ .Provides }} -{{ .Replaces }} -{{ .Requires }} +{{ end }} +{{- .Sources -}} +{{- .Conflicts -}} +{{- .Provides -}} +{{- .Replaces -}} +{{- .Requires -}} %description {{.Description}} -{{ .PrepareSources }} -{{ .BuildSteps }} -{{ .Install }} -{{ .Post }} -{{ .PreUn }} -{{ .PostUn }} -{{ .Files }} -{{ .Changelog }} + +{{ .PrepareSources -}} +{{ .BuildSteps -}} +{{ .Install -}} +{{ .Post -}} +{{ .PreUn -}} +{{ .PostUn -}} +{{ .Files -}} +{{ .Changelog -}} `))) +func optionalField(key, value string) string { + if value == "" { + return "" + } + return key + ": " + value + "\n" +} + +var tmplFuncs = map[string]any{ + "optionalField": optionalField, +} + type specWrapper struct { *dalec.Spec Target string @@ -71,6 +81,7 @@ func (w *specWrapper) Changelog() (fmt.Stringer, error) { } } + b.WriteString("\n") return b, nil } @@ -81,6 +92,7 @@ func (w *specWrapper) Provides() fmt.Stringer { for _, name := range w.Spec.Provides { fmt.Fprintln(b, "Provides:", name) } + b.WriteString("\n") return b } @@ -123,6 +135,7 @@ func (w *specWrapper) Requires() fmt.Stringer { writeDep(b, "Requires", name, constraints) } + b.WriteString("\n") return b } @@ -146,6 +159,7 @@ func (w *specWrapper) Conflicts() string { constraints := w.Spec.Conflicts[name] writeDep(b, "Conflicts", name, constraints) } + b.WriteString("\n") return b.String() } @@ -186,6 +200,9 @@ func (w *specWrapper) Sources() (fmt.Stringer, error) { fmt.Fprintf(b, "Source%d: %s.tar.gz\n", len(keys), gomodsName) } + if len(keys) > 0 { + b.WriteString("\n") + } return b, nil } @@ -256,6 +273,10 @@ func (w *specWrapper) PrepareSources() (fmt.Stringer, error) { return nil, fmt.Errorf("error preparing source %s: %w", name, err) } } + + if len(keys) > 0 { + b.WriteString("\n") + } return b, nil } @@ -297,6 +318,7 @@ func (w *specWrapper) BuildSteps() fmt.Stringer { writeStep(b, step) } + b.WriteString("\n") return b } @@ -314,6 +336,7 @@ func (w *specWrapper) PreUn() fmt.Stringer { fmt.Fprintf(b, "%%systemd_preun %s\n", serviceName) } + b.WriteString("\n") return b } @@ -333,6 +356,7 @@ func (w *specWrapper) Post() fmt.Stringer { fmt.Fprintf(b, "%%systemd_post %s\n", unitConf.ResolveName(servicePath)) } + b.WriteString("\n") return b } @@ -360,6 +384,7 @@ func (w *specWrapper) Install() fmt.Stringer { fmt.Fprintln(b, "%install") if w.Spec.Artifacts.IsEmpty() { + b.WriteString("\n") return b } @@ -473,6 +498,7 @@ func (w *specWrapper) Install() fmt.Stringer { copyArtifact(root, l, cfg) } + b.WriteString("\n") return b } @@ -481,6 +507,7 @@ func (w *specWrapper) Files() fmt.Stringer { fmt.Fprintf(b, "%%files\n") if w.Spec.Artifacts.IsEmpty() { + b.WriteString("\n") return b } @@ -591,6 +618,7 @@ func (w *specWrapper) Files() fmt.Stringer { fmt.Fprintln(b, fullDirective) } + b.WriteString("\n") return b } diff --git a/frontend/rpm/template_test.go b/frontend/rpm/template_test.go index 46413306a..c9a8df81b 100644 --- a/frontend/rpm/template_test.go +++ b/frontend/rpm/template_test.go @@ -19,9 +19,9 @@ func TestTemplateSources(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - if s.String() != "" { - t.Fatalf("unexpected sources: %s", s.String()) - } + expect := "" + actual := s.String() + assert.Equal(t, actual, expect) }) // Each source entry is prefixed by comments documenting how the source was generated @@ -65,7 +65,7 @@ func TestTemplateSources(t *testing.T) { } // File sources are not (currently) compressed, so the source is the file itself - expected := "Source0: src1\n" + expected := "Source0: src1\n\n" actual := s[len(expectedDoc):] // trim off the doc from the output if actual != expected { t.Fatalf("unexpected sources: expected %q, got: %q", expected, actual) @@ -95,7 +95,7 @@ func TestTemplateSources(t *testing.T) { t.Errorf("Expected doc:\n%q\n\n, got:\n%q\n", expectedDoc, s) } - expected := "Source0: src1.tar.gz\n" + expected := "Source0: src1.tar.gz\n\n" actual := s[len(expectedDoc):] // trim off the doc from the output if actual != expected { t.Fatalf("unexpected sources: expected %q, got: %q", expected, actual) @@ -113,12 +113,14 @@ func TestTemplateSources(t *testing.T) { t.Fatalf("unexpected error: %v", err) } s2 := out2.String() + // trim last newline from the first output since that has shifted + s = s[:len(s)-1] if !strings.HasPrefix(s2, s) { t.Fatalf("expected output to start with %q, got %q", s, out2.String()) } s2 = strings.TrimPrefix(out2.String(), s) - expected := "Source1: " + gomodsName + ".tar.gz\n" + expected := "Source1: " + gomodsName + ".tar.gz\n\n" if s2 != expected { t.Fatalf("unexpected sources: expected %q, got: %q", expected, s2) } @@ -202,7 +204,7 @@ func TestTemplateSources(t *testing.T) { // Now we should have one more entry for gomods. // Note there are 2 gomod sources but they should be combined into one entry. - expected := "Source5: " + gomodsName + ".tar.gz\n" + expected := "Source5: " + gomodsName + ".tar.gz\n\n" if s != expected { t.Fatalf("gomod: unexpected sources: expected %q, got: %q", expected, s) } @@ -250,6 +252,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %doc %{_docdir}/test-pkg/docs/README + ` assert.Equal(t, want, got) }) @@ -268,6 +271,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %doc %{_docdir}/test-pkg/README.md + ` assert.Equal(t, want, got) }) @@ -286,6 +290,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %doc %{_docdir}/test-pkg/README.md + ` assert.Equal(t, want, got) }) @@ -304,6 +309,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %license %{_licensedir}/test-pkg/LICENSE + ` assert.Equal(t, want, got) }) @@ -325,6 +331,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %license %{_licensedir}/test-pkg/licenses/LICENSE.md + ` assert.Equal(t, want, got) }) @@ -346,6 +353,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %config(noreplace) %{_sysconfdir}/sysconfig/config + ` assert.Equal(t, want, got) }) @@ -364,6 +372,7 @@ func TestTemplate_Artifacts(t *testing.T) { got := w.Files().String() want := `%files %config(noreplace) %{_sysconfdir}/config.env + ` assert.Equal(t, want, got) }) @@ -386,6 +395,7 @@ func TestTemplate_Artifacts(t *testing.T) { want := `%files %dir %{_unitdir}/foo.service.d %{_unitdir}/foo.service.d/blah.config + ` assert.Equal(t, want, got) }) @@ -413,6 +423,7 @@ func TestTemplate_Artifacts(t *testing.T) { %dir %{_unitdir}/foo.service.d %{_unitdir}/foo.service.d/blah.config %{_unitdir}/foo.service.d/test.conf + ` assert.Equal(t, want, got) }) @@ -461,7 +472,74 @@ Requires: a-no-constraints Requires: b-one-constraints < 2.0 Requires: c-multiple-constraints < 2.0 Requires: c-multiple-constraints >= 1.0 + ` assert.Equal(t, want, got) } + +func TestTemplateOptionalFields(t *testing.T) { + spec := &dalec.Spec{ + Name: "testing", + Version: "0.0.1", + Revision: "1", + Description: "A helpful tool", + License: "MIT", + } + + w := &strings.Builder{} + err := specTmpl.Execute(w, &specWrapper{Spec: spec}) + assert.NilError(t, err) + + actual := strings.TrimSpace(w.String()) + expect := strings.TrimSpace(` +Name: testing +Version: 0.0.1 +Release: 1%{?dist} +License: MIT +Summary: A helpful tool + + +%description +A helpful tool + +%install + +%files + +`) + + assert.Equal(t, expect, actual) + + w.Reset() + + spec.Packager = "Awesome Packager" + err = specTmpl.Execute(w, &specWrapper{Spec: spec}) + assert.NilError(t, err) + + actual = strings.TrimSpace(w.String()) + expect = strings.TrimSpace(` +Name: testing +Version: 0.0.1 +Release: 1%{?dist} +License: MIT +Summary: A helpful tool +Packager: Awesome Packager + + +%description +A helpful tool + +%install + +%files + +`) + + defer func() { + if t.Failed() { + t.Log(actual) + } + }() + assert.Equal(t, expect, actual) +} diff --git a/spec.go b/spec.go index 0ef135e05..e46cafb11 100644 --- a/spec.go +++ b/spec.go @@ -70,9 +70,9 @@ type Spec struct { // License is the license of the package. License string `yaml:"license" json:"license"` // Vendor is the vendor of the package. - Vendor string `yaml:"vendor" json:"vendor"` + Vendor string `yaml:"vendor,omitempty" json:"vendor,omitempty"` // Packager is the name of the person,team,company that packaged the package. - Packager string `yaml:"packager" json:"packager"` + Packager string `yaml:"packager,omitempty" json:"packager,omitempty"` // Artifacts is the list of artifacts to include in the package. Artifacts Artifacts `yaml:"artifacts,omitempty" json:"artifacts,omitempty"` diff --git a/test/azlinux_test.go b/test/azlinux_test.go index b1e78d651..0eb66d9f2 100644 --- a/test/azlinux_test.go +++ b/test/azlinux_test.go @@ -331,6 +331,7 @@ echo "$BAR" > bar.txt Description: "foo bar baz", Website: "https://foo.bar.baz", Revision: "1", + License: "MIT", PackageConfig: &dalec.PackageConfig{ Signer: &dalec.PackageSigner{ Frontend: &dalec.Frontend{