Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dynamic build requires for Foreman on EL9 #8414

Draft
wants to merge 1 commit into
base: rpm/develop
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Aug 27, 2022

Since Fedora 31 it's possible to use Dynamic BuildRequires. This removes the need to change the spec file all the time to keep in sync with the Foreman repo itself.

Once EL 8 support is dropped, this can be used. Since that's far in the future, I haven't done the same for NPM yet but obviously that's possible too.

In a proper implementation we would have tooling that's usable in all package, like a foreman-packaging RPM with reusable scripts and macros.

@ekohl
Copy link
Member Author

ekohl commented Mar 4, 2024

Rebased to resolve a conflict. Since we now have EL 9 builds, it should be possible to verify it works with COPR. Until we drop EL 8, we can't avoid commits like 212285e but at least with packit we'll have correct build requires for EL 9.

@ekohl ekohl force-pushed the rpm/develop-generate-build-requires-on-the-fly branch 3 times, most recently from c786f49 to d2a32e0 Compare March 4, 2024 16:50
@ekohl
Copy link
Member Author

ekohl commented Mar 4, 2024

I don't quite understand why this is failing.

2024-03-04T17:06:42.576Z] TASK [repoclosure : Run repoclosure] *******************************************
[2024-03-04T17:06:42.576Z] fatal: [foreman]: FAILED! => 
[2024-03-04T17:06:42.576Z]     changed: false
[2024-03-04T17:06:42.576Z]     command: dnf repoclosure --refresh --newest --config /tmp/ansible._h54ylkcrepoclosure/yum.conf
[2024-03-04T17:06:42.576Z]         --check repo0 --repo repo0 --check el9-foreman-nightly-staging --repo el9-foreman-nightly-staging
[2024-03-04T17:06:42.576Z]         --repofrompath repo0,https://download.copr.fedorainfracloud.org/results/@theforeman/foreman-nightly-staging-scratch-9f3b3342-376b-5d58-b326-307b5a3d886f/rhel-9-x86_64
[2024-03-04T17:06:42.576Z]         --repo el9-baseos --repo el9-appstream --repo el9-puppet-7 --repo el9-candlepin-4.3-staging
[2024-03-04T17:06:42.576Z]     msg: Repoclosure failed
[2024-03-04T17:06:42.576Z]     output: |-
[2024-03-04T17:06:42.576Z]         Unable to detect release version (use '--releasever' to specify release version)
[2024-03-04T17:06:42.576Z]         Added repo0 repo from https://download.copr.fedorainfracloud.org/results/@theforeman/foreman-nightly-staging-scratch-9f3b3342-376b-5d58-b326-307b5a3d886f/rhel-9-x86_64
[2024-03-04T17:06:42.576Z]         Error: Repoclosure ended with unresolved dependencies.
[2024-03-04T17:06:42.576Z]         package: foreman-3.11.0-0.3.develop.el9.src from repo0
[2024-03-04T17:06:42.576Z]           unresolved deps:
[2024-03-04T17:06:42.576Z]             nodejs-packaging

That said, it's clearly making a difference:

diff -Nut <(rpm -q --requires foreman-3.11.0-0.3.develop.el8.src.rpm) <(rpm -q --requires foreman-3.11.0-0.3.develop.el9.src.rpm)
warning: foreman-3.11.0-0.3.develop.el9.src.rpm: Header V4 RSA/SHA256 Signature, key ID f74516e7: NOKEY
warning: foreman-3.11.0-0.3.develop.el8.src.rpm: Header V4 RSA/SHA256 Signature, key ID f74516e7: NOKEY
--- /dev/fd/63	2024-03-04 21:26:45.014618994 +0100
+++ /dev/fd/62	2024-03-04 21:26:45.014618994 +0100
@@ -22,6 +22,7 @@
 (npm(webpack-stats-plugin) >= 1.0.3 with npm(webpack-stats-plugin) < 2.0.0)
 (rubygem(activerecord-session_store) >= 2.0.0 with rubygem(activerecord-session_store) < 3)
 (rubygem(ancestry) >= 4.0 with rubygem(ancestry) < 5.0)
