pip3 freeze output still confusing when stray AdjacentTempDirectorys are present #7269
Comments
triage-new-issues
bot
added
the
S: needs triage
|
I'm having the same issue. |
|
I think the right approach is to not pollute the The |
triage-new-issues
bot
removed
the
S: needs triage
|
I have #9393 ready if we want to skip directories prefaced with non-alphanumeric characters (the I really think ignoring temp directories (folders prefixed with |
|
As well as avoiding the long paths, it also makes uninstall/rollback significantly faster and more reliable. The fact that it still occasionally fails means that it would be failing anyway, but instead of junk in the directory you'd have a totally failed update/remove. Ignoring the directories in freeze seems fine to me, but the should probably be skipped by whichever function/library thinks that they are importable when they are clearly not. (Maybe also replacing ".dist-info" in the temporary name will help avoid it showing up in the scan?) Having pip attempt cleanup again on future runs would help keep things tidy, but really, I think that's a level of complexity that isn't necessary. "Do not show invalid packages in pip freeze" seems like a good generic name for this issue, as presumably there are other ways to get into this state besides having an uninstall fail. |
|
Hmm. I don't know the history of why these folders got introduced, so maybe I'm missing something, but I have some questions:
Okay - but why is the junk preferable to that? Given the choice between: A. some operation fails and B. the operation fails in exactly the same way, but also leaves a folder of undocumented junk behind why would I want option B? Even if the junk didn't confuse tools, it'd still be confusing to a user trying to make sense of the content of their
Relative to what alternative, and how so? |
|
You get option C, which is "the operation succeeds, but also leaves a folder with undocumented name containing the files that weren't able to be deleted". The alternative was to copy all the files from their original location to a backup location (possibly on a different drive, hopefully a local one, but no guarantees), copy all the metadata and permissions to the backup location (assuming it supports them), then attempt to delete all the original files. Rollback would do the same thing in reverse. Renaming the directory in-place is a trivial and equivalent operation by comparison. |
|
There is a middleground here: leave undocumented folders that can’t be automatically cleaned up, but avoid treating them like installed distributions. The root issue is actually in |
Good enough for me - this is what I meant by "skipped by whichever function/library thinks that they are importable". |
|
I'm not sure if it would do anything to fix I'm not familiar with |
|
Making the directory path longer is risky, so you'd have to account for that and add a fallback to regenerate the name if the move fails. |
|
Actually, the move will probably succeed, but trying to delete it later will fail (and some users won't be able to delete it themselves either because of the long path). |
|
Thanks for chiming in @zooba - your temp directory function is a clever approach to addressing failures. But the problems with options B and C are not with the status of the operation, but with its state. The fact that these temp directories are (1) non-deterministically named, (2) aren't differentiated from production packages, and (3) are not accounted for after their creation; result in a state where pip can't acknowledge these temp directories properly. The quick fix to this particular issue would be addressing dist-info directory consumption (with @uranusjr 's suggestion and/or PR #9393). This solves (2). For longer-term improvements:
Can we implement a similar functionality for pip? e.g., if a temp directory is found, warn user to delete it or abort. If the user doesn't have permission to delete, tell the user to run |
|
Making another directory is a non-starter - you end up back in the unreliable starting point. What's probably best is to detect the ".dist-info" suffix and corrupt that part of the name as well. A directory starting with "~" is not a valid module, but it apparently can be a valid package. Alternatively, the corrupted name could simply be a counter with no information about where it was originally, provided the name is no longer than it started (more precisely, the full path of any file in the directory does not become any longer). That way it should trivially be made to be ignored by pkg_resources. |
|
Can we try to preserve the package name when naming its temp directory? When a user sees If the name can't be longer than it started...what about just replacing |
|
I think that's less obvious that it can safely be deleted. And there was some reason we decided not to put a dot at the start, but I forget now. Would have to find the original PR I'm guessing. You also have to avoid collisions, since there's no (easy) way to know whether you can overwrite the temporary directory or not (maybe it's from an old operation, but it could be from the current one). At pip startup, you could offer to delete anything lingering, but I wouldn't hold up an uninstall or update operation if there are lingering rollback files. That just hurts the user experience for something that they really don't care about (and once pip freeze is fixed, totally won't care about). |
|
Maybe Good point on not holding up the flow for temp directories. Auto-deleting |
|
Current flow: Proposed flow |
|
Can we just ignore it during pip freeze and warn when the removal process fails during install/uninstall? People should ignore stderr output during pip freeze, but I know that many don't, so we will break people by adding more output to that task. |
What would these be, and when would them be shown? It’s easy to simply ignore these invalid directories, but showing warnings in the right places (while keeping those directories hidden) is more difficult. |
|
At the end of a pip install/uninstall process, there's a step that cleans up rollback files that weren't used. At a guess, it's a shutil.rmtree with fairly liberal error handling. The warning would be "WARNING: could not remove rollback files in $directory_name. These may be deleted manually" whenever an exception in rmtree is swallowed. |
|
Should be doable! This can be done by adding a try-except block in I’ll mark this as awaiting PR. Anyone interested, please feel free to send a PR! |
uranusjr
added
state: awaiting PR
Thanks! It would be great if we could change the invalid distribution naming convention too with this PR (see comments above...replacing random letters with dashes is just a bit confusing). |
What do you propose as an alternative? There aren’t much choices if we must keep the directory where it is, and cannot change the directory name’s length. |
|
I think the suggestion was to replace the |
Bumps [pip](https://github.com/pypa/pip) from 21.0.1 to 21.1. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p> <blockquote> <h1>21.1 (2021-04-24)</h1> <h2>Process</h2> <ul> <li>Start installation scheme migration from <code>distutils</code> to <code>sysconfig</code>. A warning is implemented to detect differences between the two implementations to encourage user reports, so we can avoid breakages before they happen.</li> </ul> <h2>Features</h2> <ul> <li>Add the ability for the new resolver to process URL constraints. (<code>[#8253](pypa/pip#8253) <https://github.com/pypa/pip/issues/8253></code>_)</li> <li>Add a feature <code>--use-feature=in-tree-build</code> to build local projects in-place when installing. This is expected to become the default behavior in pip 21.3; see <code>Installing from local packages <https://pip.pypa.io/en/stable/user_guide/#installing-from-local-packages></code>_ for more information. (<code>[#9091](pypa/pip#9091) <https://github.com/pypa/pip/issues/9091></code>_)</li> <li>Bring back the "(from versions: ...)" message, that was shown on resolution failures. (<code>[#9139](pypa/pip#9139) <https://github.com/pypa/pip/issues/9139></code>_)</li> <li>Add support for editable installs for project with only setup.cfg files. (<code>[#9547](pypa/pip#9547) <https://github.com/pypa/pip/issues/9547></code>_)</li> <li>Improve performance when picking the best file from indexes during <code>pip install</code>. (<code>[#9748](pypa/pip#9748) <https://github.com/pypa/pip/issues/9748></code>_)</li> <li>Warn instead of erroring out when doing a PEP 517 build in presence of <code>--build-option</code>. Warn when doing a PEP 517 build in presence of <code>--global-option</code>. (<code>[#9774](pypa/pip#9774) <https://github.com/pypa/pip/issues/9774></code>_)</li> </ul> <h2>Bug Fixes</h2> <ul> <li>Fixed <code>--target</code> to work with <code>--editable</code> installs. (<code>[#4390](pypa/pip#4390) <https://github.com/pypa/pip/issues/4390></code>_)</li> <li>Add a warning, discouraging the usage of pip as root, outside a virtual environment. (<code>[#6409](pypa/pip#6409) <https://github.com/pypa/pip/issues/6409></code>_)</li> <li>Ignore <code>.dist-info</code> directories if the stem is not a valid Python distribution name, so they don't show up in e.g. <code>pip freeze</code>. (<code>[#7269](pypa/pip#7269) <https://github.com/pypa/pip/issues/7269></code>_)</li> <li>Only query the keyring for URLs that actually trigger error 401. This prevents an unnecessary keyring unlock prompt on every pip install invocation (even with default index URL which is not password protected). (<code>[#8090](pypa/pip#8090) <https://github.com/pypa/pip/issues/8090></code>_)</li> <li>Prevent packages already-installed alongside with pip to be injected into an isolated build environment during build-time dependency population. (<code>[#8214](pypa/pip#8214) <https://github.com/pypa/pip/issues/8214></code>_)</li> <li>Fix <code>pip freeze</code> permission denied error in order to display an understandable error message and offer solutions. (<code>[#8418](pypa/pip#8418) <https://github.com/pypa/pip/issues/8418></code>_)</li> <li>Correctly uninstall script files (from setuptools' <code>scripts</code> argument), when installed with <code>--user</code>. (<code>[#8733](pypa/pip#8733) <https://github.com/pypa/pip/issues/8733></code>_)</li> <li>New resolver: When a requirement is requested both via a direct URL (<code>req @ URL</code>) and via version specifier with extras (<code>req[extra]</code>), the resolver will now be able to use the URL to correctly resolve the requirement with extras. (<code>[#8785](pypa/pip#8785) <https://github.com/pypa/pip/issues/8785></code>_)</li> <li>New resolver: Show relevant entries from user-supplied constraint files in the error message to improve debuggability. (<code>[#9300](pypa/pip#9300) <https://github.com/pypa/pip/issues/9300></code>_)</li> <li>Avoid parsing version to make the version check more robust against lousily debundled downstream distributions. (<code>[#9348](pypa/pip#9348) <https://github.com/pypa/pip/issues/9348></code>_)</li> <li><code>--user</code> is no longer suggested incorrectly when pip fails with a permission error in a virtual environment. (<code>[#9409](pypa/pip#9409) <https://github.com/pypa/pip/issues/9409></code>_)</li> <li>Fix incorrect reporting on <code>Requires-Python</code> conflicts. (<code>[#9541](pypa/pip#9541) <https://github.com/pypa/pip/issues/9541></code>_)</li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/pypa/pip/commit/2b2a268d25963727c2a1c805de8f0246b9cd63f6"><code>2b2a268</code></a> Bump for release</li> <li><a href="https://github.com/pypa/pip/commit/ea761a6575f37b90cf89035ee8be3808cf872184"><code>ea761a6</code></a> Update AUTHORS.txt</li> <li><a href="https://github.com/pypa/pip/commit/2edd3fdf2af2f09dce5085ef0eb54684b4f9bc04"><code>2edd3fd</code></a> Postpone a deprecation to 21.2</li> <li><a href="https://github.com/pypa/pip/commit/3cccfbf169bd35133ee25d2543659b9c1e262f8c"><code>3cccfbf</code></a> Rename mislabeled news fragment</li> <li><a href="https://github.com/pypa/pip/commit/21cd124b5d40b510295c201b9152a65ac3337a37"><code>21cd124</code></a> Fix NEWS.rst placeholder position</li> <li><a href="https://github.com/pypa/pip/commit/e46bdda9711392fec0c45c1175bae6db847cb30b"><code>e46bdda</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/9827">#9827</a> from pradyunsg/fix-git-improper-tag-handling</li> <li><a href="https://github.com/pypa/pip/commit/0e4938d269815a5bf1dd8c16e851cb1199fc5317"><code>0e4938d</code></a> 📰</li> <li><a href="https://github.com/pypa/pip/commit/ca832b2836e0bffa7cf95589acdcd71230f5834e"><code>ca832b2</code></a> Don't split git references on unicode separators</li> <li><a href="https://github.com/pypa/pip/commit/1320bac4ff80d76b8fba2c8b4b4614a40fb9c6c3"><code>1320bac</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/9814">#9814</a> from pradyunsg/revamp-ci-apr-2021-v2</li> <li><a href="https://github.com/pypa/pip/commit/e9cc23ffd97cb6d66d32dc3ec27cf832524bb33d"><code>e9cc23f</code></a> Skip checks on PRs only</li> <li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/21.0.1...21.1">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually </details>
Environment
Like some other people (e.g. this fellow, and this guy), I have a
site-packageslittered with leftover folders created byAdjacentTempDirectory, visible at the bottom of thislsoutput:Output
Description
As a consequence of the above, when I run
pip3 freeze, I get a whole load of seemingly nonsensical warnings:Warnings output
@cjerdonek's change at #6538 improves this slightly (previously there was no clue that these weird names referred to anything in the
site-packagesdirectory), but as a naive user this I'm still left unsure what I'm being told or what to do about it. When encountering this message, I'm left adrift in a couple of ways:site-packagesdirectory. It mentions the directory, and mentions the weird name, and then leaves me to guess.site-packagesdirectory! The subdirectories triggering the errors start with a~, but in the error message that~has somehow shapeshifted into a-.site-packagesdirectory somehow relate to this error message despite the above hurdles, I'm still left confused and guessing about what that implies. Why are they there at all? Should I delete them or is that going to break my Python installation somehow? Is something horribly corrupted and do I need to reinstall Python? The error message gives me no guidance; it suggests that something is wrong with the fact that these folders exist, but that's all the help I get.(The answer to my rhetorical questions in point 3 is that they're temporary folders created by Pip, possibly left behind when I killed a Pip process previously before it could clean up, and that deleting them is safe. But I'm not sure how I could've found this out without stumbling across this Stack Overflow answer.)
It's got to be possible to avoid confusing and worrying the user in this way.
A possible easy fix
Make
pip freezeignore~-prefixed folders without logging a warning. From what I understand from https://stackoverflow.com/a/57488427/1709587,~-prefixed directories are guaranteed to be temporary directories. So just treat them like they don't exist infreeze.A possible harder fix
Attack the root cause by ensuring that these temporary directories eventually get cleaned up properly, even if a
pipprocess gets uncleanly killed. I see two strategies for this:pipinvocation, even if the command is to do something unrelated (similarly to howgit, by default, sometimes decides to run garbage collection on the objects in your local repo).tempfilemodule) instead(I'm throwing both these ideas out naively, without much understanding of Pip's internals; I apologise if either of them is stupid for a reason I'm not seeing.)
The text was updated successfully, but these errors were encountered: