rpm: Don't emit optional fields when empty (#311)

* 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 <cpuguy83@gmail.com> * 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 <cpuguy83@gmail.com> --------- Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Azure · Jul 8, 2024 · 92b9acd · 92b9acd
1 parent 4b09d96
commit 92b9acd
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 37 deletions.
diff --git 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
@@ -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
@@ -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
 }