Description of problem: I think conditionals, global defines and bconds shouldn't expand in comments. Version-Release number of selected component (if applicable): rpm-4.12.0.1-6.fc21.x86_64 How reproducible: Always Steps to Reproduce: 1. Create spec with the following content: #%ifarch %{ix86} x86_64 #%bcond_without openblas #%else %bcond_with openblas #%endif Actual results: Build with openblas on x86 Expected results: Build without openblas on x86 Additional info: The current behaviour is confusing. User may forget to escape macros in comments by second %. Maybe that no macro should expand in comments, or is there any use case for it?
We don't change this behaviour right now because somebody can depend on it. But we added a warning that a macro is expanded in comment. So user will be aware that he has to escape macros also in comments if he don't want them to be expanded. Fixed upstream as 2a3f49585e5bd82d0bbfe5b9d6cdf24d9501b5cd.
(In reply to Ľuboš Kardoš from comment #1) > We don't change this behaviour right now because somebody can depend on it. > But we added a warning that a macro is expanded in comment. So user will be > aware that he has to escape macros also in comments if he don't want them to > be expanded. > > Fixed upstream as 2a3f49585e5bd82d0bbfe5b9d6cdf24d9501b5cd. This is regression: varování: Macro expanded in comment on line 16: # Executing testsuite (enabling %%check section) will cause dependency loop. It was always enough to prepend another % in front of the macro to avoid the warning. Now this is apparently ignored. With single %, rpmlint always fired warning: $ rpmlint -i rubygems.spec rubygems.spec:16: W: macro-in-comment %check There is a unescaped macro after a shell style comment in the specfile. Macros are expanded everywhere, so check if it can cause a problem in this case and escape the macro with another leading % if appropriate.
(In reply to Ľuboš Kardoš from comment #1) > Fixed upstream as 2a3f49585e5bd82d0bbfe5b9d6cdf24d9501b5cd. Btw since you include hash of the commit, it would be more useful to sent a link instead. That way I could immediately review your change.
(In reply to Ľuboš Kardoš from comment #1) > We don't change this behaviour right now because somebody can depend on it. > But we added a warning that a macro is expanded in comment. So user will be > aware that he has to escape macros also in comments if he don't want them to > be expanded. > > Fixed upstream as 2a3f49585e5bd82d0bbfe5b9d6cdf24d9501b5cd. I can't imagine any use case for macro expansion in comments. Is there any? If not, I think this could be good feature candidate for e.g. rpm 5.
(In reply to Vít Ondruch from comment #2) > (In reply to Ľuboš Kardoš from comment #1) > > We don't change this behaviour right now because somebody can depend on it. > > But we added a warning that a macro is expanded in comment. So user will be > > aware that he has to escape macros also in comments if he don't want them to > > be expanded. > > > > Fixed upstream as 2a3f49585e5bd82d0bbfe5b9d6cdf24d9501b5cd. > > This is regression: > > varování: Macro expanded in comment on line 16: # Executing testsuite > (enabling %%check section) will cause dependency loop. > > It was always enough to prepend another % in front of the macro to avoid the > warning. Now this is apparently ignored. > > With single %, rpmlint always fired warning: > > $ rpmlint -i rubygems.spec > rubygems.spec:16: W: macro-in-comment %check > There is a unescaped macro after a shell style comment in the specfile. > Macros > are expanded everywhere, so check if it can cause a problem in this case and > escape the macro with another leading % if appropriate. Now the warning is not showed when a macro is escaped by adding another % in front of the macro. Fixed in version rpm-4.12.90-4.fc24. (upstream: https://github.com/rpm-software-management/rpm/commit/655ca18630d4a89e022b51ea495fa63718303413)
This produces many unnecessary warnings, since read-only expansion is not harmful whether in comments or not. Defining (or undefining) macros in comments is the nasty part, but of course there's no way for the spec parser to know that. My (rather vague) plan was to add some sort of read/write/execute mode flags to allow controlling what the macro engine is allowed to do at a given time. For spec parsing it could drop to read-only for apparent comment lines (and warn/error on define/undefine attempt). There are other places that could benefit from restricted macro execution too.
(In reply to Panu Matilainen from comment #6) > Defining (or undefining) macros in comments is the nasty part, but of course > there's no way for the spec parser to know that. s/know that/know what an arbitrary macro might do/
(In reply to Ľuboš Kardoš from comment #1) > We don't change this behaviour right now because somebody can depend on it. How could somebody actually depend on this buggy behavior? Do you know of any real world situation? I can easily think of situations where this behavior will cause trouble (the most obvious one: you comment out a macro definition and the macro is still defined - if I understand correctly). But I can't think of any case where someone would actually want the current behavior. The whole purpose of comments in "source code" is to be read by humans and ignored by machines. The fact that rpm is parsing comments at all is simply wrong.
FWIW, rpm >= 4.15 has a real commenting primitive in the way of M4-inspired %dnl (discard to next line). As the name implies, it simply discards anything to the next newline, and macros in the discarded part are naturally not expanded. See https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/7Q3BZ6GTAKFDJMNOLNJJENMWCPQTTET7/ and https://github.com/rpm-software-management/rpm/commit/44180fc7400ea8e67a13c5581459ad13f1d14103 for more info.
I read many articles on this blog, and I really enjoy it. The topics are interesting and the writing is good. I would definitely recommend reading more of the posts. https://fallguyss.co
I am very happy to read the very useful information you provide. Thank you so much for sharing. https://aarouteplanner.io