Fix test failures against pip@master

- Add `wheel_cache` context manager helper for managing global context when creating wheel wheel_cache instances - Add optional `check_supports_wheel` argument to `Resolver.resolve()` call when expected arguments include this one - Inspect `InstallRequirement.ensure_build_location()` method and include `autodelete` kwarg if it is expected - Update argument types passed into `resolver.resolve` which, as of `pip>=20.1` will expect a list of `InstallRequirement` instances - Update `Resolver` import path to point at new location (`resolution.legacy.resolver`) - Add necessary `global_tempdir_manager` contexts in tests - Fix expected `RequirementSet.cleanup()` call after `Resolver.resolve()` which no longer applies due to use of transient `RequirementSet` - Fixes #58 - Fixes #59 - Fixes #60 - Fixes #61 - Fixes #62 Signed-off-by: Dan Ryan <[email protected]>
sarugaku · Mar 5, 2020 · 7a77147 · 7a77147
1 parent c41c537
commit 7a77147
Show file tree

Hide file tree

Showing 8 changed files with 153 additions and 99 deletions.
diff --git a/news/58.bugfix.rst b/news/58.bugfix.rst
@@ -0,0 +1,2 @@
+Added ``wheel_cache`` context manager helper for managing global context when creating wheel wheel_cache instances.
+
diff --git a/news/59.bugfix.rst b/news/59.bugfix.rst
@@ -0,0 +1,3 @@
+Fixed resolution failures due to ``Resolver.resolve`` signature updates in ``pip@master``:
+  - Automatically check for and pass ``check_supports_wheel`` argument to `Resolver.resolve()` when expected
+  - Check whether ``Resolver.resolve()`` expects a ``RequirementSet`` or ``List[InstallRequirement]`` and pass the appropriate input
diff --git a/news/60.bugfix.rst b/news/60.bugfix.rst
@@ -0,0 +1 @@
+Fixed requirement build failures due to new ``autodelete: bool`` required argument in ``InstallRequirement.ensure_build_location``.
diff --git a/news/61.bugfix.rst b/news/61.bugfix.rst
@@ -0,0 +1 @@
+Updated ``Resolver`` import path to point at new location (``legacy_resolve`` -> ``resolution.legacy.resolver``).
diff --git a/news/62.bugfix.rst b/news/62.bugfix.rst
@@ -0,0 +1 @@
+Fixed ``AttributeError`` caused by failed ``RequirementSet.cleanup()`` calls after ``Resolver.resolve()`` which is no longer valid in ``pip>=20.1``.
diff --git a/src/pip_shims/compat.py b/src/pip_shims/compat.py
@@ -2,7 +2,7 @@
 """
 Backports and helper functionality to support using new functionality.
 """
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function
 
 import atexit
 import contextlib
@@ -350,6 +350,27 @@ def ensure_resolution_dirs(**kwargs):
             yield kwargs
 
 
