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

Issues when package is installed in virtualenv #185

Closed
WhyNotHugo opened this issue Jul 24, 2021 · 21 comments
Closed

Issues when package is installed in virtualenv #185

WhyNotHugo opened this issue Jul 24, 2021 · 21 comments
Labels

Comments

@WhyNotHugo
Copy link

I'm having issues when the package I'm editing is present in my current virtualenv.

I run pip install -e . for my packages, since they're cli apps, and that's the easiest way to test them.

When the package I'm editing is installed in the current virtualenv, it's treated as third party instead of first party.

Any hints on how to work around this? Or have I come across a bug?

@asottile
Copy link
Owner

my guess is you've not properly set --application-directories -- though you haven't provided a full reproduction so I can only guess.

for what it's worth, it works correctly for me with editable installs

@WhyNotHugo
Copy link
Author

I'm not setting it. Can I configure that via setup.py / pyproject.yaml or something?

Isn't it possible to programmatically detect if the file being reordered is in pwd?

@asottile
Copy link
Owner

you can configure it via args: [...] or by passing it on the command line. you still haven't shown me your problem so I still can't help you -- please show a reproducible case so I can run it and guide you to a solution instead of having to guess at the problem.

@WhyNotHugo
Copy link
Author

It happens on this repo:

https://github.com/pimutils/vdirsyncer

Run reorder-python-imports vdirsyncer/storage/dav.py. It moves vdirsyncer into third-party. Oddly, the pre-commit hook works as intended.

@asottile
Copy link
Owner

does not reproduce:

$ virtualenv venv
created virtual environment CPython3.8.10.final.0-64 in 275ms
  creator CPython3Posix(dest=/tmp/z/vdirsyncer/venv, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/asottile/.local/share/virtualenv)
    added seed packages: pip==21.1.3, setuptools==57.1.0, wheel==0.36.2
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
$ . venv/bin/activate
$ pip install -e .
Obtaining file:///tmp/z/vdirsyncer
Collecting click<9.0,>=5.0
...
$ reorder-python-imports vdirsyncer/storage/dav.py 
$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

@WhyNotHugo
Copy link
Author

I realised I was on reorder-python-imports 2.5.0, but upgraded to 2.6.0 and it's still an issue.

vdirsyncer is also installed system-side (e.g.: via my distribution's package manager). Do you think that could be related?

@WhyNotHugo
Copy link
Author

WhyNotHugo commented Jul 25, 2021

So it seems to happen regardless of whether the virtualenv is active or not actually.

The only think I see unusual about this package is that it's installed on on my system python too (since it's a cli tool I actually use).

Reproduction:

$ git clone git@github.com:pimutils/vdirsyncer.git
Cloning into 'vdirsyncer'...
Confirm user presence for key ED25519-SK SHA256:pXQ62ghWiAKayDFMRM+CmmdwyOOrdVyuUfIJyPTwLj8
User presence confirmed
remote: Enumerating objects: 13130, done.
remote: Counting objects: 100% (900/900), done.
remote: Compressing objects: 100% (443/443), done.
remote: Total 13130 (delta 497), reused 790 (delta 437), pack-reused 12230
Receiving objects: 100% (13130/13130), 2.95 MiB | 2.74 MiB/s, done.
Resolving deltas: 100% (9202/9202), done.

$ cd vdirsyncer

$ reorder-python-imports vdirsyncer/storage/dav.py
Reordering imports in vdirsyncer/storage/dav.py

$ git diff
diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py
index 1743265..eb537cd 100644
--- a/vdirsyncer/storage/dav.py
+++ b/vdirsyncer/storage/dav.py
@@ -7,6 +7,7 @@ from inspect import signature

 import aiohttp
 import aiostream
+from vdirsyncer.exceptions import Error

 from .. import exceptions
 from .. import http
@@ -18,7 +19,6 @@ from ..http import USERAGENT
 from ..vobject import Item
 from .base import normalize_meta_value
 from .base import Storage
-from vdirsyncer.exceptions import Error


 dav_logger = logging.getLogger(__name__)

$ which reorder-python-imports
/usr/bin/reorder-python-imports

$ pip list | grep reorder
reorder-python-imports        2.6.0

@WhyNotHugo
Copy link
Author

WhyNotHugo commented Jul 25, 2021

Here's a better reproduction example. Since it requires installing something system-wide, I've used docker to isolate this from the host OS:

docker run --rm -it python:3.9 bash

Inside the container run:

pip install reorder-python-imports vdirsyncer  # Note: runs as root, so installs system-wide.
git clone https://github.com/pimutils/vdirsyncer.git
cd vdirsyncer
reorder-python-imports vdirsyncer/storage/dav.py
git diff

(In my case vdirsyncer is installed via my distro's package manager, but sudo pip install is close enough and reproduces the issue well enough).

@asottile
Copy link
Owner

yeah that makes sense then, you have a global third party installation which will always be discovered first (I would not recommend using global installs at all)

next time when making an issue to save us all time please start with your reproduction, it would have saved us all hours of time

@WhyNotHugo
Copy link
Author

I would not recommend using global installs at all

For applications that are part of the desktop (or even part of the OS itself), this is unavoidable. I'm not using sudo pip install; I'm installing it via my OS's package manager. I can't really imagine any reasonable workaround.

Did you mean to close this issue intentionally? Is there any workaround for this?

I think it might make sense to detect if the file being edited is the same one that's in the global third party installation, or would that end up being too slow?

next time when making an issue to save us all time please start with your reproduction, it would have saved us all hours of time

My bad, I mistakenly assumed the cause was elsewhere, sorry about that!

@asottile
Copy link
Owner

yes, don't use global installs -- the output is correct

@WhyNotHugo
Copy link
Author

WhyNotHugo commented Jul 25, 2021

yes, don't use global installs

I don't really understand what you're trying to propose here. If I remove all global installs, my system won't boot to my desktop any more, aside from all sorts of things breaking pretty badly.

I have vdirsyncer installed because I use it on my desktop. I also have 4 other packages installed that depend on vdirsyncer (and not all of them are Python packages either). Trying to remove it would result in all dependant packages being removed too, and I rely on them daily.

vdirsyncer is only one example of codebases on which I'm having this issue.

Uninstalling it from my system isn't an option. I'm a maintainer and a user of the same product, this can't be such a crazy scenario. Is this issue really a WONTFIX?

@asottile
Copy link
Owner

take it up with your operating system packager of reorder-python-imports then -- it's working as intended from my point of view

@WhyNotHugo
Copy link
Author

take it up with your operating system packager of reorder-python-imports then

That would be me.

it's working as intended from my point of view

Let me give another example: suppose you want to use reorder-python-imports to sort import of dnf, a package manager, and you're running Fedora (this isn't a crazy idea either: I can imagine Fedora's developers actually use Fedora).

dnf is part of the base OS. Removing it is not possible, and if you did manage to remove it, your system would be broken and require a reinstall (or some serious surgery to get it working again).

Simply "not installing" a package on one's system is not an option, especially since the package may be part of the system.

it's working as intended from my point of view

Can the "intended behaviour" be discussed so as to accommodate this scenario? I don't think I'm proposing something unreasonable here.

@asottile
Copy link
Owner

you're not understanding -- I'm not asking you to remove dnf, I'm asking you to use reorder-python-imports from your virtualenv

@WhyNotHugo
Copy link
Author

I was misunderstanding actually. 👍

So installing reorder-python-imports itself as a global install is what's unsupported? How would you advise a system package to install this? Using a dedicated virtualenv?

@asottile
Copy link
Owner

it's not that it's unsupported -- it's that when you've got a global package it's going to be identified as such:

>>> import ufw
>>> ufw.__file__
'/usr/lib/python3/dist-packages/ufw/__init__.py'
>>> from aspy.refactor_imports.classify import classify_import
>>> classify_import('ufw')
'THIRD_PARTY'

as aside: I don't recommend operating systems to try to package my things because they often don't understand the software that they are packaging and waste copious amounts of my time on problems that exist between their keyboard and their chair.

@WhyNotHugo
Copy link
Author

I've been asking politely, and I'd appreciate if you can abstain from insults.

it's not that it's unsupported -- it's that when you've got a global package it's going to be identified as such

I don't think I understand this bit. If installed globally it gives erroneous results, but it's not unsupported? I was thinking it'd be safer to add a note in the README stating that global installs are discouraged given they may result in misidentifying packages.

I don't recommend operating systems to try to package my things

Packaging tools is what allows us developers to install them. How do you envision this should happen? For example, I install reorder_python_imports since I've nvim configured to use it under the hood to reorder imports.

@asottile
Copy link
Owner

first off, the insult is not intended towards you -- there's even lower quality reports that are frequent from a certain few packagers. and when people fork / patch / install in weird layouts / etc. and then report bugs because of their mistakes it's a huge waste of time for everyone involved -- install it from pip using pip avoids the entire class of problems

I guess I'm just frustrated with this issue because you started with a very low quality vague "it's not working" with no reproduction report. it then took me three rounds of asking for a reproduction for you to tell me that your original report was factually incorrect and that you're instead globally installing everything!

I avoid the system python like the plague, especially for reasons similar to this one. it's way too easy for packages to have bad interactions with each other or for the operating system packager to make a poor decision about how things are packaged. it's fairly easy to set up a virtualenv and install tools there -- that's how I avoid this mess entirely

the results are not erroneous, it is installed as a third party package and presented to the import system as a third party package so it is identified as such

@WhyNotHugo
Copy link
Author

Regarding packagers shipping forks, and alike, I can, regrettably, relate to that. vdirsyncer itself has a history of distributions shipping a fork of an old version, and users continuously having issues with "the latest version". That's also one of the biggest reasons for the former maintainer to step down.

OHOT, I do apologise about the lack of repro steps. It happens to me on so many projects that I had a hard time pinpointing down all the relevant factors.


Oh the topic of system packages: I guess choice of distribution is key on this item. Arch has a strong policy of following upstream defaults and not applying patches, so the experiences tends to be a bit different. Versions are updated pretty fast, but also made sure to work with one another.

I'm really not sure how I'd deal with installing this without it being a system package. For one, when I install a new system (physical or not), I merely re-install the same system packages, dotfiles and that's it. It seems that I'd now require an extra step for one single package: reorder_python_imports, but I'm also unsure what those steps should be.

Installing it in a dedicated virtualenv suddenly puts the burden of manually updating it on each system, which really doesn't scale. It'd also break every time there's a major python update. Not a huge deal, but I'd rather not have such a snowflake requiring such attention.

How do you install reorder-python-imports?

@asottile
Copy link
Owner

I don't, I only use it through pre-commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants