[Rpm-maint] [rpm-software-management/rpm] Warn if a text is after %else or %endif (#625)

[pavlinamv]

> Well, c) and d) would depend on context, but most likely they'd end up in syntax error, which is ok. 

Yes, but not in all cases.


> OTOH the closest relative to spec %if's would be the C pre-processor which simply warns: warning: extra tokens at end of #else directive
>So perhaps that is the best option here afterall.

OK I think so.


>Looking at the macro expansion behavior, the current behavior is certainly very broken. Consider the following:

>%if %{val}
>%{echo:yeah}
>%else %{echo:moo}
>%{echo:nay}
>%endif

>"moo" gets echoed iff %{val} evaluates to true, which is just crazy. At this point I do have to agree with you that %else needs to be parsed before deciding whether to expand a line or not, and ditto %endif. 

As it is described in the commit message of the PR:
* a text after %else is expanded according to evaluation of the previous %if.
* a text after %endif is expanded according to evaluation of the previous %else, resp. %if if there was no %else.

This was my original reason for creating the patch.

> And if we're adding a warning for text after %else, then perhaps neither of these lines should ever be expanded. 


I looked to the code and as usually it is more complicated. There are two versions of the "line" in spec file. One before the macro expansion and the second is after the expansion. So 

1) if a line (before expansion) is:
  %else %define name value
then it is incorrect before the expansion and it looks correctly after the expansion. Thus a check of a text after %else must be done before the expansion.

2) on the other hand a macro (e.g. "%debug_package" from macros.in) can be expanded to a text containing more lines including some %if + %else lines. 
  -> I) Thus we can not decide what is the current type of the line before expansion. The final decision must be done after expansion. In the readLine function where it is done now.
  -> II) a second check of a text after %else must be done after expansion (if a warning was written in the first check, then this check should not write any warning). This should be improved in the next PR.
  -> III) There is a large area for errors when expanding %if's from a file.
Lets start with an example:

Add into a macro file:

```
%mem1 \
%if 1  \
%global blabla1 1111 \
%else \
%global blabla2 2222 \
%endif\
%{nil}
```
try:

```
rpm --eval '%blabla1' --eval '%blabla2' --eval '%mem1' --eval '%blabla1' --eval '%blabla2'
```

the result is:

```
%blabla1
%blabla2

%if 1  
%else 
%endif

1111
2222
```


So in expansion of %if - %else - %endif from macro files there is a bug. Macros are expanded no matter whether %if/%else holds or not. Thus they are defined and undefined no matter whether %if/%else holds or not. This is another and bigger problem that should be solved. But it is not part of the problem that this PR solve. 



-- 
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/625#issuecomment-461151723
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190206/dddc54f5/attachment-0001.html>