refactor(spec): add selectable legacy and structural editors

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

@@ -43,11 +43,11 @@ func ApplyOverlayToSources(
 	dryRunnable opctx.DryRunnable,
 	fs opctx.FS,
 	overlay projectconfig.ComponentOverlay,
-	sourcesDirPath, specPath string,
+	sourcesDirPath, specPath string, options ...spec.OpenOption,
 ) error {
 	// Apply the spec component, if any.
 	if overlay.ModifiesSpec() {
-		err := ApplySpecOverlayToFileInPlace(fs, overlay, specPath)
+		err := ApplySpecOverlayToFileInPlace(fs, overlay, specPath, options...)
 		if err != nil {
 			return err
 		}
@@ -78,13 +78,15 @@ func ApplyOverlayToSources(
 
 // ApplySpecOverlayToFileInPlace applies the given overlay to the specified spec file.
 // Changes are made in-place.
-func ApplySpecOverlayToFileInPlace(fs opctx.FS, overlay projectconfig.ComponentOverlay, specPath string) error {
+func ApplySpecOverlayToFileInPlace(
+	fs opctx.FS, overlay projectconfig.ComponentOverlay, specPath string, options ...spec.OpenOption,
+) error {
 	specFile, err := fs.Open(specPath)
 	if err != nil {
 		return fmt.Errorf("failed to open spec %#q for reading:\n%w", specPath, err)
 	}
 
-	openedSpec, err := spec.OpenSpec(specFile)
+	openedSpec, err := spec.OpenSpec(specFile, options...)
 	specFile.Close()
 
 	if err != nil {

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

@@ -394,6 +394,52 @@ newname package
 	}
 }
 
+func TestApplySpecOverlay_ShimConditionalRepairAcrossOverlays(t *testing.T) {
+	openedSpec, err := spec.OpenSpec(strings.NewReader(`Name: shim-unsigned-%{efiarch}
+
+%build
+%if 0%{?dbxfile}
+echo dbx
+%endif
+cd build-%{efiarch}
+cd build-%{efialtarch}
+%install
+`), spec.WithEditor(spec.EditorStructural))
+	require.NoError(t, err)
+
+	require.NoError(t, sources.ApplySpecOverlay(projectconfig.ComponentOverlay{
+		Type:        projectconfig.ComponentOverlaySearchAndReplaceInSpec,
+		SectionName: "%build",
+		Regex:       `^cd build-%\{efialtarch\}$`,
+		Replacement: "%if 0\ncd build-%{efialtarch}",
+	}, openedSpec))
+
+	var intermediate bytes.Buffer
+	require.NoError(t, openedSpec.Serialize(&intermediate))
+	assert.Contains(t, intermediate.String(), "%if 0\ncd build-%{efialtarch}\n")
+
+	require.NoError(t, sources.ApplySpecOverlay(projectconfig.ComponentOverlay{
+		Type:        projectconfig.ComponentOverlayAppendSpecLines,
+		SectionName: "%build",
+		Lines:       []string{"%endif"},
+	}, openedSpec))
+
+	var result bytes.Buffer
+	require.NoError(t, openedSpec.Serialize(&result))
+	assert.Equal(t, `Name: shim-unsigned-%{efiarch}
+
+%build
+%if 0%{?dbxfile}
+echo dbx
+%endif
+cd build-%{efiarch}
+%if 0
+cd build-%{efialtarch}
+%endif
+%install
+`, result.String())
+}
+
 func TestApplyNonSpecOverlay(t *testing.T) {
 	testCases := []struct {
 		name           string

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

@@ -8,7 +8,6 @@ import (
 	"log/slog"
 	"regexp"
 	"strconv"
-	"strings"
 
 	"github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components"
 	"github.com/microsoft/azure-linux-dev-tools/internal/global/opctx"
@@ -34,29 +33,21 @@ var staticReleasePattern = regexp.MustCompile(`^(\d+)(%\{\??dist\})?$`)
 // GetReleaseTagValue reads the Release tag value from the spec file at specPath.
 // It returns the raw value string as written in the spec (e.g. "1%{?dist}" or "%autorelease").
 // Returns [spec.ErrNoSuchTag] if no Release tag is found.
-func GetReleaseTagValue(fs opctx.FS, specPath string) (string, error) {
+func GetReleaseTagValue(fs opctx.FS, specPath string, options ...spec.OpenOption) (string, error) {
 	specFile, err := fs.Open(specPath)
 	if err != nil {
 		return "", fmt.Errorf("failed to open spec %#q:\n%w", specPath, err)
 	}
 	defer specFile.Close()
 
-	openedSpec, err := spec.OpenSpec(specFile)
+	openedSpec, err := spec.OpenSpec(specFile, options...)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse spec %#q:\n%w", specPath, err)
 	}
 
-	var releaseValue string
-
-	err = openedSpec.VisitTagsPackage("", func(tagLine *spec.TagLine, _ *spec.Context) error {
-		if strings.EqualFold(tagLine.Tag, "Release") {
-			releaseValue = tagLine.Value
-		}
-
-		return nil
-	})
+	releaseValue, err := openedSpec.GetLastTag("", "Release")
 	if err != nil {
-		return "", fmt.Errorf("failed to visit tags in spec %#q:\n%w", specPath, err)
+		return "", fmt.Errorf("failed to get Release tag from spec %#q:\n%w", specPath, err)
 	}
 
 	if releaseValue == "" {
@@ -146,7 +137,7 @@ func (p *sourcePreparerImpl) readAndBumpRelease(
 		return err
 	}
 
-	releaseValue, err := GetReleaseTagValue(p.fs, specPath)
+	releaseValue, err := GetReleaseTagValue(p.fs, specPath, spec.WithEditor(p.specEditor))
 	if err != nil {
 		return fmt.Errorf("failed to read Release tag for component %#q:\n%w",
 			component.GetName(), err)
@@ -187,7 +178,7 @@ func (p *sourcePreparerImpl) readAndBumpRelease(
 		Value: newRelease,
 	}
 
-	if err := ApplySpecOverlayToFileInPlace(p.fs, overlay, specPath); err != nil {
+	if err := ApplySpecOverlayToFileInPlace(p.fs, overlay, specPath, spec.WithEditor(p.specEditor)); err != nil {
 		return fmt.Errorf("failed to apply release bump overlay for component %#q:\n%w",
 			component.GetName(), err)
 	}

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

@@ -103,6 +103,44 @@ func TestTryBumpStaticRelease_StaticBumps(t *testing.T) {
 	assert.Contains(t, string(content), "Release: 4%{?dist}")
 }
 
+func TestTryBumpStaticRelease_BumpsLastConditionalReleaseAndRereadsIt(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	memFS := afero.NewMemMapFs()
+	preparer := newTestPreparer(memFS)
+	specDir := filepath.Join(testSourcesDir, "test-pkg")
+	require.NoError(t, fileutils.MkdirAll(memFS, specDir))
+	specPath := filepath.Join(specDir, "test-pkg.spec")
+	require.NoError(t, fileutils.WriteFile(memFS, specPath, []byte(`Name: test-pkg
+Version: 1.0.0
+%if 0
+Release: 1%{?dist}
+%else
+Release: 2%{?dist}
+%endif
+`), fileperms.PublicFile))
+
+	comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{
+		Release: projectconfig.ReleaseConfig{Calculation: projectconfig.ReleaseCalculationAuto},
+	})
+
+	require.NoError(t, preparer.tryBumpStaticRelease(comp, specDir, 3))
+
+	release, err := GetReleaseTagValue(memFS, specPath)
+	require.NoError(t, err)
+	assert.Equal(t, "5%{?dist}", release)
+
+	content, err := fileutils.ReadFile(memFS, specPath)
+	require.NoError(t, err)
+	assert.Equal(t, `Name: test-pkg
+Version: 1.0.0
+%if 0
+Release: 5%{?dist}
+%else
+Release: 5%{?dist}
+%endif
+`, string(content))
+}
+
 func TestTryBumpStaticRelease_StaticBumpsNonConditionalDist(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	memFS := afero.NewMemMapFs()

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

@@ -104,11 +104,44 @@ func TestGetReleaseTagValue(t *testing.T) {
 	for _, testCase := range []struct {
 		name, specContent, expected string
 		wantErr                     bool
+		options                     []spec.OpenOption
 	}{
-		{"static with dist", makeSpec("1%{?dist}"), "1%{?dist}", false},
-		{"autorelease", makeSpec("%autorelease"), "%autorelease", false},
-		{"braced autorelease", makeSpec("%{autorelease}"), "%{autorelease}", false},
-		{"no release tag", "Name: test-package\nVersion: 1.0.0\nSummary: Test\n", "", true},
+		{name: "static with dist", specContent: makeSpec("1%{?dist}"), expected: "1%{?dist}"},
+		{name: "autorelease", specContent: makeSpec("%autorelease"), expected: "%autorelease"},
+		{name: "braced autorelease", specContent: makeSpec("%{autorelease}"), expected: "%{autorelease}"},
+		{
+			name:        "last repeated conditional release with legacy editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\n%if 0\nRelease: 1\n%else\nRelease: 2\n%endif\n",
+			expected:    "2",
+		},
+		{
+			name:        "last repeated conditional release with structural editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\n%if 0\nRelease: 1\n%else\nRelease: 2\n%endif\n",
+			expected:    "2",
+			options:     []spec.OpenOption{spec.WithEditor(spec.EditorStructural)},
+		},
+		{
+			name:        "no release tag with legacy editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\nSummary: Test\n",
+			wantErr:     true,
+		},
+		{
+			name:        "no release tag with structural editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\nSummary: Test\n",
+			options:     []spec.OpenOption{spec.WithEditor(spec.EditorStructural)},
+			wantErr:     true,
+		},
+		{
+			name:        "empty final release tag with legacy editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\nRelease: 1\nRelease:\n",
+			wantErr:     true,
+		},
+		{
+			name:        "empty final release tag with structural editor",
+			specContent: "Name: test-package\nVersion: 1.0.0\nRelease: 1\nRelease:\n",
+			options:     []spec.OpenOption{spec.WithEditor(spec.EditorStructural)},
+			wantErr:     true,
+		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			ctx := testctx.NewCtx()
@@ -117,7 +150,7 @@ func TestGetReleaseTagValue(t *testing.T) {
 			err := fileutils.WriteFile(ctx.FS(), specPath, []byte(testCase.specContent), 0o644)
 			require.NoError(t, err)
 
-			result, err := sources.GetReleaseTagValue(ctx.FS(), specPath)
+			result, err := sources.GetReleaseTagValue(ctx.FS(), specPath, testCase.options...)
 			if testCase.wantErr {
 				require.ErrorIs(t, err, spec.ErrNoSuchTag)
 			} else {

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

@@ -24,6 +24,7 @@ import (
 	"github.com/microsoft/azure-linux-dev-tools/internal/projectconfig"
 	"github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders"
 	"github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource"
+	"github.com/microsoft/azure-linux-dev-tools/internal/rpm/spec"
 	"github.com/microsoft/azure-linux-dev-tools/internal/utils/dirdiff"
 	"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms"
 	"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils"
@@ -98,6 +99,11 @@ func WithDirtyDetection() PreparerOption {
 	}
 }
 
+// WithSpecEditor selects the [spec.EditorMode] used for source preparation.
+func WithSpecEditor(mode spec.EditorMode) PreparerOption {
+	return func(p *sourcePreparerImpl) { p.specEditor = mode }
+}
+
 // WithSkipLookaside returns a [PreparerOption] that skips all lookaside cache
 // downloads during source preparation. This includes both explicit source file
 // downloads ([SourceManager.FetchFiles]) and lookaside extraction during
@@ -156,6 +162,7 @@ func WithAllowNoHashes() PreparerOption {
 // Standard implementation of the [SourcePreparer] interface.
 type sourcePreparerImpl struct {
 	sourceManager sourceproviders.SourceManager
+	specEditor    spec.EditorMode
 	fs            opctx.FS
 	eventListener opctx.EventListener
 	dryRunnable   opctx.DryRunnable
@@ -228,6 +235,7 @@ func NewPreparer(
 
 	impl := &sourcePreparerImpl{
 		sourceManager: sourceManager,
+		specEditor:    spec.EditorLegacy,
 		fs:            fs,
 		eventListener: eventListener,
 		dryRunnable:   dryRunnable,
@@ -1390,7 +1398,7 @@ func (p *sourcePreparerImpl) applyOverlayList(
 		}
 
 		if err := ApplyOverlayToSources(
-			p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath,
+			p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath, spec.WithEditor(p.specEditor),
 		); err != nil {
 			return fmt.Errorf("failed to apply %#q overlay:\n%w", overlay.Type, err)
 		}

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

@@ -119,7 +119,7 @@ func (p *sourcePreparerImpl) addUpstreamProvenanceMacros(
 		return
 	}
 
-	version, release, err := parseSpecVersionRelease(p.fs, specPath)
+	version, release, err := parseSpecVersionRelease(p.fs, specPath, spec.WithEditor(p.specEditor))
 	if err != nil {
 		slog.Warn("Skipping upstream provenance macros; failed to parse spec",
 			"component", component.GetName(), "error", err)
@@ -245,13 +245,15 @@ func setMacroIfAbsent(macros map[string]string, name, value string) {
 // package of the spec at specPath. Values are captured verbatim (no macro
 // expansion beyond the caller's later %{?dist} substitution). Missing tags
 // yield empty strings; it is not an error for a tag to be absent.
-func parseSpecVersionRelease(fs opctx.FS, specPath string) (version, release string, err error) {
+func parseSpecVersionRelease(
+	fs opctx.FS, specPath string, options ...spec.OpenOption,
+) (version, release string, err error) {
 	data, err := fileutils.ReadFile(fs, specPath)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to read spec %#q:\n%w", specPath, err)
 	}
 
-	parsed, err := spec.OpenSpec(bytes.NewReader(data))
+	parsed, err := spec.OpenSpec(bytes.NewReader(data), options...)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to parse spec %#q:\n%w", specPath, err)
 	}

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

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/microsoft/azure-linux-dev-tools/internal/projectconfig"
+	"github.com/microsoft/azure-linux-dev-tools/internal/rpm/spec"
 	"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms"
 	"github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils"
 	"github.com/spf13/afero"
@@ -79,6 +80,48 @@ func TestParseSpecVersionRelease(t *testing.T) {
 	assert.Equal(t, "5%{?dist}", release, "release is captured verbatim, dist is expanded later")
 }
 
+func TestParseSpecVersionReleaseReadsFirstRepeatedConditionalRelease(t *testing.T) {
+	memFS := afero.NewMemMapFs()
+	require.NoError(t, fileutils.MkdirAll(memFS, provenanceWorkDir))
+	require.NoError(t, fileutils.WriteFile(memFS, filepath.Join(provenanceWorkDir, "grub2.spec"), []byte(`Name: grub2
+Version: 2.12
+%if 0
+Release: 5%{?dist}
+%else
+Release: 6%{?dist}
+%endif
+`), fileperms.PublicFile))
+
+	version, release, err := parseSpecVersionRelease(memFS, filepath.Join(provenanceWorkDir, "grub2.spec"))
+	require.NoError(t, err)
+	assert.Equal(t, "2.12", version)
+	assert.Equal(t, "5%{?dist}", release)
+}
+
+func TestParseSpecVersionReleaseSkipsEmptyRepeatedConditionalTags(t *testing.T) {
+	for _, editor := range []spec.EditorMode{spec.EditorLegacy, spec.EditorStructural} {
+		t.Run(string(editor), func(t *testing.T) {
+			memFS := afero.NewMemMapFs()
+			require.NoError(t, fileutils.MkdirAll(memFS, provenanceWorkDir))
+			require.NoError(t, fileutils.WriteFile(memFS, filepath.Join(provenanceWorkDir, "grub2.spec"), []byte(`Name: grub2
+%if 0
+Version:
+Release:
+%endif
+Version: 2.12
+Release: 5%{?dist}
+`), fileperms.PublicFile))
+
+			version, release, err := parseSpecVersionRelease(
+				memFS, filepath.Join(provenanceWorkDir, "grub2.spec"), spec.WithEditor(editor),
+			)
+			require.NoError(t, err)
+			assert.Equal(t, "2.12", version)
+			assert.Equal(t, "5%{?dist}", release)
+		})
+	}
+}
+
 func TestParseSpecVersionRelease_MissingFile(t *testing.T) {
 	_, _, err := parseSpecVersionRelease(afero.NewMemMapFs(), "/does-not-exist.spec")
 	require.Error(t, err)