Skip to content

build: For RHEL/Centos Stream drop the unavailable ragel package dependency #2282

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

Merged
merged 1 commit into from
Aug 18, 2025

Conversation

wcohen
Copy link
Contributor

@wcohen wcohen commented Aug 15, 2025

When attempting to do ./Makepkgs on Centos 9 stream and Centos 10 Stream the rpmbuild build would fail because of the unsatisfied dependency on ragel. There are two places where ragel build dependencies were listed, but only one had a conditional to turn it off on RHEL/Centos. Added a conditional to the remaining place to allow ./Makepkgs to work on those platforms.

@kmcdonell
Copy link
Member

@wcohen is ragel available on any version of rhel?
and is the rhel rpm macro set for the centos platforms where ragel is not available?

My only concern is that %if 0%{?rhel} == 0 may be too broad for some rhel versions or not broad enough for some centos versions.

@wcohen
Copy link
Contributor Author

wcohen commented Aug 15, 2025

The only versions of ragel I see for RHEL are in EPEL and are limited to RHEL8/9. No ragel packages built for RHEL10. Having a RHEL/Centos stream package dependent on an EPEL package won't work very well. There was a separate pcp.spec file being used for the RHEL build and it excludes the ragel build dependency for pcp-pmda-statsd subpackage.

@kmcdonell
Copy link
Member

@wcohen and whatever the outcome, there may need to be matching changes to remove references to ragel in qa/admin/package-list files ... and quick inspection shows official RH packaging of ragel stopped after rhel6 (!), but
ragel is in EPEL 8 and EPEL 9 but not EPEL 10.

So it looks like your Centos 9 case needs an install from EPEL before building and your patch needs to be restricted to
0%{?rhel} > 9 ... this will work for RHEL 10 or later, and I'm guessing CentOS Stream 10 will also have the rhel rpm macro and the value 10.

@kmcdonell
Copy link
Member

I don't think this: Having a RHEL/Centos stream package dependent on an EPEL package won't work very well is necessarily correct for pcp.spec.in. For the distros, they are likely to be using internal tool chains and processes driven off the source rpm and may or may not use pcp.spec.in (RH uses redhat.spec for example). For everyone else (including all our QA outside RH) Makepkgs is used and using EPEL or other 3rd party packages as all OK here.

There is no one-size-fits all here (and the arguments are similar, but not the same for non-rpm builds), but I think this is a reasonable set of guidelines for some rpm package foo that we'd like to use:

  1. if the build will not work without foo then it should be in a Build-Requires clause of pcp.spec.in and in the qa/admin/package-lists for this platform
  2. if the build will work without foo (even if there is some functionality that is not included), then foo should be in the qa/admin/package-lists for this platform; in this case a minimal build will be without the functionality that foo enables, and a bells-and-whistles build will have used list-packages -m before Makepkgs and the functionality that foo enables will be included

Similar arguments apply to the Requires (instead of Build Requires) clause and QA (instead of build) (which is a superset of common use scenarios).

@natoscott
Copy link
Member

This one is also a bit of a special case - even without ragel the build works fine and all functionality is enabled.
Ragel is used only by the statsd PMDA, which we also need on RHEL, so the workaround was to have a checked-in version of its output that is used when ragel cannot be found.
Given that, in this special case I recommend we go with whatever the simplest solution is - as long as ragel is run on most platforms (i.e. without restricted packages available like RHEL), we should be fine.

@natoscott natoscott merged commit 78f260f into performancecopilot:main Aug 18, 2025
21 checks passed
@wcohen wcohen deleted the wcohen/centos_srpm branch August 18, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants