Improve retrieval of information for packages with two python specific wheels

poetry/puzzle/provider.py

@@ -35,6 +35,7 @@ from poetry.utils.env import Env
 from poetry.utils.env import EnvCommandError
 from poetry.utils.env import EnvCommandError
 from poetry.utils.setup_reader import SetupReader
 from poetry.utils.setup_reader import SetupReader
 
 
+from poetry.version.markers import MarkerUnion
 from poetry.vcs.git import Git
 from poetry.vcs.git import Git
 
 
 from .exceptions import CompatibilityError
 from .exceptions import CompatibilityError
@@ -516,34 +517,19 @@ class Provider:
             for constraint, _deps in by_constraint.items():
             for constraint, _deps in by_constraint.items():
                 new_markers = []
                 new_markers = []
                 for dep in _deps:
                 for dep in _deps:
-                    pep_508_dep = dep.to_pep_508(False)
-                    if ";" not in pep_508_dep:
+                    marker = dep.marker.without_extras()
+                    if marker.is_empty():
+                        # No marker or only extras
                         continue
                         continue
 
 
-                    markers = pep_508_dep.split(";")[1].strip()
-                    if not markers:
-                        # One of the constraint has no markers
-                        # so this means we don't actually need to merge
-                        new_markers = []
-                        break
-
-                    new_markers.append("({})".format(markers))
+                    new_markers.append(marker)
 
 
                 if not new_markers:
                 if not new_markers:
-                    dependencies += _deps
                     continue
                     continue
 
 
                 dep = _deps[0]
                 dep = _deps[0]
-                new_requirement = "{}; {}".format(
-                    dep.to_pep_508(False).split(";")[0], " or ".join(new_markers)
-                )
-                new_dep = dependency_from_pep_508(new_requirement)
-                if dep.is_optional() and not dep.is_activated():
-                    new_dep.deactivate()
-                else:
-                    new_dep.activate()
-
-                by_constraint[constraint] = [new_dep]
+                dep.marker = dep.marker.union(MarkerUnion(*new_markers))
+                by_constraint[constraint] = [dep]
 
 
                 continue
                 continue
 
 

poetry/repositories/legacy_repository.py

@@ -16,6 +16,7 @@ except ImportError:
 
 
     unescape = HTMLParser().unescape
     unescape = HTMLParser().unescape
 
 
+from collections import defaultdict
 from typing import Generator
 from typing import Generator
 from typing import Optional
 from typing import Optional
 from typing import Union
 from typing import Union
@@ -344,23 +345,15 @@ class LegacyRepository(PyPiRepository):
                     name, version
                     name, version
                 )
                 )
             )
             )
-        urls = {}
+        urls = defaultdict(list)
         hashes = []
         hashes = []
-        default_link = links[0]
         for link in links:
         for link in links:
             if link.is_wheel:
             if link.is_wheel:
-                m = wheel_file_re.match(link.filename)
-                python = m.group("pyver")
-                platform = m.group("plat")
-                if python == "py2.py3" and platform == "any":
-                    urls["bdist_wheel"] = link.url
-            elif link.filename.endswith(".tar.gz"):
-                urls["sdist"] = link.url
-            elif (
-                link.filename.endswith((".zip", ".bz2", ".xz", ".Z", ".tar"))
-                and "sdist" not in urls
+                urls["bdist_wheel"].append(link.url)
+            elif link.filename.endswith(
+                (".tar.gz", ".zip", ".bz2", ".xz", ".Z", ".tar")
             ):
             ):
-                urls["sdist"] = link.url
+                urls["sdist"].append(link.url)
 
 
             hash = link.hash
             hash = link.hash
             if link.hash_name == "sha256":
             if link.hash_name == "sha256":
@@ -368,18 +361,6 @@ class LegacyRepository(PyPiRepository):
 
 
         data["digests"] = hashes
         data["digests"] = hashes
 
 
-        if not urls:
-            if default_link.is_wheel:
-                urls["bdist_wheel"] = default_link.url
-            elif default_link.filename.endswith(".tar.gz"):
-                urls["sdist"] = default_link.url
-            elif (
-                default_link.filename.endswith((".zip", ".bz2")) and "sdist" not in urls
-            ):
-                urls["sdist"] = default_link.url
-            else:
-                return data
-
         info = self._get_info_from_urls(urls)
         info = self._get_info_from_urls(urls)
 
 
         data["summary"] = info["summary"]
         data["summary"] = info["summary"]

