Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Vít Ondruch
Ah, ok, so this works, but not for the `%endif`. I cannot say I grasped it from the documentation update :/ -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996#issuecomment-2022749958 You are receiving this because you are subscribed to

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
@pmatilai commented on this pull request. > @@ -246,8 +246,8 @@ static int expandMacrosInSpecBuf(rpmSpec spec, int strip) if ((condition) && (!condition->withArgs)) { const char *s = lbuf + condition->textLen; SKIPSPACE(s); - if (s[0]) -

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Vít Ondruch
> > `%dnl` already works for this purpose, doesn't it? > > It doesn't, because this check happens before macro expansion. This fails: ~~~ $ cat license-subpackages.spec Summary: Demonstration package for mining licenses from subpackages Name: license-subpackages Version: 1 Release: 1%{?dist}

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
Merged #2996 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996#event-12265396584 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
@pmatilai commented on this pull request. > @@ -246,8 +246,8 @@ static int expandMacrosInSpecBuf(rpmSpec spec, int strip) if ((condition) && (!condition->withArgs)) { const char *s = lbuf + condition->textLen; SKIPSPACE(s); - if (s[0]) -

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Florian Festi
@ffesti commented on this pull request. > @@ -246,8 +246,8 @@ static int expandMacrosInSpecBuf(rpmSpec spec, int strip) if ((condition) && (!condition->withArgs)) { const char *s = lbuf + condition->textLen; SKIPSPACE(s); - if (s[0]) - rpmlog(RPMLOG_WARNING,

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
@pmatilai commented on this pull request. > @@ -246,8 +246,8 @@ static int expandMacrosInSpecBuf(rpmSpec spec, int strip) if ((condition) && (!condition->withArgs)) { const char *s = lbuf + condition->textLen; SKIPSPACE(s); - if (s[0]) -

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Florian Festi
@ffesti pushed 1 commit. fc4c5ef5aaae4a0e360cde24c13647ef4ed8be16 Make junk after conditionals an error -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996/files/6bbb6a39e662d32fa0876c3cafcb091509200c09..fc4c5ef5aaae4a0e360cde24c13647ef4ed8be16 You are receiving this

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
@pmatilai commented on this pull request. > @@ -246,8 +246,8 @@ static int expandMacrosInSpecBuf(rpmSpec spec, int strip) if ((condition) && (!condition->withArgs)) { const char *s = lbuf + condition->textLen; SKIPSPACE(s); - if (s[0]) -

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
> `%dnl` already works for this purpose, doesn't it? It doesn't, because this check happens before macro expansion. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996#issuecomment-2022362161 You are receiving this because you are

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Vít Ondruch
`%dnl` already works for this purpose, doesn't it? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996#issuecomment-2022331640 You are receiving this because you are subscribed to this thread. Message ID:

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Florian Festi
Done. @dmnks: This needs to go into the compatibility notes of the 4.20 release. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2996#issuecomment-2022305618 You are receiving this because you are subscribed to this thread. Message ID:

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-27 Thread Panu Matilainen
Lets worry about the other stuff separately, this is the thing that hurts the most by far because it used to work, even if for the wrong reasons. Making non-comments an error now that we do allow comments is indeed the right thing to do :+1: -- Reply to this email directly or view it on

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-26 Thread Florian Festi
I am open to allow comments elsewhere, too. This will probably require multiple independent patches. This one is for conditionals. (Although I am fine to do that in this PR) I would not allow comments in the section contents per se. But basically allow them for the native RPM parts and leave

Re: [Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-26 Thread Panu Matilainen
Heh, that's indeed a pretty simple cure for this. The "problem" of course is that it's inconsistent with the rest of the spec where comments are only allowed at the beginning of lines, but seeing how widely this is in use and how many pointless warnings packagers suffer for it... seems well

[Rpm-maint] [rpm-software-management/rpm] Allow comments after conditionals (PR #2996)

2024-03-26 Thread Florian Festi
Still issue an warning for everything else. Now that comments are allowed again may be we should issue an error for those cases. Resolves: #829 You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/2996 -- Commit Summary -- *