Introduce special handling of distro context#4895
Conversation
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
LecrisUT
added
status | blocked
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a custom TmtContext and a plugin-based distro context system to support advanced distribution comparisons for CentOS, Fedora, and RHEL. It migrates existing context usage to TmtContext, which now handles case-insensitivity by default. Feedback identifies a regex typo in the CentOS pattern and logic errors in CentOS and RHEL comparison methods that prevent correct cross-distro equality matching and cause asymmetric results.
| @provides_distro_context("centos") | ||
| @container | ||
| class CentosDistro(RhelLikeDistro): | ||
| _DISTRO_PATTERN = re.compile(r"^centos-?(?P<stream>stream)?-?(?P<version>[\d.]*])$") |
There was a problem hiding this comment.
The regular expression for CentOS distros contains a typo: a literal ] is included at the end of the version named group. This will cause the pattern to fail for standard CentOS names like centos-9, causing them to fall back to UnknownDistro and lose specific comparison logic.
| _DISTRO_PATTERN = re.compile(r"^centos-?(?P<stream>stream)?-?(?P<version>[\d.]*])$") | |
| _DISTRO_PATTERN = re.compile(r"^centos-?(?P<stream>stream)?-?(?P<version>[\d.]*)$") |
| # centos < rhel with higher version | ||
| if version_comp == -1: | ||
| return -1 | ||
| # centos > rhel with same version or lower | ||
| return 1 |
There was a problem hiding this comment.
The comparison logic returns 1 when versions are equal (version_comp == 0), which means centos-stream-10 == rhel-10 will evaluate to False. This contradicts the PR description which explicitly states that such cross-distro validations should be true.
| # centos < rhel with higher version | |
| if version_comp == -1: | |
| return -1 | |
| # centos > rhel with same version or lower | |
| return 1 | |
| # centos < rhel with higher version | |
| if version_comp == -1: | |
| return -1 | |
| # centos == rhel with same version | |
| if version_comp == 0: | |
| return 0 | |
| # centos > rhel with lower version | |
| return 1 |
| # rhel > centos with lower version | ||
| if version_comp == 1: | ||
| return 1 | ||
|
|
||
| # rhel < centos with same version or higher | ||
| return -1 |
There was a problem hiding this comment.
This logic returns -1 when versions are equal, meaning rhel-10 == centos-10 will be False. To align with the PR's goal of supporting cross-distro equality, it should return 0 when the versions match.
| # rhel > centos with lower version | |
| if version_comp == 1: | |
| return 1 | |
| # rhel < centos with same version or higher | |
| return -1 | |
| # rhel > centos with lower version | |
| if version_comp == 1: | |
| return 1 | |
| # rhel == centos with same version | |
| if version_comp == 0: | |
| return 0 | |
| # rhel < centos with higher version | |
| return -1 |
| assert other.version is None # narrow type | ||
|
|
||
| # rhel is only rhel | ||
| return other.family == "rhel" |
There was a problem hiding this comment.
The unversioned equality check is asymmetric: CentosDistro considers itself equal to rhel, but RhelDistro does not consider itself equal to centos. While this might reflect a one-way compatibility (CentOS as a superset of RHEL), it can lead to confusing results in context matching where == is expected to be symmetric.
| return other.family == "rhel" | |
| return other.family in ("rhel", "centos") |
2 participants
A special handling for
distrois needed because there are cross-distro validations that should be true likefedora-eln == centosorcentos-stream-10 == rhel-10.Depends-on #4894
Pull Request Checklist