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/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/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) +} 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) } 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