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

Do not add user site-packages directory to sys.path (RHEL-26646) #2074

Conversation

jrohel
Copy link
Contributor

@jrohel jrohel commented Apr 3, 2024

Adds -s to shebang lines.
The -s flag ensures that the user’s Python packages (e.g. installed by pip install --user) don’t interfere with the RPM installed software.
According to Fedora Python Packaging Guidelines, the flag is added using %py3_shebang_fix macro.

Note:
DNF supports plugins. There is a risk that the change will break a custom plugins that require something from PIP.
Therefore, the change is only in the .spec file and is only allowed for Fedora >= 41 and RHEL >= 10.

@jrohel jrohel force-pushed the fix/dont_add_usr_local_python_modules branch 2 times, most recently from 8aa5298 to ab22cb9 Compare April 4, 2024 06:20
@ppisar ppisar self-assigned this Apr 4, 2024
dnf.spec Show resolved Hide resolved
@ppisar
Copy link
Contributor

ppisar commented Apr 4, 2024

Adds -s to shebang lines. The -s flag ensures that the user’s Python packages (e.g. installed by pip install --user, or just placed in the current directory)

Current directory is controlled by -P option. Not -s option. Remove these words from the commit message.

@jrohel
Copy link
Contributor Author

jrohel commented Apr 4, 2024

@ppisar

Current directory is controlled by -P option.

Probably not just the current directory. Python Packaging Guidelines https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_shebangs states (I'll comment the code):

# Don't add -P to Python shebangs
# The executable Python scripts in /usr/share/opt-viewer/ import each other
%undefine _py3_shebang_P

Anyway, I copied the text from the Python Packaging Guidelines and they probably have a mistake in the description (I found another error there already yesterday).
I delete or just placed in the current directory from the text. Thanks for the notice.

Adds `-s` to shebang lines.
The `-s` flag ensures that the user’s Python packages (e.g. installed by
pip install --user) don’t interfere with the RPM installed software.
According to Fedora Python Packaging Guidelines, the flag is added using
`%py3_shebang_fix` macro.

Note:
DNF supports plugins. There is a risk that the change will break
a custom plugins that require something from PIP.
Therefore, the change is only in the .spec file and is only allowed
for Fedora >= 41 and RHEL >= 10.
@jrohel jrohel force-pushed the fix/dont_add_usr_local_python_modules branch from ab22cb9 to 0a90976 Compare April 4, 2024 09:40
Copy link
Contributor

@ppisar ppisar left a comment

Choose a reason for hiding this comment

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

Good. I verified it by building on Fedora 41 and inspecting sys.path Python object.

@ppisar ppisar merged commit caa17e3 into rpm-software-management:master Apr 4, 2024
7 of 11 checks passed
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