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

Remove site file usage #2165

Closed
jaraco opened this issue May 29, 2020 · 8 comments · Fixed by #2166
Closed

Remove site file usage #2165

jaraco opened this issue May 29, 2020 · 8 comments · Fixed by #2166

Comments

@jaraco
Copy link
Member

jaraco commented May 29, 2020

In #1998 and pypa/virtualenv#1638 and other places, we learned that:

  • virtualenv will not be superseded by venv.
  • virtualenv 20 still (and more aggressively) uses a site.py file to hack into Python.
  • As a result, virtualenv is the only package allowed to use site.py and setuptools needs to drop support for eggs on PYTHONPATH taking precedence over other packages on sys.path or any other features that rely on site.py.

@gaborbernat Does that sound about right?

@gaborbernat
Copy link
Contributor

gaborbernat commented May 29, 2020

Sounds about right for python 2. On python 3 virtualenv does not use site.py at all. I'd say site.py is reserved for OS usage though, consider using the site customize file which was designed for such usage.

@jaraco
Copy link
Member Author

jaraco commented May 29, 2020

I see. That's a little more awkward then. That means that setuptools on Python 2.7 is incompatible with virtualenv 20. So users are kinda screwed anyway, since setuptools doesn't declare a dependency on virtualenv and can't really declare an incompatibility.

That means the only way to support virtualenv 20 is to remove this functionality from setuptools 44.x. I'm a little reluctant to do that as it doesn't give a lot of wiggle room to navigate between Python 2 compatibility and virtualenv compatibility. That is, if setuptools removes this functionality in setuptools 44.2, 44.1.1 becomes the last supported version with this functionality (for Python 2). I guess that's okay if no one misses it.

Okay, I think I have a plan:

  • Remove this functionality on Setuptools N+1 (Python 3 only).
  • Work through any issues reported by users.
  • Assuming all issues are relatively workable, backport the removal to Setuptools 44.x.

jaraco added a commit that referenced this issue May 29, 2020
@jaraco jaraco mentioned this issue May 29, 2020
2 tasks
commodo added a commit to commodo/packages that referenced this issue Oct 13, 2020
Drop patch 016-adjust-config-paths.patch
Implemented this differently; it was kind of on my todo-list, since this
patch is a workaround for not dealing with PLATFORM_TRIPLET on OpenWrt.

Refreshed other patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 13, 2020
Drop patch 016-adjust-config-paths.patch
Implemented this differently; it was kind of on my todo-list, since this
patch is a workaround for not dealing with PLATFORM_TRIPLET on OpenWrt.

Refreshed other patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 13, 2020
Drop patch 016-adjust-config-paths.patch
Using PYTHON_FOR_BUILD variable for that.

Refreshed other patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 13, 2020
Drop patch 016-adjust-config-paths.patch
Using PYTHON_FOR_BUILD variable for that.

Refreshed other patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 19, 2020
Refreshed patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 19, 2020
Refreshed patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
commodo added a commit to commodo/packages that referenced this issue Oct 19, 2020
Refreshed patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
jmarcet pushed a commit to jmarcet/packages that referenced this issue Oct 19, 2020
Refreshed patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
@asavah
Copy link

asavah commented Oct 21, 2020

This change should be reverted as it will affect multiple users used to "export PYTHONPATH" - "it works!" magic without the need of sitecustomize.py and more complex hacks.
Just wait until distros get released with setuptools 49.x+ ...

pprindeville pushed a commit to pprindeville/packages that referenced this issue Dec 19, 2020
Refreshed patches.

Dropped 'patches-setuptools/004-site-patch.patch'
Does not apply anymore. Setuptools has removed site.py support:
   pypa/setuptools#2165
If this is still needed, we may need to re-think it's implementation.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
cript0nauta added a commit to cript0nauta/pynixify that referenced this issue Jan 5, 2021
Since setuptools v49, acceptance tests were failing to import pynixify.
After a long session of debugging, the problem turned out to be that I
needed to add a __init__.py file.

Root cause of the regression: pypa/setuptools#2165
cript0nauta added a commit to cript0nauta/pynixify that referenced this issue Jan 5, 2021
Since setuptools v49, acceptance tests were failing to import pynixify.
After a long session of debugging, the problem turned out to be that I
needed to add a __init__.py file.

