diff --git a/news/4692.bugfix.rst b/news/4692.bugfix.rst new file mode 100644 index 00000000000..0b5e6300680 --- /dev/null +++ b/news/4692.bugfix.rst @@ -0,0 +1 @@ +This change adds a warning for situations where pip is run with sudo. diff --git a/news/4727.bugfix.rst b/news/4727.bugfix.rst new file mode 100644 index 00000000000..0b5e6300680 --- /dev/null +++ b/news/4727.bugfix.rst @@ -0,0 +1 @@ +This change adds a warning for situations where pip is run with sudo. diff --git a/news/7269.bugfix.rst b/news/7269.bugfix.rst new file mode 100644 index 00000000000..84acf2ae721 --- /dev/null +++ b/news/7269.bugfix.rst @@ -0,0 +1 @@ +This change fixes pip freeze by suppressing warnings for invalid packages. diff --git a/news/7307.bugfix.rst b/news/7307.bugfix.rst new file mode 100644 index 00000000000..1dec29a24e4 --- /dev/null +++ b/news/7307.bugfix.rst @@ -0,0 +1 @@ +This change prevents the unintended creation of temporary package directories. diff --git a/news/7450.bugfix.rst b/news/7450.bugfix.rst new file mode 100644 index 00000000000..1dec29a24e4 --- /dev/null +++ b/news/7450.bugfix.rst @@ -0,0 +1 @@ +This change prevents the unintended creation of temporary package directories. diff --git a/news/7763.bugfix.rst b/news/7763.bugfix.rst new file mode 100644 index 00000000000..1dec29a24e4 --- /dev/null +++ b/news/7763.bugfix.rst @@ -0,0 +1 @@ +This change prevents the unintended creation of temporary package directories. diff --git a/news/7940.bugfix.rst b/news/7940.bugfix.rst new file mode 100644 index 00000000000..0b5e6300680 --- /dev/null +++ b/news/7940.bugfix.rst @@ -0,0 +1 @@ +This change adds a warning for situations where pip is run with sudo. diff --git a/news/9235.bugfix.rst b/news/9235.bugfix.rst new file mode 100644 index 00000000000..84acf2ae721 --- /dev/null +++ b/news/9235.bugfix.rst @@ -0,0 +1 @@ +This change fixes pip freeze by suppressing warnings for invalid packages. diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index cd97ecb6042..6d863941a4b 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -721,11 +721,16 @@ def create_os_error_message(error, show_traceback, using_user_site): if error.errno == errno.EACCES: user_option_part = "Consider using the `--user` option" permissions_part = "Check the permissions" + chown_part = ("Oftentimes packages are unnecessarily installed with " + "`sudo pip`, which sets the folder owner to root. " + "In this case, consider changing the owner to $USER: " + "`$ sudo chown -R $USER [package folder]`") if not using_user_site: parts.extend([ user_option_part, " or ", - permissions_part.lower(), + permissions_part.lower(), ".\n", + chown_part ]) else: parts.append(permissions_part) diff --git a/src/pip/_internal/req/req_uninstall.py b/src/pip/_internal/req/req_uninstall.py index eea2508db86..95d5c790b1c 100644 --- a/src/pip/_internal/req/req_uninstall.py +++ b/src/pip/_internal/req/req_uninstall.py @@ -398,8 +398,21 @@ def remove(self, auto_confirm=False, verbose=False): for_rename = compress_for_rename(self.paths) for path in sorted(compact(for_rename)): - moved.stash(path) logger.debug('Removing file or directory %s', path) + try: + moved.stash(path) + except PermissionError as ex: + raise UninstallationError( + "{!r}\nFailed to uninstall {!r} because " + "this user lacks permissions to the package " + "folder. Consider using the `--user` option or " + "check the permissions.\nOftentimes packages " + "are unnecessarily installed with `sudo pip`, " + "which sets the folder owner to root. In this " + "case, consider changing the owner to $USER: " + "`$ sudo chown -R $USER [package folder]`" + .format(str(ex), dist_name_version) + ) for pth in self.pth.values(): pth.remove() diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index bd205ada5db..a03f035245c 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -327,6 +327,11 @@ def renames(old, new): """Like os.renames(), but handles renaming across devices.""" # Implementation borrowed from os.renames(). head, tail = os.path.split(new) + if not os.access(old, os.W_OK): + raise PermissionError( + 'Rename aborted. User does not have write access to ' + old + ) + if head and tail and not os.path.exists(head): os.makedirs(head) @@ -458,6 +463,7 @@ def user_test(d): return [d for d in working_set if local_test(d) and d.key not in skip and + '-' not in d.key[0] and editable_test(d) and editables_only_test(d) and user_test(d) diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index 5d3d4968619..2f27722374c 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -123,9 +123,12 @@ def fake_install(pkgname, dest): valid_pkgnames = ('middle-dash', 'middle_underscore', 'middle.dot') invalid_pkgnames = ( - '-leadingdash', '_leadingunderscore', '.leadingdot', 'trailingdash-', 'trailingunderscore_', 'trailingdot.' ) + ignored_pkgnames = ( + '-leadingdash', '_leadingunderscore', '.leadingdot' + ) + for pkgname in valid_pkgnames + invalid_pkgnames: fake_install(pkgname, script.site_packages_path) result = script.pip('freeze', expect_stderr=True) @@ -142,6 +145,9 @@ def fake_install(pkgname, dest): 'distribution {}...'.format(dist_repr) ) _check_output(result.stderr, expected) + for pkgname in ignored_pkgnames: + assert(pkgname not in result.stdout) + assert(pkgname not in result.stderr) # Also check that the parse error details occur at least once. # We only need to find one occurrence to know that exception details diff --git a/tests/unit/test_command_install.py b/tests/unit/test_command_install.py index 66eb8ef3881..cefeba5b86c 100644 --- a/tests/unit/test_command_install.py +++ b/tests/unit/test_command_install.py @@ -89,9 +89,13 @@ def test_rejection_for_location_requirement_options(): # show_traceback = True, using_user_site = False (OSError("Illegal byte sequence"), True, False, 'Could not' ' install packages due to an OSError.\n'), - (OSError(errno.EACCES, "No file permission"), True, False, 'Could' - ' not install packages due to an OSError.\nConsider using the' - ' `--user` option or check the permissions.\n'), + (OSError(errno.EACCES, "No file permission"), True, False, + 'Could not install packages due to an OSError.' + '\nConsider using the `--user` option or check the permissions.' + '\nOftentimes packages are unnecessarily installed with `sudo pip`, ' + 'which sets the folder owner to root. In this ' + 'case, consider changing the owner to $USER: ' + '`$ sudo chown -R $USER [package folder]`.\n'), # show_traceback = False, using_user_site = True (OSError("Illegal byte sequence"), False, True, 'Could not' ' install packages due to an OSError: Illegal byte' @@ -104,9 +108,13 @@ def test_rejection_for_location_requirement_options(): ' install packages due to an OSError: Illegal byte sequence' '\n'), (OSError(errno.EACCES, "No file permission"), False, False, - 'Could not install packages due to an OSError: [Errno 13] No' - ' file permission\nConsider using the `--user` option or check the' - ' permissions.\n'), + 'Could not install packages due to an OSError: ' + '[Errno 13] No file permission\nConsider using the `--user` ' + 'option or check the permissions.' + '\nOftentimes packages are unnecessarily installed with `sudo pip`, ' + 'which sets the folder owner to root. In this ' + 'case, consider changing the owner to $USER: ' + '`$ sudo chown -R $USER [package folder]`.\n') ]) def test_create_os_error_message( error, show_traceback, using_user_site, expected