Merge pull request #4524 from pradyunsg/misc-1

Misc Changes

pip/basecommand.py

@@ -174,17 +174,13 @@ class Command(object):
             # logging enabled. These use both pip._vendor and the bare names
             # for the case where someone unbundles our libraries.
             "loggers": dict(
-                (
-                    name,
-                    {
-                        "level": (
-                            "WARNING"
-                            if level in ["INFO", "ERROR"]
-                            else "DEBUG"
-                        ),
-                    },
-                )
-                for name in ["pip._vendor", "distlib", "requests", "urllib3"]
+                (name, {
+                    "level": (
+                        "WARNING" if level in ["INFO", "ERROR"] else "DEBUG"
+                    )
+                }) for name in [
+                    "pip._vendor", "distlib", "requests", "urllib3"
+                ]
             ),
         })
 

pip/commands/install.py

@@ -220,7 +220,7 @@ class InstallCommand(RequirementCommand):
                 logger.warning(
                     "The directory '%s' or its parent directory is not owned "
                     "by the current user and caching wheels has been "
-                    "disabled. check the permissions and owner of that "
+                    "disabled. Check the permissions and owner of that "
                     "directory. If executing pip with sudo, you may want "
                     "sudo's -H flag.",
                     options.cache_dir,
@@ -254,12 +254,7 @@ class InstallCommand(RequirementCommand):
                 )
 
                 try:
