Skip to content

Commit

Permalink
Issue #2478 - topological install order.
Browse files Browse the repository at this point in the history
This is needed for setup-requires, since without it its possible
to cause installation to fail in sort-circuit scenarios such as
the added functional test case demonstrates.
  • Loading branch information
rbtcollins committed Mar 25, 2015
1 parent a91c35b commit 493911e
Show file tree
Hide file tree
Showing 18 changed files with 130 additions and 18 deletions.
1 change: 1 addition & 0 deletions pip/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ def url_name(self):

@property
def setup_py(self):
assert self.source_dir, "No source dir for %s" % self
try:
import setuptools # noqa
except ImportError:
Expand Down
81 changes: 63 additions & 18 deletions pip/req/req_set.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import absolute_import

from collections import defaultdict
import functools
import itertools
import logging
Expand Down Expand Up @@ -273,6 +274,8 @@ def __init__(self, build_dir, src_dir, download_dir, upgrade=False,
if wheel_download_dir:
wheel_download_dir = normalize_path(wheel_download_dir)
self.wheel_download_dir = wheel_download_dir
# Maps from install_req -> dependencies_of_install_req
self._dependencies = defaultdict(list)

def __str__(self):
reqs = [req for req in self.requirements.values()
Expand All @@ -287,29 +290,55 @@ def __repr__(self):
return ('<%s object; %d requirement(s): %s>'
% (self.__class__.__name__, len(reqs), reqs_str))

def add_requirement(self, install_req):
if not install_req.match_markers():
def add_requirement(self, install_req, parent_req_name=None):
"""Add install_req as a requirement to install.
:param parent_req_name: The name of the requirement that needed this
added. The name is used because when multiple unnamed requirements
resolve to the same name, we could otherwise end up with dependency
links that point outside the Requirements set. parent_req must
already be added.
:return: Additional requirements to scan. That is either [] if
the requirement is not applicable, or [install_req] if the
requirement is applicable and has just been added.
"""
name = install_req.name
if ((not name or not self.has_requirement(name))
and not install_req.match_markers()):
# Only log if we haven't already got install_req from somewhere.
logger.debug("Ignore %s: markers %r don't match",
install_req.name, install_req.markers)
return
return []

name = install_req.name
install_req.as_egg = self.as_egg
install_req.use_user_site = self.use_user_site
install_req.target_dir = self.target_dir
install_req.pycompile = self.pycompile
if not name:
# url or path requirement w/o an egg fragment
self.unnamed_requirements.append(install_req)
return [install_req]
else:
if self.has_requirement(name):
if parent_req_name is None and self.has_requirement(name):
raise InstallationError(
'Double requirement given: %s (already in %s, name=%r)'
% (install_req, self.get_requirement(name), name))
self.requirements[name] = install_req
# FIXME: what about other normalizations? E.g., _ vs. -?
if name.lower() != name:
self.requirement_aliases[name.lower()] = name
if not self.has_requirement(name):
# Add requirement
self.requirements[name] = install_req
# FIXME: what about other normalizations? E.g., _ vs. -?
if name.lower() != name:
self.requirement_aliases[name.lower()] = name
result = [install_req]
else:
# Canonicalise to the already-added object
install_req = self.get_requirement(name)
# No need to scan, this is a duplicate requirement.
result = []
if parent_req_name:
parent_req = self.get_requirement(parent_req_name)
self._dependencies[parent_req].append(install_req)
return result

def has_requirement(self, project_name):
for name in project_name, project_name.lower():
Expand Down Expand Up @@ -610,23 +639,20 @@ def _prepare_file(self, finder, req_to_install):
more_reqs = []

def add_req(subreq):
if self.has_requirement(subreq.project_name):
# FIXME: check for conflict
return
subreq = InstallRequirement(
sub_install_req = InstallRequirement(
str(subreq),
req_to_install,
isolated=self.isolated,
)
more_reqs.append(subreq)
self.add_requirement(subreq)
more_reqs.extend(self.add_requirement(
sub_install_req, req_to_install.name))

# We add req_to_install before its dependencies, so that when
# to_install is calculated, which reverses the order,
# req_to_install is installed after its dependencies.
if not self.has_requirement(req_to_install.name):
# 'unnamed' requirements will get added here
self.add_requirement(req_to_install)
self.add_requirement(req_to_install, None)

if not self.ignore_dependencies:
if getattr(dist, 'setup_requires', None):
Expand Down Expand Up @@ -688,13 +714,32 @@ def _pip_has_created_build_dir(self):
)
)

def _to_install(self):
"""Create the installation order.
We install the user specified things in the order given, except when
dependencies require putting other things first.
"""
order = []
ordered_reqs = set()

def schedule(req):
if req.satisfied_by or req in ordered_reqs:
return
ordered_reqs.add(req)
for dep in self._dependencies[req]:
schedule(dep)
order.append(req)
for install_req in self.requirements.values():
schedule(install_req)
return order

def install(self, install_options, global_options=(), *args, **kwargs):
"""
Install everything in this set (after having downloaded and unpacked
the packages)
"""
to_install = [r for r in self.requirements.values()[::-1]
if not r.satisfied_by]
to_install = self._to_install()

# DISTRIBUTE TO SETUPTOOLS UPGRADE HACK (1 of 3 parts)
# move the distribute-0.7.X wrapper to the end because it does not
Expand Down
11 changes: 11 additions & 0 deletions tests/data/packages/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ requires SetupRequires2[a,b], as using extras for local paths is currently
broken (issue 1236). Ideally SetupRequires3 would have the extras itself
and no requires-dist (to test declarative extras as sole requirements).

TopoRequires[123][-0.0.1.tar.gz]
--------------------------------

These are used for testing topological handling of requirements: we have
TopoRequires, which is setup-required by TopoRequires2 and TopoRequires3
and finally TopoRequires4 which install-requires both TopoRequires2 and 3
and also setup-Requires TopoRequires.
This creates a diamond where no matter which way we walk without topological
awareness we'll end up attempting to install TopoRequires after one of
TopoRequires2, TopoRequires3 or TopoRequires4. (prefix iteration works as its
topological, suffix iteration likewise, infix breaks).

simple[2]-[123].0.tar.gz
------------------------
Expand Down
Binary file added tests/data/packages/TopoRequires-0.0.1.tar.gz
Binary file not shown.
7 changes: 7 additions & 0 deletions tests/data/packages/TopoRequires/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from setuptools import setup

setup(
name='TopoRequires',
version='0.0.1',
packages=['toporequires'],
)
Empty file.
Binary file added tests/data/packages/TopoRequires2-0.0.1.tar.gz
Binary file not shown.
4 changes: 4 additions & 0 deletions tests/data/packages/TopoRequires2/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[metadata]
name = TopoRequires2
requires-setup =
TopoRequires
8 changes: 8 additions & 0 deletions tests/data/packages/TopoRequires2/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from setuptools import setup
import toporequires

setup(
name='TopoRequires2',
version='0.0.1',
packages=['toporequires2'],
)
Empty file.
Binary file added tests/data/packages/TopoRequires3-0.0.1.tar.gz
Binary file not shown.
4 changes: 4 additions & 0 deletions tests/data/packages/TopoRequires3/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[metadata]
name = TopoRequires3
requires-setup =
TopoRequires
8 changes: 8 additions & 0 deletions tests/data/packages/TopoRequires3/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from setuptools import setup
import toporequires

setup(
name='TopoRequires3',
version='0.0.1',
packages=['toporequires3'],
)
Empty file.
7 changes: 7 additions & 0 deletions tests/data/packages/TopoRequires4/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[metadata]
name = TopoRequires4
requires-dist =
TopoRequires2
TopoRequires3
requires-setup =
TopoRequires
8 changes: 8 additions & 0 deletions tests/data/packages/TopoRequires4/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from setuptools import setup
import toporequires

setup(
name='TopoRequires4',
version='0.0.1',
packages=['toporequires4'],
)
Empty file.
9 changes: 9 additions & 0 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,3 +764,12 @@ def test_install_declarative_extras(script, data):
assert 'Running setup.py install for simple2\n' in str(res), str(res)
assert 'Running setup.py install for SetupRequires3\n' in str(res), \
str(res)


def test_install_topological_sort(script, data):
to_install = data.packages.join('TopoRequires4')
args = ['install'] + [to_install, '-f', data.packages]
res = str(script.pip(*args, expect_error=False))
order1 = 'TopoRequires, TopoRequires2, TopoRequires3, TopoRequires4'
order2 = 'TopoRequires, TopoRequires3, TopoRequires2, TopoRequires4'
assert order1 in res or order2 in res, res

0 comments on commit 493911e

Please sign in to comment.