Skip to content

Account for circular expansion of tags#539

Merged
centosinfra-prod-github-app[bot] merged 1 commit into
mainfrom
fix
May 26, 2026
Merged

Account for circular expansion of tags#539
centosinfra-prod-github-app[bot] merged 1 commit into
mainfrom
fix

Conversation

@nforro
Copy link
Copy Markdown
Member

@nforro nforro commented May 25, 2026

RELEASE NOTES BEGIN

Fixed an issue where the value of a tag could have been incorrectly expanded if the spec file contained a macro definition shadowing the tag name, e.g.:

%global release 12
%global release_string %{release}%{?dist}

Release: %{release_string}

In this case, with dist being .fc44, Specfile.expanded_release returned 12.fc44.fc44 instead of 12.fc44.

RELEASE NOTES END

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses circular macro expansion issues in RPM tags by manually deleting tag macros before expansion and invalidating the parser cache. The review feedback highlights that the current implementation in specfile.py is ineffective because the use of extra_macros triggers a re-parse that restores the deleted macros. Key recommendations include refactoring the duplicated logic into a helper method to improve maintainability, ensuring consistent cache invalidation, using %{nil} for efficient context loading, and expanding test coverage to verify all affected paths.

Comment thread specfile/specfile.py
Comment thread specfile/specfile.py Outdated
Comment thread specfile/tags.py
Comment thread tests/integration/test_specfile.py
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
@nforro
Copy link
Copy Markdown
Member Author

nforro commented May 25, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses circular macro expansion issues by manually managing the RPM macro stack and invalidating the parser cache. Review feedback suggests ensuring that the parser cache is consistently invalidated whenever the macro stack is modified, specifically in add_changelog_entry and expanded_release. There is also a concern regarding the performance impact of frequent cache invalidations, which may lead to O(N^2) complexity when processing multiple tags.

Comment thread specfile/specfile.py
Comment thread specfile/specfile.py
Comment thread specfile/tags.py
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@nforro nforro added the mergeit Merge via Zuul label May 26, 2026
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@centosinfra-prod-github-app centosinfra-prod-github-app Bot merged commit 20f90f9 into main May 26, 2026
49 of 52 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in Packit pull requests May 26, 2026
@nforro nforro deleted the fix branch May 26, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mergeit Merge via Zuul

Projects

Development

Successfully merging this pull request may close these issues.

3 participants