[Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Supports the same expressions as spec %if conditions because, only this returns
the result as a string instead of a boolean.
This obviously *screams* to be wired into macro conditional syntax, but that's
left as an exercise for later.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/817
-- Commit Summary --
* Resurrect parseExpressionString()
* Export the expression parsing functions and move to librpmio
* Add %{expr:...} macro for parsing expressions
-- File Changes --
M build/Makefile.am (2)
M build/parseSpec.c (2)
M build/rpmbuild_internal.h (8)
M doc/manual/macros (1)
M rpmio/Makefile.am (2)
R rpmio/expression.c (48)
M rpmio/macro.c (5)
M rpmio/rpmmacro.h (15)
M tests/rpmmacro.at (21)
-- Patch Links --
https://github.com/rpm-software-management/rpm/pull/817.patch
https://github.com/rpm-software-management/rpm/pull/817.diff
--
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/817
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Forgot to update POTFILES.in. Thank you distcheck for catching it early... -- 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/817#issuecomment-521558880___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Functionality is OK, but error messages can be improved. I tried macros:
%{expr:0 || b} %{expr:4 &|| 0} %{expr:6 + 9 - o -=iu}
in a spec file and error messages were:
error: types must match
error: syntax error while parsing &&
error: types must match
--
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/817#issuecomment-522017572___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
@pmatilai pushed 1 commit.
22331d303765509f1856352713a8a4de74b1085e Add %{expr:...} macro for parsing
expressions
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/817/files/0efdee6fec3848bfd3a4ccf160df553f3a081c2d..22331d303765509f1856352713a8a4de74b1085e
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
The error messages are simply the same as you get from spec %if conditionals, there are no messages added / changed in this PR. That's not to say the messages couldn't maybe be improved, but any change needs to account for the fact that they're shared between two quite different contexts. There was an actual bug wrt errors in that expression errors did not raise a macro-level error. Fixed now. -- 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/817#issuecomment-522440554___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Oh and BTW, the reason for doing this *just now* is that it might be kinda necessary for resolving #804 in the macro space instead of placing all the calculations on the C side. Of course technically Lua could be used for those same calculations but we I don't want to make Lua a hard dependency. The other reason of course is the macro conditional syntax stuff. -- 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/817#issuecomment-522441966___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
> The error messages are simply the same as you get from spec %if conditionals,
> there are no messages added / changed in this PR. That's not to say the
> messages couldn't maybe be improved, but any change needs to account for the
> fact that they're shared between two quite different contexts.
Not entirely. If there is a problem in spec %if conditional, then two error
messages are emitted e.g.:
```
error: syntax error while parsing ==
error: /data/SPECS/iftest.spec:22: bad %if condition: =
```
But if the same wrong conditional is used after _%{expr_ (e.g. %{expr:=}) only
the first error message is emitted:
```
error: syntax error while parsing ==
```
> There was an actual bug wrt errors in that expression errors did not raise a
> macro-level error. Fixed now.
Yes this is corrected now.
--
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/817#issuecomment-523323551___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
And now with improved expression error messages in both specs and macros. -- 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/817#issuecomment-523410815___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
It is definitely better.
I tried a multiline expression:
```
rpm --eval '%{expr: 0 || 0 ||
0 || 0 |o| 0}'
```
and **'^'** does not point to the expected position:
```
+error: syntax error while parsing ||: 0 || 0 ||
+ 0 || 0 |o| 0
+error:^
```
--
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/817#issuecomment-523428755___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Yes, I'm sure you can construct arbitrary cases where it's not going to point to the right place. Making the expression error reports *perfect* is not in the scope of this PR, so either I rip the marker support out or we leave it the way it is. -- 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/817#issuecomment-523430128___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Yes, but usually it is better to know the limitations of the current solution before it is merged. I prefer to rip the marker support. But you can choose whether rip it or not. -- 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/817#issuecomment-523435330___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
At the risk of setting precedent that makes users expect friendly error messages from rpm, I'll keep it. It's just a cosmetical thing in any case and can be removed without other risk or harm if it turns out to be overly problematic. -- 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/817#issuecomment-523438191___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %{expr:...} macro for parsing expressions (#817)
Merged #817 into master. -- 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/817#event-2572929323___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