-                    if (not wheel or not options.cache_dir):
-                        # on -d don't do complex things like building
-                        # wheels, and don't try to build wheels when wheel is
-                        # not installed.
-                        requirement_set.prepare_files(finder)
-                    else:
+                    if wheel and options.cache_dir:
                         # build wheels before install.
                         wb = WheelBuilder(
                             requirement_set,
@@ -270,6 +265,11 @@ class InstallCommand(RequirementCommand):
                         # Ignore the result: a failed wheel will be
                         # installed from the sdist/vcs whatever.
                         wb.build(autobuilding=True)
+                    else:
+                        # on -d don't do complex things like building
+                        # wheels, and don't try to build wheels when wheel is
+                        # not installed.
+                        requirement_set.prepare_files(finder)
 
                     requirement_set.install(
                         install_options,
@@ -332,59 +332,65 @@ class InstallCommand(RequirementCommand):
                         requirement_set.cleanup_files()
 
         if options.target_dir:
-            ensure_dir(options.target_dir)
-
-            # Checking both purelib and platlib directories for installed
-            # packages to be moved to target directory
-            lib_dir_list = []
-
-            purelib_dir = distutils_scheme('', home=temp_target_dir)['purelib']
-            platlib_dir = distutils_scheme('', home=temp_target_dir)['platlib']
-            data_dir = distutils_scheme('', home=temp_target_dir)['data']
-
-            if os.path.exists(purelib_dir):
-                lib_dir_list.append(purelib_dir)
-            if os.path.exists(platlib_dir) and platlib_dir != purelib_dir:
-                lib_dir_list.append(platlib_dir)
-            if os.path.exists(data_dir):
-                lib_dir_list.append(data_dir)
-
-            for lib_dir in lib_dir_list:
-                for item in os.listdir(lib_dir):
-                    if lib_dir == data_dir:
-                        ddir = os.path.join(data_dir, item)
-                        if any(s.startswith(ddir) for s in lib_dir_list[:-1]):
-                            continue
-                    target_item_dir = os.path.join(options.target_dir, item)
-                    if os.path.exists(target_item_dir):
-                        if not options.upgrade:
-                            logger.warning(
-                                'Target directory %s already exists. Specify '
-                                '--upgrade to force replacement.',
-                                target_item_dir
-                            )
-                            continue
-                        if os.path.islink(target_item_dir):
-                            logger.warning(
-                                'Target directory %s already exists and is '
-                                'a link. Pip will not automatically replace '
-                                'links, please remove if replacement is '
-                                'desired.',
-                                target_item_dir
-                            )
-                            continue
-                        if os.path.isdir(target_item_dir):
-                            shutil.rmtree(target_item_dir)
-                        else:
-                            os.remove(target_item_dir)
+            self._handle_target_dir(
+                options.target_dir, temp_target_dir, options.upgrade
+            )
 
-                    shutil.move(
-                        os.path.join(lib_dir, item),
-                        target_item_dir
-                    )
-            shutil.rmtree(temp_target_dir)
         return requirement_set
 
+    def _handle_target_dir(self, target_dir, temp_target_dir, upgrade):
+        ensure_dir(target_dir)
+
+        # Checking both purelib and platlib directories for installed
+        # packages to be moved to target directory
+        lib_dir_list = []
+
+        purelib_dir = distutils_scheme('', home=temp_target_dir)['purelib']
+        platlib_dir = distutils_scheme('', home=temp_target_dir)['platlib']
+        data_dir = distutils_scheme('', home=temp_target_dir)['data']
+
+        if os.path.exists(purelib_dir):
+            lib_dir_list.append(purelib_dir)
+        if os.path.exists(platlib_dir) and platlib_dir != purelib_dir:
+            lib_dir_list.append(platlib_dir)
+        if os.path.exists(data_dir):
+            lib_dir_list.append(data_dir)
+
+        for lib_dir in lib_dir_list:
+            for item in os.listdir(lib_dir):
+                if lib_dir == data_dir:
+                    ddir = os.path.join(data_dir, item)
+                    if any(s.startswith(ddir) for s in lib_dir_list[:-1]):
+                        continue
+                target_item_dir = os.path.join(target_dir, item)
+                if os.path.exists(target_item_dir):
+                    if not upgrade:
+                        logger.warning(
+                            'Target directory %s already exists. Specify '
+                            '--upgrade to force replacement.',
+                            target_item_dir
+                        )
+                        continue
+                    if os.path.islink(target_item_dir):
+                        logger.warning(
+                            'Target directory %s already exists and is '
+                            'a link. Pip will not automatically replace '
+                            'links, please remove if replacement is '
+                            'desired.',
+                            target_item_dir
+                        )
+                        continue
+                    if os.path.isdir(target_item_dir):
+                        shutil.rmtree(target_item_dir)
+                    else:
+                        os.remove(target_item_dir)
+
+                shutil.move(
+                    os.path.join(lib_dir, item),
+                    target_item_dir
+                )
+        shutil.rmtree(temp_target_dir)
+
 
 def get_lib_location_guesses(*args, **kwargs):
     scheme = distutils_scheme('', *args, **kwargs)

pip/req/req_set.py

@@ -401,49 +401,48 @@ class RequirementSet(object):
         """
         # Check whether to upgrade/reinstall this req or not.
         req_to_install.check_if_exists()
-        if req_to_install.satisfied_by:
-            upgrade_allowed = self._is_upgrade_allowed(req_to_install)
-
-            # Is the best version is installed.
-            best_installed = False
-
-            if upgrade_allowed:
-                # For link based requirements we have to pull the
-                # tree down and inspect to assess the version #, so
-                # its handled way down.
-                if not (self.force_reinstall or req_to_install.link):
-                    try:
-                        finder.find_requirement(
-                            req_to_install, upgrade_allowed)
-                    except BestVersionAlreadyInstalled:
-                        best_installed = True
-                    except DistributionNotFound:
-                        # No distribution found, so we squash the
-                        # error - it will be raised later when we
-                        # re-try later to do the install.
-                        # Why don't we just raise here?
-                        pass
-
-                if not best_installed:
-                    # don't uninstall conflict if user install and
-                    # conflict is not user install
-                    if not (self.use_user_site and not
-                            dist_in_usersite(req_to_install.satisfied_by)):
-                        req_to_install.conflicts_with = \
-                            req_to_install.satisfied_by
-                    req_to_install.satisfied_by = None
-
-            # Figure out a nice message to say why we're skipping this.
-            if best_installed:
-                skip_reason = 'already up-to-date'
-            elif self.upgrade_strategy == "only-if-needed":
-                skip_reason = 'not upgraded as not directly required'
-            else:
-                skip_reason = 'already satisfied'
+        if not req_to_install.satisfied_by:
+            return None
+        upgrade_allowed = self._is_upgrade_allowed(req_to_install)
+
+        # Is the best version is installed.
+        best_installed = False
 
-            return skip_reason
+        if upgrade_allowed:
+            # For link based requirements we have to pull the
+            # tree down and inspect to assess the version #, so
+            # its handled way down.
+            if not (self.force_reinstall or req_to_install.link):
+                try:
+                    finder.find_requirement(
+                        req_to_install, upgrade_allowed)
+                except BestVersionAlreadyInstalled:
+                    best_installed = True
+                except DistributionNotFound:
+                    # No distribution found, so we squash the
+                    # error - it will be raised later when we
+                    # re-try later to do the install.
+                    # Why don't we just raise here?
+                    pass
+
+            if not best_installed:
+                # don't uninstall conflict if user install and
+                # conflict is not user install
+                if not (self.use_user_site and not
+                        dist_in_usersite(req_to_install.satisfied_by)):
+                    req_to_install.conflicts_with = \
+                        req_to_install.satisfied_by
+                req_to_install.satisfied_by = None
+
+        # Figure out a nice message to say why we're skipping this.
+        if best_installed:
+            skip_reason = 'already up-to-date'
+        elif self.upgrade_strategy == "only-if-needed":
+            skip_reason = 'not upgraded as not directly required'
         else:
-            return None
+            skip_reason = 'already satisfied'
+
+        return skip_reason
 
     def _prepare_file(self,
                       finder,
@@ -684,7 +683,7 @@ class RequirementSet(object):
                 self.add_requirement(req_to_install, None)
 
             if not ignore_dependencies:
-                if (req_to_install.extras):
+                if req_to_install.extras:
                     logger.debug(
                         "Installing extra requirements: %r",
                         ','.join(req_to_install.extras),
@@ -741,6 +740,7 @@ class RequirementSet(object):
             for dep in self._dependencies[req]:
                 schedule(dep)
             order.append(req)
+
         for install_req in self.requirements.values():
             schedule(install_req)
         return order
@@ -775,14 +775,20 @@ class RequirementSet(object):
                         **kwargs
                     )
                 except:
+                    should_rollback = (
+                        requirement.conflicts_with and
+                        not requirement.install_succeeded
+                    )
                     # if install did not succeed, rollback previous uninstall
-                    if (requirement.conflicts_with and not
-                            requirement.install_succeeded):
+                    if should_rollback:
                         requirement.uninstalled_pathset.rollback()
                     raise
                 else:
-                    if (requirement.conflicts_with and
-                            requirement.install_succeeded):
+                    should_commit = (
+                        requirement.conflicts_with and
+                        requirement.install_succeeded
+                    )
+                    if should_commit:
                         requirement.uninstalled_pathset.commit()
                 requirement.remove_temporary_source()