From b58703f3bddb82ea0150826bca43a4c849dfb472 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Wed, 25 Mar 2015 13:53:10 +1300 Subject: [PATCH] Issue #2478 - topological install order. 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. --- pip/req/req_install.py | 1 + pip/req/req_set.py | 80 ++++++++++++++---- tests/data/packages/README.txt | 11 +++ tests/data/packages/TopoRequires-0.0.1.tar.gz | Bin 0 -> 746 bytes tests/data/packages/TopoRequires/setup.py | 7 ++ .../TopoRequires/toporequires/__init__.py | 0 .../data/packages/TopoRequires2-0.0.1.tar.gz | Bin 0 -> 769 bytes tests/data/packages/TopoRequires2/setup.cfg | 4 + tests/data/packages/TopoRequires2/setup.py | 8 ++ .../TopoRequires2/toporequires2/__init__.py | 0 .../data/packages/TopoRequires3-0.0.1.tar.gz | Bin 0 -> 792 bytes tests/data/packages/TopoRequires3/setup.cfg | 4 + tests/data/packages/TopoRequires3/setup.py | 8 ++ .../TopoRequires3/toporequires3/__init__.py | 0 tests/data/packages/TopoRequires4/setup.cfg | 7 ++ tests/data/packages/TopoRequires4/setup.py | 8 ++ .../TopoRequires4/toporequires4/__init__.py | 0 tests/functional/test_install.py | 9 ++ 18 files changed, 129 insertions(+), 18 deletions(-) create mode 100644 tests/data/packages/TopoRequires-0.0.1.tar.gz create mode 100644 tests/data/packages/TopoRequires/setup.py create mode 100644 tests/data/packages/TopoRequires/toporequires/__init__.py create mode 100644 tests/data/packages/TopoRequires2-0.0.1.tar.gz create mode 100644 tests/data/packages/TopoRequires2/setup.cfg create mode 100644 tests/data/packages/TopoRequires2/setup.py create mode 100644 tests/data/packages/TopoRequires2/toporequires2/__init__.py create mode 100644 tests/data/packages/TopoRequires3-0.0.1.tar.gz create mode 100644 tests/data/packages/TopoRequires3/setup.cfg create mode 100644 tests/data/packages/TopoRequires3/setup.py create mode 100644 tests/data/packages/TopoRequires3/toporequires3/__init__.py create mode 100644 tests/data/packages/TopoRequires4/setup.cfg create mode 100644 tests/data/packages/TopoRequires4/setup.py create mode 100644 tests/data/packages/TopoRequires4/toporequires4/__init__.py diff --git a/pip/req/req_install.py b/pip/req/req_install.py index 81da2671be6..8ed5e347a00 100644 --- a/pip/req/req_install.py +++ b/pip/req/req_install.py @@ -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: diff --git a/pip/req/req_set.py b/pip/req/req_set.py index e293a1a8d0e..3a514184d5d 100644 --- a/pip/req/req_set.py +++ b/pip/req/req_set.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +from collections import defaultdict import functools import itertools import logging @@ -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() @@ -287,13 +290,26 @@ 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 @@ -301,15 +317,28 @@ def add_requirement(self, install_req): 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(): @@ -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): @@ -688,13 +714,31 @@ 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 diff --git a/tests/data/packages/README.txt b/tests/data/packages/README.txt index b2f574210e0..f9d8fde6bcf 100644 --- a/tests/data/packages/README.txt +++ b/tests/data/packages/README.txt @@ -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 ------------------------ diff --git a/tests/data/packages/TopoRequires-0.0.1.tar.gz b/tests/data/packages/TopoRequires-0.0.1.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..58182befe111fa283d9a100366664b9732dd732e GIT binary patch literal 746 zcmVv z#`B!NBI1QjDyf|dQIU93w(2S3jg0{}PV>)L*Jlu?THHivAWMwhR5Ag8sfArIGY~GrwF( z|9;=OhyH|GeM{+YQJZK6f&P!7zu&vCPRZh%!A)+iDcFWbX2<9xfU?CU7({4STbkDZ* z=eh=-vj6KLlZRn1N+;R&ivE9C{ilxQq5eN9{g3uf4qv`LGUYefcK)}mrvA1~`pEyA zpepwI&8i>lzU{pqz1i3ATog<081tN`W1a?=ejKG2;+9+U^TPZhvUJI`sv~~PXFM*y zqV*;f`TutR-y~#f{Xebw-&X#2o%#MB{$D?6#^C>Z`Tw`{+w%UOx()w3)P?^yLcNY6 z7r_5Z{ap1^&y@vm7ivE8yeD6fUFL=zE;GKanVX@#_MkiG5^x|MgMLT?FC)gQ# zogXmF|LXs0C%6XxTb|YMzvWt}{~O^0mu$==>wUa(`@oF=_H_i`5U>(Gm-{G1id>mE;76kkX8qJXEO_Sh>ff+EUg`L-%} z5HmT+isU*1@2<)JS4Tp@By8&)ch-N;Zv6l6QMDz2|2M#?O7Q+X!Jp}ph5q?8ipR## zC@)lA;Oa9eFT^ZWeZnl7CxJ2u1OB7_U!l^sQUB-u@7pQ_|D*n+{sRC20000000000 c000000000000000001=j3A5j>7ywWJ05wE~{Qv*} literal 0 HcmV?d00001 diff --git a/tests/data/packages/TopoRequires/setup.py b/tests/data/packages/TopoRequires/setup.py new file mode 100644 index 00000000000..6cd29a7b656 --- /dev/null +++ b/tests/data/packages/TopoRequires/setup.py @@ -0,0 +1,7 @@ +from setuptools import setup + +setup( + name='TopoRequires', + version='0.0.1', + packages=['toporequires'], +) diff --git a/tests/data/packages/TopoRequires/toporequires/__init__.py b/tests/data/packages/TopoRequires/toporequires/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/packages/TopoRequires2-0.0.1.tar.gz b/tests/data/packages/TopoRequires2-0.0.1.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..61aaef45a919026418875f063d357d2254f87033 GIT binary patch literal 769 zcmV+c1OEIUiwFor(h*ex|72-%bT3qIaBos&adl~OWpgquFfK4IF)nmrascg_TW^~% z7>2o>e+9}-HmOj|QH_*~X=imys~)zCszQ`FjFr>Ci55-!?=vK<32K&Zqcl_B=Z5&R zLlk{}?9XtPX6Xt0It_DH=!&LlD&3b_MiV%WRbHuMX{*;7iRzYS8WyFRDQVO)EM1Z< zsRO13_i|a5^7M-3+}}8ETYvpW>g#{P*)93!B9_;&{RhauUiKsZyODp_4U>?&uA0sN zA^)~*-XZ@Q)ooqmU)LQ|k~QT2LGr&AQ&|uwVKPbgJM^DgwgdlnEdQrR=O-^-ovQpB zZ`XgrZ06rEMK}EKfP(R9ruvhBEIws(vLXG)hi{a3qt{1d`}Ei}WXv*_j9KE(-6%}1 zi{FjSu5$HDktUn^#U|xOY{sJMCT(w7kA^?+|F-P4wExqF|CVi7@V^TP6oCJC^M7r{ zZMFWVcEf+u5Fz;A0c#s476bUd+5fsCYybRjInDZCHy!xj1>bvN;pQx2Ucq_;dE!L{ z+mm~ND4%C6FTykt(H=WE@DIjL?+1i=y#Ch*z}EfO9i!pDZrj-ZJK+Q4-q_=w@^NAN zK&GljMqbPY^4dHgzXnv3gq%*}*vsbw`FwOZI{GvsZ_}75nHR8|(DNz(oaW88!eTFs zZldo)pC!f0*Iu&1&%=yYzg7p2BactgJiduQ-h1-DeiGEyZ*29S>b3uxMJ-A6YBrw# zcfur3W4TH=Pt&N7!&saL_##4c)ou literal 0 HcmV?d00001 diff --git a/tests/data/packages/TopoRequires2/setup.cfg b/tests/data/packages/TopoRequires2/setup.cfg new file mode 100644 index 00000000000..448eb63bf4f --- /dev/null +++ b/tests/data/packages/TopoRequires2/setup.cfg @@ -0,0 +1,4 @@ +[metadata] +name = TopoRequires2 +requires-setup = + TopoRequires diff --git a/tests/data/packages/TopoRequires2/setup.py b/tests/data/packages/TopoRequires2/setup.py new file mode 100644 index 00000000000..44bf1e1b1dc --- /dev/null +++ b/tests/data/packages/TopoRequires2/setup.py @@ -0,0 +1,8 @@ +from setuptools import setup +import toporequires + +setup( + name='TopoRequires2', + version='0.0.1', + packages=['toporequires2'], +) diff --git a/tests/data/packages/TopoRequires2/toporequires2/__init__.py b/tests/data/packages/TopoRequires2/toporequires2/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/packages/TopoRequires3-0.0.1.tar.gz b/tests/data/packages/TopoRequires3-0.0.1.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..37cf3f6928f026d90d6fb682607c492c73ed498a GIT binary patch literal 792 zcmV+z1Lyo7iwFo#(h*ex|72-%bT3qIaBos&adl~OWpgtvFfK4IF)nmrascg_+iuf9 z5QcrrK82N=fKRz1K)Rax6)V@!R*-b7T0cgJ?)HjWxXscVAxzg$>5 zyRj9`{4&K({>cq zRytsoOP;B!k|h@+ljF7LHtp9xq`v+moZR96T*z6fr&n$I-*K$9{x_IW`=8p(Ed1|M z$5d1b|35m@EQ!?0;WA0WTn(Z$$>g<#EEckek!%qogj8EkiCPfzNDTXp5bEz#J9Clc zK@t!9RYWa}DIZ^QU*yBH{_-u&{`n4hMij6m`fu2D;rm(;Pm|pT>Hl8&-(a?DA^&fg z|Bnw&k6yew*5x=hx>1|cca$_WOI0|J2DZe zh$kW*UwL5=U*^9$n_gu4mpqBr?H3iq3&mW7)lC}pWY_b8IFO##Vh@A-f2a3a(SNaA z(tlAW4BN37^xp+&yP*Hy)PE!6wpssaQS+Pnw@kx;{=1-2IFSsX|MmK>JF?d2Kh$)}Gkg8}Cir-VX@o|J8r}2e5Vh z8?L#cf5UN6|2yG>kbJ@=*FIjWeV{U(k`a%@KyAbUxj9e;2|1oc5znp$>gi~Ibnt0J z-X@XIQtpde)AO19oMfwOO+-8hZ>{fxv551fue}ucI15r)J*_q#hFngQEV{KoypN>+ z<}l^q&$$2mxBOuKyB+?2_%Lnr{HOLmWrkyv`QLCI