poetry/repositories/pypi_repository.py

 import logging
 import logging
 import os
 import os
-import re
 import tarfile
 import tarfile
 import zipfile
 import zipfile
 
 
 import pkginfo
 import pkginfo
 
 
 from bz2 import BZ2File
 from bz2 import BZ2File
+from collections import defaultdict
 from gzip import GzipFile
 from gzip import GzipFile
 from typing import Dict
 from typing import Dict
 from typing import List
 from typing import List
@@ -31,6 +31,7 @@ from requests import session
 from poetry.locations import CACHE_DIR
 from poetry.locations import CACHE_DIR
 from poetry.packages import dependency_from_pep_508
 from poetry.packages import dependency_from_pep_508
 from poetry.packages import Package
 from poetry.packages import Package
+from poetry.packages.utils.link import Link
 from poetry.semver import parse_constraint
 from poetry.semver import parse_constraint
 from poetry.semver import VersionConstraint
 from poetry.semver import VersionConstraint
 from poetry.semver import VersionRange
 from poetry.semver import VersionRange
@@ -39,8 +40,10 @@ from poetry.utils._compat import Path
 from poetry.utils._compat import to_str
 from poetry.utils._compat import to_str
 from poetry.utils.helpers import parse_requires
 from poetry.utils.helpers import parse_requires
 from poetry.utils.helpers import temporary_directory
 from poetry.utils.helpers import temporary_directory
+from poetry.utils.patterns import wheel_file_re
 from poetry.utils.setup_reader import SetupReader
 from poetry.utils.setup_reader import SetupReader
 from poetry.version.markers import InvalidMarker
 from poetry.version.markers import InvalidMarker
+from poetry.version.markers import parse_marker
 
 
 from .exceptions import PackageNotFound
 from .exceptions import PackageNotFound
 from .repository import Repository
 from .repository import Repository
@@ -306,74 +309,14 @@ class PyPiRepository(Repository):
             # or badly set metadata when uploading
             # or badly set metadata when uploading
             # So, we need to make sure there is actually no
             # So, we need to make sure there is actually no
             # dependencies by introspecting packages
             # dependencies by introspecting packages
-            urls = {}
+            urls = defaultdict(list)
             for url in json_data["urls"]:
             for url in json_data["urls"]:
-                # Only get sdist and universal wheels if they exist
+                # Only get sdist and wheels if they exist
                 dist_type = url["packagetype"]
                 dist_type = url["packagetype"]
                 if dist_type not in ["sdist", "bdist_wheel"]:
                 if dist_type not in ["sdist", "bdist_wheel"]:
                     continue
                     continue
 
 
-                if dist_type == "sdist" and "sdist" not in urls:
-                    urls[url["packagetype"]] = url["url"]
-                    continue
-
-                if "bdist_wheel" in urls:
-                    continue
-
-                # If bdist_wheel, check if it's universal
-                filename = url["filename"]
-                if not re.search(r"-py2\.py3-none-any.whl", filename):
-                    continue
-
-                urls[dist_type] = url["url"]
-
-            if "sdist" in urls and "bdist_wheel" not in urls:
-                # If can't found a universal wheel
-                # but we found an sdist, inspect the sdist first
-                info = self._get_info_from_urls(urls)
-                if info["requires_dist"]:
-                    data["requires_dist"] = info["requires_dist"]
-
-                    if not data["requires_python"]:
-                        data["requires_python"] = info["requires_python"]
-
-                    return data
-                else:
-                    del urls["sdist"]
-
-            if not urls:
-                # If we don't have urls, we try to take the first one
-                # we find and go from there
-                if not json_data["urls"]:
-                    return data
-
-                for url in json_data["urls"]:
-                    # Only get sdist and universal wheels if they exist
-                    dist_type = url["packagetype"]
-
-                    if dist_type != "bdist_wheel":
-                        continue
-
-                    urls[url["packagetype"]] = url["url"]
-
-                    break
-
-            if not urls or "bdist_wheel" not in urls:
-                # If we don't have urls, we try to take the first one
-                # we find and go from there
-                if not json_data["urls"]:
-                    return data
-
-                for url in json_data["urls"]:
-                    # Only get sdist and universal wheels if they exist
-                    dist_type = url["packagetype"]
-
-                    if dist_type != "bdist_wheel":
-                        continue
-
-                    urls[url["packagetype"]] = url["url"]
-
-                    break
+                urls[dist_type].append(url["url"])
 
 
             if not urls:
             if not urls:
                 return data
                 return data
@@ -398,27 +341,98 @@ class PyPiRepository(Repository):
 
 
     def _get_info_from_urls(
     def _get_info_from_urls(
         self, urls
         self, urls
-    ):  # type: (Dict[str, str]) -> Dict[str, Union[str, List, None]]
+    ):  # type: (Dict[str, List[str]]) -> Dict[str, Union[str, List, None]]
+        # Checking wheels first as they are more likely to hold
+        # the necessary information
         if "bdist_wheel" in urls:
         if "bdist_wheel" in urls:
-            self._log(
-                "Downloading wheel: {}".format(
-                    urlparse.urlparse(urls["bdist_wheel"]).path.rsplit("/")[-1]
-                ),
-                level="debug",
+            # Check fo a universal wheel
+            wheels = urls["bdist_wheel"]
+
+            universal_wheel = None
+            universal_python2_wheel = None
+            universal_python3_wheel = None
+            platform_specific_wheels = []
+            for wheel in wheels:
+                link = Link(wheel)
+                m = wheel_file_re.match(link.filename)
+                if not m:
+                    continue
+
+                pyver = m.group("pyver")
+                abi = m.group("abi")
+                plat = m.group("plat")
+                if abi == "none" and plat == "any":
+                    # Universal wheel
+                    if pyver == "py2.py3":
+                        # Any Python
+                        universal_wheel = wheel
+                    elif pyver == "py2":
+                        universal_python2_wheel = wheel
+                    else:
+                        universal_python3_wheel = wheel
+                else:
+                    platform_specific_wheels.append(wheel)
+
+            if universal_wheel is not None:
+                return self._get_info_from_wheel(universal_wheel)
+
+            info = {}
+            if universal_python2_wheel and universal_python3_wheel:
+                info = self._get_info_from_wheel(universal_python2_wheel)
+
+                py3_info = self._get_info_from_wheel(universal_python3_wheel)
+                if py3_info["requires_dist"]:
+                    if not info["requires_dist"]:
+                        info["requires_dist"] = py3_info["requires_dist"]
+
+                        return info
+
+                    py2_requires_dist = set(
+                        dependency_from_pep_508(r).to_pep_508()
+                        for r in info["requires_dist"]
                     )
                     )
-            return self._get_info_from_wheel(urls["bdist_wheel"])
+                    py3_requires_dist = set(
+                        dependency_from_pep_508(r).to_pep_508()
+                        for r in py3_info["requires_dist"]
+                    )
+                    base_requires_dist = py2_requires_dist & py3_requires_dist
+                    py2_only_requires_dist = py2_requires_dist - py3_requires_dist
+                    py3_only_requires_dist = py3_requires_dist - py2_requires_dist
+
+                    # Normalizing requires_dist
+                    requires_dist = list(base_requires_dist)
+                    for requirement in py2_only_requires_dist:
+                        dep = dependency_from_pep_508(requirement)
+                        dep.marker = dep.marker.intersect(
+                            parse_marker("python_version == '2.7'")
+                        )
+                        requires_dist.append(dep.to_pep_508())
 
 
-        self._log(
-            "Downloading sdist: {}".format(
-                urlparse.urlparse(urls["sdist"]).path.rsplit("/")[-1]
-            ),
-            level="debug",
+                    for requirement in py3_only_requires_dist:
+                        dep = dependency_from_pep_508(requirement)
+                        dep.marker = dep.marker.intersect(
+                            parse_marker("python_version >= '3'")
                         )
                         )
-        return self._get_info_from_sdist(urls["sdist"])
+                        requires_dist.append(dep.to_pep_508())
+
+                    info["requires_dist"] = sorted(list(set(requires_dist)))
+
+            if info:
+                return info
+
+            if platform_specific_wheels and "sdist" not in urls:
+                # Pick the first wheel available and hope for the best
+                return self._get_info_from_wheel(platform_specific_wheels[0])
+
+        return self._get_info_from_sdist(urls["sdist"][0])
 
 
     def _get_info_from_wheel(
     def _get_info_from_wheel(
         self, url
         self, url
     ):  # type: (str) -> Dict[str, Union[str, List, None]]
     ):  # type: (str) -> Dict[str, Union[str, List, None]]
+        self._log(
+            "Downloading wheel: {}".format(urlparse.urlparse(url).path.rsplit("/")[-1]),
+            level="debug",
+        )
         info = {"summary": "", "requires_python": None, "requires_dist": None}
         info = {"summary": "", "requires_python": None, "requires_dist": None}
 
 
         filename = os.path.basename(urlparse.urlparse(url).path.rsplit("/")[-1])
         filename = os.path.basename(urlparse.urlparse(url).path.rsplit("/")[-1])
@@ -447,6 +461,10 @@ class PyPiRepository(Repository):
     def _get_info_from_sdist(
     def _get_info_from_sdist(
         self, url
         self, url
     ):  # type: (str) -> Dict[str, Union[str, List, None]]
     ):  # type: (str) -> Dict[str, Union[str, List, None]]
+        self._log(
+            "Downloading sdist: {}".format(urlparse.urlparse(url).path.rsplit("/")[-1]),
+            level="debug",
+        )
         info = {"summary": "", "requires_python": None, "requires_dist": None}
         info = {"summary": "", "requires_python": None, "requires_dist": None}
 
 
         filename = os.path.basename(urlparse.urlparse(url).path)
         filename = os.path.basename(urlparse.urlparse(url).path)

tests/repositories/fixtures/legacy/ipython.html

+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Links for ipython</title>
+  </head>
+  <body>
+    <h1>Links for ipython</h1>
+    <a href="https://files.pythonhosted.org/packages/52/19/aadde98d6bde1667d0bf431fb2d22451f880aaa373e0a241c7e7cb5815a0/ipython-5.7.0-py2-none-any.whl#sha256=707d1bbfc81e41e39ead1012af931bec6f80357b87e520af352e539cf5961dc0">ipython-5.7.0-py2-none-any.whl</a><br/>
+    <a href="https://files.pythonhosted.org/packages/c7/b6/03e0b5b0972e6161d16c4cec8d41a20372bd0634f8cb4cc0c984b8a91db6/ipython-5.7.0-py3-none-any.whl#sha256=fc0464e68f9c65cd8c453474b4175432cc29ecb6c83775baedf6dbfcee9275ab">ipython-5.7.0-py3-none-any.whl</a><br/>
+    <a href="https://files.pythonhosted.org/packages/3c/fd/559fead731a29eaa55cc235c8029807b2520976a937c30e9ee603f3bb566/ipython-5.7.0.tar.gz#sha256=8db43a7fb7619037c98626613ff08d03dda9d5d12c84814a4504c78c0da8323c">ipython-5.7.0.tar.gz</a><br/>
+  </body>
+</html>
+<!--SERIAL 4837213-->

tests/repositories/test_legacy_repository.py

@@ -169,3 +169,39 @@ def test_get_package_information_sets_appropriate_python_versions_if_wheels_only
     assert package.name == "futures"
     assert package.name == "futures"
     assert package.version.text == "3.2.0"
     assert package.version.text == "3.2.0"
     assert package.python_versions == ">=2.6, <3"
     assert package.python_versions == ">=2.6, <3"
+
+
+def test_get_package_from_both_py2_and_py3_specific_wheels():
+    repo = MockRepository()
+
+    package = repo.package("ipython", "5.7.0")
+
+    assert "ipython" == package.name
+    assert "5.7.0" == package.version.text
+    assert "*" == package.python_versions
+
+    expected = [
+        Dependency("appnope", "*"),
+        Dependency("backports.shutil-get-terminal-size", "*"),
+        Dependency("colorama", "*"),
+        Dependency("decorator", "*"),
+        Dependency("pathlib2", "*"),
+        Dependency("pexpect", "*"),
+        Dependency("pickleshare", "*"),
+        Dependency("prompt-toolkit", ">=1.0.4,<2.0.0"),
+        Dependency("pygments", "*"),
+        Dependency("setuptools", ">=18.5"),
+        Dependency("simplegeneric", ">0.8"),
+        Dependency("traitlets", ">=4.2"),
+        Dependency("win-unicode-console", ">=0.5"),
+    ]
+    assert expected == package.requires
+
+    assert 'python_version == "2.7"' == str(package.requires[1].marker)
+    assert 'sys_platform == "win32" and python_version < "3.6"' == str(
+        package.requires[12].marker
+    )
+    assert 'python_version == "2.7" or python_version == "3.3"' == str(
+        package.requires[4].marker
+    )
+    assert 'sys_platform != "win32"' == str(package.requires[5].marker)