Skip to content

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Oct 7, 2025

Python 3.9 is already an old version which is still available on prominent distros and thus an explicit installation need not be a hard requirement. Also, python3dist(flake8) is not seen as a system dependency during the build process other than the tox environment for detecting linting errors.

Python 3.9 is already an old version which is still available on
prominent distros and thus an explicit installation need not be
a hard requirement. Also, `python3dist(flake8)` is not seen as
a system dependency during the build process other than the tox
environment for detecting linting errors.

Signed-off-by: Anoop C S <anoopcs@samba.org>
@phlogistonjohn
Copy link
Collaborator

Dropping flake8 from this list is fine, however the intent of python3.9 is not to pin to an older python but to enable tox to test multiple versions (essentially 3.9 - our oldest supported version; and whatever is the newest).

The issue is that this behavior works fine on fedora as intended and it works with centos 9 because that's the version that ships with centos 9. However, on el10 it fails. We may need to make this part conditional on the base distribution.

@phlogistonjohn
Copy link
Collaborator

It's a bit more complex but I was thinking something more along the lines of:

diff --git a/tests/container/build.sh b/tests/container/build.sh
index ddf3f0c..8d588a5 100755
--- a/tests/container/build.sh
+++ b/tests/container/build.sh
@@ -88,19 +88,19 @@ setup_update() {
 task_sys_deps() {
     info "installing system packages"
     OS_VER=$(source /etc/os-release && echo "${ID}-${VERSION_ID}")
+    local use_distro
     case "${OS_VER}" in
         centos*)
             info "detected centos (stream): ${OS_VER}"
-            use_centos=true
+            use_distro="el${VERSION_ID},centos"
         ;;
         rhel*)
             info "detected rhel: ${OS_VER}"
-            use_centos=
-            use_rhel=true
+            use_distro="el${VERSION_ID},rhel"
         ;;
         fedora*)
             info "detected fedora: ${OS_VER}"
-            use_centos=
+            use_distro=fedora
         ;;
         *)
             info "unknown platform: ${OS_VER}"
@@ -124,7 +124,6 @@ task_sys_deps() {
         python3.9 \
         samba-common-tools \
         rpm-build \
-        'python3dist(flake8)' \
         'python3dist(inotify-simple)' \
         'python3dist(mypy)' \
         'python3dist(pytest)' \
@@ -134,14 +133,20 @@ task_sys_deps() {
         'python3dist(wheel)' \
     )
 
-    if [ "$use_centos" ]; then
-        "${dnf_cmd}" install -y epel-release
-        yum_args=(--enablerepo=crb)
-        pkgs+=(pyproject-rpm-macros)
-    fi
-    if [ "$use_rhel" ]; then
-        pkgs+=(pyproject-rpm-macros)
-    fi
+    case "${use_distro}" in
+        fedora|el9,centos)
+            pkgs+=(python3.9)
+        ;;&
+        # common for all centos
+        *,centos)
+            "${dnf_cmd}" install -y epel-release
+            yum_args=(--enablerepo=crb)
+        ;;&
+        # common for all EL distros
+        el*)
+            pkgs+=(pyproject-rpm-macros)
+        ;;
+    esac
     "${dnf_cmd}" "${yum_args[@]}" install -y "${pkgs[@]}"
     "${dnf_cmd}" clean all
 }

The latter case tries to make the common parts really common, but I really don't care about using the fancy syntax, just the logic.

@phlogistonjohn
Copy link
Collaborator

@andrewschoen ping

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Oct 8, 2025

Closing in favour of #177.

@anoopcs9 anoopcs9 closed this Oct 8, 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.

2 participants