[opensuse-factory] New rpm macros to test package versions in spec files
Hi, I've written a few rpm macros that will help to write more readable spec files which will require less maintenance work. Consider you have a patch that has to be applied only with gcc 7. You'd propably do something like: %if 0%{?suse_version} > 1320 %patch0 -p1 %endif That makes sense only once you know suse_version is a number that somehow relates to the distribution version, once you know >1320 means Tumbleweed and once you know that Tumbleweed currently has gcc 7 (which only happens during a timeframe). It works, it's useful, but it's not nice. This is just an example but for other cases you'd probably have to mix it with checks for 0%{?leap_version}, 0%{?sle_version} and maybe also 0%{?is_opensuse}. With the new macros I just submitted you can do: %if %{pkg_version_at_least gcc 7} %patch0 -p1 %endif Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require less maintenance work when any distribution changes other package versions (think Leap 15 changing to gcc 7 in that example above) since it allows you not to think about what package versions a distribution has, but about the package versions themselves. Instead of package names, you can also use the macros using bracket capabilities like: %if %{pkg_version_at_least cmake(Qt5Core) 5.7} And of course, in more complex expressions: %if %{pkg_version_at_least gcc 6} && %{pkg_version_less_than gcc 7.1} Note that the package/bracket capability name doesn't need to necessarily appear in a BuildRequires statement, it's enough that it's installed in the system where the spec file is being built, so it can also be an indirect requirement. Of course, when I say "installed in the system" I mean in the chroot system where obs builds packages. Also, the version comparison is provided by rpm, so it works with all kind of version strings like 1.0alpha, 1.0rc2 and such things. The full list of macros added and simple descriptions is: %pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version Those are the high-level macros, but it's also possible to use the low-level version of those macros (though I would recommend to use the high-level macros whenever possible): %rpm_vercmp : Accepts two versions as parameters and returns -1, 0, 1 if the first version is less than, equal or greater than the second version respectively. If the first version number is the string ~~~, it just returns the same string in order to propagate errors. %pkg_version : Accepts a package or capability name as argument and returns the version number of the installed package. If no package provides the argument, it returns the string ~~~. %pkg_version_cmp : Accepts a package or capability name as first argument and a version number as second argument and returns -1, 0, 1 or ~~~ . The number values have the same meaning as in %rpm_vercmp and the ~~~ string is returned if the package or capability can't be found. Also, if you check for a package that is not installed in the system, the package build will fail with something like: [ 24s] error: /home/abuild/rpmbuild/SOURCES/libid3tag.spec:88: bad %if condition Btw, I have to say thanks to Dominique and Michael Schroeder who helped me get started on writing those macros and helped improve them. Btw2, The real case issue that initiated this was a problem I had with libid3tag since I found out a patch had to be applied only in a Staging project that tested gperf 3.1 while both Factory and the devel project for libid3tag still used gperf 3.0 where applying the patch would break the build. So, any opinion/suggestion about these macros? Greetings, -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne? (Besides "not equals" being grammatically incorrect...) -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 12:26, Jan Engelhardt wrote:
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne?
I thought about that, but it's not actually a "requirement" so %requires_* is not accurate and may clash with other macro names in the future. I also thought about using %pkg_version_ge _gt ... but I thought what I submitted was more readable and you read the code more times than you write it :). I'm open to change the names in that sense though if it's generally accepted that those names are preferred.
(Besides "not equals" being grammatically incorrect...)
Ouch, thanks for noticing, I'll change that to _not_equal. -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Antonio Larrosa <alarrosa@suse.de> writes:
On 31/05/17 12:26, Jan Engelhardt wrote:
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne?
I thought about that, but it's not actually a "requirement" so %requires_* is not accurate and may clash with other macro names in the future. I also thought about using %pkg_version_ge _gt ... but I thought what I submitted was more readable and you read the code more times than you write it :). I'm open to change the names in that sense though if it's generally accepted that those names are preferred.
And can that be in form %pkg_version gcc <= 7.0 or there are limits on rpm side? -- Nikola -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, May 31, 2017 at 12:37:44PM +0200, Nikola Pajkovsky wrote:
And can that be in form
%pkg_version gcc <= 7.0
or there are limits on rpm side?
Hey, I like that. It's nicely readable. And yes, rpm can do it. The trick is that <= and 7.0 are arguments to %pkg_version, i.e. your example is actually %{pkg_version gcc <= 7.0}. It somewhat conflicts with "%pkg_version gcc" expanding to the version of gcc, though. Cheers, Michael. -- Michael Schroeder mls@suse.de SUSE LINUX GmbH, GF Jeff Hawn, HRB 16746 AG Nuernberg main(_){while(_=~getchar())putchar(~_-1/(~(_|32)/13*2-11)*13);} -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 12:37, Nikola Pajkovsky wrote:
Antonio Larrosa <alarrosa@suse.de> writes:
On 31/05/17 12:26, Jan Engelhardt wrote:
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne?
I thought about that, but it's not actually a "requirement" so %requires_* is not accurate and may clash with other macro names in the future. I also thought about using %pkg_version_ge _gt ... but I thought what I submitted was more readable and you read the code more times than you write it :). I'm open to change the names in that sense though if it's generally accepted that those names are preferred.
And can that be in form
%pkg_version gcc <= 7.0
or there are limits on rpm side?
Hmm, I think that's a very good idea. Just note pkg_version is the macro that returns the package version, so I'll call it %pkg_version_test if that's ok for you. That could very well replace the %pkg_version_at_least/ %pkg_version_at_most/... "high-level" macros with something probably even more readable and equally short. -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 15:43, Antonio Larrosa wrote:
On 31/05/17 12:37, Nikola Pajkovsky wrote:
Antonio Larrosa <alarrosa@suse.de> writes:
On 31/05/17 12:26, Jan Engelhardt wrote:
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne?
I thought about that, but it's not actually a "requirement" so %requires_* is not accurate and may clash with other macro names in the future. I also thought about using %pkg_version_ge _gt ... but I thought what I submitted was more readable and you read the code more times than you write it :). I'm open to change the names in that sense though if it's generally accepted that those names are preferred.
And can that be in form
%pkg_version gcc <= 7.0
or there are limits on rpm side?
Hmm, I think that's a very good idea. Just note pkg_version is the macro that returns the package version, so I'll call it %pkg_version_test if that's ok for you. That could very well replace the %pkg_version_at_least/ %pkg_version_at_most/... "high-level" macros with something probably even more readable and equally short.
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number? a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other? 2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not? a) pkg_version_test b) pkg_version_check c) pkg_version d) is_pkg_version e) test_pkg_version other? Note that 1.a and 2.c can't be chosen at the same time. If you have any other proposal, just add a letter and your proposal. In my case, I'd vote for 1.a and 2.a . Greetings, -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
2017.05.31 18:01, Antonio Larrosa rašė:
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number? a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other?
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not? a) pkg_version_test b) pkg_version_check c) pkg_version d) is_pkg_version e) test_pkg_version other?
Note that 1.a and 2.c can't be chosen at the same time. If you have any other proposal, just add a letter and your proposal. In my case, I'd vote for 1.a and 2.a .
I like idea to have way to get package versions, that can be used either in conditions, either in other places as values for variables. +1 for 1.a. -- Regards -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, 2017-05-31 at 17:01 +0200, Antonio Larrosa wrote:
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number? a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other?
a) pkg_version
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not? a) pkg_version_test b) pkg_version_check c) pkg_version d) is_pkg_version e) test_pkg_version other?
f) pkg_vcmp (pkg_version_compare might even be the spelled out long variant if needed) vcmp is also used by zypper for example for a very similar action; having the same acronym sounds reasonable to me. cheers, Dominique
On mercredi, 31 mai 2017 20.07:26 h CEST Dominique Leuenberger / DimStar wrote:
On Wed, 2017-05-31 at 17:01 +0200, Antonio Larrosa wrote:
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number?
a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other?
a) pkg_version
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not?
f) pkg_vcmp (pkg_version_compare might even be the spelled out long variant if needed)
vcmp is also used by zypper for example for a very similar action; having the same acronym sounds reasonable to me.
cheers, Dominique
For the same reason a) and f) -- Bruno Friedmann Ioda-Net Sàrl www.ioda-net.ch Bareos Partner, openSUSE Member, fsfe fellowship GPG KEY : D5C9B751C4653227 irc: tigerfoot -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Bruno Friedmann <bruno@ioda-net.ch> writes:
On mercredi, 31 mai 2017 20.07:26 h CEST Dominique Leuenberger / DimStar wrote:
On Wed, 2017-05-31 at 17:01 +0200, Antonio Larrosa wrote:
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number?
a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other?
a) pkg_version
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not?
f) pkg_vcmp (pkg_version_compare might even be the spelled out long variant if needed)
vcmp is also used by zypper for example for a very similar action; having the same acronym sounds reasonable to me.
I would go with pkg_version pkg_versioncmp -- Nikola -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Hello, On May 31 17:01 Antonio Larrosa wrote (excerpt):
Just wondering... let's do a small poll. How would you call ... 1) The macro that has one argument (package name) and returns its version number? a) pkg_version b) pkg_version_number c) package_version d) pkg_ver other?
c) package_version because I prefer readability over abbreviations because in the whole IT we have way too many abbreviations.
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not? a) pkg_version_test b) pkg_version_check c) pkg_version d) is_pkg_version e) test_pkg_version other?
other because I would like to see a word that is already commonly used for comparison functionality like "if" or "cmp" i.e. something like package_version_compare or for those who think abbreviations are really better: pkg_version_cmp pkg_ver_cmp pkg_vcmp pvcmp pvif pif nice to see how abbreviations make things more and more incomprehensible ;-) Kind Regards Johannes Meixner -- SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard, Graham Norton - HRB 21284 (AG Nuernberg) -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 17:01, Antonio Larrosa wrote:
On 31/05/17 15:43, Antonio Larrosa wrote:
On 31/05/17 12:37, Nikola Pajkovsky wrote: [...]
Hmm, I think that's a very good idea. Just note pkg_version is the macro that returns the package version, so I'll call it %pkg_version_test if that's ok for you. That could very well replace the %pkg_version_at_least/ %pkg_version_at_most/... "high-level" macros with something probably even more readable and equally short.
Just wondering... let's do a small poll.
I change my vote to 1.a and 2.f since I like Dominique's idea.
How would you call ... 1) The macro that has one argument (package name) and returns its version number?
a) pkg_version : 5 votes b) pkg_version_number c) package_version : 1 vote d) pkg_ver
2) The macro that has 3 arguments (package_name, operator, version number) and tests whether the condition is true or not? a) pkg_version_test b) pkg_version_check c) pkg_version d) is_pkg_version e) test_pkg_version f) pkg_vcmp : 3 votes g) pkg_versioncmp : 1 vote h) package_version_compare : 1 vote
So... the winners are: pkg_version and pkg_vcmp which I submitted at https://build.opensuse.org/request/show/501101 Thanks for sending your opinions! -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2017-05-31 12:33, Antonio Larrosa wrote:
%pkg_version_not_equals : package != version
So, any opinion/suggestion about these macros?
So much to type. Can't we just have something similar to %requires_ge, _gt, _le, _lt, _eq, _ne?
I thought about that, but it's not actually a "requirement"
Note the "similar to" ;-)
I also thought about using %pkg_version_ge _gt ... but [..] more readable and you read the code more times than you write it :). I'm open to change the names in that sense though if it's generally accepted that those names are preferred.
(Besides "not equals" being grammatically incorrect...)
Ouch, thanks for noticing, I'll change that to _not_equal.
That's my point - writing it out using natural language inherently leads to "inconsistencies". ("equals" and "not equal" is one, "less than" and "at most" is the other. This is why I instantly thought of LT and LE instead.) Some of the spec generators floating around also use the symbolic notation (<=, =>) from the get-go without having to resort to lt/gt/etc. I think it was %{rubygem_requires blah >= 5}. If your pkg_version macro could even do that, that would be even better than having to discuss not_equals, not_equal and NE. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, May 31, 2017 at 6:23 AM, Antonio Larrosa <alarrosa@suse.de> wrote:
Hi,
I've written a few rpm macros that will help to write more readable spec files which will require less maintenance work.
Consider you have a patch that has to be applied only with gcc 7. You'd propably do something like:
%if 0%{?suse_version} > 1320 %patch0 -p1 %endif
That makes sense only once you know suse_version is a number that somehow relates to the distribution version, once you know >1320 means Tumbleweed and once you know that Tumbleweed currently has gcc 7 (which only happens during a timeframe). It works, it's useful, but it's not nice. This is just an example but for other cases you'd probably have to mix it with checks for 0%{?leap_version}, 0%{?sle_version} and maybe also 0%{?is_opensuse}.
With the new macros I just submitted you can do:
%if %{pkg_version_at_least gcc 7} %patch0 -p1 %endif
Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require less maintenance work when any distribution changes other package versions (think Leap 15 changing to gcc 7 in that example above) since it allows you not to think about what package versions a distribution has, but about the package versions themselves.
Instead of package names, you can also use the macros using bracket capabilities like:
%if %{pkg_version_at_least cmake(Qt5Core) 5.7}
And of course, in more complex expressions:
%if %{pkg_version_at_least gcc 6} && %{pkg_version_less_than gcc 7.1}
Note that the package/bracket capability name doesn't need to necessarily appear in a BuildRequires statement, it's enough that it's installed in the system where the spec file is being built, so it can also be an indirect requirement. Of course, when I say "installed in the system" I mean in the chroot system where obs builds packages. Also, the version comparison is provided by rpm, so it works with all kind of version strings like 1.0alpha, 1.0rc2 and such things.
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
Those are the high-level macros, but it's also possible to use the low-level version of those macros (though I would recommend to use the high-level macros whenever possible):
%rpm_vercmp : Accepts two versions as parameters and returns -1, 0, 1 if the first version is less than, equal or greater than the second version respectively. If the first version number is the string ~~~, it just returns the same string in order to propagate errors.
%pkg_version : Accepts a package or capability name as argument and returns the version number of the installed package. If no package provides the argument, it returns the string ~~~.
%pkg_version_cmp : Accepts a package or capability name as first argument and a version number as second argument and returns -1, 0, 1 or ~~~ . The number values have the same meaning as in %rpm_vercmp and the ~~~ string is returned if the package or capability can't be found.
Also, if you check for a package that is not installed in the system, the package build will fail with something like:
[ 24s] error: /home/abuild/rpmbuild/SOURCES/libid3tag.spec:88: bad %if condition
Btw, I have to say thanks to Dominique and Michael Schroeder who helped me get started on writing those macros and helped improve them.
Btw2, The real case issue that initiated this was a problem I had with libid3tag since I found out a patch had to be applied only in a Staging project that tested gperf 3.1 while both Factory and the devel project for libid3tag still used gperf 3.0 where applying the patch would break the build.
So, any opinion/suggestion about these macros?
Greetings,
Not that I don't appreciate the technical ingenuity that is involved in making these kinds of macros, but this feels like miles of bad road. It makes it really easy for people to do dumb patches and then just let them build up. And among other things, conditional patch application makes for builds with more "surprises" in them. I also feel like this just makes it easier to not try to properly fix things. In my experience, most of the time, a proper fix for things like compiler/Qt issues typically don't break older compilers or Qt versions. In general, I think we should be doing *way less* conditional patching, not more. Then again, I'm not sure how much "working with upstream projects" is promoted in openSUSE compared to Fedora (where most of my packaging experience comes from). -- 真実はいつも一つ！/ Always, there's only one truth! -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 20:20, Neal Gompa wrote:
Then again, I'm not sure how much "working with upstream projects" is promoted in openSUSE compared to Fedora (where most of my packaging experience comes from).
Its strongly encouraged and recommended wherever possible :-), so the number of patches we have sitting around indefinitely should be small, the number of patches sitting around until the next release of upstream software can be much higher though. -- Simon Lees (Simotek) http://simotek.net Emergency Update Team keybase.io/simotek SUSE Linux Adelaide Australia, UTC+10:30 GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
On Wed, 2017-05-31 at 06:50 -0400, Neal Gompa wrote:
Not that I don't appreciate the technical ingenuity that is involved in making these kinds of macros, but this feels like miles of bad road. It makes it really easy for people to do dumb patches and then just let them build up. And among other things, conditional patch application makes for builds with more "surprises" in them.
I also feel like this just makes it easier to not try to properly fix things. In my experience, most of the time, a proper fix for things like compiler/Qt issues typically don't break older compilers or Qt versions.
In general, I think we should be doing *way less* conditional patching, not more.
Then again, I'm not sure how much "working with upstream projects" is promoted in openSUSE compared to Fedora (where most of my packaging experience comes from).
Conditional patches certainly have to be the exception - but every so often you find upstream porting stuff to a new version of libFOO and just requiring it afterwards. Because they can. We often build a package for multiple versions and try to care not only for 'newest' - just think about Leap, where a lot of things is not 'fresh of the press' So if we have a pkg A that builds with libFOO in Leap, then in TW with get an update of libFOO+n and A's upstream simply ports to that API, without actually caring for backwards compatibility (and yes, those are many) - we can apply the UPSTREAM patch on our package without breaking the Leap build - because there we don't apply it. And instead of quering the 'suse_version' as we did so far, we actually immediately gain the advantage that somebody building a branch somewhere with newer packages of libFOO added, the patch will be automatically applied. (The example of course only holds true for libFOO being API incompatible to libFOO+n - and A's upstream simply dropping support for libFOO in favor of libFOO+n) But it's still better than all the variants of %if %{suse_version} - where basically almost NEVER is the SUSE target version really the identifying factory for some change - so those macros help align the code to what the writer actually really wanted in first place ( Cheers, Dominique
On Wed, May 31, 2017 at 10:12 AM, Dominique Leuenberger / DimStar <dimstar@opensuse.org> wrote:
On Wed, 2017-05-31 at 06:50 -0400, Neal Gompa wrote:
Not that I don't appreciate the technical ingenuity that is involved in making these kinds of macros, but this feels like miles of bad road. It makes it really easy for people to do dumb patches and then just let them build up. And among other things, conditional patch application makes for builds with more "surprises" in them.
I also feel like this just makes it easier to not try to properly fix things. In my experience, most of the time, a proper fix for things like compiler/Qt issues typically don't break older compilers or Qt versions.
In general, I think we should be doing *way less* conditional patching, not more.
Then again, I'm not sure how much "working with upstream projects" is promoted in openSUSE compared to Fedora (where most of my packaging experience comes from).
Conditional patches certainly have to be the exception - but every so often you find upstream porting stuff to a new version of libFOO and just requiring it afterwards. Because they can.
We often build a package for multiple versions and try to care not only for 'newest' - just think about Leap, where a lot of things is not 'fresh of the press'
So if we have a pkg A that builds with libFOO in Leap, then in TW with get an update of libFOO+n and A's upstream simply ports to that API, without actually caring for backwards compatibility (and yes, those are many) - we can apply the UPSTREAM patch on our package without breaking the Leap build - because there we don't apply it.
Or... you know, just only develop to target Factory. The model you present makes it hard to enforce a factory-first model. When you are backporting from "the future" (as Factory is, relative to Leap/SLE), it makes sense to branch the package for "the past" (relative to Factory) and incorporate the patches that way. The back-and-forth is not sane.
And instead of quering the 'suse_version' as we did so far, we actually immediately gain the advantage that somebody building a branch somewhere with newer packages of libFOO added, the patch will be automatically applied.
(The example of course only holds true for libFOO being API incompatible to libFOO+n - and A's upstream simply dropping support for libFOO in favor of libFOO+n)
But it's still better than all the variants of %if %{suse_version} - where basically almost NEVER is the SUSE target version really the identifying factory for some change - so those macros help align the code to what the writer actually really wanted in first place (
It seems like this is being made more complicated than it should be. -- 真実はいつも一つ！/ Always, there's only one truth! -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Am Wed, 31 May 2017 12:23:13 +0200 schrieb Antonio Larrosa <alarrosa@suse.de>:
Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require
Is that 'gcc.rpm is 7.0' or 'gcc-7 binary is available'? I think it encourages people to not fix the actual code. It will likely break 'quilt setup', like most conditionals in %prep do. Olaf
On 31/05/17 12:56, Olaf Hering wrote:
Am Wed, 31 May 2017 12:23:13 +0200 schrieb Antonio Larrosa <alarrosa@suse.de>:
Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require
Is that 'gcc.rpm is 7.0' or 'gcc-7 binary is available'?
The macros just work with package names or "provided" capabilities so "gcc" as that's the package name.
I think it encourages people to not fix the actual code.
Can you be more explicit? Why would those macros encourage not to fix anything?
It will likely break 'quilt setup', like most conditionals in %prep do.
That's a good point. Although I'm not sure why these macros make the problem worse than it currently is, and if you're complaining about conditionals in %prep in general and how they break quilt, I guess that should be discussed in another thread. Just not to mix issues. -- Antonio Larrosa
On 2017-05-31 13:15, Antonio Larrosa wrote:
That's a good point. Although I'm not sure why these macros make the problem worse than it currently is, and if you're complaining about conditionals in %prep in general and how they break quilt, I guess that should be discussed in another thread.
I'm working heavily with quilt and was hitting that here and there. I found out that one problem is that the macros used in .spec files need to be available on the system running quilt. Usually in /etc/rpm/macros.* or under /usr/lib/rpm/ So when I am working with Factory packages on a 42.2 host, some newer macros will be missing. And if I do not have some latex-foo package installed that provides specific macros, the same applies. osc build takes care to make it all work in the chroot/kvm but nothing does for quilt on the outside/host. Ciao Bernhard M.
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
Consider you have a patch that has to be applied only with gcc 7.
Coming back to what Neil Gompa and Olaf Hering mentioned (gcc v7 rpm vs gcc-7 v7 rpm), if you have something that needs to be changed for gcc 7, you are probably better off with #if __GNUC__ >= 7 ... #else ... #endif -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 31/05/17 13:13, Jan Engelhardt wrote:
On Wednesday 2017-05-31 12:23, Antonio Larrosa wrote:
Consider you have a patch that has to be applied only with gcc 7.
Coming back to what Neil Gompa and Olaf Hering mentioned (gcc v7 rpm vs gcc-7 v7 rpm), if you have something that needs to be changed for gcc 7, you are probably better off with
#if __GNUC__ >= 7 ... #else ... #endif
Please, this was only an example. Don't fix your focus on gcc and think of other uses. I mentioned the real example that got me to work on this (libid3tag needing a patch only in a Staging project because that was the only place gperf 3.1 was being used to build it so I had to break the package build in the devel project while the staging project was being accepted). Also, think of packages that need a patch for SLE12 because it uses an older cmake/whatever version when we don't want to apply the patch because it's SLE12 but because of whatever package version SLE12 uses. It's those cases the macros try to help with. -- Antonio Larrosa -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, May 31, 2017 at 6:23 AM, Antonio Larrosa <alarrosa@suse.de> wrote:
Hi,
I've written a few rpm macros that will help to write more readable spec files which will require less maintenance work.
Consider you have a patch that has to be applied only with gcc 7. You'd propably do something like:
%if 0%{?suse_version} > 1320 %patch0 -p1 %endif
That makes sense only once you know suse_version is a number that somehow relates to the distribution version, once you know >1320 means Tumbleweed and once you know that Tumbleweed currently has gcc 7 (which only happens during a timeframe). It works, it's useful, but it's not nice. This is just an example but for other cases you'd probably have to mix it with checks for 0%{?leap_version}, 0%{?sle_version} and maybe also 0%{?is_opensuse}.
With the new macros I just submitted you can do:
%if %{pkg_version_at_least gcc 7} %patch0 -p1 %endif
Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require less maintenance work when any distribution changes other package versions (think Leap 15 changing to gcc 7 in that example above) since it allows you not to think about what package versions a distribution has, but about the package versions themselves.
Instead of package names, you can also use the macros using bracket capabilities like:
%if %{pkg_version_at_least cmake(Qt5Core) 5.7}
And of course, in more complex expressions:
%if %{pkg_version_at_least gcc 6} && %{pkg_version_less_than gcc 7.1}
Note that the package/bracket capability name doesn't need to necessarily appear in a BuildRequires statement, it's enough that it's installed in the system where the spec file is being built, so it can also be an indirect requirement. Of course, when I say "installed in the system" I mean in the chroot system where obs builds packages. Also, the version comparison is provided by rpm, so it works with all kind of version strings like 1.0alpha, 1.0rc2 and such things.
The full list of macros added and simple descriptions is:
%pkg_version_at_least : Arguments are a package/capability name and a version number and returns true if package >= version %pkg_version_at_most : package <= version %pkg_version_equals : package == version %pkg_version_greater_than : package > version %pkg_version_less_than : package < version %pkg_version_not_equals : package != version
Those are the high-level macros, but it's also possible to use the low-level version of those macros (though I would recommend to use the high-level macros whenever possible):
%rpm_vercmp : Accepts two versions as parameters and returns -1, 0, 1 if the first version is less than, equal or greater than the second version respectively. If the first version number is the string ~~~, it just returns the same string in order to propagate errors.
%pkg_version : Accepts a package or capability name as argument and returns the version number of the installed package. If no package provides the argument, it returns the string ~~~.
%pkg_version_cmp : Accepts a package or capability name as first argument and a version number as second argument and returns -1, 0, 1 or ~~~ . The number values have the same meaning as in %rpm_vercmp and the ~~~ string is returned if the package or capability can't be found.
Also, if you check for a package that is not installed in the system, the package build will fail with something like:
[ 24s] error: /home/abuild/rpmbuild/SOURCES/libid3tag.spec:88: bad %if condition
Btw, I have to say thanks to Dominique and Michael Schroeder who helped me get started on writing those macros and helped improve them.
Btw2, The real case issue that initiated this was a problem I had with libid3tag since I found out a patch had to be applied only in a Staging project that tested gperf 3.1 while both Factory and the devel project for libid3tag still used gperf 3.0 where applying the patch would break the build.
So, any opinion/suggestion about these macros?
With the current approach it is easy to see a macro is out of date and no longer needed. These macros seem like they would hang around for much longer. At a minimum the style guide should call for a comment before each usage as to when the conditional should be removed from the spec file. Otherwise, I hate to see what our spec files will look like 5 years from now. Greg -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Hello, On May 31 12:23 Antonio Larrosa wrote (excerpt):
Consider you have a patch that has to be applied only with gcc 7. You'd propably do something like:
%if 0%{?suse_version} > 1320 %patch0 -p1 %endif
That makes sense only once you know suse_version is a number that somehow relates to the distribution version, once you know >1320 means Tumbleweed and once you know that Tumbleweed currently has gcc 7 (which only happens during a timeframe). It works, it's useful, but it's not nice. This is just an example but for other cases you'd probably have to mix it with checks for 0%{?leap_version}, 0%{?sle_version} and maybe also 0%{?is_opensuse}.
With the new macros I just submitted you can do:
%if %{pkg_version_at_least gcc 7} %patch0 -p1 %endif
Which is actually what you want to do: apply your patch when the gcc version is 7.0 or newer. I think this makes spec files more readable and require less maintenance work when any distribution changes other package versions (think Leap 15 changing to gcc 7 in that example above) since it allows you not to think about what package versions a distribution has, but about the package versions themselves.
...
So, any opinion/suggestion about these macros?
Great! I do very much appreciate such kind of test macros! FYI cf. what I had written about "... using product version tests in spec files is fundamentally inappropriate because such tests do not test for the actual requirement. Therefore the plain test is meaningless ..." in https://lists.opensuse.org/opensuse-packaging/2015-06/msg00027.html In general I think any version test is fundamentally inappropriate because because it does not test for the actual requirement (assuming the plain version is not what is actually needed). Therefore - strictly speaking - even a test like %if %{pkg_version_at_least gcc 7} does not test for the actual final requirement but is it a big step forward into the right direction. Furthermore in practice a package version string is probably often the only thing that is directly available for testing with reasonable effort so that I think in practice a test like %if %{pkg_version_at_least gcc 7} is probably fully sufficient for the next 10 years or so :-) Kind Regards Johannes Meixner -- SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard, Graham Norton - HRB 21284 (AG Nuernberg) -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
participants (13)
-
Antonio Larrosa -
Bernhard M. Wiedemann -
Bruno Friedmann -
Dominique Leuenberger / DimStar -
Greg Freemyer -
Jan Engelhardt -
Johannes Meixner -
Michael Schroeder -
Neal Gompa -
Nikola Pajkovsky -
Olaf Hering -
opensuse.lietuviu.kalba -
Simon Lees