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

use sys.executable #68

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

use sys.executable #68

wants to merge 7 commits into from

Conversation

br-olf
Copy link

@br-olf br-olf commented Mar 14, 2023

Fixed nasty bug

@br-olf
Copy link
Author

br-olf commented Mar 14, 2023

How to add PR labels?

@ssbarnea ssbarnea added the bug Something isn't working label Mar 14, 2023
system_python = "/usr/bin/python%s" % ".".join(
[str(item) for item in platform.python_version_tuple()[0:2]]
)
system_python = sys.executable
Copy link
Member

@ssbarnea ssbarnea Mar 14, 2023

Choose a reason for hiding this comment

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

This will break badly as it will pick a local python when run from within a virtualenv, and it fail to discover the system python, which is likely the only one that might have the _selinux binary extension installed.

Copy link
Author

Choose a reason for hiding this comment

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

I wrote a more elaborate fix, is this one acceptable?

@br-olf
Copy link
Author

br-olf commented Mar 15, 2023

I rewrote my PR to look for selinux site-package in all provided places and it should now be easy to add further places to look into.
Please review the PR now.

@br-olf
Copy link
Author

br-olf commented Mar 15, 2023

BTW, this now also adds support for virtualenvs

@br-olf
Copy link
Author

br-olf commented Mar 28, 2023

@ssbarnea Do you have any plans of merging this?

@fbicknel
Copy link

This (gh pr checkout 68) fixed my problem using the selinux package with pyenv with a virtualenv. I vote for merge.

@br-olf br-olf requested a review from ssbarnea December 1, 2023 14:06
@gonzalemario
Copy link

Hi. I think this is working because it's reloading on itself and the check for is_selinux_enabled() is always the local one, not from the original library. I tested this on a selinux enabled system and I always receive a '0', which it's the return call from the local method instead.

If I'm understanding correctly, this shim library should reload using selinux bindings if they are installed. If this is correct, I understand why we cannot merge this.

Please correct me if I'm wrong.

ps: I also looking for a solution to this problem. I can help if you guys let me.

@gonzalemario
Copy link

ansible now from version 8, loads selinux differently. Basically it's using ctypes to get the .so

I might understand why the main developers haven't payed attention to this PR

@cinnion
Copy link

cinnion commented Oct 15, 2024

Can someone please get this reviewed and merged?!! #17 not only fixed virtualenv setups, but broke more standard installs such as those done from source, which install under /usr/local/bin by default (and might even break RHEL SCL installs which would install under /opt/rh... I have just started using those, so have not checked). See my comment on #69 for more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants