@mlschroe pushed 1 commit.
630966ce35c39ac5a8c2a2583099a4165ac15c6f Implement short-circuit for logical
and ternary operators
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
This adds %[ expr ] as a new means to do expression expansion in rpm. Unlike
%{expr:} the expression is expanded in the parser, so we are safe against
expansion results messing up the syntax and also can to short-circuiting.
You can view, comment on, or merge this pull request online at:
Much better, thanks. As I don't see a better way to do it with systemd...
Thanks for the patch!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Merged #831 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/831#event-2643367507___
Rpm-maint mailing list
Hmm. It's on the subtle side, and such things aren't the easiest to communicate
to the packager community at large. But lets see...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Merged #838 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/838#event-2643253274___
Rpm-maint mailing list
Okay, looks nice and contained to me. Thanks a bunch!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
mlschroe commented on this pull request.
> + result = v1->data.i != 0;
+ break;
+ case VALUE_TYPE_STRING:
+ result = v1->data.s[0] != '\0';
+ break;
+ default:
+ goto err;
+}
+valueFree(v1);
+if (rdToken(state))
+ goto err;
+v1 =
pmatilai commented on this pull request.
> + result = v1->data.i != 0;
+ break;
+ case VALUE_TYPE_STRING:
+ result = v1->data.s[0] != '\0';
+ break;
+ default:
+ goto err;
+}
+valueFree(v1);
+if (rdToken(state))
+ goto err;
+v1 =
@mlschroe: Thank you. The PR looks good to me.
@pmatilai After the mentioned example there is a list of operators that can be
used in a condition after
```
%if
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
I added the ternary operator to the list of supported operators.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Conan-Kudo approved this pull request.
--
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/836#pullrequestreview-289845857___
@pavlinamv , what do you mean by changing those conditionals in the
documentation? I don't see them needing changing because of this, but maybe I'm
missing something.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
No, don't drop it. It's still useful if you need to do expansion before calling
the expression parser.
I.e. you have either 'expand first, then don't expand in the expression parser'
or 'expand in the expression parser':
```
%define test 1 + 2
%{expr:%test}
# the next line expands twice!
Merged #844 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/844#event-2643132460___
Rpm-maint mailing list
pmatilai approved this pull request.
Agreed. Thanks for the patch.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@pmatilai pushed 1 commit.
df5d5b712b6210702c5b399f716780331b5a8b47 Trap division by zero in expression
parser
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
pmatilai commented on this pull request.
> @@ -438,6 +438,10 @@ static Value doMultiplyDivide(ParseState state)
if (valueIsInteger(v1)) {
int i1 = v1->data.i, i2 = v2->data.i;
+ if ((i2 == 0) && (TOK_DIVIDE)) {
Ugh, yeah:
```
$ ./rpm --eval "%{expr:1*0}"
error: division by
mlschroe commented on this pull request.
> @@ -438,6 +438,10 @@ static Value doMultiplyDivide(ParseState state)
if (valueIsInteger(v1)) {
int i1 = v1->data.i, i2 = v2->data.i;
+ if ((i2 == 0) && (TOK_DIVIDE)) {
Is `(TOK_DIVIDE)` supposed to be `(op == TOK_DIVIDE)`?
--
You
Could you change file
```
rpm/doc/manual/spec
```
in section "Conditionals" the text after example:
```
%if 0%{?fedora} > 10 || 0%{?rhel} > 7
```
according of the PR?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
The %if lines in the specfile are expanded before the expressions
get evaluated, so the re-expansion of strings is surprising.
Its also not done for integers, which makes it inconsistent.
The original expression parser seems to have been written
without taking the upfront expansion into account,
Hmm, but GH doesn't let me merge despite resolving the conflicts. What a mess,
probably my fault.
I'll handle via #844 separately.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Closed #833.
--
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/833#event-2643037718___
Rpm-maint mailing list
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/844
-- Commit Summary --
* Trap division by zero in expression parser
-- File Changes --
M rpmio/expression.c (4)
M tests/rpmmacro.at (2)
-- Patch Links --
pmatilai approved this pull request.
--
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/833#pullrequestreview-289822247___
Closed #843.
--
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/843#event-2643005419___
Rpm-maint mailing list
Merged the conflict manually.
Thanks.
--
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/833#issuecomment-532626179___
Rpm-maint
@pmatilai pushed 1 commit.
8ac1d0b720b57b02904108b17e90e81d0a48afff Merge branch 'master' into divisionnil
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Reopened #843.
--
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/843#event-2642992993___
Rpm-maint mailing list
Closed #843.
--
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/843#event-2642978604___
Rpm-maint mailing list
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/843
-- Commit Summary --
* Trap division by zero in expression parser
-- File Changes --
M rpmio/expression.c (4)
M tests/rpmmacro.at (2)
-- Patch Links --
pmatilai approved this pull request.
Heh, seems you took my remark about being nice rather literally, with "please"
and all in the message :D
Anyway, works for me, thanks for the patch!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
Merged #840 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/840#event-2642953609___
Rpm-maint mailing list
Meh. Error messages are not about telling people what they may or may not do
when they *just did*. The message should simply be "error: division by zero"
and no further lecturing on the subject.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly
@mlschroe pushed 1 commit.
cd764dbd89f56c7714d94d601c70453d1191ac2d Remove TOK_IDENTIFIER support from
expression parsing
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Right, I'll add a different error message for bare strings.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
(Sorry for the bad copy with ``. Seems I patched it correctly when
I did the change in the fedora chroot I use for testing but then messed it up
when re-doing it in the git tree.)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on
FWIW, the amount of breakage in Fedora is on similar level. Like noted, no
problems with that.
If we want to be nice about it, we could add a warning about it to 4.15, but
that's another topic.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly
@pavlinamv pushed 1 commit.
8e919739e766840626d17b0891bdc27ac6be776e Prevent dividing by 0 in the
expression evaluation
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Oh, Panu already decided to go with it. Sorry for the noise ;)
I'll change the message and also do a rebase to fix the conflict.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
("Lots" meaning 26 spec files in Factory, which actually isn't that much. If
Panu decides to drop support for bare strings I'm also fine with that ;) )
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
So I grepped through the SUSE specs, and it seems we're stuck with bare
literals. Lots of specs using
things like `%if %llvm == yes` or `%if %{_lib} == lib64` or `%_arch == s390x`.
So I'll redo this pull request so that it keeps that functionality.
--
You are receiving this because you are
Merged #842 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/842#event-2642566615___
Rpm-maint mailing list
pmatilai approved this pull request.
Right, I guess the only case where %define/%global can be followed by something
is the "silent grouping" with {} eg ```rpm --eval "%define foo {bar}
%{warn:issue}" --eval "%foo"``` and that behavior doesn't change here.
--
You are receiving this because
Merged #839 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/839#event-264296___
Rpm-maint mailing list
…f0f6bbf
state is a pointer in this function, we dont want a pointer to
it.
Fixes garbage getting printed in the error message. Add a testcase too.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/842
-- Commit Summary --
Thank you. It solves the problem.
--
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/838#issuecomment-532561398___
Rpm-maint
Requiring quotes around strings explicitly seems only a good thing to me. And
since that's how it originally was, only accidentally changed in that commit so
this is actually a regression fix :)
It does trip up more than just a handful of specs in Fedora, but not a major
breakage and like
Note that if we add %[] for expressions then I think we should drop %{expr:...}
entirely, it hardly serves any purpose then.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
I'm thinking that with these new developments, we'd better revert %{expr:..}
entirely from 4.15, we don't want people adopting it if the behavior is just
about to change. Pulling it into 4.15 felt a bit hasty anyway, apparently for a
good reason.
Along with that change I think we can sneak in
Merged #841 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/841#event-2642318082___
Rpm-maint mailing list
Ack. Thanks for the patch.
--
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/841#issuecomment-532543277___
Rpm-maint mailing list
Yes, and I'm asking that you mention it is the default format to make people
aware that it's something they don't need to use. It's basically a side-effect
of a code cleanup (in commit 2bfc0fad3876bcee0143664cae9f0f0107a86a33), not
something that ever needed using or advertising.
--
You are
Heaves forbid, no. Not at this point in the release process.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
54 matches
Mail list logo
