RFC: Factory review policy change regarding multi-specfile packages
Hi all, As part of the setup of ALP products into build service, we would like to change Factory review policy regarding multi-specfile package: Currently, packages containing multiple specfiles require creating links to another package, with the name of secondary specfiles. For instance: on Build service, package foo contains: -foo.spec -bar.spec Only foo.spec will be built. For bar.spec to be built, a linkpac is needed from foo => bar. Creation of this link is handled by OBS when accepting a sr. With ALP, our goal is to migrate source management to git backend in the (hopefully) near future (there won't be any immediate change on how to do package submissions, don't worry ;). To do that and after discussing with Build Service project managers, it would be easier (and cleaner) to add to packages with multiple specfiles a _multibuild file to handle additional specfile, without the need of linkpac. In the example above, package foo would contains: - foo.spec - bar.spec - _multibuild _multibuild content: <multibuild> <flavor>bar.spec</flavor> </multibuild> (there is no need to mention foo.spec). If there is an agreement, all multi-spec Factory packages will need to be migrated, factory-auto bot will need to be adapted to reject submission without multi-spec. Hopefully, this will be a one-time only cleanup. Questions ? Comments ? -- Frederic CROZAT Enterprise Linux OS and Containers Architect SUSE
On Mon, Apr 24, Frederic Crozat via openSUSE Factory wrote:
If there is an agreement, all multi-spec Factory packages will need to be migrated, factory-auto bot will need to be adapted to reject submission without multi-spec.
Hopefully, this will be a one-time only cleanup.
Questions ? Comments ?
I really like that. I saw it far too often in the past, that BS got confused with multiple spec files, there are left overs after delete, etc. Makes everything much cleaner and easier. Thorsten -- Thorsten Kukuk, Distinguished Engineer, Senior Architect, Future Technologies SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nuernberg, Germany Managing Director: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman (HRB 36809, AG Nürnberg)
On Monday 2023-04-24 15:01, Frederic Crozat via openSUSE Factory wrote:
For instance: on Build service, package foo contains: -foo.spec -bar.spec
Only foo.spec will be built. [..] factory-auto bot will need to be adapted to reject submission without multi-spec.
There is a legit case where packages contain multiple .spec but no _multibuild, which is when e.g. package foo contains: - foo-Fedora_38.spec - foo.spec The review bot should not reject what obssched deems acceptable.
On Mon, 2023-04-24 at 16:10 +0200, Jan Engelhardt wrote:
On Monday 2023-04-24 15:01, Frederic Crozat via openSUSE Factory wrote:
For instance: on Build service, package foo contains: -foo.spec -bar.spec
Only foo.spec will be built. [..] factory-auto bot will need to be adapted to reject submission without multi-spec.
There is a legit case where packages contain multiple .spec but no _multibuild, which is when e.g. package foo contains:
- foo-Fedora_38.spec - foo.spec
The review bot should not reject what obssched deems acceptable.
Distro submit policies (ant thus what factory-auto enforces) are by definition stricter than what OBS/RPM can build. Your example does not give proper justfication to NOT do it though: already now, if any package were to include those two spec files, the 2nd one would be auto-linked on accept into Factory (actually not by OBS itself, but by the staging accept tooling) Either don't submit this to Factory or have special tags in the fedora_38.spec %if 0%{?suse_version} BuildArch: do-not-build %endif Defacto, nothing different to what we had so far - the staging tooling would be messing up badly in this case. Cheers Dominique
On Mon, 24 Apr 2023, Frederic Crozat wrote:
Hi all,
As part of the setup of ALP products into build service, we would like to change Factory review policy regarding multi-specfile package:
Currently, packages containing multiple specfiles require creating links to another package, with the name of secondary specfiles.
For instance: on Build service, package foo contains: -foo.spec -bar.spec
Only foo.spec will be built. For bar.spec to be built, a linkpac is needed from foo => bar.
Creation of this link is handled by OBS when accepting a sr.
With ALP, our goal is to migrate source management to git backend in the (hopefully) near future (there won't be any immediate change on how to do package submissions, don't worry ;).
To do that and after discussing with Build Service project managers, it would be easier (and cleaner) to add to packages with multiple specfiles a _multibuild file to handle additional specfile, without the need of linkpac.
In the example above, package foo would contains: - foo.spec - bar.spec - _multibuild
_multibuild content: <multibuild> <flavor>bar.spec</flavor> </multibuild>
(there is no need to mention foo.spec).
If there is an agreement, all multi-spec Factory packages will need to be migrated, factory-auto bot will need to be adapted to reject submission without multi-spec.
Hopefully, this will be a one-time only cleanup.
Questions ? Comments ?
There's a big usability issue when you have many .spec files (for example see gcc13). On osc commit some of the services that are run automatically are quadratic in the number of spec files and multibuild flavors which causes a commit to take many minutes (unless you use --noservice). There's an old github issue about this but nobody cares. So I'd say this particular "mode" of _multibuild isn't really supported first-class. Richard.
On Tue, 2023-04-25 at 06:05 +0000, Richard Biener wrote:
There's a big usability issue when you have many .spec files (for example see gcc13). On osc commit some of the services that are run automatically are quadratic in the number of spec files and multibuild flavors which causes a commit to take many minutes (unless you use --noservice). There's an old github issue about this but nobody cares.
So I'd say this particular "mode" of _multibuild isn't really supported first-class.
Richard.
Thanks for bumping this. The issue referenced is https://github.com/openSUSE/obs-service-source_validator/issues/103 I think the easiest fix is to 'not do the flavor expansion matrix' when there is more than one spec file found. OBS does not do the FLAVOR expansion / build neither in this case as far as we know: https://github.com/openSUSE/open-build-service/issues/8847 So: the package either uses flavors (one spec file and BUILD_FLAVOR) in which case the validator has to run over all flavors OR the package has more than one spec files, in which case the validator should treat them all as single spec files without flavor expansion. That should be 'simple enough' to fix in the source-validator. Cheers, Dominique
On Tue, 25 Apr 2023, Dominique Leuenberger wrote:
On Tue, 2023-04-25 at 06:05 +0000, Richard Biener wrote:
There's a big usability issue when you have many .spec files (for example see gcc13). On osc commit some of the services that are run automatically are quadratic in the number of spec files and multibuild flavors which causes a commit to take many minutes (unless you use --noservice). There's an old github issue about this but nobody cares.
So I'd say this particular "mode" of _multibuild isn't really supported first-class.
Richard.
Thanks for bumping this. The issue referenced is https://github.com/openSUSE/obs-service-source_validator/issues/103
I think the easiest fix is to 'not do the flavor expansion matrix' when there is more than one spec file found. OBS does not do the FLAVOR expansion / build neither in this case as far as we know: https://github.com/openSUSE/open-build-service/issues/8847
So: the package either uses flavors (one spec file and BUILD_FLAVOR) in which case the validator has to run over all flavors OR the package has more than one spec files, in which case the validator should treat them all as single spec files without flavor expansion.
That should be 'simple enough' to fix in the source-validator.
source-validator could also simply look for @BUILD_FLAVOR@ in each .spec
and not expand if that isn't even mentioned.
Richard.
--
Richard Biener
On Tue, 2023-04-25 at 13:17 +0000, Richard Biener wrote: > On Tue, 25 Apr 2023, Dominique Leuenberger wrote: > > > On Tue, 2023-04-25 at 06:05 +0000, Richard Biener wrote: > > > There's a big usability issue when you have many .spec files (for > > > example see gcc13). On osc commit some of the services that are > > > run automatically are quadratic in the number of spec files and > > > multibuild flavors which causes a commit to take many minutes > > > (unless you use --noservice). There's an old github issue about > > > this but nobody cares. > > > > > > So I'd say this particular "mode" of _multibuild isn't really > > > supported first-class. > > > > > > Richard. > > > > Thanks for bumping this. The issue referenced is > > > > https://github.com/openSUSE/obs-service-source_validator/issues/103 > > > > I think the easiest fix is to 'not do the flavor expansion matrix' > > when > > there is more than one spec file found. OBS does not do the FLAVOR > > expansion / build neither in this case as far as we know: > > https://github.com/openSUSE/open-build-service/issues/8847 > > > > So: the package either uses flavors (one spec file and > > BUILD_FLAVOR) in > > which case the validator has to run over all flavors > > OR > > the package has more than one spec files, in which case the > > validator > > should treat them all as single spec files without flavor > > expansion. > > > > That should be 'simple enough' to fix in the source-validator. > > source-validator could also simply look for @BUILD_FLAVOR@ in each > .spec > and not expand if that isn't even mentioned. > > Richard. > That's also a valid condition, indeed. Created a quick patch. Result is promising Without patch - on gcc13 sources: > time osc service ra source_validator - package has check-build.sh: (unchanged) - package has gcc13-rpmlintrc: (unchanged) - package has gcc13-testresults-rpmlintrc: (unchanged) real 3m31.572s user 3m0.255s sys 0m32.047s with patch: > time osc service ra source_validator - package has check-build.sh: (unchanged) - package has gcc13-rpmlintrc: (unchanged) - package has gcc13-testresults-rpmlintrc: (unchanged) real 0m9.608s user 0m8.035s sys 0m1.667s I guess that's something we can live with. Will create a PR against source-validator shortly Cheers, Dominique
On Tue, 25 Apr 2023, Dominique Leuenberger wrote: > On Tue, 2023-04-25 at 13:17 +0000, Richard Biener wrote: > > On Tue, 25 Apr 2023, Dominique Leuenberger wrote: > > > > > On Tue, 2023-04-25 at 06:05 +0000, Richard Biener wrote: > > > > There's a big usability issue when you have many .spec files (for > > > > example see gcc13). On osc commit some of the services that are > > > > run automatically are quadratic in the number of spec files and > > > > multibuild flavors which causes a commit to take many minutes > > > > (unless you use --noservice). There's an old github issue about > > > > this but nobody cares. > > > > > > > > So I'd say this particular "mode" of _multibuild isn't really > > > > supported first-class. > > > > > > > > Richard. > > > > > > Thanks for bumping this. The issue referenced is > > > > > > https://github.com/openSUSE/obs-service-source_validator/issues/103 > > > > > > I think the easiest fix is to 'not do the flavor expansion matrix' > > > when > > > there is more than one spec file found. OBS does not do the FLAVOR > > > expansion / build neither in this case as far as we know: > > > https://github.com/openSUSE/open-build-service/issues/8847 > > > > > > So: the package either uses flavors (one spec file and > > > BUILD_FLAVOR) in > > > which case the validator has to run over all flavors > > > OR > > > the package has more than one spec files, in which case the > > > validator > > > should treat them all as single spec files without flavor > > > expansion. > > > > > > That should be 'simple enough' to fix in the source-validator. > > > > source-validator could also simply look for @BUILD_FLAVOR@ in each > > .spec > > and not expand if that isn't even mentioned. > > > > Richard. > > > > That's also a valid condition, indeed. Created a quick patch. Result is > promising > > Without patch - on gcc13 sources: > > > time osc service ra source_validator > - package has check-build.sh: (unchanged) > - package has gcc13-rpmlintrc: (unchanged) > - package has gcc13-testresults-rpmlintrc: (unchanged) > > real 3m31.572s > user 3m0.255s > sys 0m32.047s > > > with patch: > > time osc service ra source_validator > - package has check-build.sh: (unchanged) > - package has gcc13-rpmlintrc: (unchanged) > - package has gcc13-testresults-rpmlintrc: (unchanged) > > real 0m9.608s > user 0m8.035s > sys 0m1.667s > > > I guess that's something we can live with. Nice! > Will create a PR against source-validator shortly Thanks, Richard.
1. not sure that using the same element for multiple build descriptions and flavors within a build description is a good idea 2. how would you define something like ``` foo.spec: - flavor: postgresql15 - flavor: postgresql14 bar.spec: - flavor: postgresql15 - flavor: postgresql14 ``` with your new syntax? -- Always remember: Never accept the world as it appears to be. Dare to see it for what it could be. The world can always use more heroes.
Le mardi 25 avril 2023 à 13:42 +0200, Marcus Rückert a écrit :
1. not sure that using the same element for multiple build descriptions and flavors within a build description is a good idea
This is wanted, according to Adrian (see https://github.com/openSUSE/open-build-service/issues/8847#issuecomment-5701... where package xml tag is considered obsolete)
2. how would you define something like
``` foo.spec: - flavor: postgresql15 - flavor: postgresql14
bar.spec: - flavor: postgresql15 - flavor: postgresql14 ```
with your new syntax?
Again, it is not my new syntax, we are only leveraging OBS features (see https://openbuildservice.org/help/manuals/obs-user-guide/cha.obs.multibuild.... ) Maybe Adrian or mls could share some lights on how to achieve what you are asking for ? -- Frederic CROZAT Enterprise Linux OS and Containers Architect SUSE
On Mon, Apr 24, 2023 at 01:01:41PM +0000, Frederic Crozat via openSUSE Factory wrote:
_multibuild content: <multibuild> <flavor>bar.spec</flavor> </multibuild>
You're supposed to drop the ".spec" suffix in the flavor. It should be just: <multibuild> <flavor>bar</flavor> </multibuild> (The bar.spec works by accident. The .spec is treated like a maintenance incident suffix.) Cheers, Michael. -- Michael Schroeder SUSE Software Solutions Germany GmbH mls@suse.de GF: Ivo Totev HRB 36809, AG Nuernberg main(_){while(_=~getchar())putchar(~_-1/(~(_|32)/13*2-11)*13);}
Le mardi 25 avril 2023 à 12:34 +0000, Michael Schroeder a écrit :
On Mon, Apr 24, 2023 at 01:01:41PM +0000, Frederic Crozat via openSUSE Factory wrote:
_multibuild content: <multibuild> <flavor>bar.spec</flavor> </multibuild>
You're supposed to drop the ".spec" suffix in the flavor. It should be just:
<multibuild> <flavor>bar</flavor> </multibuild>
(The bar.spec works by accident. The .spec is treated like a maintenance incident suffix.)
Thanks for the clarification ! -- Frederic CROZAT Enterprise Linux OS and Containers Architect SUSE
Hi, On 4/25/23 08:34, Michael Schroeder wrote:
On Mon, Apr 24, 2023 at 01:01:41PM +0000, Frederic Crozat via openSUSE Factory wrote:
_multibuild content: <multibuild> <flavor>bar.spec</flavor> </multibuild>
You're supposed to drop the ".spec" suffix in the flavor. It should be just:
<multibuild> <flavor>bar</flavor> </multibuild>
(The bar.spec works by accident. The .spec is treated like a maintenance incident suffix.)
So we know of another use case, is the intention then to solve only 1 use case and ignore the second use case with multi flavor expansion? Wouldn't it make sense, and I do not know the implementation, that while touching that part of the code to address both use cases? Thanks, Robert -- Robert Schweikert MAY THE SOURCE BE WITH YOU Distinguished Engineer LINUX Technical Team Lead Public Cloud rjschwei@suse.com IRC: robjo
participants (8)
-
Dominique Leuenberger -
Frederic Crozat -
Jan Engelhardt -
Marcus Rückert -
Michael Schroeder -
Richard Biener -
Robert Schweikert -
Thorsten Kukuk