+@contextlib.contextmanager
+def wheel_cache(
+    wheel_cache_provider,  # type: TShimmedFunc
+    tempdir_manager_provider,  # type: TShimmedFunc
+    cache_dir,  # type: str
+    format_control=None,  # type: Any
+    format_control_provider=None,  # type: Optional[TShimmedFunc]
+):
+    tempdir_manager_provider = resolve_possible_shim(tempdir_manager_provider)
+    wheel_cache_provider = resolve_possible_shim(wheel_cache_provider)
+    format_control_provider = resolve_possible_shim(format_control_provider)
+    if not format_control and not format_control_provider:
+        raise TypeError("Format control or provider needed for wheel cache!")
+    if not format_control:
+        format_control = format_control_provider(None, None)
+    with ExitStack() as ctx:
+        ctx.enter_context(tempdir_manager_provider())
+        wheel_cache = wheel_cache_provider(cache_dir, format_control)
+        yield wheel_cache
+
+
 def partial_command(shimmed_path, cmd_mapping=None):
     # type: (Type, Optional[TShimmedCmdDict]) -> Union[Type[TCommandInstance], functools.partial]
     """
@@ -1022,7 +1043,7 @@ def get_resolver(
         assert isinstance(install_cmd_provider, (type, functools.partial))
         install_cmd = install_cmd_provider()
     if options is None and install_cmd is not None:
-        options = install_cmd.parser.parse_args([])  # type: ignore
+        options, _ = install_cmd.parser.parse_args([])  # type: ignore
     for arg, val in install_cmd_dependency_map.items():
         if arg not in required_args:
             continue
@@ -1072,7 +1093,7 @@ def get_resolver(
     return resolver_fn(**resolver_kwargs)  # type: ignore
 
 
-def resolve(
+def resolve(  # noqa:C901
     ireq,  # type: TInstallRequirement
     reqset_provider=None,  # type: Optional[TShimmedFunc]
     req_tracker_provider=None,  # type: Optional[TShimmedFunc]
@@ -1102,6 +1123,7 @@ def resolve(
     wheel_download_dir=None,  # type: Optional[str]
     wheel_cache=None,  # type: Optional[TWheelCache]
     require_hashes=None,  # type: bool
+    check_supported_wheels=True,  # type: bool
 ):
     # (...) -> Set[TInstallRequirement]
     """
