Repository navigation
fix(spec): group patch-add tags with existing Patch tags - #366
Izan (IzanVil) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Fixes RPM spec patch-add placement so new patch tags stay grouped with existing patch tags.
Changes:
- Refactors family-based tag insertion.
- Applies grouped insertion to
AddPatchEntry. - Adds regression tests for grouping and fallback scenarios.
| File | Summary |
|---|---|
internal/rpm/spec/edit.go |
Implements family-aware patch insertion. |
internal/rpm/spec/edit_test.go |
Adds regression coverage for patch placement scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thank you Izan (@IzanVil) for opening this PR! We are currently reviewing our usage of the overlay engine so cosmetic changes may take a bit of time to get in. We will keep you posted. |
Move the body of InsertTag into insertTagAfterFamily with a flag that controls whether insertion falls back to the last tag of any kind when no tag of the same family exists. InsertTag keeps its current behavior. Signed-off-by: IzanVil <soyoquienmas@outlook.com>
When a spec has no %patchlist section, AddPatchEntry appended the new PatchN tag at the end of the preamble, typically right before %description, instead of next to the existing Patch tags. Insert the new tag after the last Patch tag of the target package, moving past any enclosing conditional block. Specs with a %patchlist section still append there, and specs without Patch tags keep appending to the end of the preamble. Fixes microsoft#284 Signed-off-by: IzanVil <soyoquienmas@outlook.com>
…sertTag Signed-off-by: IzanVil <soyoquienmas@outlook.com>
Signed-off-by: IzanVil <soyoquienmas@outlook.com>
|
Rebased onto current |
b6f7cf9 to
75983a4
Compare
Fixes #284
Problem
When a spec has no
%patchlistsection, thepatch-addoverlay appends the newPatchN:tag at the very end of the preamble (typically right before%description) instead of next to the existing Patch tags.AddPatchEntryfell back toAddTag, which always appends to the end of the section.Changes
Following the same style as #235 / #345, this reuses the family-based insertion that
InsertTagalready uses forSourcetags:refactor(spec): move the body ofInsertTagintoinsertTagAfterFamily, with a flag controlling whether it falls back to the last tag of any kind when no tag of the same family exists.InsertTagkeeps its current behavior.fix(spec):AddPatchEntrynow inserts the newPatchN:after the last Patch tag of the target package, moving past an enclosing%if/%endifso the new tag stays unconditional. Without a same-family match it does not fall back to "any tag", so it keeps today'sAddTagbehavior.Against the acceptance criteria:
patch-addare placed next to existing Patch/PatchN tags.%patchlistsection continue to append there.Note: already-rendered specs that use
patch-addon top of existing Patch tags will see thePatchN:line move next to the other Patch tags the next time they are rendered. This is the intended cosmetic change.Testing
TestAddPatchEntryininternal/rpm/spec/edit_test.go: unnumberedPatch:followed by a conditionalBuildRequiresblock (the libsolv shape from the issue), multiple numbered Patch tags, last Patch tag inside a conditional,%patchlistpresent, and no existing Patch tags. The three grouping cases fail before the fix and pass after it.TestInsertTagandTestAddPatchEntry_EmptySpecpass unchanged.mage unitandmage check lintpass, on each commit.