Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-24 Thread Panu Matilainen
Merged #853 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/853#event-2657687248___ Rpm-maint mailing list

Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-24 Thread pavlinamv
The PR looks good to me. -- 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/853#issuecomment-534511744___ Rpm-maint mailing list

Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-24 Thread Panu Matilainen
After poking around a bit, I ended up dropping the argument length checking entirely: it's not an *error* to echo an empty string, and similarly it is not an *error* to ask for dirname of an empty string. And so on - seems better left alone in reality. On-disk filenames cannot be empty so

Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-24 Thread Panu Matilainen
Yup, the thing with g vs gn is that g reflects whether ':' is there or not, and gn reflects whether there's something after it: macros that do not accept arguments must not have g, and macros that do need to have both (but non-zero gn implies non-NULL g). And yeah, ```%{echo:%{nil}}``` would

Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-23 Thread Michael Schroeder
No objections from my side. Note that `%{echo:%nil}` would still work. I guess you tested `gn` instead of `g` on purpose? You may also want to add a test in doFoo so that this also gets checked after expansion. Or maybe only check this in doFoo? -- You are receiving this because you are

Re: [Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-23 Thread Panu Matilainen
Note that while for most built-ins such as %{getenv:...} a non-empty argument is clearly required, but there are some cases where an empty argument could be allowed, such as an empty %{echo:} which this turns into an error. I could be convinced to add a third form where the argument is

[Rpm-maint] [rpm-software-management/rpm] Codify built-in macro argument acceptance (#853)

2019-09-23 Thread Panu Matilainen
Built-in macros either take arguments via %{foo:...} or dont, raise errors on unexpected and missing arguments. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/853 -- Commit Summary -- * Codify built-in macro argument