Root cause of the regression: pypa/setuptools#2165
@cript0nauta
Copy link

cript0nauta commented Mar 2, 2021

This change broke development environments using nix-shell, which offers a feature similar to running setup.py develop. If I manually copy the site.py generated by older versions of setuptools into the new environment I can fix it, but this is clearly a hack.

I'm not sure how to proceed to properly fix this issue when using nix-shell. @jaraco would you be able to help recommending workarounds to solve this problem?

@jaraco
Copy link
Member Author

jaraco commented Mar 3, 2021

For sure. Can you help me isolate the issue? I don't know enough about nix to give any meaningful advice.

I found with this Dockerfile, inspired downstream issue, I can replicate the failure:

from nixos/nix
RUN nix-env --install git
RUN git clone https://github.com/pypa/sampleproject
WORKDIR sampleproject
RUN echo $'with import <nixpkgs> {};\nwith python38Packages;\n\nbuildPythonPackage rec {\n  name = "sampleproject";\n  src = ./.;\n  propagatedBuildInputs = [ peppercorn ];\n}' > shell.nix
CMD nix-shell --command 'sample'

(output)

I'm not sure how nix works or how to inspect what it's doing. Is it possible you could distill some commands that I could run outside of nixos that have the same effect as what nix is doing (but in another Unix environment)?

It does look to me like pip install is being used to invoke setup.py develop, so perhaps pypa/pip#9636 and the subsequent planned fix for Setuptools will address the issue.

@cript0nauta
Copy link

cript0nauta commented Mar 3, 2021

In order to isolate the problem and replicate it without using Nix, i built the following shellscript to be run inside the sampleproject directory:

# Uncomment to make the command succeed
# pip install 'setuptools<49'
# 
tmp_path=$(mktemp -d)
export PATH="$tmp_path/bin:$PATH"
export PYTHONPATH="$tmp_path/lib/python3.9/site-packages:$PYTHONPATH"
mkdir -p "$tmp_path/lib/python3.9/site-packages"
eval "python3 -m pip install -e . --prefix $tmp_path --no-build-isolation >&2"
sample

I ran this code inside a standard Python docker container. When using older versions of setuptools, sample runs successfully. When using newer versions, it fails to find the sampleproject module.

Note: The above script is based in the code used by nix-shell to build the environment: https://github.com/NixOS/nixpkgs/blob/1e2b6695cf8c71b3a0605bf0edd53d8d19d4545e/pkgs/development/interpreters/python/hooks/setuptools-build-hook.sh#L27.

@limeytexan
Copy link

limeytexan commented Mar 17, 2021

@jaraco I was wondering if you had any thoughts on the above, or an estimate for when you might be able to look into it? I have a team of Nix+Python developers who are anxious to have a working development shell again. Most importantly, it would be good to know one way or another whether you expect the required functionality to be restored because if not we'll need to go back to the drawing board to look for a different solution on the Nix side. Many thanks.

@jaraco
Copy link
Member Author

jaraco commented Mar 20, 2021

I regret I haven't gotten to this yet. I believe this deserves a new issue.

I'd like to come up with a solution other than to revert that old and crufty site.py hack. Let's identify the root expectation that's missed and then devise a solution based on today's packaging landscape.

jaraco added a commit that referenced this issue Mar 21, 2021
kdesysadmin pushed a commit to KDE/krita that referenced this issue Jun 17, 2021
Since setuptools 49.0.0 [1], their easy_install command no longer installs
site.py to PYTHONPATH directories. This didn't hit me (or anyone using,
as per 3rdparty, stock Python 3.8.1) because that version ships
Setuptools 41; but anyone using v49 onwards would've run on importlib
errors when trying to use sip-build or any of our 3rdparty tools.
This bug can be easily triggered by making a virtual environment using
virtualenv and then executing build.cmd inside it.

This commit reuses Iván's sitecustomize.py for macOS and adds
the entry suggested in [2] to inject our directory.

While at it, let's ensure all PYTHONPATH entries are sanitized and
stored in KRITA_PYTHONPATH, not just ours.

[1]: https://setuptools.readthedocs.io/en/latest/history.html#v49-0-0

[2]: pypa/setuptools#2165

CCMAIL: kimageshop@kde.org
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 a pull request may close this issue.

5 participants