Skip to content

Add Almalinux 9.5 as a supported toolchain version #940

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.

Sign up for GitHub

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

Closed
nicbadiu wants to merge 2 commits into from
Closed

Add Almalinux 9.5 as a supported toolchain version #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbfac1e
Add Almalinux 9.5 as a supported toolchain version
nicbadiu Feb 24, 2025
3cb6dc4
Add Almalinux 9.5 as a supported `MakeRpm` version
nicbadiu Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/make_rpm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,10 @@ def CallRpmBuild(self, dirname, rpmbuild_args, debuginfo_type):
'--define', '_builddir %s/BUILD' % dirname,
]

if debuginfo_type in ["fedora40", "centos7", "centos9", "almalinux9.3"]:
if debuginfo_type in ["fedora40", "centos7", "centos9", "almalinux9.3", "almalinux9.5"]:
Copy link
Collaborator

@aiuto aiuto Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be unmaintainable at some point.
Let's fix this rather than add to the clutter.

How about you change the configure to add a new property besides debuginfo_type. Something like

OR.... why do we even care about the type. It looks like we always want to undefine debugsrc_pkgs if we have set debuginfo_type. So perhaps this can just reduce to

if debuginfo_type: 
  args += ...

Copy link
Contributor

@kellyma2 kellyma2 Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few nichy behaviour that this controls. That said, from what we've observed so far it looks like there's two variants (fedora (40?) vs alma, centos, etc). Originally when I wrote this I was concerned that there might be more variation, but it increasingly feels like this can be simplified down to this until we get any indications otherwise.

Thoughts?

Copy link
Contributor Author

@nicbadiu nicbadiu Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is definitely the 'fix' for this behavior - and is based on empirical testing for Almalinux 9.5

We are using a forked version so there is no urgency to have this merged upstream.

Perhaps we should first clarify what problems this is trying to address and pause this PR and cancel it once we agree on the way to handle the problem at hand in a more future proof manner.

Copy link
Collaborator

@aiuto aiuto Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will defer the opinion about what is right to you two. I don't build enough RPMs to on enough platforms to have anything useful to say. Other than the desired to have conditional logic based on the property of something rather than the name of the thing.

Copy link
Contributor Author

@nicbadiu nicbadiu Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need to separate is only in terms of Fedora vs CentOS as agreed offline.

Superseding this PR with #942

args += ['--undefine', '_debugsource_packages']

if debuginfo_type in ["centos7", "centos9", "almalinux9.3"]:
if debuginfo_type in ["centos7", "centos9", "almalinux9.3", "almalinux9.5"]:
args += ['--define', 'buildsubdir .']

if debuginfo_type == "fedora40":
Expand Down
2 changes: 1 addition & 1 deletion toolchains/rpm/rpmbuild_configure.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def _parse_release_info(release_info):
return os_name, os_version

KNOWN_DEBUGINFO_VERSIONS = {
"almalinux": ["9.3"],
"almalinux": ["9.3", "9.5"],
"centos": ["7", "9"],
"fedora": ["40"],
}
Expand Down