A lot of related code has changed in the meanwhile and this PR carries an
unnecessary amount of historical baggage by now. I'll close this one,
@pavlinamv do create a new PR for restarting the %elif work to get a fresh
start.
--
You are receiving this because you are subscribed to this
Closed #613.
--
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/613#event-2353002024___
Rpm-maint mailing list
This is on hold for now, @pavlinamv has more important assignments at the
moment.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Thank you for pointing out that this should be rebased. I have a slightly
different plan. #625 already contains a non-trivial refactorization, that is
useful for %elif. My next step will be to create an additional PR that will
contain a correction of a current minor %else parsing problem + as a
@pavlinamv #625 was merged, so could you rebase this on top of current git
master?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
I am waiting with a new version of this PR for finishing of PR #625.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Oh and BTW, a general piece of advise that applies everywhere, including here:
when possible, use data structures instead of code logic.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
As mentioned in the warn-after-else PR, one of the things that bugs me here is
multiple places to parse the same tokens. Have a function that parses the "line
type", which you can then use in both place, maybe borrow a chapter from the
macro engine builtins work?
The other coding style eyesore
Reopened #613.
--
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/613#event-2117852085___
Rpm-maint mailing list
The pull request is reopened (PR #618 will be closed).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
pmatilai commented on this pull request.
> spec->readStack = rl;
spec->line[0] = '\0';
+} else if (isElif) {
+ spec->readStack->reading = match && spec->readStack->readable;
+ if (spec->readStack->reading)
+ spec->readStack->readable = 0;
+
Corrected version is in PR #618.
--
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/613#issuecomment-453772152___
Rpm-maint mailing
Closed #613.
--
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/613#event-2070220111___
Rpm-maint mailing list
pavlinamv commented on this pull request.
> spec->readStack = rl;
spec->line[0] = '\0';
+} else if (isElif) {
+ spec->readStack->reading = match && spec->readStack->readable;
+ if (spec->readStack->reading)
+ spec->readStack->readable = 0;
+
pmatilai commented on this pull request.
> spec->readStack = rl;
spec->line[0] = '\0';
+} else if (isElif) {
+ spec->readStack->reading = match && spec->readStack->readable;
+ if (spec->readStack->reading)
+ spec->readStack->readable = 0;
+
pavlinamv commented on this pull request.
> spec->readStack = rl;
spec->line[0] = '\0';
+} else if (isElif) {
+ spec->readStack->reading = match && spec->readStack->readable;
+ if (spec->readStack->reading)
+ spec->readStack->readable = 0;
+
pavlinamv commented on this pull request.
> @@ -360,7 +364,10 @@ do { \
int readLine(rpmSpec spec, int strip)
{
char *s;
-int match;
+char *z;
+int match = 0;
+int isIf = 0;
+int isElif = 0;
Yes a line can be `%if` line, `%elif` line or "another" line.
Yes, in some
pmatilai commented on this pull request.
> spec->readStack = rl;
spec->line[0] = '\0';
+} else if (isElif) {
+ spec->readStack->reading = match && spec->readStack->readable;
+ if (spec->readStack->reading)
+ spec->readStack->readable = 0;
+
pmatilai commented on this pull request.
> @@ -360,7 +364,10 @@ do { \
int readLine(rpmSpec spec, int strip)
{
char *s;
-int match;
+char *z;
+int match = 0;
+int isIf = 0;
+int isElif = 0;
A line cannot be both an if and an elif, so there should only really be one
If this comment is about patch
"Move checking whether %if condition will be resolved to the right place"
then the reply is:
I will improve the commit message. It should not speak about the "right place".
But it is not just refactoring because of `%elif`. The current place is not
optimal because:
pavlinamv commented on this pull request.
> lbuf = spec->lbuf;
SKIPSPACE(lbuf);
if (lbuf[0] == '#')
isComment = 1;
- /* Don't expand macros (eg. %define) in false branch of %if clause */
-if (!spec->readStack->reading)
+/* Don't expand macros after %elif
pavlinamv commented on this pull request.
> @@ -421,8 +422,8 @@ int readLine(rpmSpec spec, int strip)
match = parseExpressionBoolean(s);
if (match < 0) {
rpmlog(RPMLOG_ERR,
- _("%s:%d: bad %%if condition\n"),
-
pmatilai commented on this pull request.
How can the current place be "wrong" and the new place "right" if there's no
functional change?
If it's just refactoring for the next steps, then the commit message should say
so.
--
You are receiving this because you are subscribed to this thread.
pmatilai commented on this pull request.
> @@ -421,8 +422,8 @@ int readLine(rpmSpec spec, int strip)
match = parseExpressionBoolean(s);
if (match < 0) {
rpmlog(RPMLOG_ERR,
- _("%s:%d: bad %%if condition\n"),
-
pmatilai commented on this pull request.
> lbuf = spec->lbuf;
SKIPSPACE(lbuf);
if (lbuf[0] == '#')
isComment = 1;
- /* Don't expand macros (eg. %define) in false branch of %if clause */
-if (!spec->readStack->reading)
+/* Don't expand macros after %elif
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/613#pullrequestreview-189678463___
There's no mechanism for tracking build-side features, we just let things
explode.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Won't we need an `rpmlib()` dependency emitted for source packages that have
`%elif`? Or do we just typically ignore this and let things explode on their
own?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
28 matches
Mail list logo
