fix: Source9999 could be added in the wrong place, causing build breaks (#463)

Author: tobiasb-ms

docs/reference/overlays.md

@@ -15,6 +15,7 @@ These overlays modify `.spec` files using the structured spec parser, allowing p
 | Type | Description | Required Fields |
 |------|-------------|-----------------|
 | `spec-add-tag` | Adds a tag to the spec; **fails if the tag already exists** | `tag`, `value` |
+| `spec-insert-tag` | Inserts a tag after the last tag of the same family (e.g., `Source9999` after the last `Source*`); falls back to after the last tag of any kind, then appends to the section end. **Fails if targeting a sub-package that doesn't exist.** | `tag`, `value` |
 | `spec-set-tag` | Sets a tag value; replaces if exists, adds if not | `tag`, `value` |
 | `spec-update-tag` | Updates an existing tag; **fails if the tag doesn't exist** | `tag`, `value` |
 | `spec-remove-tag` | Removes a tag from the spec; **fails if the tag doesn't exist** | `tag` |
@@ -50,8 +51,8 @@ successfully makes a replacement to at least one matching file.
 |-------|----------|-------------|---------|
 | Type | `type` | **Required.** The overlay type to apply | All overlays |
 | Description | `description` | Human-readable explanation documenting the need for the change; helps identify overlays in error messages | All (optional) |
-| Tag | `tag` | The spec tag name (e.g., `BuildRequires`, `Requires`, `Version`) | `spec-add-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` |
-| Value | `value` | The tag value to set, or value to match for removal | `spec-add-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching) |
+| Tag | `tag` | The spec tag name (e.g., `BuildRequires`, `Requires`, `Version`) | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` |
+| Value | `value` | The tag value to set, or value to match for removal | `spec-add-tag`, `spec-insert-tag`, `spec-set-tag`, `spec-update-tag`, `spec-remove-tag` (optional for matching) |
 | Section | `section` | The spec section to target (e.g., `%build`, `%install`, `%files`, `%description`) | `spec-prepend-lines`, `spec-append-lines`, `spec-search-replace` (optional) |
 | Package | `package` | The sub-package name for multi-package specs; omit to target the main package | All spec overlays (optional) |
 | Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace` |
@@ -74,6 +75,22 @@ tag = "BuildRequires"
 value = "some-devel-package"
 ```
 
+### Inserting a Tag by Family
+
+Use `spec-insert-tag` to place a tag after the last existing tag of the same family rather than appending it to the end of the section. The "family" is determined by stripping trailing digits from the tag name (case-insensitive), so `Source0`, `Source1`, and `Source` all belong to the `Source` family.
+
+This is useful when tag placement matters — for example, ensuring a new `Source` tag doesn't end up after macros like `%fontpkg` or inside `%if` conditionals:
+
+```toml
+[[components.mypackage.overlays]]
+type = "spec-insert-tag"
+description = "Add macros file as a source"
+tag = "Source9999"
+value = "macros.azl.macros"
+```
+
+If no tags from the same family exist, the tag is placed after the last tag of any kind. If there are no tags at all, it falls back to `spec-add-tag` behavior (appending to the section end).
+
 ### Setting a Version
 
 Use `spec-set-tag` when you want to set a value regardless of whether the tag exists:

internal/app/azldev/core/sources/overlays.go

@@ -125,6 +125,11 @@ func ApplySpecOverlay(overlay projectconfig.ComponentOverlay, openedSpec *spec.S
 		if err != nil {
 			return fmt.Errorf("failed to add tag %#q to spec:\n%w", overlay.Tag, err)
 		}
+	case projectconfig.ComponentOverlayInsertSpecTag:
+		err := openedSpec.InsertTag(overlay.PackageName, overlay.Tag, overlay.Value)
+		if err != nil {
+			return fmt.Errorf("failed to insert tag %#q into spec:\n%w", overlay.Tag, err)
+		}
 	case projectconfig.ComponentOverlayUpdateSpecTag:
 		err := openedSpec.UpdateExistingTag(overlay.PackageName, overlay.Tag, overlay.Value)
 		if err != nil {

internal/app/azldev/core/sources/overlays_test.go

@@ -116,6 +116,35 @@ BuildRequires: added-package
 				PackageName: "i-do-not-exist",
 			},
 			spec: `Name: name
+`,
+			errorExpected: true,
+		},
+		{
+			name: "insert tag after same family",
+			overlay: projectconfig.ComponentOverlay{
+				Type:  projectconfig.ComponentOverlayInsertSpecTag,
+				Tag:   "Source9999",
+				Value: "macros.azl.macros",
+			},
+			spec: `Name: name
+Source0: test.tar.gz
+BuildRequires: gcc
+`,
+			result: `Name: name
+Source0: test.tar.gz
+Source9999: macros.azl.macros
+BuildRequires: gcc
+`,
+		},
+		{
+			name: "insert tag to non-existent package",
+			overlay: projectconfig.ComponentOverlay{
+				Type:        projectconfig.ComponentOverlayInsertSpecTag,
+				Tag:         "Source9999",
+				Value:       "macros.azl.macros",
+				PackageName: "i-do-not-exist",
+			},
+			spec: `Name: name
 `,
 			errorExpected: true,
 		},

internal/app/azldev/core/sources/sourceprep.go

@@ -86,15 +86,14 @@ func NewPreparer(
 func (p *sourcePreparerImpl) PrepareSources(
 	ctx context.Context, component components.Component, outputDir string, applyOverlays bool,
 ) error {
-	// Download any explicitly configured source files first.
-	// Files defined here take precedence over any sources implicitly defined by the fedora upstream.
+	// Use the source manager to fetch source files (archives, patches, etc.)
 	err := p.sourceManager.FetchFiles(ctx, component, outputDir)
 	if err != nil {
 		return fmt.Errorf("failed to fetch source files for component %#q:\n%w",
 			component.GetName(), err)
 	}
 
-	// Fetch the component sources.
+	// Use the source manager to fetch the component (spec file and sidecar files).
 	err = p.sourceManager.FetchComponent(ctx, component, outputDir)
 	if err != nil {
 		return fmt.Errorf("failed to fetch sources for component %#q:\n%w",
@@ -290,10 +289,9 @@ func synthesizeMacroLoadOverlays(macrosFileName string) ([]projectconfig.Compone
 		{
 			// Ensure that the macros file is manifested as a source in the spec so that
 			// mock and other tools know it needs to be present in the build root.
-			// Ideally we'd dynamically compute a unique source tag here, but for now
-			// we just use a high number to stay simple. If a conflict is found, this
-			// overlay application will fail.
-			Type:  projectconfig.ComponentOverlayAddSpecTag,
+			// Use InsertSpecTag to place it after the last existing Source* tag, avoiding
+			// misplacement after macros like %fontpkg or inside %if conditionals.
+			Type:  projectconfig.ComponentOverlayInsertSpecTag,
 			Tag:   "Source9999", // Use a high number to avoid conflicts with existing sources.
 			Value: macrosFileName,
 		},

internal/projectconfig/overlay.go

@@ -15,7 +15,7 @@ import (
 // ComponentOverlay represents an overlay that may be applied to a component's spec and/or its sources.
 type ComponentOverlay struct {
 	// The type of overlay to apply.
-	Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"`
+	Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"`
 	// Human readable description of overlay; primarily present to document the need for the change.
 	Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay"`
 
@@ -64,6 +64,7 @@ func (c *ComponentOverlay) WithAbsolutePaths(referenceDir string) (result *Compo
 // also require spec modifications.
 func (c *ComponentOverlay) ModifiesSpec() bool {
 	return c.Type == ComponentOverlayAddSpecTag ||
+		c.Type == ComponentOverlayInsertSpecTag ||
 		c.Type == ComponentOverlaySetSpecTag ||
 		c.Type == ComponentOverlayUpdateSpecTag ||
 		c.Type == ComponentOverlayRemoveSpecTag ||
@@ -93,6 +94,10 @@ type ComponentOverlayType string
 const (
 	// ComponentOverlayAddSpecTag is an overlay that adds a tag to the spec; fails if the tag already exists.
 	ComponentOverlayAddSpecTag ComponentOverlayType = "spec-add-tag"
+	// ComponentOverlayInsertSpecTag is an overlay that inserts a tag into the spec, placing it
+	// after the last existing tag from the same family (e.g., Source9999 after the last Source* tag).
+	// Falls back to after the last tag of any kind, then to appending at the section end.
+	ComponentOverlayInsertSpecTag ComponentOverlayType = "spec-insert-tag"
 	// ComponentOverlaySetSpecTag is an overlay that sets a tag to the spec. If the tag already exists, replaces
 	// its existing value; otherwise, adds the tag.
 	ComponentOverlaySetSpecTag ComponentOverlayType = "spec-set-tag"
@@ -172,7 +177,8 @@ func (c *ComponentOverlay) Validate() error {
 	}
 
 	switch c.Type {
-	case ComponentOverlayAddSpecTag, ComponentOverlaySetSpecTag, ComponentOverlayUpdateSpecTag:
+	case ComponentOverlayAddSpecTag, ComponentOverlayInsertSpecTag,
+		ComponentOverlaySetSpecTag, ComponentOverlayUpdateSpecTag:
 		if c.Tag == "" {
 			return missingField("tag")
 		}

internal/projectconfig/overlay_test.go

@@ -47,6 +47,34 @@ func TestComponentOverlay_Validate(t *testing.T) {
 			errorExpected: true,
 			errorContains: "value",
 		},
+		// spec-insert-tag tests
+		{
+			name: "spec-insert-tag valid",
+			overlay: projectconfig.ComponentOverlay{
+				Type:  projectconfig.ComponentOverlayInsertSpecTag,
+				Tag:   "Source9999",
+				Value: "macros.azl.macros",
+			},
+			errorExpected: false,
+		},
+		{
+			name: "spec-insert-tag missing tag",
+			overlay: projectconfig.ComponentOverlay{
+				Type:  projectconfig.ComponentOverlayInsertSpecTag,
+				Value: "some-value",
+			},
+			errorExpected: true,
+			errorContains: "tag",
+		},
+		{
+			name: "spec-insert-tag missing value",
+			overlay: projectconfig.ComponentOverlay{
+				Type: projectconfig.ComponentOverlayInsertSpecTag,
+				Tag:  "Source9999",
+			},
+			errorExpected: true,
+			errorContains: "value",
+		},
 		// spec-set-tag tests
 		{
 			name: "spec-set-tag valid",
@@ -355,6 +383,7 @@ func TestComponentOverlay_Validate(t *testing.T) {
 func TestComponentOverlay_ModifiesSpec(t *testing.T) {
 	specOverlayTypes := []projectconfig.ComponentOverlayType{
 		projectconfig.ComponentOverlayAddSpecTag,
+		projectconfig.ComponentOverlayInsertSpecTag,
 		projectconfig.ComponentOverlaySetSpecTag,
 		projectconfig.ComponentOverlayUpdateSpecTag,
 		projectconfig.ComponentOverlayRemoveSpecTag,

internal/rpm/spec/edit.go

@@ -167,6 +167,114 @@ func (s *Spec) AddTag(packageName string, tag string, value string) (err error)
 	return s.AppendLinesToSection(sectionName, packageName, []string{fmt.Sprintf("%s: %s", tag, value)})
 }
 
+// tagFamily returns the "family" prefix of a tag name by stripping any trailing digits.
+// For example, "Source9999" returns "source", "Patch100" returns "patch", and
+// "BuildRequires" returns "buildrequires". The result is always lowercased.
+func tagFamily(tag string) string {
+	lower := strings.ToLower(tag)
+
+	// Strip trailing digits.
+	end := len(lower)
+	for end > 0 && lower[end-1] >= '0' && lower[end-1] <= '9' {
+		end--
+	}
+
+	// If the entire tag is digits, return the full lowered tag.
+	if end == 0 {
+		return lower
+	}
+
+	return lower[:end]
+}
+
+// InsertTag inserts a tag into the spec, placing it after the last existing tag from the
+// same "family" (e.g., Source9999 is placed after the last Source* tag). If no tags from
+// the same family exist, the tag is placed after the last tag of any kind. If there are no
+// tags at all, it falls back to [AddTag] behavior (appending to the section end).
+//
+// The tag family is determined by stripping trailing digits from the tag name
+// (case-insensitive). For example, "Source0", "Source1", and "Source" all belong to the
+// "source" family.
+//
+// Note: When inserting into a sub-package (non-empty packageName), the corresponding
+// %package section must already exist in the spec; otherwise, an [ErrSectionNotFound]
+// error is returned.
+func (s *Spec) InsertTag(packageName string, tag string, value string) error {
+	slog.Debug("Inserting tag to spec", "package", packageName, "tag", tag, "value", value)
+
+	family := tagFamily(tag)
+	newLine := fmt.Sprintf("%s: %s", tag, value)
+
+	// First pass: find the best insertion point.
+	// Track both the last same-family tag line and the last tag of any kind.
+	lastFamilyTagLineNum := -1
+	lastAnyTagLineNum := -1
+	sectionFound := false
+
+	sectionName := ""
+	if packageName != "" {
+		sectionName = "%package"
+	}
+
+	err := s.Visit(func(ctx *Context) error {
+		// Track whether the target section exists.
+		if ctx.Target.TargetType == SectionStartTarget {
+			if ctx.CurrentSection.SectName == sectionName && ctx.CurrentSection.Package == packageName {
+				sectionFound = true
+			}
+		}
+
+		if ctx.Target.TargetType != SectionLineTarget {
+			return nil
+		}
+
+		// Only consider tags in the target section/package.
+		if ctx.CurrentSection.SectName != sectionName || ctx.CurrentSection.Package != packageName {
+			return nil
+		}
+
+		if ctx.Target.Line.Parsed.GetType() != Tag {
+			return nil
+		}
+
+		tagLine, ok := ctx.Target.Line.Parsed.(*TagLine)
+		if !ok {
+			return nil
+		}
+
+		lastAnyTagLineNum = ctx.CurrentLineNum
+
+		if tagFamily(tagLine.Tag) == family {
+			lastFamilyTagLineNum = ctx.CurrentLineNum
+		}
+
+		return nil
+	})
+	if err != nil {
+		return fmt.Errorf("failed to scan spec for tag insertion point:\n%w", err)
+	}
+
+	if !sectionFound {
+		return fmt.Errorf("section %#q (package=%#q) not found:\n%w", sectionName, packageName, ErrSectionNotFound)
+	}
+
+	// Determine insertion point: prefer same-family, then any tag, then fall back to AddTag.
+	insertAfterLine := lastFamilyTagLineNum
+	if insertAfterLine < 0 {
+		insertAfterLine = lastAnyTagLineNum
+	}
+
+	if insertAfterLine < 0 {
+		// No tags at all — fall back to AddTag behavior.
+		return s.AddTag(packageName, tag, value)
+	}
+
+	// Insert after the found line (0-indexed, so insertAfterLine+1).
+	s.InsertLinesAt([]string{newLine}, insertAfterLine+1)
+
+	return nil
+}
+
 // PrependLinesToSection prepends the given lines to the start of the specified section, placing
 // them just after the section header (or at the top of the file in the global section). An error
 // is returned if the identified section cannot be found in the spec.