+(rubygem(apipie-dsl) >= 2.6.1)
 (rubygem(apipie-rails) >= 0.8.0 with rubygem(apipie-rails) < 2)
 (rubygem(audited) >= 5.0 with rubygem(audited) < 6.0)
 (rubygem(bcrypt) >= 3.1 with rubygem(bcrypt) < 4.0)
@@ -41,6 +42,7 @@
 (rubygem(ldap_fluff) >= 0.5.0 with rubygem(ldap_fluff) < 1.0)
 (rubygem(logging) >= 1.8.0 with rubygem(logging) < 3.0.0)
 (rubygem(mail) >= 2.7 with rubygem(mail) < 3.0)
+(rubygem(net-ldap) >= 0.16.0)
 (rubygem(oauth) >= 1.0 with rubygem(oauth) < 2.0)
 (rubygem(patternfly-sass) >= 3.59.4 with rubygem(patternfly-sass) < 3.60.0)
 (rubygem(po_to_json) >= 1.1 with rubygem(po_to_json) < 2.0)
@@ -48,6 +50,7 @@
 (rubygem(rack-cors) >= 1.1 with rubygem(rack-cors) < 2.0)
 (rubygem(rails) >= 6.1.6 with rubygem(rails) < 6.2.0)
 (rubygem(rails-i18n) >= 7.0 with rubygem(rails-i18n) < 8.0)
+(rubygem(rdoc) < 6.4)
 (rubygem(responders) >= 3.0 with rubygem(responders) < 4.0)
 (rubygem(rest-client) >= 2.0.0 with rubygem(rest-client) < 3)
 (rubygem(rexml) or ruby-default-gems < 3.0)
@@ -72,21 +75,20 @@
 nodejs >= 14
 nodejs-packaging
 rpmlib(CompressedFileNames) <= 3.0.4-1
+rpmlib(DynamicBuildRequires) <= 4.15.0-1
 rpmlib(FileDigests) <= 4.6.0-1
 rpmlib(RichDependencies) <= 4.12.0-1
 rubygem(activerecord-nulldb-adapter)
-rubygem(apipie-dsl) >= 2.6.1
 rubygem(bigdecimal)
 rubygem(bundler_ext)
 rubygem(daemons)
 rubygem(facter)
 rubygem(get_process_mem)
 rubygem(graphql-batch)
-rubygem(net-ldap) >= 0.16.0
 rubygem(net-ping)
 rubygem(net-scp)
 rubygem(net-ssh)
 rubygem(rake) >= 0.8.3
-rubygem(rdoc) < 6.4
+rubygem(rss)
 rubygems
 systemd

Technically it's even more correct, because it understands RUBY_VERSION conditionals. Note rss is in there for EL9, and that's because of https://github.com/theforeman/foreman/blob/bbca47389a95e85f71e6a31c6be33c7733bdbed5/Gemfile#L52

@evgeni
Copy link
Member

evgeni commented Mar 5, 2024

I don't quite understand why this is failing.

Because nodejs-packaging is in a module, and we don't enable that in closure.
But also, because it's looking at the SRPM, and we actually don't care about SRPM closure in prod.

@ekohl ekohl force-pushed the rpm/develop-generate-build-requires-on-the-fly branch from d2a32e0 to 1a58edb Compare March 5, 2024 12:31
Since Fedora 31 it's possible to use Dynamic BuildRequires[1]. This
removes the need to change the spec file all the time to keep in sync
with the Foreman repo itself.

Once EL 8 support is dropped, the legacy way can be dropped. Until then
it's hidden behind conditionals so at least EL 9 is up to date.

In a proper implementation we would have tooling that's usable in all
package, like a foreman-packaging RPM with reusable scripts and macros.

[1]: https://fedoraproject.org/wiki/Changes/DynamicBuildRequires
@ekohl ekohl force-pushed the rpm/develop-generate-build-requires-on-the-fly branch from 1a58edb to 3c51fa5 Compare March 5, 2024 12:57
@ekohl ekohl changed the title Showcase dynamic build requires Use dynamic build requires for Foreman on EL9 Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants