pypa / pipenv

Python Development Workflow for Humans.
https://pipenv.pypa.io
MIT License
24.78k stars 1.86k forks source link

Major pipenv uninstall --all behavior change #6200

Open dan-hook opened 1 month ago

dan-hook commented 1 month ago

Issue description

Running pipenv uninstall --all (the command) with the latest version of pipenv uninstalls all packages in the [packages] group and removes them from the Pipfile. It doesn't uninstall packages from any other group.

Expected result

I had been relying on the behavior last seen in Release v2023.12.1, where running the command produced output like:

Un-installing all [dev-packages] and [packages]...
Found 47 installed package(s), purging...
Found existing installation: six 1.16.0
Uninstalling six-1.16.0:
  Successfully uninstalled six-1.16.0
Found existing installation: Jinja2 3.1.4
Uninstalling Jinja2-3.1.4:
  Successfully uninstalled Jinja2-3.1.4
Found existing installation: cryptography 42.0.8
Uninstalling cryptography-42.0.8:
  Successfully uninstalled cryptography-42.0.8
Found existing installation: packaging 24.1
...
Environment now purged and fresh!

There's a minor bug in that output in that it only lists that it's uninstalling from the [dev-packages] and [packages] group, but that's just a display issue. It does actually uninstall all packages in all groups from the virtual environment and leaves the Pipfile untouched. This is consistent with the documentation.

Actual result

Un-installing all [packages]...
Uninstalling src...
Found existing installation: src 1.0
Uninstalling src-1.0:
  Successfully uninstalled src-1.0

For this project, the [packages] group only contains one package, src-1.0. All the other packages are in either [dev-packages] or [layer-packages]. The current version of pipenv only uninstalls packages from the [packages] group and removes them from the Pipfile as well.

Steps to replicate

Create a Pipfile with several groups:

[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

# For local development, run
# pipenv install --categories="packages layer-packages dev-packages"

[packages]
src = {editable = true, path = "./src"}

[requires]
python_version = "3.11"

[dev-packages]
moto = ">=5.0.0"
pytest = "*"
pytest-monitor = "*"
mypy = "*"
boto3-stubs = "*"
pystache = "*"
types-requests = "*"
pytest-mock = "*"

[layer-packages]
boto3 = "*"
urllib3 = "*"
botocore = "*"
appsync-client = {path = "./../../../../../packages/appsync_client"}
pandas = "2.2.2"
openpyxl = "3.1.5"
aws-lambda-powertools = "*"

Install all of the packages:

pipenv install --categories="packages layer-packages dev-packages"

Now run the command:

pipenv uninstall --all

Examine the Pipfile, and see that the [packages] group is empty, but the other groups are still populated. Examine the site-packages in the virtualenv and note that the all group packages are still there.

Notes on the code

It appears that the do_purge function is now an orphan. There's no active code that will display Environment now purged and fresh!.

dan-hook commented 1 month ago

My best guess is that this bug was introduced in this PR.

matteius commented 1 month ago

Bummer I missed this -- if someone wants to take a shot at fixing it, I would be super appreciative. I head to SF for work this weekend and won't be able to look into it for at least a week.

matteius commented 4 weeks ago

@dan-hook could you take a look at https://github.com/pypa/pipenv/pull/6209 and help me determine if this is the right behavioral fix.

matteius commented 3 days ago

I am confused on one point -- it should leave the entries in the Pipfile right when using --all or --all-dev? It seems there is a really old test assertion for --all-dev that checks it removes everything from the Pipfile dev, which feels weird and like a difference between what --all and --all-dev was supposed to do. I am thinking both flags should not clear Pipfile entires for that category.

dan-hook commented 2 days ago

I've never used --all-dev, but I recall seeing the same strange behavior in the docs. Since a change in --all behavior broke my workflow, I'd be sympathetic to anyone who had come to rely on the behavior of --all-dev. If the current --all-dev behavior is useful, it might be worth considering adding a -c, --categories option to uninstall, since uninstall --categories dev seems like it should remove all packages from [dev-packages] in the Pipfile.

matteius commented 20 hours ago

I'd be sympathetic to anyone who had come to rely on the behavior of --all-dev

Yeah me too, I restored that part of the PR -- that is the super hard thing about inheriting mainteance of this project, has been how undocumented and not always logical all the use cases have been and evolved. Something I hope to improve eventually is the documentation.