feat(overlays): add spec-set-macros overlay#134
Draft
reubeno wants to merge 1 commit intomicrosoft:mainfrom
Draft
feat(overlays): add spec-set-macros overlay#134reubeno wants to merge 1 commit intomicrosoft:mainfrom
reubeno wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new spec-set-macros overlay to declaratively update RPM %global / %define macro definitions in spec files, reducing reliance on fragile regex-based spec-search-replace overlays.
Changes:
- Introduces
spec-set-macrosoverlay type with config/schema support (macrosmap with optionalkind). - Implements
Spec.SetMacro()in the RPM spec editor and wires it into overlay application with deterministic macro application order. - Updates user-facing config docs and regenerates the JSON schema.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/azldev.schema.json | Adds schema support for spec-set-macros and MacroSetSpec. |
| internal/rpm/spec/edit.go | Adds SetMacro editing primitive + new macro-related errors. |
| internal/rpm/spec/edit_test.go | Adds unit tests for SetMacro. |
| internal/projectconfig/overlay.go | Adds overlay config fields/types + validation for macro setting entries. |
| internal/projectconfig/overlay_test.go | Adds validation tests + marks new overlay as spec-modifying. |
| internal/app/azldev/core/sources/overlays.go | Applies spec-set-macros overlays in stable sorted order. |
| internal/app/azldev/core/sources/overlays_test.go | Adds integration tests for applying macro overlays end-to-end. |
| docs/user/reference/config/overlays.md | Documents the new overlay type and configuration. |
Adds a new declarative overlay type for setting %global / %define macro values in RPM spec files, replacing ~25 fragile spec-search-replace regex toggles across azurelinux (gcc alone has 8 macro toggles). The overlay accepts a map of macro names to settings and updates each existing macro definition in place. The directive form (%global vs %define) is auto-detected from the spec; an optional `kind` field forces a specific form. The overlay fails if any named macro is not present in the spec, catching typos and upstream removals. Implementation notes / divergences from the design doc: - No inline-comment preservation. RPM does not treat `#` mid-line on %global/%define lines as a comment, so "preserving" trailing `#` text would silently truncate legitimate macro values. - Uses a new ErrNoSuchMacro in the spec package rather than wrapping ErrOverlayDidNotApply, matching the existing ErrNoSuchTag / ErrPatternNotFound convention in internal/rpm/spec/edit.go. - Stricter post-name match guard `(\s|$)` instead of `\b` so a request for `build_ada` does not match `%define build_ada()` (function-like macro) or `%global build_adapter` (shared prefix). - Multi-line definitions (lines ending in `\` continuation) are detected and rejected with ErrUnsupportedMacroDefinition rather than silently corrupting the spec by orphaning continuation lines. - Macro-name validation (no whitespace, `%`, or parentheses) and multi-line value rejection happen at config load time. - Multiple occurrences of the same macro are all updated; per-macro iteration order is sorted for deterministic rendering. - No section / sub-package scoping in v1 since macros are typically at top level in real specs; can be added later if a need arises. Tests: 14 SetMacro unit tests (incl. shared-prefix, function-like, multi-line, kind override, multiple occurrences), 10 new validation cases, 5 new integration cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4d612d2 to
e9dce20
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new declarative overlay type,
spec-set-macros, for setting%global/%definemacro values in RPM spec files. This replaces ~25 fragilespec-search-replaceregex toggles acrossazurelinux—gcc.comp.tomlalone has 8 such macro flips today.Before
After
Behavior
valueand an optionalkind(globalordefine) that forces a specific directive form.%globalor%define.%globalinside an%ifblock stays indented). Inter-token whitespace is normalized to single spaces.Implementation notes / divergences from the design doc
The design doc lives at the repository's
TODO/01-spec-set-macros.md(not part of this PR). A few intentional divergences:#mid-line on%global/%definelines as a comment — the#and everything after it is part of the macro value. "Preserving" a trailing# commentas the design doc suggested would silently truncate legitimate values. We just replace the value portion in full.ErrNoSuchMacroin the spec package rather than wrappingErrOverlayDidNotApply. This matches the existingErrNoSuchTag/ErrPatternNotFoundconvention ininternal/rpm/spec/edit.go.(\s|$)instead of\b. A request forbuild_adawould otherwise match%define build_ada()(function-like macro) or%global build_adapter(shared prefix). The custom guard avoids this.\continuation get a clearErrUnsupportedMacroDefinitionerror; a single-line rewrite would orphan continuation lines.%, or parentheses. Multi-line values are also rejected at load time.Testing
SetMacrounit tests (internal/rpm/spec/edit_test.go) covering shared-prefix non-match, function-like non-match, multi-line rejection, kind override, multiple occurrences, whitespace tolerance, value-with-spaces, indentation preservation.internal/projectconfig/overlay_test.go) for the new schema.internal/app/azldev/core/sources/overlays_test.go) exercising the full overlay-application path including the multi-macrogcc-style case.mage fix all,mage check all,mage unit,mage buildall pass locally.schemas/azldev.schema.json) regenerated.Migration
Out of scope for this PR; a follow-up PR in
azurelinuxwill migrate the affected*.comp.tomlfiles (gcc, qemu, firefox, libsoup3, grub2, nbdkit, systemd, the python-* docs toggles, etc.).Coordination
Independent of TODO 02 (
spec-insert-lines-after/before+matchcardinality) and TODO 03 (spec-deps). No ordering required between them.