@@ -1163,6 +1185,8 @@ def resolve(
     :param Optional[TWheelCache] wheel_cache: The wheel cache to use, defaults to None
     :param bool require_hashes: Whether to require hashes when resolving. Defaults to
         False.
+    :param bool check_supported_wheels: Whether to check support of wheels before including
+        them in resolution.
     :return: A dictionary mapping requirements to corresponding
         :class:`~pip._internal.req.req_install.InstallRequirement`s
     :rtype: :class:`~pip._internal.req.req_install.InstallRequirement`
@@ -1239,7 +1263,9 @@ def resolve(
             kwargs["cache_dir"], format_control
         )  # type: ignore
         ireq.is_direct = True  # type: ignore
-        ireq.build_location(kwargs["build_dir"])  # type: ignore
+        build_location_kwargs = {"build_dir": kwargs["build_dir"], "autodelete": True}
+        call_function_with_correct_args(ireq.build_location, **build_location_kwargs)
+        # ireq.build_location(kwargs["build_dir"])  # type: ignore
         if reqset_provider is None:
             raise TypeError(
                 "cannot resolve without a requirement set provider... failed!"
@@ -1292,11 +1318,23 @@ def resolve(
             wheel_cache=wheel_cache,
             **resolver_args
         )  # type: ignore
-        reqset.add_requirement(ireq)
         resolver.require_hashes = kwargs.get("require_hashes", False)  # type: ignore
-        resolver.resolve(reqset)  # type: ignore
-        results = reqset.requirements
-        reqset.cleanup_files()
+        _, required_resolver_args = get_method_args(resolver.resolve)
+        resolver_args = []
+        if "requirement_set" in required_resolver_args.args:
+            reqset.add_requirement(ireq)
+            resolver_args.append(reqset)
+        elif "root_reqs" in required_resolver_args.args:
+            resolver_args.append([ireq])
+        if "check_supported_wheels" in required_resolver_args.args:
+            resolver_args.append(check_supported_wheels)
+        result_reqset = resolver.resolve(*resolver_args)  # type: ignore
+        if result_reqset is None:
+            result_reqset = reqset
+        results = result_reqset.requirements
+        cleanup_fn = getattr(reqset, "cleanup_files", None)
+        if cleanup_fn is not None:
+            cleanup_fn()
         return results
 
 

diff --git a/src/pip_shims/models.py b/src/pip_shims/models.py
@@ -1048,7 +1048,8 @@ def import_pip():
 
 Resolver = ShimmedPathCollection("Resolver", ImportTypes.CLASS)
 Resolver.create_path("resolve.Resolver", "7.0.0", "19.1.1")
-Resolver.create_path("legacy_resolve.Resolver", "19.1.2", "9999")
+Resolver.create_path("legacy_resolve.Resolver", "19.1.2", "20.0.89999")
+Resolver.create_path("resolution.legacy.resolver.Resolver", "20.0.99999", "99999")
 
 SafeFileCache = ShimmedPathCollection("SafeFileCache", ImportTypes.CLASS)
 SafeFileCache.create_path("network.cache.SafeFileCache", "19.3.0", "9999")

diff --git a/tests/test_instances.py b/tests/test_instances.py
@@ -304,8 +304,8 @@ def test_resolution(tmpdir, PipCommand):
     wheel_download_dir = CACHE_DIR.mkdir("wheels")
     pkg_download_dir = CACHE_DIR.mkdir("pkgs")
     results = None
-    wheel_cache = WheelCache(USER_CACHE_DIR, FormatControl(None, None))
     if parse_version(pip_version) < parse_version("10.0"):
+        wheel_cache = WheelCache(USER_CACHE_DIR, FormatControl(None, None))
         reqset = RequirementSet(
             build_dir.strpath,
             source_dir.strpath,
@@ -346,16 +346,18 @@ def test_resolution(tmpdir, PipCommand):
             )
         else:
             resolver_kwargs["session"] = session
-        if parse_version(pip_version) >= parse_version("19.3"):
-            make_install_req = partial(
-                install_req_from_req_string,
-                isolated=False,
-                wheel_cache=wheel_cache,
-                # use_pep517=use_pep517,
-            )
-            resolver_kwargs["make_install_req"] = make_install_req
-        else:
-            resolver_kwargs.update({"isolated": False, "wheel_cache": wheel_cache})
+        with global_tempdir_manager():
+            wheel_cache = WheelCache(USER_CACHE_DIR, FormatControl(None, None))
+            if parse_version(pip_version) >= parse_version("19.3"):
+                make_install_req = partial(
+                    install_req_from_req_string,
+                    isolated=False,
+                    wheel_cache=wheel_cache,
+                    # use_pep517=use_pep517,
+                )
+                resolver_kwargs["make_install_req"] = make_install_req
+            else:
+                resolver_kwargs.update({"isolated": False, "wheel_cache": wheel_cache})
         resolver = None
         preparer = None
         with global_tempdir_manager(), get_requirement_tracker() as req_tracker:
@@ -372,7 +374,10 @@ def test_resolution(tmpdir, PipCommand):
             resolver = Resolver(**resolver_kwargs)
             resolver.require_hashes = False
             results = resolver._resolve_one(reqset, ireq)
-            reqset.cleanup_files()
+            try:
+                reqset.cleanup_files()
+            except AttributeError:
+                pass
     results = set(results)
     result_names = [r.name for r in results]
     assert "chardet" in result_names
@@ -401,8 +406,9 @@ def test_frozen_req():
 
 def test_wheel_cache():
     fc = FormatControl(None, None)
-    w = WheelCache(USER_CACHE_DIR, fc)
-    assert w.__class__.__name__ == "WheelCache"
+    with global_tempdir_manager():
+        w = WheelCache(USER_CACHE_DIR, fc)
+        assert w.__class__.__name__ == "WheelCache"
 
 
 def test_vcs_support():
@@ -481,86 +487,87 @@ def test_wheelbuilder(tmpdir, PipCommand):
     source_dir = tmpdir.mkdir("source_dir")
     download_dir = tmpdir.mkdir("download_dir")
     wheel_download_dir = CACHE_DIR.mkdir("wheels")
-    wheel_cache = WheelCache(USER_CACHE_DIR, FormatControl(None, None))
-    kwargs = {
-        "build_dir": build_dir.strpath,
-        "src_dir": source_dir.strpath,
-        "download_dir": download_dir.strpath,
-        "wheel_download_dir": wheel_download_dir.strpath,
-        "wheel_cache": wheel_cache,
-    }
-    if parse_version(pip_version) > parse_version("19.99.99"):
-        kwargs.update(
-            {"finder": finder, "require_hashes": False, "use_user_site": False,}
+    with global_tempdir_manager():
+        wheel_cache = WheelCache(USER_CACHE_DIR, FormatControl(None, None))
+        kwargs = {
+            "build_dir": build_dir.strpath,
+            "src_dir": source_dir.strpath,
+            "download_dir": download_dir.strpath,
+            "wheel_download_dir": wheel_download_dir.strpath,
+            "wheel_cache": wheel_cache,
+        }
+        if parse_version(pip_version) > parse_version("19.99.99"):
+            kwargs.update(
+                {"finder": finder, "require_hashes": False, "use_user_site": False,}
+            )
+        ireq = InstallRequirement.from_editable(
+            "git+https://github.com/urllib3/[email protected]#egg=urllib3"
         )
-    ireq = InstallRequirement.from_editable(
-        "git+https://github.com/urllib3/[email protected]#egg=urllib3"
-    )
-    ireq.populate_link(finder, False, False)
-    ireq.ensure_has_source_dir(kwargs["src_dir"])
-    # Ensure the remote artifact is downloaded locally. For wheels, it is
-    # enough to just download because we'll use them directly. For an sdist,
-    # we need to unpack so we can build it.
-    unpack_kwargs = {
-        "session": session,
-        "hashes": ireq.hashes(True),
-        "link": ireq.link,
-        "location": ireq.source_dir,
-        "download_dir": kwargs["download_dir"],
-    }
-    if parse_version(pip_version) < parse_version("19.2.0"):
-        unpack_kwargs["only_download"] = ireq.is_wheel
-    if parse_version(pip_version) >= parse_version("10"):
-        unpack_kwargs["progress_bar"] = "off"
-    if not is_file_url(ireq.link):
-        shim_unpack(**unpack_kwargs)
-    output_file = None
-    ireq.is_direct = True
-    build_wheel_kwargs = {
-        "finder": finder,
-        "req": ireq,
-        "output_dir": output_dir.strpath,
-        "session": session,
-        "build_dir": build_dir,
-        "src_dir": source_dir,
-        "download_dir": download_dir,
-        "wheel_download_dir": wheel_download_dir,
-        "wheel_cache": wheel_cache,
-    }
-    if parse_version(pip_version) < parse_version("10"):
-        kwargs["session"] = session
-        reqset = RequirementSet(**kwargs)
-        build_wheel_kwargs["reqset"] = reqset
-        # XXX: We can skip all of the intervening steps and go straight to the
-        # wheel generation bit
+        ireq.populate_link(finder, False, False)
+        ireq.ensure_has_source_dir(kwargs["src_dir"])
+        # Ensure the remote artifact is downloaded locally. For wheels, it is
+        # enough to just download because we'll use them directly. For an sdist,
+        # we need to unpack so we can build it.
+        unpack_kwargs = {
+            "session": session,
+            "hashes": ireq.hashes(True),
+            "link": ireq.link,
+            "location": ireq.source_dir,
+            "download_dir": kwargs["download_dir"],
+        }
+        if parse_version(pip_version) < parse_version("19.2.0"):
+            unpack_kwargs["only_download"] = ireq.is_wheel
+        if parse_version(pip_version) >= parse_version("10"):
+            unpack_kwargs["progress_bar"] = "off"
+        if not is_file_url(ireq.link):
+            shim_unpack(**unpack_kwargs)
+        output_file = None
         ireq.is_direct = True
-        builder = WheelBuilder(reqset, finder)
-        output_file = builder._build_one(ireq, output_dir.strpath)
-    else:
+        build_wheel_kwargs = {
+            "finder": finder,
+            "req": ireq,
+            "output_dir": output_dir.strpath,
+            "session": session,
+            "build_dir": build_dir,
+            "src_dir": source_dir,
+            "download_dir": download_dir,
+            "wheel_download_dir": wheel_download_dir,
+            "wheel_cache": wheel_cache,
+        }
+        if parse_version(pip_version) < parse_version("10"):
+            kwargs["session"] = session
+            reqset = RequirementSet(**kwargs)
+            build_wheel_kwargs["reqset"] = reqset
+            # XXX: We can skip all of the intervening steps and go straight to the
+            # wheel generation bit
+            ireq.is_direct = True
+            builder = WheelBuilder(reqset, finder)
+            output_file = builder._build_one(ireq, output_dir.strpath)
+        else:
 
-        kwargs.update(
-            {"progress_bar": "off", "build_isolation": False,}
-        )
-        if parse_version(pip_version) > parse_version("19.99.99"):
-            downloader = Downloader(session=session, progress_bar="off")
-            kwargs.pop("progress_bar", None)
-            kwargs["downloader"] = downloader
             kwargs.update(
-                {"use_user_site": False, "require_hashes": False,}
+                {"progress_bar": "off", "build_isolation": False,}
             )
-        wheel_cache = kwargs.pop("wheel_cache")
-        with get_requirement_tracker() as req_tracker:
-            if req_tracker:
-                kwargs["req_tracker"] = req_tracker
-            preparer = RequirementPreparer(**kwargs)
-            builder_args = [preparer, wheel_cache]
-            if parse_version(pip_version) < parse_version("19.3"):
-                builder_args = [finder] + builder_args
-            if parse_version(pip_version) < parse_version("19.3.9"):
-                builder = WheelBuilder(*builder_args)
-                output_file = builder._build_one(ireq, output_dir.strpath)
-            else:
-                output_file = build_one(ireq, output_dir.strpath, [], [])
+            if parse_version(pip_version) > parse_version("19.99.99"):
+                downloader = Downloader(session=session, progress_bar="off")
+                kwargs.pop("progress_bar", None)
+                kwargs["downloader"] = downloader
+                kwargs.update(
+                    {"use_user_site": False, "require_hashes": False,}
+                )
+            wheel_cache = kwargs.pop("wheel_cache")
+            with get_requirement_tracker() as req_tracker:
+                if req_tracker:
+                    kwargs["req_tracker"] = req_tracker
+                preparer = RequirementPreparer(**kwargs)
+                builder_args = [preparer, wheel_cache]
+                if parse_version(pip_version) < parse_version("19.3"):
+                    builder_args = [finder] + builder_args
+                if parse_version(pip_version) < parse_version("19.3.9"):
+                    builder = WheelBuilder(*builder_args)
+                    output_file = builder._build_one(ireq, output_dir.strpath)
+                else:
+                    output_file = build_one(ireq, output_dir.strpath, [], [])
     # XXX: skipping to here is functionally the same and should pass all tests
     # output_file = build_wheel(**build_wheel_kwargs)
     assert output_file, output_file
@@ -618,4 +625,4 @@ def test_stdlib_pkgs():
 def test_get_session():
     cmd = InstallCommand()
     sess = get_session(install_cmd=cmd)
-    assert type(sess).__base__.__qualname__ == "Session"
+    assert type(sess).__base__.__name__ == "Session"
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Added ``wheel_cache`` context manager helper for managing global context when creating wheel wheel_cache instances.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fixed requirement build failures due to new ``autodelete: bool`` required argument in ``InstallRequirement.ensure_build_location``.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Updated ``Resolver`` import path to point at new location (``legacy_resolve`` -> ``resolution.legacy.resolver``).
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fixed ``AttributeError`` caused by failed ``RequirementSet.cleanup()`` calls after ``Resolver.resolve()`` which is no longer valid in ``pip>=20.1``.