Skip to content
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

SSSDConfig: use 'setuptools' instead of 'distutils' #6581

Closed
wants to merge 2 commits into from

Conversation

alexey-tikhonov
Copy link
Member

The Python standard library distutils module will be removed from Python 3.12+

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Feb 18, 2023
@alexey-tikhonov
Copy link
Member Author

Simple replacement with setuptools doesn't work for centos8.
Maybe we can just postpone this till centos8 will be backed up by LTS branch only.

@alexey-tikhonov
Copy link
Member Author

@pbrezina, let's remove centos-8 from CI targets for 'master' branch?

@alexey-tikhonov alexey-tikhonov marked this pull request as ready for review September 8, 2023 10:12
@pbrezina
Copy link
Member

pbrezina commented Sep 8, 2023

@pbrezina, let's remove centos-8 from CI targets for 'master' branch?

We can, but... besides this PR, there is nothing that prevents us running tests against c8 and we will still have lots of backports to 2.9. Currently we only run CI on push event for older branches since we do not open pull requests if there is no conflict. I wonder if it would be better to keep running c8 or start require pull requests for backports (can be automated if there is no conflict).

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Sep 8, 2023

@pbrezina, let's remove centos-8 from CI targets for 'master' branch?

We can, but... besides this PR, there is nothing that prevents us running tests against c8 and we will still have lots of backports to 2.9. Currently we only run CI on push event

Valid point...
Can we condition targets based on backport-to/no-backport labels?

Going forward, we might have more examples where 2.10+ patches won't work on centos-8.

@pbrezina
Copy link
Member

pbrezina commented Sep 8, 2023

@pbrezina, let's remove centos-8 from CI targets for 'master' branch?

We can, but... besides this PR, there is nothing that prevents us running tests against c8 and we will still have lots of backports to 2.9. Currently we only run CI on push event

Valid point... Can we condition targets based on backport-to/no-backport labels?

Going forward, we might have more examples where 2.10+ patches won't work on centos-8.

It won't work, unless it actually uses sssd-2-9 branch to run the test. In other words, we need to move this way, but it is going to be way more complex. We want to have one pull request and if it has backport label then check if there is no conflict and then run PR CI on target distros with backported code.

Or do it (maybe) simple by opening pull request with the backport. Which can be automatically merged, if all checks pass, without review, I think. It's unknown waters. But this is the way that projects on github usually go.

@alexey-tikhonov
Copy link
Member Author

Fail is centos8 specific but manifests only during 'make intgcheck':

Checking .pth file support in /tmp/sssd-intg.bs6XRVwA/lib/python2.7/site-packages/
/usr/bin/python2 -E -c pass
TEST FAILED: /tmp/sssd-intg.bs6XRVwA/lib/python2.7/site-packages/ does NOT support .pth files
error: bad install directory or PYTHONPATH

You are attempting to install a package to a directory that is not
on PYTHONPATH and which Python does not read ".pth" files from.  The
installation directory you specified (via --install-dir, --prefix, or
the distutils default setting) was:

    /tmp/sssd-intg.bs6XRVwA/lib/python2.7/site-packages/

and your PYTHONPATH environment variable currently contains:

    ''

Here are some of your options for correcting the problem:

* You can choose a different installation directory, i.e., one that is
  on PYTHONPATH or supports .pth files

* You can add the installation directory to the PYTHONPATH environment
  variable.  (It must then also be on PYTHONPATH whenever you run
  Python and want to use the package(s) you are installing.)

* You can set up the installation directory to support ".pth" files by
  using one of the approaches described here:

  https://setuptools.readthedocs.io/en/latest/easy_install.html#custom-installation-locations


Please make the appropriate changes for your system and try again.
make[6]: *** [Makefile:47590: install-exec-hook] Error 1

The Python standard library distutils module will be removed from Python 3.12+
@pbrezina
Copy link
Member

I took the liberty to rebase this and added a patch that fixes the centos8 issue, at least locally. Let's see if CI will pass.

@alexey-tikhonov
Copy link
Member Author

I took the liberty to rebase this and added a patch that fixes the centos8 issue, at least locally. Let's see if CI will pass.

I tried something similar (9c65997 and other attempts).
Hope you'll have a better luck.

@pbrezina pbrezina force-pushed the setuptools branch 3 times, most recently from fdb27e1 to 01dc27d Compare September 14, 2023 09:25
@pbrezina pbrezina force-pushed the setuptools branch 2 times, most recently from 15faa56 to 01f5ccb Compare September 14, 2023 13:03
@pbrezina
Copy link
Member

@alexey-tikhonov It's green now. I'm not sure if it is worth it though.

@alexey-tikhonov
Copy link
Member Author

I'm not sure if it is worth it though.

Maybe as a way to postpone solving proper target platform selection...

Copy link
Contributor

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

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

Ack, thank you.

@alexey-tikhonov
Copy link
Member Author

ACK

@alexey-tikhonov
Copy link
Member Author

Pushed PR: #6581

  • master
    • 61bf109 - SSSDConfig: set PYTHONPATH to make setuptools work on centos8
    • 9efd79b - SSSDConfig: use 'setuptools' instead of 'distutils'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only. Pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants