pavlinamv commented on this pull request.
> lbuf = spec->lbuf;
SKIPSPACE(lbuf);
if (lbuf[0] == '#')
isComment = 1;
- /* Don't expand macros (eg. %define) in false branch of %if clause */
- if (!spec->readStack->reading)
+ /* Don't expand macros after %elif (resp. %elifarch, %elifos) in false
branch */
+ if (ISMACROWITHARG(lbuf, "%elif")) {
+ if (!spec->readStack->readable)
The need to test for `ISMACROWITHARG(lbuf, "%elif")` in that place is caused by
the current implementation of the parser and not by tracking by "reading" and
"readable".
Parsing of a line in spec file consists of these for us important actions (in
this order):
1) storing the line from file into buffer
(function `copyNextLineFromOFI`)
2) deciding whether the line should be expanded
(beginning of function `expandMacrosInSpecBuf`)
3) if it should be expanded then expansion of the line
(second part of function `expandMacrosInSpecBuf`)
4) checking if the line starts with a rpm conditional and according of it
doing appropriate actions
(second part of function `readLine`)
Before adding `%elif`, the decision 2) whether the line should be expanded
depends only on previous lines of the spec file. But after adding `%elif` it
depends on the new line stored in the buffer, too. Because if the line starts
with `%elif` then `if !readable` line must not be expanded, otherwise it must
be expanded.
If there should be a variable that is used for deciding whether the new line
should be expanded or not, then the variable must be set after 1). So it can be
set
a) in the second part of function `copyNextLineFromOFI` before calling
`expandMacrosInSpecBuf`, or
b) in the first part of `expandMacrosInSpecBuf`.
In the current patch the test is done in position b) and instead of setting a
variable and then deciding according to it, the test is straightforwardly used.
Thus I do not see any improvement in changing tracking variables to allow
decision 2).
To be precise I should add into `expandMacrosInSpecBuf` tests for
`ISMACROWITHARG(s, "%elifarch")` and `ISMACROWITHARG(s, "%elifos")` too. And to
be extremely precise with solve all corner cases I should add `ISMACRO(s,
"%endif")` and `ISMACRO(s, "%else")` too. Because they all can change the
potential to expand in that line. ( But usually only after `%elif` is expected
to be a macro).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/613#discussion_r246021478_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
