From 7a9abe9249dee3bedb3cb8db419628d65ad669de Mon Sep 17 00:00:00 2001 From: Thien Trung Vuong Date: Wed, 9 Sep 2026 01:14:54 +0000 Subject: [PATCH 1/3] feat(sources): add a transactional rpmdev-bumpspec runner Keep rpmdev-bumpspec external while providing one deterministic host operation for release updates. The runner isolates user-specific state, forwards the effective RPM context, and validates each mutation by comparing structured source EVRs with the host RPM implementation. Treat every operation as a transaction: command, parsing, identity, and ordering failures restore the original spec bytes. Focused in-memory tests cover runtime discovery, sidecar context, build macros, target propagation, strict output contracts, RPM ordering, and rollback. --- internal/app/azldev/core/sources/bumpspec.go | 401 +++++++++++++ .../core/sources/bumpspec_internal_test.go | 536 ++++++++++++++++++ 2 files changed, 937 insertions(+) create mode 100644 internal/app/azldev/core/sources/bumpspec.go create mode 100644 internal/app/azldev/core/sources/bumpspec_internal_test.go diff --git a/internal/app/azldev/core/sources/bumpspec.go b/internal/app/azldev/core/sources/bumpspec.go new file mode 100644 index 00000000..296dd27d --- /dev/null +++ b/internal/app/azldev/core/sources/bumpspec.go @@ -0,0 +1,401 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "bytes" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/externalcmd" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" +) + +const ( + // RPMDevBumpspecBinary is the executable used to make one spec release bump. + RPMDevBumpspecBinary = "rpmdev-bumpspec" + + rpmdevPackagerBinary = "rpmdev-packager" + rpmBinary = "rpm" + rpmspecBinary = "rpmspec" + python3Binary = "python3" + + rpmautospecNoBumpOutput = "RPMAutoSpec usage detected, not changing the spec file." + + rpmdevBumpspecBinDirName = ".rpmdev-bumpspec-bin" + rpmdevBumpspecRPMWrapper = "rpm" + + rpmdevBumpspecEVRSeparator = "\x1f" + rpmdevBumpspecEVRQueryFormat = "%{NAME}\x1f%{EPOCHNUM}\x1f%{VERSION}\x1f%{RELEASE}\n" + rpmdevBumpspecEVRFieldCount = 4 + rpmLabelCompareProgram = "import rpm, sys\n" + + "print(rpm.labelCompare(tuple(sys.argv[1:4]), tuple(sys.argv[4:7])))\n" +) + +// ErrRPMDevBumpspecNoMutation is returned when rpmdev-bumpspec exits successfully without +// producing a newer source package EVR. +var ErrRPMDevBumpspecNoMutation = errors.New("rpmdev-bumpspec did not produce a newer source package EVR") + +// RPMDevBumpspecSourceEVR is the source package identity evaluated by the host RPM implementation. +type RPMDevBumpspecSourceEVR struct { + Name string + Epoch string + Version string + Release string +} + +// RPMDevBumpspecRequest describes one deterministic host rpmdev-bumpspec operation. HomeDir must +// be a request-local, initially empty directory whose lifecycle and cleanup are caller-owned. +type RPMDevBumpspecRequest struct { + SpecPath string + Comment string + Identity string + Datestamp string + HomeDir string + Build projectconfig.ComponentBuildConfig + TargetArch string +} + +// RunRPMDevBumpspec executes one deterministic host rpmdev-bumpspec operation. rpmdevtools remains +// an external host dependency because its GPL-licensed release behavior, including fallback forms, +// is intentionally not reimplemented by azldev. +// +//nolint:cyclop // The transaction explicitly preserves every actionable failure boundary. +func RunRPMDevBumpspec(ctx opctx.Ctx, request RPMDevBumpspecRequest) (err error) { + if err := validateRPMDevBumpspecRequest(request); err != nil { + return err + } + + if err := ctx.Err(); err != nil { + return fmt.Errorf("rpmdev-bumpspec operation cancelled before execution:\n%w", err) + } + + if err := requireRPMDevBumpspecRuntime(ctx); err != nil { + return err + } + + if ctx.DryRun() { + return runRPMDevBumpspecCommand(ctx, request) + } + + beforeBytes, err := fileutils.ReadFile(ctx.FS(), request.SpecPath) + if err != nil { + return fmt.Errorf("failed to save spec %#q before rpmdev-bumpspec:\n%w", request.SpecPath, err) + } + + defer func() { + if err == nil { + return + } + + if restoreErr := fileutils.WriteFile( + ctx.FS(), request.SpecPath, beforeBytes, fileperms.PublicFile, + ); restoreErr != nil { + err = fmt.Errorf("rpmdev-bumpspec failed and restoring original spec %#q also failed:\n%w", + request.SpecPath, errors.Join(err, restoreErr)) + } + }() + + if err := prepareRPMDevBumpspecHome(ctx, request); err != nil { + return err + } + + beforeEVR, err := evaluateRPMDevBumpspecSourceEVR(ctx, request) + if err != nil { + return fmt.Errorf("failed to evaluate source package EVR before rpmdev-bumpspec for spec %#q:\n%w", + request.SpecPath, err) + } + + if err := runRPMDevBumpspecCommand(ctx, request); err != nil { + return err + } + + afterEVR, err := evaluateRPMDevBumpspecSourceEVR(ctx, request) + if err != nil { + return fmt.Errorf("failed to evaluate source package EVR after rpmdev-bumpspec for spec %#q:\n%w", + request.SpecPath, err) + } + + if beforeEVR.Name != afterEVR.Name || beforeEVR.Epoch != afterEVR.Epoch || beforeEVR.Version != afterEVR.Version { + return fmt.Errorf("rpmdev-bumpspec unexpectedly changed source package identity for spec %#q "+ + "(before: %#v, after: %#v):\n%w", request.SpecPath, beforeEVR, afterEVR, ErrRPMDevBumpspecNoMutation) + } + + if err := requireRPMDevBumpspecReleaseIncrease(ctx, request, beforeEVR, afterEVR); err != nil { + return err + } + + return nil +} + +func requireRPMDevBumpspecRuntime(ctx opctx.Ctx) error { + for _, binary := range rpmdevBumpspecBinaries() { + if !ctx.CommandInSearchPath(binary) { + return fmt.Errorf( + "required runtime executable %#q was not found on PATH; install a behaviorally compatible "+ + "'rpmdevtools' runtime (tested: 9.6 / rpmdev-bumpspec 1.0.13):\n%w", + binary, externalcmd.ErrMissingExecutable, + ) + } + } + + return nil +} + +func runRPMDevBumpspecCommand(ctx opctx.Ctx, request RPMDevBumpspecRequest) error { + rawCmd := exec.CommandContext(ctx, RPMDevBumpspecBinary, + "-c", request.Comment, + "-u", request.Identity, + "-d", request.Datestamp, + request.SpecPath, + ) + rawCmd.Env = rpmdevBumpspecEnv(ctx, request) + + var stdout, stderr bytes.Buffer + + rawCmd.Stdout = &stdout + rawCmd.Stderr = &stderr + + cmd, err := ctx.Command(rawCmd) + if err != nil { + return fmt.Errorf("failed to create rpmdev-bumpspec command:\n%w", err) + } + + if err := cmd.Run(ctx); err != nil { + return fmt.Errorf( + "rpmdev-bumpspec failed for spec %#q (stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, stdout.String(), stderr.String(), err, + ) + } + + if reportsRPMAutoSpecNoBump(stdout.String(), stderr.String()) { + return fmt.Errorf( + "rpmdev-bumpspec reported no release bump for spec %#q (stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, stdout.String(), stderr.String(), ErrRPMDevBumpspecNoMutation, + ) + } + + return nil +} + +func prepareRPMDevBumpspecHome(ctx opctx.Ctx, request RPMDevBumpspecRequest) error { + if err := fileutils.MkdirAll(ctx.FS(), request.HomeDir); err != nil { + return fmt.Errorf("failed to create isolated HOME %#q:\n%w", request.HomeDir, err) + } + + isHomeEmpty, err := fileutils.IsDirEmpty(ctx.FS(), request.HomeDir) + if err != nil { + return fmt.Errorf("failed to inspect isolated HOME %#q:\n%w", request.HomeDir, err) + } + + if !isHomeEmpty { + return fmt.Errorf("isolated HOME %#q must be empty before running rpmdev-bumpspec", request.HomeDir) + } + + rpmWrapperPath := filepath.Join(request.HomeDir, rpmdevBumpspecBinDirName, rpmdevBumpspecRPMWrapper) + if err := fileutils.MkdirAll(ctx.FS(), filepath.Dir(rpmWrapperPath)); err != nil { + return fmt.Errorf("failed to create isolated RPM wrapper directory %#q:\n%w", filepath.Dir(rpmWrapperPath), err) + } + + if err := fileutils.WriteFile(ctx.FS(), rpmWrapperPath, []byte(rpmdevBumpspecWrapper(request)), + fileperms.PublicExecutable); err != nil { + return fmt.Errorf("failed to create isolated RPM wrapper %#q:\n%w", rpmWrapperPath, err) + } + + return nil +} + +func rpmdevBumpspecWrapper(request RPMDevBumpspecRequest) string { + args := append([]string{"rpm"}, rpmdevBumpspecContextArgs(request)...) + + quoted := make([]string, len(args)) + for index, arg := range args { + quoted[index] = shellQuote(arg) + } + + return "#!/bin/sh\n" + + "PATH=\"$RPMDEV_BUMPSPEC_ORIGINAL_PATH\"\n" + + "export PATH\n" + + "exec " + strings.Join(quoted, " ") + " \"$@\"\n" +} + +func rpmdevBumpspecContextArgs(request RPMDevBumpspecRequest) []string { + args := []string{ + "--define", "_sourcedir " + filepath.Dir(request.SpecPath), + "--define", "_specdir " + filepath.Dir(request.SpecPath), + } + if request.TargetArch != "" { + args = append(args, "--target="+request.TargetArch) + } + + macros := buildMacrosMap(request.Build) + + macroNames := make([]string, 0, len(macros)) + for name := range macros { + macroNames = append(macroNames, name) + } + + sort.Strings(macroNames) + + for _, name := range macroNames { + args = append(args, "--define", name+" "+macros[name]) + } + + return args +} + +func shellQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" +} + +func evaluateRPMDevBumpspecSourceEVR(ctx opctx.Ctx, request RPMDevBumpspecRequest) (RPMDevBumpspecSourceEVR, error) { + args := append(rpmdevBumpspecContextArgs(request), + "-q", "--srpm", + "--define", "dist %{nil}", + "--qf", rpmdevBumpspecEVRQueryFormat, + request.SpecPath, + ) + rawCmd := exec.CommandContext(ctx, rpmspecBinary, args...) + rawCmd.Env = rpmdevBumpspecEnv(ctx, request) + + var stdout, stderr bytes.Buffer + + rawCmd.Stdout = &stdout + rawCmd.Stderr = &stderr + + cmd, err := ctx.Command(rawCmd) + if err != nil { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf("failed to create host rpmspec source-EVR query:\n%w", err) + } + + if err := cmd.Run(ctx); err != nil { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf( + "host rpmspec source-EVR query failed for spec %#q (stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, stdout.String(), stderr.String(), err) + } + + result, err := parseRPMDevBumpspecSourceEVR(stdout.String()) + if err != nil { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf("invalid host rpmspec source-EVR query output for spec %#q "+ + "(stdout: %#q, stderr: %#q):\n%w", request.SpecPath, stdout.String(), stderr.String(), err) + } + + return result, nil +} + +func parseRPMDevBumpspecSourceEVR(output string) (RPMDevBumpspecSourceEVR, error) { + if !strings.HasSuffix(output, "\n") || strings.Count(output, "\n") != 1 || strings.Contains(output, "\r") { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf( + "expected exactly one terminal newline in source EVR output, got %#q", output) + } + + fields := strings.Split(strings.TrimSuffix(output, "\n"), rpmdevBumpspecEVRSeparator) + if len(fields) != rpmdevBumpspecEVRFieldCount { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf("expected exactly four source EVR fields, got %#q", output) + } + + for _, field := range fields { + if field == "" { + return RPMDevBumpspecSourceEVR{}, fmt.Errorf("source EVR output contains an empty field: %#q", output) + } + } + + if fields[1] == "(none)" { + fields[1] = "0" + } + + return RPMDevBumpspecSourceEVR{Name: fields[0], Epoch: fields[1], Version: fields[2], Release: fields[3]}, nil +} + +func requireRPMDevBumpspecReleaseIncrease( + ctx opctx.Ctx, request RPMDevBumpspecRequest, before, after RPMDevBumpspecSourceEVR, +) error { + rawCmd := exec.CommandContext(ctx, python3Binary, "-c", rpmLabelCompareProgram, + before.Epoch, before.Version, before.Release, after.Epoch, after.Version, after.Release) + rawCmd.Env = rpmdevBumpspecEnv(ctx, request) + + var stdout, stderr bytes.Buffer + + rawCmd.Stdout = &stdout + rawCmd.Stderr = &stderr + + cmd, err := ctx.Command(rawCmd) + if err != nil { + return fmt.Errorf("failed to create python RPM release comparison:\n%w", err) + } + + if err := cmd.Run(ctx); err != nil { + return fmt.Errorf("host RPM release comparison failed for spec %#q "+ + "(before: %#v, after: %#v, stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, before, after, stdout.String(), stderr.String(), errors.Join(ErrRPMDevBumpspecNoMutation, err)) + } + + switch comparison := stdout.String(); comparison { + case "-1\n": + return nil + case "0\n", "1\n": + return fmt.Errorf("host RPM release comparison rejected non-increasing release for spec %#q "+ + "(before: %#v, after: %#v, comparison: %#q, stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, before, after, comparison, stdout.String(), stderr.String(), ErrRPMDevBumpspecNoMutation) + default: + err := fmt.Errorf("expected exactly '-1\\n', '0\\n', or '1\\n', got %#q", comparison) + + return fmt.Errorf("host RPM release comparison produced invalid output for spec %#q "+ + "(before: %#v, after: %#v, stdout: %#q, stderr: %#q):\n%w", + request.SpecPath, before, after, stdout.String(), stderr.String(), errors.Join(ErrRPMDevBumpspecNoMutation, err)) + } +} + +func rpmdevBumpspecEnv(ctx opctx.Ctx, request RPMDevBumpspecRequest) []string { + originalPath := ctx.OSEnv().Getenv("PATH") + + return []string{ + "PATH=" + filepath.Join(request.HomeDir, rpmdevBumpspecBinDirName) + string(os.PathListSeparator) + originalPath, + "HOME=" + request.HomeDir, + "LC_ALL=C", + "TZ=UTC", + "RPMDEV_BUMPSPEC_ORIGINAL_PATH=" + originalPath, + } +} + +func reportsRPMAutoSpecNoBump(stdout, stderr string) bool { + return strings.Contains(stdout, rpmautospecNoBumpOutput) || strings.Contains(stderr, rpmautospecNoBumpOutput) +} + +func rpmdevBumpspecBinaries() []string { + return []string{RPMDevBumpspecBinary, rpmdevPackagerBinary, rpmBinary, rpmspecBinary, python3Binary} +} + +func validateRPMDevBumpspecRequest(request RPMDevBumpspecRequest) error { + if request.SpecPath == "" || !filepath.IsAbs(request.SpecPath) { + return fmt.Errorf("rpmdev-bumpspec spec path %#q must be absolute", request.SpecPath) + } + + if request.HomeDir == "" || !filepath.IsAbs(request.HomeDir) { + return fmt.Errorf("rpmdev-bumpspec HOME path %#q must be absolute", request.HomeDir) + } + + if strings.ContainsRune(request.HomeDir, os.PathListSeparator) { + return fmt.Errorf("rpmdev-bumpspec HOME path %#q must not contain PATH list separator %#q", + request.HomeDir, string(os.PathListSeparator)) + } + + for _, field := range []struct{ name, value string }{ + {"comment", request.Comment}, {"identity", request.Identity}, {"datestamp", request.Datestamp}, + } { + if strings.TrimSpace(field.value) == "" { + return fmt.Errorf("rpmdev-bumpspec '%s' must not be empty", field.name) + } + } + + return nil +} diff --git a/internal/app/azldev/core/sources/bumpspec_internal_test.go b/internal/app/azldev/core/sources/bumpspec_internal_test.go new file mode 100644 index 00000000..bdd87310 --- /dev/null +++ b/internal/app/azldev/core/sources/bumpspec_internal_test.go @@ -0,0 +1,536 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "context" + "errors" + "fmt" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/externalcmd" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testBumpspecPath = "/sources/component.spec" + testBumpspecHome = "/work/rpmdev-home" + testBumpspecOriginalSpec = "Name: component\nVersion: 1.0\nRelease: 4\n" +) + +func newBumpspecRequest() RPMDevBumpspecRequest { + return RPMDevBumpspecRequest{ + SpecPath: testBumpspecPath, Comment: "- rebuilt", + Identity: "Azure Linux Packaging Team ", + Datestamp: "Mon Jan 06 2025", HomeDir: testBumpspecHome, + Build: projectconfig.ComponentBuildConfig{ + With: []string{"feature"}, Without: []string{"legacy"}, + Defines: map[string]string{"zeta": "2", "alpha": "1"}, Undefines: []string{"ambient"}, + }, + } +} + +func newBumpspecCtx() *testctx.TestCtx { + osEnv := testctx.NewTestOSEnv() + osEnv.SetEnv("PATH", "/test/bin") + + ctx := testctx.NewCtx(testctx.WithOSEnv(osEnv)) + for _, binary := range rpmdevBumpspecBinaries() { + ctx.CmdFactory.RegisterCommandInSearchPath(binary) + } + + return ctx +} + +func writeBumpspecSpec(t *testing.T, ctx *testctx.TestCtx, content string) { + t.Helper() + require.NoError(t, fileutils.MkdirAll(ctx.FS(), filepath.Dir(testBumpspecPath))) + require.NoError(t, fileutils.WriteFile(ctx.FS(), testBumpspecPath, []byte(content), fileperms.PublicFile)) +} + +func evrOutput(name, epoch, release string) string { + return strings.Join([]string{name, epoch, "1.0", release}, rpmdevBumpspecEVRSeparator) + "\n" +} + +func isEVRQuery(cmd *exec.Cmd) bool { + return len(cmd.Args) >= 3 && cmd.Args[0] == rpmspecBinary && + slices.Contains(cmd.Args, "-q") && slices.Contains(cmd.Args, "--srpm") +} + +func isReleaseComparison(cmd *exec.Cmd) bool { + return len(cmd.Args) >= 2 && cmd.Args[0] == python3Binary && cmd.Args[1] == "-c" +} + +func TestRunRPMDevBumpspec_UsesHostEVRAndFixedContext(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + before := "Name: component\nVersion: 1.0\nRelease: 4%{?dist}\n%changelog\n" + after := "Name: component\nVersion: 1.0\nRelease: 5%{?dist}\n%changelog\n- rebuilt\n" + + writeBumpspecSpec(t, ctx, before) + + queries := 0 + + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queries++ + + release := "5" + if queries == 1 { + release = "4" + } + + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", release)) + case cmd.Path == RPMDevBumpspecBinary: + assert.Equal(t, []string{ + RPMDevBumpspecBinary, "-c", "- rebuilt", "-u", + "Azure Linux Packaging Team ", "-d", "Mon Jan 06 2025", + testBumpspecPath, + }, cmd.Args) + require.NoError(t, fileutils.WriteFile(ctx.FS(), testBumpspecPath, []byte(after), fileperms.PublicFile)) + case isReleaseComparison(cmd): + assert.Equal(t, []string{python3Binary, "-c", rpmLabelCompareProgram, "0", "1.0", "4", "0", "1.0", "5"}, cmd.Args) + _, _ = fmt.Fprintln(cmd.Stdout, "-1") + default: + return fmt.Errorf("unexpected command: %#v", cmd.Args) + } + + return nil + } + + require.NoError(t, RunRPMDevBumpspec(ctx, request)) + content, err := fileutils.ReadFile(ctx.FS(), testBumpspecPath) + require.NoError(t, err) + assert.Equal(t, after, string(content)) + + wrapperPath := filepath.Join(testBumpspecHome, rpmdevBumpspecBinDirName, rpmdevBumpspecRPMWrapper) + wrapper, err := fileutils.ReadFile(ctx.FS(), wrapperPath) + require.NoError(t, err) + assert.Contains(t, string(wrapper), "PATH=\"$RPMDEV_BUMPSPEC_ORIGINAL_PATH\"") + assert.Contains(t, string(wrapper), "'--define' '_sourcedir /sources'") + assert.NotContains(t, string(wrapper), "'--with'") + assert.NotContains(t, string(wrapper), "'--without'") + assert.Contains(t, string(wrapper), + "'--define' '_with_feature 1' '--define' '_without_legacy 1' '--define' 'alpha 1' '--define' 'zeta 2'") + assert.Equal(t, 2, queries) +} + +func TestRunRPMDevBumpspec_RestoresOriginalBytesOnEveryFailure(t *testing.T) { + for _, testCase := range []struct { + name string + handler func(*exec.Cmd, *testctx.TestCtx) error + }{ + { + name: "pre mutation evaluation failure", + handler: func(cmd *exec.Cmd, _ *testctx.TestCtx) error { + if isEVRQuery(cmd) { + return errors.New("before query failed") + } + + return errors.New("bumpspec must not run") + }, + }, + { + name: "command failure after mutation", + handler: func(cmd *exec.Cmd, ctx *testctx.TestCtx) error { + if isEVRQuery(cmd) { + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", "4")) + + return nil + } + if cmd.Path == RPMDevBumpspecBinary { + require.NoError(t, fileutils.WriteFile(ctx.FS(), testBumpspecPath, + []byte("Name: component\nVersion: 1.0\nRelease: 5\n"), fileperms.PublicFile)) + + return errors.New("bumpspec failed") + } + + return nil + }, + }, + { + name: "post mutation evaluation failure", + handler: func(cmd *exec.Cmd, ctx *testctx.TestCtx) error { + if isEVRQuery(cmd) { + if strings.Contains(stringMustRead(t, ctx, testBumpspecPath), "Release: 5") { + return errors.New("after query failed") + } + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", "4")) + + return nil + } + if cmd.Path == RPMDevBumpspecBinary { + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, + []byte("Name: component\nVersion: 1.0\nRelease: 5\n"), fileperms.PublicFile) + } + + return nil + }, + }, + { + name: "unchanged malformed or decreased release", + handler: func(cmd *exec.Cmd, ctx *testctx.TestCtx) error { + if isEVRQuery(cmd) { + release := "4" + if strings.Contains(stringMustRead(t, ctx, testBumpspecPath), "Release: 5") { + release = "" + } + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", release)) + + return nil + } + if cmd.Path == RPMDevBumpspecBinary { + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, + []byte("Name: component\nVersion: 1.0\nRelease: 5\n"), fileperms.PublicFile) + } + + return nil + }, + }, + { + name: "identity changes", + handler: func(cmd *exec.Cmd, ctx *testctx.TestCtx) error { + if isEVRQuery(cmd) { + if strings.Contains(stringMustRead(t, ctx, testBumpspecPath), "Release: 5") { + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("other", "0", "5")) + } else { + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", "4")) + } + + return nil + } + if cmd.Path == RPMDevBumpspecBinary { + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, + []byte("Name: component\nVersion: 1.0\nRelease: 5\n"), fileperms.PublicFile) + } + + return nil + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + original := "Name: component\nVersion: 1.0\nRelease: 4\n%changelog\n" + writeBumpspecSpec(t, ctx, original) + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { return testCase.handler(cmd, ctx) } + require.Error(t, RunRPMDevBumpspec(ctx, request)) + assert.Equal(t, original, stringMustRead(t, ctx, testBumpspecPath)) + }) + } +} + +func TestRunRPMDevBumpspec_RequiresRPMReleaseIncrease(t *testing.T) { + for _, testCase := range []struct { + name string + comparisonOut string + comparisonErr error + wantSuccess bool + }{ + {name: "newer result", comparisonOut: "-1\n", wantSuccess: true}, + {name: "unchanged result", comparisonOut: "0\n"}, + {name: "older result", comparisonOut: "1\n"}, + {name: "noncanonical negative result", comparisonOut: "-01\n"}, + {name: "noncanonical positive result", comparisonOut: "+1\n"}, + {name: "leading space", comparisonOut: " -1\n"}, + {name: "trailing space", comparisonOut: "-1 \n"}, + {name: "empty result"}, + {name: "extra newline", comparisonOut: "-1\n\n"}, + {name: "CRLF", comparisonOut: "-1\r\n"}, + {name: "arbitrary text", comparisonOut: "not an integer\n"}, + {name: "comparison command failure", comparisonErr: errors.New("python failed")}, + } { + t.Run(testCase.name, func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + original := testBumpspecOriginalSpec + after := "Name: component\nVersion: 1.0\nRelease: 5\n" + + writeBumpspecSpec(t, ctx, original) + + queryCount := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queryCount++ + + release := "4" + if queryCount == 2 { + release = "5" + } + + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "(none)", release)) + case cmd.Path == RPMDevBumpspecBinary: + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, []byte(after), fileperms.PublicFile) + case isReleaseComparison(cmd): + if testCase.comparisonErr != nil { + return testCase.comparisonErr + } + + _, _ = fmt.Fprint(cmd.Stdout, testCase.comparisonOut) + default: + return fmt.Errorf("unexpected command: %#v", cmd.Args) + } + + return nil + } + + err := RunRPMDevBumpspec(ctx, request) + assert.Equal(t, testCase.wantSuccess, err == nil) + + if !testCase.wantSuccess { + require.ErrorIs(t, err, ErrRPMDevBumpspecNoMutation) + assert.Equal(t, original, stringMustRead(t, ctx, testBumpspecPath)) + } else { + assert.Equal(t, after, stringMustRead(t, ctx, testBumpspecPath)) + } + + assert.Equal(t, 2, queryCount) + }) + } +} + +func TestRunRPMDevBumpspec_RuntimeAndDryRun(t *testing.T) { + t.Run("missing python is actionable", func(t *testing.T) { + ctx := testctx.NewCtx() + for _, binary := range []string{RPMDevBumpspecBinary, rpmdevPackagerBinary, rpmBinary, rpmspecBinary} { + ctx.CmdFactory.RegisterCommandInSearchPath(binary) + } + + err := RunRPMDevBumpspec(ctx, newBumpspecRequest()) + require.ErrorIs(t, err, externalcmd.ErrMissingExecutable) + assert.Contains(t, err.Error(), python3Binary) + }) + t.Run("dry run delegates only bumpspec", func(t *testing.T) { + ctx := newBumpspecCtx() + ctx.DryRunValue = true + request := newBumpspecRequest() + + writeBumpspecSpec(t, ctx, "Name: component\nVersion: 1.0\nRelease: 4\n") + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + assert.Equal(t, RPMDevBumpspecBinary, cmd.Path) + + return nil + } + require.NoError(t, RunRPMDevBumpspec(ctx, request)) + }) + t.Run("cancelled before runtime discovery", func(t *testing.T) { + base, cancel := context.WithCancel(context.Background()) + cancel() + + ctx := testctx.NewCtx(func(ctx *testctx.TestCtx) { ctx.Ctx = base }) + require.ErrorIs(t, RunRPMDevBumpspec(ctx, newBumpspecRequest()), context.Canceled) + }) +} + +func TestParseRPMDevBumpspecSourceEVR_RequiresOneCanonicalRecord(t *testing.T) { + valid := evrOutput("component", "(none)", "4") + + for _, testCase := range []struct { + name string + output string + wantEVR RPMDevBumpspecSourceEVR + }{ + { + name: "valid record with one terminal newline", output: valid, + wantEVR: RPMDevBumpspecSourceEVR{Name: "component", Epoch: "0", Version: "1.0", Release: "4"}, + }, + {name: "missing terminal newline", output: strings.TrimSuffix(valid, "\n")}, + {name: "extra terminal newline", output: valid + "\n"}, + {name: "CRLF", output: strings.TrimSuffix(valid, "\n") + "\r\n"}, + {name: "stdout prefix before Name", output: "banner\n" + valid}, + {name: "newline in Name", output: evrOutput("component\nextra", "0", "4")}, + {name: "CR in Name", output: evrOutput("component\rextra", "0", "4")}, + {name: "newline in Epoch", output: evrOutput("component", "0\nextra", "4")}, + {name: "CR in Epoch", output: evrOutput("component", "0\rextra", "4")}, + {name: "newline in Version", output: "component" + rpmdevBumpspecEVRSeparator + "0" + + rpmdevBumpspecEVRSeparator + "1.0\nextra" + rpmdevBumpspecEVRSeparator + "4\n"}, + {name: "CR in Version", output: "component" + rpmdevBumpspecEVRSeparator + "0" + + rpmdevBumpspecEVRSeparator + "1.0\rextra" + rpmdevBumpspecEVRSeparator + "4\n"}, + {name: "newline in Release", output: evrOutput("component", "0", "4\nextra")}, + {name: "CR in Release", output: evrOutput("component", "0", "4\rextra")}, + {name: "suffix after record", output: valid + "suffix"}, + {name: "wrong field count", output: "component" + rpmdevBumpspecEVRSeparator + "0\n"}, + {name: "empty field", output: evrOutput("component", "0", "")}, + } { + t.Run(testCase.name, func(t *testing.T) { + evr, err := parseRPMDevBumpspecSourceEVR(testCase.output) + if testCase.wantEVR == (RPMDevBumpspecSourceEVR{}) { + require.Error(t, err) + + return + } + + require.NoError(t, err) + assert.Equal(t, testCase.wantEVR, evr) + }) + } +} + +func TestRunRPMDevBumpspec_RestoresOriginalBytesAfterMalformedEVROutput(t *testing.T) { + for _, malformedOutput := range []string{ + "banner\n" + evrOutput("component", "0", "5"), + evrOutput("component", "0", "5") + "\n", + } { + t.Run(fmt.Sprintf("%q", malformedOutput), func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + original := testBumpspecOriginalSpec + after := "Name: component\nVersion: 1.0\nRelease: 5\n" + + writeBumpspecSpec(t, ctx, original) + + queryCount := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queryCount++ + if queryCount == 1 { + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", "4")) + } else { + _, _ = fmt.Fprint(cmd.Stdout, malformedOutput) + } + case cmd.Path == RPMDevBumpspecBinary: + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, []byte(after), fileperms.PublicFile) + } + + return nil + } + + require.Error(t, RunRPMDevBumpspec(ctx, request)) + assert.Equal(t, original, stringMustRead(t, ctx, testBumpspecPath)) + }) + } +} + +func TestRunRPMDevBumpspec_UsesOneSRPMRecordAndRollsBackMultipleRecords(t *testing.T) { + t.Run("source record is accepted for a multi-subpackage spec", func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + before := "Name: component\nVersion: 1.0\nRelease: 4\n%package child\nSummary: child\n" + after := strings.Replace(before, "Release: 4", "Release: 5", 1) + writeBumpspecSpec(t, ctx, before) + + queryCount := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queryCount++ + + release := "4" + if queryCount == 2 { + release = "5" + } + + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", release)) + case cmd.Path == RPMDevBumpspecBinary: + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, []byte(after), fileperms.PublicFile) + case isReleaseComparison(cmd): + _, _ = fmt.Fprintln(cmd.Stdout, "-1") + } + + return nil + } + + require.NoError(t, RunRPMDevBumpspec(ctx, request)) + assert.Equal(t, after, stringMustRead(t, ctx, testBumpspecPath)) + assert.Equal(t, 2, queryCount) + }) + + t.Run("multiple records are rejected and restored", func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + original := testBumpspecOriginalSpec + writeBumpspecSpec(t, ctx, original) + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + if isEVRQuery(cmd) { + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", "4")+evrOutput("component-child", "0", "4")) + } + + return nil + } + + require.Error(t, RunRPMDevBumpspec(ctx, request)) + assert.Equal(t, original, stringMustRead(t, ctx, testBumpspecPath)) + }) +} + +func TestRunRPMDevBumpspec_AppliesTargetToQueryAndWrapper(t *testing.T) { + for _, target := range []string{"x86_64", "aarch64"} { + t.Run(target, func(t *testing.T) { + ctx := newBumpspecCtx() + request := newBumpspecRequest() + request.TargetArch = target + original := "Name: component\nVersion: 1.0\nRelease: %{?target}\n" + writeBumpspecSpec(t, ctx, original) + + queryCount := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + assert.Contains(t, cmd.Args, "--target="+target) + + queryCount++ + + release := "9" + if target == "aarch64" { + release = "5" + } + + if queryCount == 2 { + release += ".1" + } + + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("component", "0", release)) + case cmd.Path == RPMDevBumpspecBinary: + return fileutils.WriteFile(ctx.FS(), testBumpspecPath, + []byte("Name: component\nVersion: 1.0\nRelease: changed\n"), fileperms.PublicFile) + case isReleaseComparison(cmd): + _, _ = fmt.Fprintln(cmd.Stdout, "-1") + } + + return nil + } + + require.NoError(t, RunRPMDevBumpspec(ctx, request)) + wrapper, err := fileutils.ReadFile(ctx.FS(), + filepath.Join(testBumpspecHome, rpmdevBumpspecBinDirName, rpmdevBumpspecRPMWrapper)) + require.NoError(t, err) + assert.Contains(t, string(wrapper), "'--target="+target+"'") + }) + } +} + +func TestRPMDevBumpspecContextArgs_UsesEffectiveBuildMacros(t *testing.T) { + request := newBumpspecRequest() + request.Build.Defines["_with_feature"] = "explicit" + request.Build.Undefines = append(request.Build.Undefines, "_without_legacy") + + assert.Equal(t, []string{ + "--define", "_sourcedir /sources", + "--define", "_specdir /sources", + "--define", "_with_feature explicit", + "--define", "alpha 1", + "--define", "zeta 2", + }, rpmdevBumpspecContextArgs(request)) +} + +func stringMustRead(t *testing.T, ctx *testctx.TestCtx, path string) string { + t.Helper() + + content, err := fileutils.ReadFile(ctx.FS(), path) + require.NoError(t, err) + + return string(content) +} From ec71059820fa488bc4944cdaf7387374e58f593f Mon Sep 17 00:00:00 2001 From: Thien Trung Vuong Date: Wed, 9 Sep 2026 01:16:04 +0000 Subject: [PATCH 2/3] feat(sources): use rpmdev-bumpspec for release updates Keep fingerprint-derived synthetic changes as the source of N while delegating non-autorelease Release and changelog updates to the transactional rpmdev-bumpspec runner. Build, render, and prepare-sources share the same host operation and available build target context. Remove the custom static-integer parser and release-tag overlay. Each requested bump must produce a strictly newer source Release, and any failed operation restores the complete pre-sequence spec while existing manual and autorelease modes remain unchanged. --- internal/app/azldev/cmds/component/build.go | 1 + .../azldev/cmds/component/preparesources.go | 7 + internal/app/azldev/cmds/component/render.go | 1 + internal/app/azldev/core/sources/release.go | 182 ++++------ .../core/sources/release_internal_test.go | 337 +++++++++--------- .../app/azldev/core/sources/release_test.go | 44 --- .../app/azldev/core/sources/sourceprep.go | 22 +- 7 files changed, 276 insertions(+), 318 deletions(-) diff --git a/internal/app/azldev/cmds/component/build.go b/internal/app/azldev/cmds/component/build.go index 60aed20f..5d056925 100644 --- a/internal/app/azldev/cmds/component/build.go +++ b/internal/app/azldev/cmds/component/build.go @@ -271,6 +271,7 @@ func buildComponent( if !options.WithoutGitRepo { preparerOpts = append(preparerOpts, sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), + sources.WithRPMDevBumpspec(env, env.WorkDir(), options.MockConfigOpts["target_arch"]), sources.WithDirtyDetection(), ) } diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index ac48bdca..2072c78b 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -130,6 +130,12 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er "synthetic history requires overlays to be applied") } + if !options.WithoutGitRepo && !options.SkipOverlays { + if err := fileutils.MkdirAll(env.FS(), env.WorkDir()); err != nil { + return fmt.Errorf("failed to create work directory %#q:\n%w", env.WorkDir(), err) + } + } + preparerOpts := buildPreparerOptions(env, distro, options) preparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, preparerOpts...) @@ -157,6 +163,7 @@ func buildPreparerOptions( if !options.WithoutGitRepo && !options.SkipOverlays { opts = append(opts, sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), + sources.WithRPMDevBumpspec(env, env.WorkDir(), ""), sources.WithDirtyDetection(), ) } diff --git a/internal/app/azldev/cmds/component/render.go b/internal/app/azldev/cmds/component/render.go index 163da0dd..5603f3b7 100644 --- a/internal/app/azldev/cmds/component/render.go +++ b/internal/app/azldev/cmds/component/render.go @@ -525,6 +525,7 @@ func prepareComponentSources( // sidecar files are needed for rendering. preparerOpts := []sources.PreparerOption{ sources.WithGitRepo(env, env.LockReader(), distro.Version.ReleaseVer), + sources.WithRPMDevBumpspec(env, env.WorkDir(), ""), sources.WithDirtyDetection(), sources.WithSkipLookaside(), sources.WithUpstreamProvenance(sources.FedoraDistTag(distro.Ref.Name, distro.Version.ReleaseVer)), diff --git a/internal/app/azldev/core/sources/release.go b/internal/app/azldev/core/sources/release.go index 55dcad6c..8d64e77b 100644 --- a/internal/app/azldev/core/sources/release.go +++ b/internal/app/azldev/core/sources/release.go @@ -4,36 +4,30 @@ package sources import ( + "context" + "errors" "fmt" "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" "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" ) -// autoreleasePattern matches the %autorelease macro invocation in a Release tag value. -// This covers: -// - bare form: %autorelease -// - braced form: %{autorelease} -// - braced form with arguments: %{autorelease -e asan} -// - conditional form (no fallback): %{?autorelease} -var autoreleasePattern = regexp.MustCompile(`%(\{[?]?autorelease($|[}\s])|autorelease($|\s))`) +const ( + rpmDevBumpspecIdentity = "Azure Linux Packaging Team " + rpmDevBumpspecComment = "- rebuilt" + rpmDevBumpspecDate = "Mon Jan 06 2025" +) -// staticReleasePattern matches only the two Release tag forms we can safely -// auto-bump: a bare integer (e.g. "1") or an integer followed by a -// dist macro (e.g. "5%{?dist}" or "5%{dist}"). Any other suffix — dotted -// segments, unknown macros, etc. — is rejected so the component must use -// 'release.calculation = "manual"'. -var staticReleasePattern = regexp.MustCompile(`^(\d+)(%\{\??dist\})?$`) +var autoreleasePattern = regexp.MustCompile(`%(\{[?]?autorelease($|[}\s])|autorelease($|\s))`) // 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) { specFile, err := fs.Open(specPath) if err != nil { @@ -66,81 +60,33 @@ func GetReleaseTagValue(fs opctx.FS, specPath string) (string, error) { return releaseValue, nil } -// ReleaseUsesAutorelease reports whether the given Release tag value uses the -// %autorelease macro (either bare or braced form). +// ReleaseUsesAutorelease reports whether the given Release tag value uses the %autorelease macro. func ReleaseUsesAutorelease(releaseValue string) bool { return autoreleasePattern.MatchString(releaseValue) } -// BumpStaticRelease increments the leading integer in a static Release tag value -// by the given commit count. -func BumpStaticRelease(releaseValue string, commitCount int) (string, error) { - matches := staticReleasePattern.FindStringSubmatch(releaseValue) - if matches == nil { - return "", fmt.Errorf("release value %#q does not start with an integer", releaseValue) +//nolint:cyclop,funlen // The transaction boundaries preserve all four release modes and failure paths. +func (p *sourcePreparerImpl) tryBumpStaticRelease( + ctx context.Context, component components.Component, sourcesDirPath string, changes []FingerprintChange, +) (err error) { + if err := ctx.Err(); err != nil { + return fmt.Errorf("release bump cancelled before execution:\n%w", err) } - currentRelease, err := strconv.Atoi(matches[1]) - if err != nil { - return "", fmt.Errorf("failed to parse release number from %#q:\n%w", releaseValue, err) + if len(changes) == 0 { + return nil } - newRelease := currentRelease + commitCount - suffix := matches[2] - - return fmt.Sprintf("%d%s", newRelease, suffix), nil -} - -// tryBumpStaticRelease manages the Release tag based on the component's release -// calculation mode. It may bump, skip, or auto-detect depending on configuration: -// -// - "manual": no-op — component manages its own release numbering. -// - "autorelease": no-op — rpmautospec resolves the release from git history. -// - "static": always bumps the static integer release by commitCount. -// - "auto": auto-detects from the spec's Release tag value; skips if -// %autorelease is found, otherwise bumps the static integer. -func (p *sourcePreparerImpl) tryBumpStaticRelease( - component components.Component, - sourcesDirPath string, - commitCount int, -) error { calc := component.GetConfig().Release.Calculation - switch calc { - case projectconfig.ReleaseCalculationManual: - slog.Debug("Component uses manual release calculation; skipping static release bump", - "component", component.GetName()) - + case projectconfig.ReleaseCalculationManual, projectconfig.ReleaseCalculationAutorelease: return nil - - case projectconfig.ReleaseCalculationAutorelease: - slog.Debug("Component uses autorelease calculation; skipping static release bump", - "component", component.GetName()) - - return nil - - case projectconfig.ReleaseCalculationStatic: - return p.readAndBumpRelease(component, sourcesDirPath, commitCount, true) - - case projectconfig.ReleaseCalculationAuto: - return p.readAndBumpRelease(component, sourcesDirPath, commitCount, false) - + case projectconfig.ReleaseCalculationAuto, projectconfig.ReleaseCalculationStatic: + // Continue with automatic release handling. default: - return fmt.Errorf("component %#q has unknown release calculation mode %#q", - component.GetName(), calc) + return fmt.Errorf("component %#q has unknown release calculation mode %#q", component.GetName(), calc) } -} -// readAndBumpRelease reads the Release tag from the spec and bumps its static integer. -// When requireStaticRelease is true (explicit static mode), encountering %autorelease -// produces an error telling the user to switch to 'release.calculation = "autorelease"'. -// When false (auto mode), specs using %autorelease are silently skipped. -func (p *sourcePreparerImpl) readAndBumpRelease( - component components.Component, - sourcesDirPath string, - commitCount int, - requireStaticRelease bool, -) error { specPath, err := p.resolveSpecPath(component, sourcesDirPath) if err != nil { return err @@ -148,49 +94,73 @@ func (p *sourcePreparerImpl) readAndBumpRelease( releaseValue, err := GetReleaseTagValue(p.fs, specPath) if err != nil { - return fmt.Errorf("failed to read Release tag for component %#q:\n%w", - component.GetName(), err) + return fmt.Errorf("failed to read Release tag for component %#q:\n%w", component.GetName(), err) } if ReleaseUsesAutorelease(releaseValue) { - if requireStaticRelease { - return fmt.Errorf( - "component %#q has 'release.calculation = \"static\"' but its Release tag "+ - "uses %%autorelease; set 'release.calculation = \"autorelease\"' instead", - component.GetName()) + if calc == projectconfig.ReleaseCalculationStatic { + return fmt.Errorf("component %#q has 'release.calculation = \"static\"' but its Release tag uses %%autorelease; "+ + "set 'release.calculation = \"autorelease\"' instead", component.GetName()) } - slog.Debug("Spec uses %%autorelease; skipping static release bump", - "component", component.GetName()) - return nil } - newRelease, err := BumpStaticRelease(releaseValue, commitCount) - if err != nil { - return fmt.Errorf( - "component %#q has a non-standard Release tag value %#q that cannot be auto-bumped; "+ - "set 'release.calculation = \"manual\"' in the component configuration "+ - "and add a \"spec-set-tag\" overlay for the Release tag if needed:\n%w", - component.GetName(), releaseValue, err) + if p.bumpspecCtx == nil || p.bumpspecScratchDir == "" { + return fmt.Errorf("component %#q requires rpmdev-bumpspec release handling, but its host runner is not configured", + component.GetName()) } - slog.Info("Bumping static release", - "component", component.GetName(), - "oldRelease", releaseValue, - "newRelease", newRelease, - "commitCount", commitCount) + if err := fileutils.MkdirAll(p.bumpspecCtx.FS(), p.bumpspecScratchDir); err != nil { + return fmt.Errorf("failed to create rpmdev-bumpspec scratch parent %#q:\n%w", p.bumpspecScratchDir, err) + } - overlay := projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayUpdateSpecTag, - Tag: "Release", - Value: newRelease, + originalBytes, err := fileutils.ReadFile(p.fs, specPath) + if err != nil { + return fmt.Errorf("failed to save original spec %#q before release bumps:\n%w", specPath, err) } - if err := ApplySpecOverlayToFileInPlace(p.fs, overlay, specPath); err != nil { - return fmt.Errorf("failed to apply release bump overlay for component %#q:\n%w", - component.GetName(), err) + defer func() { + if err == nil { + return + } + + if restoreErr := fileutils.WriteFile(p.fs, specPath, originalBytes, fileperms.PublicFile); restoreErr != nil { + err = fmt.Errorf("release bump failed and restoring original spec %#q also failed:\n%w", + specPath, errors.Join(err, restoreErr)) + } + }() + + for operation := range changes { + homeDir, mkErr := fileutils.MkdirTemp(p.bumpspecCtx.FS(), p.bumpspecScratchDir, "azldev-bumpspec-") + if mkErr != nil { + return fmt.Errorf("failed to create rpmdev-bumpspec scratch directory:\n%w", mkErr) + } + + request := RPMDevBumpspecRequest{ + SpecPath: specPath, Comment: rpmDevBumpspecComment, Identity: rpmDevBumpspecIdentity, + Datestamp: rpmDevBumpspecDate, HomeDir: homeDir, Build: component.GetConfig().Build, + TargetArch: p.bumpspecTargetArch, + } + runErr := RunRPMDevBumpspec(p.bumpspecCtx, request) //nolint:contextcheck + + cleanupErr := p.bumpspecCtx.FS().RemoveAll(homeDir) + if runErr != nil { + if cleanupErr != nil { + return fmt.Errorf("failed release bump operation %d and scratch cleanup:\n%w", + operation+1, errors.Join(runErr, cleanupErr)) + } + + return fmt.Errorf("failed to bump release for component %#q at operation %d:\n%w", + component.GetName(), operation+1, runErr) + } + + if cleanupErr != nil { + return fmt.Errorf("failed to clean rpmdev-bumpspec scratch directory %#q:\n%w", homeDir, cleanupErr) + } } + slog.Info("Bumped release with rpmdev-bumpspec", "component", component.GetName(), "count", len(changes)) + return nil } diff --git a/internal/app/azldev/core/sources/release_internal_test.go b/internal/app/azldev/core/sources/release_internal_test.go index 6a37e078..85813cd1 100644 --- a/internal/app/azldev/core/sources/release_internal_test.go +++ b/internal/app/azldev/core/sources/release_internal_test.go @@ -4,10 +4,17 @@ package sources import ( + "context" + "errors" + "fmt" + "os/exec" "path/filepath" + "strconv" + "strings" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components/components_testutils" + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" @@ -19,22 +26,35 @@ import ( const testSourcesDir = "/sources" -func newTestPreparer(memFS afero.Fs) *sourcePreparerImpl { - return &sourcePreparerImpl{ - fs: memFS, - } +func releaseTestPreparer(t *testing.T) (*sourcePreparerImpl, *testctx.TestCtx) { + t.Helper() + + ctx := newBumpspecCtx() + + return &sourcePreparerImpl{fs: ctx.FS(), bumpspecCtx: ctx, bumpspecScratchDir: "/work/missing"}, ctx } -func writeTestSpec(t *testing.T, memFS afero.Fs, name, release string) { +func writeTestSpec(t *testing.T, memFS afero.Fs, name, content string) string { t.Helper() - specDir := filepath.Join(testSourcesDir, name) - require.NoError(t, fileutils.MkdirAll(memFS, specDir)) + specPath := filepath.Join(testSourcesDir, name, name+".spec") + require.NoError(t, fileutils.MkdirAll(memFS, filepath.Dir(specPath))) + require.NoError(t, fileutils.WriteFile(memFS, specPath, []byte(content), fileperms.PublicFile)) + + return specPath +} - specPath := filepath.Join(specDir, name+".spec") - content := []byte("Name: " + name + "\nVersion: 1.0.0\nRelease: " + release + "\nSummary: Test\nLicense: MIT\n") +func mockReleaseComponent( + ctrl *gomock.Controller, name string, calculation projectconfig.ReleaseCalculation, +) *components_testutils.MockComponent { + comp := components_testutils.NewMockComponent(ctrl) + comp.EXPECT().GetName().AnyTimes().Return(name) + comp.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Release: projectconfig.ReleaseConfig{Calculation: calculation}, + Build: projectconfig.ComponentBuildConfig{Defines: map[string]string{"fixture": "1"}}, + }) - require.NoError(t, fileutils.WriteFile(memFS, specPath, content, fileperms.PublicFile)) + return comp } func mockComponent( @@ -47,175 +67,162 @@ func mockComponent( return comp } -func TestTryBumpStaticRelease_ManualSkips(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - comp := mockComponent(ctrl, "kernel", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationManual, - }, - }) - - // No spec file needed — should skip before reading anything. - err := preparer.tryBumpStaticRelease(comp, testSourcesDir, 3) - require.NoError(t, err) +func testChanges() []FingerprintChange { + return []FingerprintChange{ + {CommitMetadata: CommitMetadata{Hash: "first", Timestamp: 1, Message: "different"}}, + {CommitMetadata: CommitMetadata{Hash: "second", Timestamp: 2, Message: "metadata"}}, + } } -func TestTryBumpStaticRelease_AutoreleaseSkips(t *testing.T) { +func TestTryBumpStaticRelease_UsesEVRTransactionsAndFixedMetadata(t *testing.T) { ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "test-pkg", "%autorelease") - - comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationAuto, - }, - }) + preparer, ctx := releaseTestPreparer(t) + original := "Name: test-pkg\nVersion: 1.0\nRelease: 4%{?dist}\n%changelog\n" + specPath := writeTestSpec(t, ctx.FS(), "test-pkg", original) + comp := mockReleaseComponent(ctrl, "test-pkg", projectconfig.ReleaseCalculationAuto) + queries := 0 + bumps := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queries++ + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("test-pkg", "0", strconv.Itoa(3+(queries+1)/2))) + case cmd.Path == RPMDevBumpspecBinary: + bumps++ + + assert.Equal(t, "- rebuilt", cmd.Args[2]) + assert.Equal(t, "Mon Jan 06 2025", cmd.Args[6]) + content := stringMustRead(t, ctx, specPath) + content = strings.Replace(content, fmt.Sprintf("Release: %d%%{?dist}", 3+bumps), + fmt.Sprintf("Release: %d%%{?dist}", 4+bumps), 1) + require.NoError(t, fileutils.WriteFile(ctx.FS(), specPath, []byte(content), fileperms.PublicFile)) + case isReleaseComparison(cmd): + _, _ = fmt.Fprintln(cmd.Stdout, "-1") + default: + return fmt.Errorf("unexpected command: %#v", cmd.Args) + } + + return nil + } - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "test-pkg"), 3) + require.NoError(t, preparer.tryBumpStaticRelease(context.Background(), comp, + filepath.Join(testSourcesDir, "test-pkg"), testChanges())) + assert.Equal(t, 2, bumps) + assert.Equal(t, 4, queries) + assert.Contains(t, stringMustRead(t, ctx, specPath), "Release: 6%{?dist}") + entries, err := afero.ReadDir(ctx.FS(), "/work/missing") require.NoError(t, err) + assert.Empty(t, entries) } -func TestTryBumpStaticRelease_StaticBumps(t *testing.T) { +func TestTryBumpStaticRelease_RollsBackWholeSequence(t *testing.T) { ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "test-pkg", "1%{?dist}") - - comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationAuto, - }, - }) - - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "test-pkg"), 3) - require.NoError(t, err) + preparer, ctx := releaseTestPreparer(t) + original := "Name: osbs-client\nVersion: 1.0\n%if 0\n%global release 23\n%else\n" + + "%global release 6\n%endif\nRelease: %{release}\n" + specPath := writeTestSpec(t, ctx.FS(), "osbs-client", original) + comp := mockReleaseComponent(ctrl, "osbs-client", projectconfig.ReleaseCalculationAuto) + queries := 0 + bumps := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queries++ + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("osbs-client", "0", "6")) + case cmd.Path == RPMDevBumpspecBinary: + bumps++ + + content := stringMustRead(t, ctx, specPath) + if bumps == 1 { + return fileutils.WriteFile(ctx.FS(), specPath, []byte(strings.Replace(content, "release 23", "release 24", 1)), + fileperms.PublicFile) + } + + return fileutils.WriteFile(ctx.FS(), specPath, []byte(strings.Replace(content, "release 6", "release 7", 1)), + fileperms.PublicFile) + case isReleaseComparison(cmd): + _, _ = fmt.Fprintln(cmd.Stdout, "0") + } + + return nil + } - // Verify the spec was updated. - specPath := filepath.Join(testSourcesDir, "test-pkg", "test-pkg.spec") - content, err := fileutils.ReadFile(memFS, specPath) - require.NoError(t, err) - assert.Contains(t, string(content), "Release: 4%{?dist}") + err := preparer.tryBumpStaticRelease( + context.Background(), comp, filepath.Join(testSourcesDir, "osbs-client"), testChanges()) + require.ErrorIs(t, err, ErrRPMDevBumpspecNoMutation) + assert.Equal(t, original, stringMustRead(t, ctx, specPath), "inactive release mutation must be rolled back") + assert.Equal(t, 1, bumps) } -func TestTryBumpStaticRelease_StaticBumpsNonConditionalDist(t *testing.T) { +func TestTryBumpStaticRelease_RollsBackAfterKthFailure(t *testing.T) { ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "test-pkg", "1%{dist}") - - comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationAuto, - }, - }) - - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "test-pkg"), 3) - require.NoError(t, err) - - specPath := filepath.Join(testSourcesDir, "test-pkg", "test-pkg.spec") - content, err := fileutils.ReadFile(memFS, specPath) - require.NoError(t, err) - assert.Contains(t, string(content), "Release: 4%{dist}") -} - -func TestTryBumpStaticRelease_NonStandardErrorsWithoutManual(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "kernel", "%{pkg_release}") - - comp := mockComponent(ctrl, "kernel", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationAuto, - }, - }) + preparer, ctx := releaseTestPreparer(t) + original := "Name: test-pkg\nVersion: 1.0\nRelease: 4\n" + specPath := writeTestSpec(t, ctx.FS(), "test-pkg", original) + comp := mockReleaseComponent(ctrl, "test-pkg", projectconfig.ReleaseCalculationAuto) + queries := 0 + bumps := 0 + ctx.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { + switch { + case isEVRQuery(cmd): + queries++ + + release := "4" + if queries > 1 { + release = "5" + } + + _, _ = fmt.Fprint(cmd.Stdout, evrOutput("test-pkg", "0", release)) + case cmd.Path == RPMDevBumpspecBinary: + bumps++ + if bumps == 2 { + return errors.New("second operation failed") + } + + return fileutils.WriteFile(ctx.FS(), specPath, + []byte("Name: test-pkg\nVersion: 1.0\nRelease: 5\n"), fileperms.PublicFile) + case isReleaseComparison(cmd): + _, _ = fmt.Fprintln(cmd.Stdout, "-1") + } + + return nil + } - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "kernel"), 3) + err := preparer.tryBumpStaticRelease( + context.Background(), comp, filepath.Join(testSourcesDir, "test-pkg"), testChanges()) require.Error(t, err) - assert.Contains(t, err.Error(), "cannot be auto-bumped") - assert.Contains(t, err.Error(), "release.calculation") -} - -func TestTryBumpStaticRelease_NonStandardSucceedsWithManual(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "kernel", "%{pkg_release}") - - comp := mockComponent(ctrl, "kernel", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationManual, - }, - }) - - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "kernel"), 3) - require.NoError(t, err) -} - -func TestTryBumpStaticRelease_ExplicitAutoreleaseSkips(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - // Spec has a static release, but config says autorelease — should skip. - comp := mockComponent(ctrl, "gvisor", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationAutorelease, - }, - }) - - // No spec file needed — should skip before reading anything. - err := preparer.tryBumpStaticRelease(comp, testSourcesDir, 3) - require.NoError(t, err) -} - -func TestTryBumpStaticRelease_ExplicitStaticBumps(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - writeTestSpec(t, memFS, "test-pkg", "1%{?dist}") - - comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationStatic, - }, - }) - - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "test-pkg"), 3) - require.NoError(t, err) - - // Verify the spec was updated. - specPath := filepath.Join(testSourcesDir, "test-pkg", "test-pkg.spec") - content, err := fileutils.ReadFile(memFS, specPath) - require.NoError(t, err) - assert.Contains(t, string(content), "Release: 4%{?dist}") + assert.Equal(t, 2, bumps) + assert.Equal(t, original, stringMustRead(t, ctx, specPath)) } -func TestTryBumpStaticRelease_ExplicitStaticErrorsOnAutorelease(t *testing.T) { - ctrl := gomock.NewController(t) - memFS := afero.NewMemMapFs() - preparer := newTestPreparer(memFS) - - // Spec uses %autorelease, but config says static — should error. - writeTestSpec(t, memFS, "test-pkg", "%autorelease") - - comp := mockComponent(ctrl, "test-pkg", &projectconfig.ComponentConfig{ - Release: projectconfig.ReleaseConfig{ - Calculation: projectconfig.ReleaseCalculationStatic, - }, - }) - - err := preparer.tryBumpStaticRelease(comp, filepath.Join(testSourcesDir, "test-pkg"), 3) - require.Error(t, err) - assert.Contains(t, err.Error(), `release.calculation = "autorelease"`) +func TestTryBumpStaticRelease_ModesAndZeroChanges(t *testing.T) { + for _, testCase := range []struct { + name, release string + calculation projectconfig.ReleaseCalculation + changes []FingerprintChange + wantErr bool + }{ + {"zero", "4", projectconfig.ReleaseCalculationAuto, nil, false}, + {"manual", "4", projectconfig.ReleaseCalculationManual, testChanges(), false}, + {"explicit autorelease", "4", projectconfig.ReleaseCalculationAutorelease, testChanges(), false}, + {"auto autorelease", "%{autorelease -e asan}", projectconfig.ReleaseCalculationAuto, testChanges(), false}, + {"static autorelease mismatch", "%autorelease", projectconfig.ReleaseCalculationStatic, testChanges(), true}, + } { + t.Run(testCase.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + preparer, ctx := releaseTestPreparer(t) + comp := mockReleaseComponent(ctrl, "test-pkg", testCase.calculation) + specPath := writeTestSpec(t, ctx.FS(), "test-pkg", + "Name: test-pkg\nVersion: 1.0\nRelease: "+testCase.release+"\n") + + err := preparer.tryBumpStaticRelease( + context.Background(), comp, filepath.Join(testSourcesDir, "test-pkg"), testCase.changes) + if testCase.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Contains(t, stringMustRead(t, ctx, specPath), "Release: "+testCase.release) + } + }) + } } diff --git a/internal/app/azldev/core/sources/release_test.go b/internal/app/azldev/core/sources/release_test.go index 13a0444c..da3befa8 100644 --- a/internal/app/azldev/core/sources/release_test.go +++ b/internal/app/azldev/core/sources/release_test.go @@ -52,50 +52,6 @@ func TestReleaseUsesAutorelease(t *testing.T) { } } -func TestBumpStaticRelease(t *testing.T) { - for _, testCase := range []struct { - name, value string - commits int - expected string - wantErr bool - }{ - // Accepted forms: bare integer or integer + dist macro. - {"simple integer", "1", 3, "4", false}, - {"with conditional dist tag", "1%{?dist}", 2, "3%{?dist}", false}, - {"non-conditional dist tag", "1%{dist}", 2, "3%{dist}", false}, - {"larger base", "10%{?dist}", 5, "15%{?dist}", false}, - {"single commit", "1%{?dist}", 1, "2%{?dist}", false}, - - // Rejected: no leading integer. - {"no leading int", "%{?dist}", 1, "", true}, - {"empty string", "", 1, "", true}, - - // Rejected: unknown macros in suffix. - {"other macros", "17%{someothermacro}%{?dist}", 3, "", true}, - {"macro before dist", "0%{rc_subver}%{?dist}", 1, "", true}, - - // Rejected: dotted decimal releases. - {"dotted with beta suffix", "1.39.b1%{?dist}", 3, "", true}, - {"dotted simple", "1.2%{?dist}", 2, "", true}, - {"dotted no suffix", "1.10", 5, "", true}, - {"dotted zero prefix", "0.1", 1, "", true}, - - // Rejected: trailing dot. - {"trailing dot before dist", "1.%{?dist}", 1, "", true}, - {"trailing dot no suffix", "1.", 1, "", true}, - } { - t.Run(testCase.name, func(t *testing.T) { - result, err := sources.BumpStaticRelease(testCase.value, testCase.commits) - if testCase.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - assert.Equal(t, testCase.expected, result) - } - }) - } -} - func TestGetReleaseTagValue(t *testing.T) { makeSpec := func(release string) string { return "Name: test-package\nVersion: 1.0.0\nRelease: " + release + "\nSummary: Test\n" diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 4d32c983..0763e9bf 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -83,6 +83,18 @@ func WithGitRepo( } } +// WithRPMDevBumpspec enables deterministic host release bumps using the supplied operation context. +// scratchDir must be outside component staging directories because synthetic history stages all files +// below the component source directory. targetArch is optional; an empty value preserves the host RPM +// default for callers that do not have a resolved build target. +func WithRPMDevBumpspec(ctx opctx.Ctx, scratchDir, targetArch string) PreparerOption { + return func(p *sourcePreparerImpl) { + p.bumpspecCtx = ctx + p.bumpspecScratchDir = scratchDir + p.bumpspecTargetArch = targetArch + } +} + // WithDirtyDetection returns a [PreparerOption] that enables uncommitted-change // detection during synthetic history generation. When set, the current input // fingerprint is compared against the committed lock file; if they differ, a @@ -199,6 +211,10 @@ type sourcePreparerImpl struct { // %fedora_upstream_release. Nil disables autorelease resolution (the release // macro is skipped for such specs). Set via [WithMockProcessor]. autoreleaseResolver autoreleaseResolver + + bumpspecCtx opctx.Ctx + bumpspecScratchDir string + bumpspecTargetArch string } // NewPreparer creates a new [SourcePreparer] instance. All positional arguments @@ -529,9 +545,9 @@ func (p *sourcePreparerImpl) trySyntheticHistory( return nil } - // Adjust the Release tag before staging changes. See [tryBumpStaticRelease] - // for the handling of %autorelease, static integers, and non-standard values. - if err := p.tryBumpStaticRelease(component, sourcesDirPath, len(changes)); err != nil { + // Adjust the Release tag before staging changes. The ordered fingerprint + // changes are the authoritative source of one deterministic bump per change. + if err := p.tryBumpStaticRelease(ctx, component, sourcesDirPath, changes); err != nil { return fmt.Errorf("failed to apply release bump:\n%w", err) } From 68053f0c1be8726de5870814fcc3c94c58e69011 Mon Sep 17 00:00:00 2001 From: Thien Trung Vuong Date: Wed, 9 Sep 2026 01:17:08 +0000 Subject: [PATCH 3/3] docs: document rpmdev-bumpspec release handling Describe the tested host rpmdev-bumpspec workflow, fixed transitional changelog metadata, and strict source-EVR acceptance checks. Clarify that conditional and fallback forms are attempted natively but ineffective mutations fail and restore the original spec. Keep emitted component guidance and configuration comments aligned with the implementation while documenting the existing manual and autorelease exceptions and the prepare-sources opt-out. --- docs/user/reference/config/components.md | 31 +++++++++++++++++-- .../app/azldev/agentskill/agentskill_test.go | 9 ++++++ .../content/build-component.md.tmpl | 8 +++-- .../agentskill/content/comp-toml.md.tmpl | 26 +++++++++++++--- internal/projectconfig/component.go | 9 +++--- 5 files changed, 69 insertions(+), 14 deletions(-) diff --git a/docs/user/reference/config/components.md b/docs/user/reference/config/components.md index 06484002..91aa2a45 100644 --- a/docs/user/reference/config/components.md +++ b/docs/user/reference/config/components.md @@ -112,9 +112,9 @@ The `[components..release]` section controls how azldev manages the Releas | Mode | Behavior | |------|----------| -| `auto` | Auto-detects from the spec's Release tag value. If `%autorelease` is found, rpmautospec handles it. If a static integer is found, optionally followed by `%{?dist}` or `%{dist}`, it is bumped by the synthetic commit count. | +| `auto` | Auto-detects `%autorelease` and leaves it to rpmautospec. All other Release forms are bumped by `rpmdev-bumpspec`, once for each fingerprint-derived synthetic change. | | `autorelease` | Explicitly declares the spec uses `%autorelease`. Skips all Release manipulation. Use this for specs with conditional `%autorelease`/`%else` fallbacks that confuse auto-detection. | -| `static` | Explicitly declares the spec uses a static integer release. Bumps it by the synthetic commit count only when the Release tag is an integer, optionally followed by `%{?dist}` or `%{dist}`. Non-integer or other non-standard Release values (for example, `%{pkg_release}`) require `manual` or an overlay. | +| `static` | Requires a non-`%autorelease` Release and invokes `rpmdev-bumpspec` once for each fingerprint-derived synthetic change. rpmdev-bumpspec natively attempts integer, dotted, macro, conditional, and fallback forms; azldev accepts the operation only when host RPM evaluation proves the source Release is strictly newer. Inactive conditional definitions or otherwise ineffective mutations fail and restore the original spec. | | `manual` | Skips all automatic Release manipulation. Use for components that manage their own release numbering (e.g. kernel). | Most components use `auto` (the default) and need no release configuration. Examples: @@ -129,6 +129,33 @@ calculation = "autorelease" calculation = "manual" ``` +### Host requirement and troubleshooting + +Automatic non-`%autorelease` release handling requires `rpmdev-bumpspec`, +`rpmdev-packager`, `rpm`, `rpmspec`, and `python3` with the RPM Python module. The +tested implementation is `rpmdevtools` 9.6 (`rpmdev-bumpspec` 1.0.13), but azldev +accepts a behaviorally compatible newer implementation: it evaluates exactly the +source package EVR with `rpmspec --srpm` before and after every bump and verifies that +RPM orders the new Release strictly higher. azldev converts `build.with` and +`build.without` to their effective `_with_` and `_without_` macro +definitions, then applies explicit defines and undefines with the same precedence as +the build. A component build also passes `target_arch` from `--mock-config-opt` to +both evaluations and the wrapped RPM command; render and `prepare-sources` use the +host RPM target when no target is otherwise available. azldev uses normal host vendor +macros, an isolated HOME, and fixed locale/timezone. If render, build, or +`prepare-sources` (which creates dist-git by default; `--without-git` opts out) +reports a missing or incompatible tool, provision the host runtime rather than adding +a Release overlay. + +This is a transitional mutation engine: locks and synthetic history still determine +the ordered fingerprint changes, and azldev invokes `rpmdev-bumpspec` once per +change. Each invocation uses the fixed Azure Linux Packaging Team identity, `- rebuilt` +comment, and `Mon Jan 06 2025` datestamp, so fingerprint author/message/time data do +not affect generated release or changelog bytes. The tool may update both `Release:` +(or its preferred release macro) and `%changelog`; `%autorelease` remains unchanged. +This does not introduce lock-free release calculation or redesign final changelog +ordering. + ## Render Configuration The `[components..render]` section controls rendering behavior for a component. diff --git a/internal/app/azldev/agentskill/agentskill_test.go b/internal/app/azldev/agentskill/agentskill_test.go index 42172f89..693fdf00 100644 --- a/internal/app/azldev/agentskill/agentskill_test.go +++ b/internal/app/azldev/agentskill/agentskill_test.go @@ -119,6 +119,15 @@ func TestUpdateComponentSkillStagesRenderedOutputBeforeAmend(t *testing.T) { "both amend workflows must stage the post-commit render") } +func TestComponentSkillDocumentsRPMDevBumpspecReleaseHandling(t *testing.T) { + doc, err := agentskill.SkillDocument("azldev-comp-toml", testParams()) + require.NoError(t, err) + + assert.Contains(t, doc, "rpmdevtools` 9.6") + assert.Contains(t, doc, "fingerprint-derived synthetic change") + assert.Contains(t, doc, "only when host RPM evaluation proves the") +} + func TestImageSkillDocumentsRuntimeConfigOverride(t *testing.T) { doc, err := agentskill.SkillDocument("azldev-image", testParams()) require.NoError(t, err) diff --git a/internal/app/azldev/agentskill/content/build-component.md.tmpl b/internal/app/azldev/agentskill/content/build-component.md.tmpl index fe7b4de5..d4b9232b 100644 --- a/internal/app/azldev/agentskill/content/build-component.md.tmpl +++ b/internal/app/azldev/agentskill/content/build-component.md.tmpl @@ -43,8 +43,12 @@ Finalize with `azldev comp update -p ` before opening a PR (see the ## Debugging build failures -1. **Render error mentioning a non-standard `Release` tag** — a release-calculation - issue; see the `azldev-comp-toml` skill. +1. **Render error mentioning `rpmdev-bumpspec` or RPM macros** — provision + `rpmdev-bumpspec`, `rpmdev-packager`, `rpm`, `rpmspec`, and `python3-rpm`. + rpmdevtools 9.6 (`rpmdev-bumpspec` 1.0.13) is tested; behaviorally compatible + newer tools are accepted. azldev evaluates one source EVR with `rpmspec --srpm` + and verifies RPM ordering, using effective bcond macro definitions and the build + target when one is configured; see the `azldev-comp-toml` skill. 2. **Overlay did not apply as expected** — `azldev comp diff-sources -p ` shows what the overlays actually change. 3. **Inspect the build environment** — `azldev comp build -p --preserve-buildenv diff --git a/internal/app/azldev/agentskill/content/comp-toml.md.tmpl b/internal/app/azldev/agentskill/content/comp-toml.md.tmpl index 709ba3b1..6f3ffddc 100644 --- a/internal/app/azldev/agentskill/content/comp-toml.md.tmpl +++ b/internal/app/azldev/agentskill/content/comp-toml.md.tmpl @@ -68,14 +68,17 @@ without = ["plugin_rhsm"] # disable %bcond_with conditionals `release.calculation` controls the `Release:` tag. There are four modes: -- `auto` (default) — auto-detect whether the spec uses `%autorelease` or a static - release and handle it accordingly. Correct for most packages. +- `auto` (default) — leave `%autorelease` unchanged; otherwise invoke + `rpmdev-bumpspec` once for each fingerprint-derived synthetic change. - `autorelease` — force `%autorelease` handling (use when auto-detection misreads a spec that wraps `%autorelease` in a conditional). -- `static` — force static-integer handling and bump the integer on render (the - inverse of `autorelease`). +- `static` — require a non-`%autorelease` release and invoke `rpmdev-bumpspec`. + It natively attempts integer, dotted, macro, conditional, and fallback Release + forms, but azldev accepts an operation only when host RPM evaluation proves the + source Release is strictly newer. Inactive conditional definitions or otherwise + ineffective mutations fail and restore the original spec. - `manual` — you own the `Release:` value. Use this only when render fails with a - "non-standard Release tag" error. **A `manual` component is not bumped by the + automatic Release tool. **A `manual` component is not bumped by the render/commit/amend cycle, so increment its release yourself in the same change** (see the `azldev-update-component` skill). @@ -84,6 +87,19 @@ without = ["plugin_rhsm"] # disable %bcond_with conditionals calculation = "manual" ``` +Non-`%autorelease` rendering requires `rpmdev-bumpspec`, `rpmdev-packager`, `rpm`, +`rpmspec`, and `python3` with its RPM module. `rpmdevtools` 9.6 / +`rpmdev-bumpspec` 1.0.13 is the tested implementation; newer behaviorally compatible +tools are accepted only when `rpmspec --srpm` evaluates source +Name/Epoch/Version/Release before and after the operation and RPM orders the Release +strictly higher. Build bconds are evaluated as effective `_with_` and +`_without_` macro definitions, with explicit defines and undefines retaining +their usual precedence. A build's `--mock-config-opt target_arch=...` is also passed +to both evaluations and the wrapped RPM command. azldev uses normal host vendor macros +and fixed team identity/comment/datestamp inputs, so fingerprint metadata does not +alter the generated release or changelog bytes. Locks and synthetic history still +determine the number and order of bumps. + ## Render configuration `render.skip-file-filter = true` keeps all source and patch files during render. diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 846051c9..be81d8a7 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -280,7 +280,7 @@ type ReleaseCalculation string const ( // ReleaseCalculationAuto is the default. azldev auto-detects whether the spec uses - // %autorelease or a static integer release, and handles each accordingly. + // %autorelease or another Release form and handles each accordingly. ReleaseCalculationAuto ReleaseCalculation = "auto" // ReleaseCalculationAutorelease explicitly declares that the spec uses %autorelease. @@ -289,10 +289,9 @@ const ( // fallbacks that confuse auto-detection. ReleaseCalculationAutorelease ReleaseCalculation = "autorelease" - // ReleaseCalculationStatic explicitly declares that the spec uses a static - // release tag. azldev parses and bumps the release value during rendering. - // Use this for specs with conditional Release tags where auto-detection - // picks the wrong branch but the static release logic still works correctly. + // ReleaseCalculationStatic explicitly declares that the spec uses a non-%autorelease + // release tag. azldev invokes rpmdev-bumpspec and accepts a mutation only when host + // RPM evaluation proves the source Release is strictly newer. ReleaseCalculationStatic ReleaseCalculation = "static" // ReleaseCalculationManual skips all automatic Release tag manipulation. Use this for