Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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 it on GitHub: https://github.com/rpm-software-management/rpm/pull/840#pullrequestreview-289812867___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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 Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
@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: https://github.com/rpm-software-management/rpm/pull/840/files/08e34e0929d7278c3b7e6d988845d90c90156893..cd764dbd89f56c7714d94d601c70453d1191ac2d ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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: https://github.com/rpm-software-management/rpm/pull/840#issuecomment-532607547___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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 or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/840#issuecomment-532603556___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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: https://github.com/rpm-software-management/rpm/pull/840#issuecomment-532598443___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
("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:
https://github.com/rpm-software-management/rpm/pull/840#issuecomment-532597604___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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 subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/840#issuecomment-532595542___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
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 noted, easy to fix and the fix is correct everywhere so its all for a good cause. The referenced commit id seems wrong though, the right one seems to be 3ad99fcba52fcc5e8ab636d2f1760c945cdfbf19. Care to fix that? -- 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#issuecomment-532557041___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
I was about to write that this still works, but it really depends on to what `%var` expands. It'll most likely be an error and you need to write `%if "%var" != "string"`. -- 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#issuecomment-532290796___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
@mlschroe Wait, even *I* use `%if %var != "string"`... That won't work anymore? -- 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#issuecomment-532286213___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
The problem is that many people do not know that `%var != string` is not valid syntax. Even Panu used `a!=b` in the test suite. I prefer ripping out the code because the currently only `[a-zA-Z][a-zA-Z0-9]*` is supported (it's supposed to match an identifier, but note the missing `_`) and thus it will confuse people. If we decide to keep the functionality for backwards compatibility, I'll do a different patch that also removes TOK_IDENTIFIER but instead directly generates TOK_STRING without calling rpmExpand(). -- 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#issuecomment-532279131___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Remove TOK_IDENTIFIER support from expression parsing (#840)
Conan-Kudo approved this pull request. I guess if no one noticed for ~19 years... -- 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#pullrequestreview-289277566___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
