[Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
There's a lot of expansion going on in the %expr macro. Witness this:
./rpm --eval '%{expr: ""}'
%
The string gets expanded
- at the start of doFoo when it expands the argument
- in rdToken when it parses a string
- at the end of doFoo because of issue #313
I'm somewhat surprised that rdToken expands strings and that this hasn't messed
up some `%if` statements yet.
It's also somewhat inconsistent that the integer case does not do expansion.
I kind of like to have the expansion in rdToken instead of doFoo because:
- It makes `"%str"` work when %str contains a `"` character.
- It allows to have short circuit semantics
--
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/834___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
Here's my proposal:
1) turn off default string expansion in rdToken
This is an incompatible change, but it's clearly the right thing and I
can't imaging somebody
actually using this in a %if statement
2) Add optional expansion for the integer case
3) Add a new macro that does not expand the macro body in `doFoo`, but instead
calls rpmExprStr with a flag that enables expansion in rdToken.
We could use `%{( )}` as new macro, as proposed in the thread about the `?
:` operator.
4) Add '? : ' support to the expression expansion code
With that changes we can do `%{( %foo ? "%bar" : "%baz" )}`
Does that make sense? Should I create a 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/issues/834#issuecomment-531171559___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
How about using `%[ ]` for expression expansion? We'd have:
`%{ ... }`: macro expansion
`%( ... )`: shell expansion
`%[ ... ]`: expression expansions
I did a grep over SUSE's spec files and there was no conflict. Could someone
please do this
for Fedora as well?
--
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/834#issuecomment-531700421___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
@mlschroe: I like this idea. I checked Fedora spec files and there was no conflict with "%[... ]". -- 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/834#issuecomment-531788382___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
Yeah, %[] is nice. -- 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/834#issuecomment-532075022___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
Ok, I'll create a pull request tomorrow. Is it ok to add a `flags` argument to rpmExprBool()/rpmExprStr() or is the API fixed and we need new functions? I'm asking because they are in rpm-4.15.x and you probably don't want to cherry pick this into 4.15? The flags would be `RPMEXPR_EXPAND_STRING` and `RPMEXPR_EXPAND_INTEGER`. Or we can drop the default string expansion and just have a `RPMEXPR_EXPAND` flag. -- 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/834#issuecomment-532282944___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
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 an added flags argument to those
patches. This will need an rc2 anyway.
--
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/834#issuecomment-532548605___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
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:
https://github.com/rpm-software-management/rpm/issues/834#issuecomment-532549129___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
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!
%{expand:%%[ %test ]}
```
--
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/834#issuecomment-532639968___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
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: https://github.com/rpm-software-management/rpm/issues/834#issuecomment-532655408___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] expression expansion (#834)
Closed #834. -- 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/834#event-2654226267___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
