[Rpm-maint] [rpm-software-management/rpm] Unclosed %if (when defined inside %define) (#1198)

[Michal Domonkos]

This issue stems from the fact that the line continuation marker `\` has *different* semantics in the spec-level context and in a macro definition. On the spec level, it is used to break long `%if` statements into multiple lines. Inside macro definitions, it's the whole body that's broken down. The patch in 5f4fdce interprets these markers equally, though.

This becomes a problem when a `%define` or `%global` macro is encountered in a false branch of an `%if` statement and is therefore left unexpanded (for obvious reasons); the spec parser just continues scanning the macro's body as if it were part of the spec itself: it joins the inner `%if` statement into a single line, including the inner `%endif` (since all of the lines end with an `\`) and finally tries to match it against a corresponding `%endif`, which fails as there's none.

The remedy here would be to completely skip the macro's body in case it's unexpanded. In fact, there's a very simple and elegant (yet non-obvious) way to do this: when joining multi-line `%if` conditionals, treat `%define` as a such a conditional; that way, the whole macro (when unexpanded) collapses into a single line and its content is not interpreted. I have a working patch here: https://github.com/dmnks/rpm/commit/9c1c592d40777868d672a531b49c63fb6dd6ec84

That being said, it turns out that conditionals inside macros are *unsupported*. The macro expander does *not* interpret them at all. For example, the following construct would print "hello":

```
%define test() \
%if 0 \
%{echo:hello} \
%endif

%test
```

This is also mentioned (though not entirely clearly) in the `/doc/manual/spec` file:
```
%if-conditionals are not macros, and are unlikely to yield expected results
if used in them.
```

@ignatenkobrain, is there a specific use case for such conditionals that I'm missing? While there is a simple fix for this (as described above) which shouldn't introduce any side-effects (but I am not 100% sure yet), it doesn't make much sense to apply it if the use case being fixed is in fact unsupported.

-- 
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/issues/1198#issuecomment-660079015
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20200717/b6985836/attachment.html>