diff --git a/internal/app/azldev/core/sources/overlays.go b/internal/app/azldev/core/sources/overlays.go index 44c2ca5c5..91bac3bab 100644 --- a/internal/app/azldev/core/sources/overlays.go +++ b/internal/app/azldev/core/sources/overlays.go @@ -30,6 +30,8 @@ import ( // overlay that found no matches). var ErrOverlayDidNotApply = errors.New("overlay did not apply to target") +const componentOverlayAddSource projectconfig.ComponentOverlayType = "internal-source-add" + // isSpecFile returns true if the given file path refers to a spec file. func isSpecFile(filePath string) bool { return strings.HasSuffix(filePath, ".spec") @@ -46,7 +48,7 @@ func ApplyOverlayToSources( sourcesDirPath, specPath string, ) error { // Apply the spec component, if any. - if overlay.ModifiesSpec() { + if overlay.ModifiesSpec() || overlay.Type == componentOverlayAddSource { err := ApplySpecOverlayToFileInPlace(fs, overlay, specPath) if err != nil { return err @@ -130,6 +132,11 @@ func ApplySpecOverlay(overlay projectconfig.ComponentOverlay, openedSpec *spec.S if err != nil { return fmt.Errorf("failed to insert tag %#q into spec:\n%w", overlay.Tag, err) } + case componentOverlayAddSource: + err := openedSpec.AddSourceEntry(overlay.Value) + if err != nil { + return fmt.Errorf("failed to add source entry to spec:\n%w", err) + } case projectconfig.ComponentOverlayUpdateSpecTag: err := openedSpec.UpdateExistingTag(overlay.PackageName, overlay.Tag, overlay.Value) if err != nil { diff --git a/internal/app/azldev/core/sources/overlays_test.go b/internal/app/azldev/core/sources/overlays_test.go index 5e6a26d4d..646cfd0e6 100644 --- a/internal/app/azldev/core/sources/overlays_test.go +++ b/internal/app/azldev/core/sources/overlays_test.go @@ -136,6 +136,22 @@ BuildRequires: gcc Source0: test.tar.gz Source9999: macros.azl.macros BuildRequires: gcc +`, + }, + { + name: "add source avoids occupied preferred number", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayType("internal-source-add"), + Value: "macros.azl.macros", + }, + spec: `Name: name +Source9999: upstream.file +BuildRequires: gcc +`, + result: `Name: name +Source9999: upstream.file +Source10000: macros.azl.macros +BuildRequires: gcc `, }, { diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 4d32c9834..d627bf135 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -426,26 +426,34 @@ func (p *sourcePreparerImpl) applyArchiveOverlayGroup( return repackedArchives, nil } -// collectOverlays gathers all overlays for a component into a single ordered slice: -// macros-load first, then user overlays, followed by check-skip and file-header overlays. +// collectOverlays gathers all overlays for a component into a single ordered slice: the +// macros-load directive first, then user overlays, then check-skip overlays, then the +// macros source registration, and finally the file-header overlay. The macros source +// registration is deliberately ordered after user overlays so it claims the next free +// source number without colliding with any sources the user added. func (p *sourcePreparerImpl) collectOverlays( component components.Component, macrosFileName string, ) ([]projectconfig.ComponentOverlay, error) { config := component.GetConfig() - var allOverlays []projectconfig.ComponentOverlay + var ( + allOverlays []projectconfig.ComponentOverlay + macroSourceOverlays []projectconfig.ComponentOverlay + ) if macrosFileName != "" { - macroOverlays, err := synthesizeMacroLoadOverlays(macrosFileName) + loadDirective, sourceRegistration, err := synthesizeMacroLoadOverlays(macrosFileName) if err != nil { return nil, fmt.Errorf("failed to compute macros load overlays:\n%w", err) } - allOverlays = append(allOverlays, macroOverlays...) + allOverlays = append(allOverlays, loadDirective...) + macroSourceOverlays = sourceRegistration } allOverlays = append(allOverlays, config.Overlays...) allOverlays = append(allOverlays, synthesizeCheckSkipOverlays(config.Build.Check)...) + allOverlays = append(allOverlays, macroSourceOverlays...) allOverlays = append(allOverlays, generateFileHeaderOverlay()...) return allOverlays, nil @@ -1288,19 +1296,25 @@ func renderMacrosFile(macros map[string]string) string { return strings.Join(lines, "\n") + "\n" } -func synthesizeMacroLoadOverlays(macrosFileName string) ([]projectconfig.ComponentOverlay, error) { +// synthesizeMacroLoadOverlays returns the overlays that wire a component's generated +// macros file into its spec. The load-directive overlay prepends the %{load:...} line; the +// source-registration overlay adds the macros file as a numbered Source. They are returned +// separately so the caller can apply the source registration after user overlays, letting +// it claim the next free source number without colliding with sources the user added. +func synthesizeMacroLoadOverlays( + macrosFileName string, +) (loadDirective, sourceRegistration []projectconfig.ComponentOverlay, err error) { // Basic check that the macros file name is valid and doesn't require escaping. if strings.ContainsFunc(macrosFileName, func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '.' && r != '-' && r != '_' && r != '+' }) { - return nil, fmt.Errorf( + return nil, nil, fmt.Errorf( "macros file name %#q contains invalid characters; does the component name contain invalid characters?", macrosFileName, ) } - // We inject an overlay to prepend a line to the spec to load the macros file. - return []projectconfig.ComponentOverlay{ + loadDirective = []projectconfig.ComponentOverlay{ { // Prepend the %{load:...} directive to the spec. Type: projectconfig.ComponentOverlayPrependSpecLines, @@ -1311,16 +1325,18 @@ func synthesizeMacroLoadOverlays(macrosFileName string) ([]projectconfig.Compone "", }, }, + } + + sourceRegistration = []projectconfig.ComponentOverlay{ { // 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. - // 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. + Type: componentOverlayAddSource, Value: macrosFileName, }, - }, nil + } + + return loadDirective, sourceRegistration, nil } // generateFileHeaderOverlay generates an overlay that prepends a header to the spec. diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index 8276689c3..579cc7afa 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -7,6 +7,7 @@ import ( "errors" "os" "path/filepath" + "regexp" "strings" "testing" @@ -94,11 +95,11 @@ func TestPrepareSources_Success(t *testing.T) { require.NoError(t, err) assert.False(t, exists, "macros file should not be created when there are no macros") - // Verify spec does NOT contain macro load or Source9999. + // Verify spec does not contain a macro load directive or a macros source entry. specContents, err := fileutils.ReadFile(ctx.FS(), outputSpecPath) require.NoError(t, err) assert.NotContains(t, string(specContents), "%{load:%{_sourcedir}/"+macrosFileName+"}") - assert.NotContains(t, string(specContents), "Source9999") + assert.NotContains(t, string(specContents), macrosFileName) } // TestPrepareSources_ArchiveOverlayRehashesSourcesEntry is an end-to-end check @@ -423,14 +424,51 @@ func TestPrepareSources_WritesMacrosFile(t *testing.T) { require.NoError(t, err) assert.Contains(t, string(contents), "%_with_feature 1") - // Verify spec has macro load directive and Source9999 tag. + // Verify spec has macro load directive and a collision-free source tag. specPath := filepath.Join(testOutputDir, "my-package.spec") specContents, err := fileutils.ReadFile(ctx.FS(), specPath) require.NoError(t, err) specStr := string(specContents) assert.Contains(t, specStr, "%{load:%{_sourcedir}/my-package"+sources.MacrosFileExtension+"}") - assert.Contains(t, specStr, "Source9999") + // Assert the macros file is registered under some SourceN tag, without pinning the number + // (the allocator picks the next free source number based on the spec's existing sources). + macrosSourcePattern := regexp.MustCompile( + `(?m)^Source[0-9]+: my-package` + regexp.QuoteMeta(sources.MacrosFileExtension) + `$`, + ) + assert.Regexp(t, macrosSourcePattern, specStr) +} + +func TestPrepareSources_MacroSourceTagCollision(t *testing.T) { + const testOutputDir = "/output" + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + ctx := testctx.NewCtx() + + component.EXPECT().GetName().AnyTimes().Return("my-package") + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Build: projectconfig.ComponentBuildConfig{With: []string{"feature"}}, + }) + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, testOutputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, testOutputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, outputDir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(outputDir, "my-package.spec"), + []byte("Name: my-package\nSource9999: upstream.file\n"), fileperms.PublicFile, + ) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + require.NoError(t, preparer.PrepareSources(ctx, component, testOutputDir, true)) + + specContents, err := fileutils.ReadFile(ctx.FS(), filepath.Join(testOutputDir, "my-package.spec")) + require.NoError(t, err) + assert.Contains(t, string(specContents), "Source9999: upstream.file") + assert.Contains(t, string(specContents), "Source10000: my-package"+sources.MacrosFileExtension) } // Tests for GenerateMacrosFileContents - these test content generation in isolation. diff --git a/internal/rpm/spec/edit.go b/internal/rpm/spec/edit.go index 0fdd056c3..15df87c51 100644 --- a/internal/rpm/spec/edit.go +++ b/internal/rpm/spec/edit.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "math" "regexp" "strconv" "strings" @@ -631,6 +632,77 @@ func ParsePatchTagNumber(tag string) (int, bool) { return num, true } +// ParseSourceTagNumber checks if the given tag name is a SourceN tag (case-insensitive) +// and returns the numeric suffix N. Returns -1, false if the tag is not a SourceN tag +// or the suffix is not a valid non-negative integer. +func ParseSourceTagNumber(tag string) (int, bool) { + suffix, found := strings.CutPrefix(strings.ToLower(tag), "source") + if !found || suffix == "" { + return -1, false + } + + num, err := strconv.Atoi(suffix) + if err != nil || num < 0 { + return -1, false + } + + return num, true +} + +// AddSourceEntry registers a source in the spec. It prefers the conventional high slot +// (Source9999) when that number is free, matching azldev's historical output so already-rendered +// specs stay byte-identical (idempotent). When Source9999 is already taken, it falls back to the +// next number after the highest existing source tag. Automatically numbered bare Source tags are +// included when determining occupancy and the highest number. +func (s *Spec) AddSourceEntry(filename string) error { + const preferredSourceTagNumber = 9999 + + highest := -1 + unnumberedCount := 0 + preferredOccupied := false + + err := s.VisitTags(func(tagLine *TagLine, _ *Context) error { + num, isSourceTag := ParseSourceTagNumber(tagLine.Tag) + if isSourceTag { + if num > highest { + highest = num + } + + if num == preferredSourceTagNumber { + preferredOccupied = true + } + } else if strings.EqualFold(tagLine.Tag, "source") { + unnumberedCount++ + } + + return nil + }) + if err != nil { + return fmt.Errorf("failed to scan for existing source tags:\n%w", err) + } + + // Bare Source tags auto-number sequentially from 0, occupying slots 0..unnumberedCount-1. + if unnumberedCount-1 > highest { + highest = unnumberedCount - 1 + } + + if unnumberedCount > preferredSourceTagNumber { + preferredOccupied = true + } + + num := preferredSourceTagNumber + + if preferredOccupied { + if highest == math.MaxInt { + return errors.New("cannot allocate SourceN tag after maximum integer tag number") + } + + num = highest + 1 + } + + return s.InsertTag("", fmt.Sprintf("Source%d", num), filename) +} + // HasSection returns true if the spec contains a section with the given name. // The comparison is exact (case-sensitive), consistent with [AppendLinesToSection]. func (s *Spec) HasSection(sectionName string) (bool, error) { diff --git a/internal/rpm/spec/edit_test.go b/internal/rpm/spec/edit_test.go index 959c5f104..336f7143e 100644 --- a/internal/rpm/spec/edit_test.go +++ b/internal/rpm/spec/edit_test.go @@ -5,6 +5,8 @@ package spec_test import ( "bytes" + "fmt" + "math" "strings" "testing" "time" @@ -1340,6 +1342,81 @@ func TestGetHighestPatchTagNumber(t *testing.T) { } } +func TestAddSourceEntry(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "prefers Source9999 when it is free", + input: "Name: test\nSource0: source.tar.gz\nVersion: 1.0\n", + expected: "Name: test\nSource0: source.tar.gz\nSource9999: macros.azl.macros\nVersion: 1.0\n", + }, + { + name: "prefers Source9999 when no sources exist", + input: "Name: test\nVersion: 1.0\n", + expected: "Name: test\nVersion: 1.0\nSource9999: macros.azl.macros\n", + }, + { + name: "avoids occupied preferred source number", + input: "Name: test\nSource9999: upstream.file\nVersion: 1.0\n", + expected: "Name: test\nSource9999: upstream.file\nSource10000: macros.azl.macros\nVersion: 1.0\n", + }, + { + name: "source tags are case insensitive", + input: "Name: test\nSOURCE9999: upstream.file\nsource9998: another.file\n", + expected: "Name: test\nSOURCE9999: upstream.file\nsource9998: another.file\nSource10000: macros.azl.macros\n", + }, + { + name: "bare source reserves automatically numbered slot", + input: "Name: test\nSource: source.tar.gz\nSource9999: upstream.file\n", + expected: "Name: test\nSource: source.tar.gz\nSource9999: upstream.file\nSource10000: macros.azl.macros\n", + }, + { + name: "allocates after source numbers above preferred range", + input: "Name: test\nSource9999: upstream.file\nSource10133: texlive.file\n", + expected: "Name: test\nSource9999: upstream.file\nSource10133: texlive.file\nSource10134: macros.azl.macros\n", + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + specFile, err := spec.OpenSpec(strings.NewReader(testCase.input)) + require.NoError(t, err) + + err = specFile.AddSourceEntry("macros.azl.macros") + require.NoError(t, err) + + var output strings.Builder + require.NoError(t, specFile.Serialize(&output)) + assert.Equal(t, testCase.expected, output.String()) + }) + } +} + +func TestAddSourceEntry_ManyBareSources(t *testing.T) { + input := "Name: test\n" + strings.Repeat("Source: automatically-numbered.tar.gz\n", 10001) + specFile, err := spec.OpenSpec(strings.NewReader(input)) + require.NoError(t, err) + + require.NoError(t, specFile.AddSourceEntry("macros.azl.macros")) + + var output strings.Builder + require.NoError(t, specFile.Serialize(&output)) + assert.Contains(t, output.String(), "Source10001: macros.azl.macros\n") +} + +func TestAddSourceEntry_MaximumIntegerTag(t *testing.T) { + // Source9999 occupied forces the fallback path; SourceMaxInt makes highest+1 overflow. + input := fmt.Sprintf("Name: test\nSource9999: upstream.file\nSource%d: max.file\n", math.MaxInt) + specFile, err := spec.OpenSpec(strings.NewReader(input)) + require.NoError(t, err) + + err = specFile.AddSourceEntry("macros.azl.macros") + require.ErrorContains(t, err, "cannot allocate SourceN tag after maximum integer tag number") +} + func TestRemoveTagsMatching(t *testing.T) { tests := []struct { name string @@ -1538,6 +1615,32 @@ func TestParsePatchTagNumber(t *testing.T) { } } +func TestParseSourceTagNumber(t *testing.T) { + tests := []struct { + tag string + expectedNum int + expectedOK bool + }{ + {"Source0", 0, true}, + {"Source9999", 9999, true}, + {"source5", 5, true}, + {"SOURCE10000", 10000, true}, + {"Source-1", -1, false}, + {"Source", -1, false}, + {"SourceFoo", -1, false}, + {"Patch0", -1, false}, + {"", -1, false}, + } + + for _, testCase := range tests { + t.Run(testCase.tag, func(t *testing.T) { + num, ok := spec.ParseSourceTagNumber(testCase.tag) + assert.Equal(t, testCase.expectedNum, num) + assert.Equal(t, testCase.expectedOK, ok) + }) + } +} + func TestVisitTags(t *testing.T) { input := `Name: main-pkg Version: 1.0