Skip to content

Add Almalinux 9.5 as a supported toolchain version#940

Closed
nicbadiu wants to merge 2 commits intobazelbuild:mainfrom
nicbadiu:patch-1
Closed

Add Almalinux 9.5 as a supported toolchain version#940
nicbadiu wants to merge 2 commits intobazelbuild:mainfrom
nicbadiu:patch-1

Conversation

@nicbadiu
Copy link
Contributor

No description provided.

@nicbadiu nicbadiu marked this pull request as ready for review February 24, 2025 10:32
]

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

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

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

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

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

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

@nicbadiu nicbadiu closed this Feb 26, 2025
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

Comments