From 7a77147987be2cabef8a9f52eb47a4afb6fa9eb1 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 5 Mar 2020 17:15:55 -0500 Subject: [PATCH] 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 --- news/58.bugfix.rst | 2 + news/59.bugfix.rst | 3 + news/60.bugfix.rst | 1 + news/61.bugfix.rst | 1 + news/62.bugfix.rst | 1 + src/pip_shims/compat.py | 54 ++++++++++-- src/pip_shims/models.py | 3 +- tests/test_instances.py | 187 +++++++++++++++++++++------------------- 8 files changed, 153 insertions(+), 99 deletions(-) create mode 100644 news/58.bugfix.rst create mode 100644 news/59.bugfix.rst create mode 100644 news/60.bugfix.rst create mode 100644 news/61.bugfix.rst create mode 100644 news/62.bugfix.rst diff --git a/news/58.bugfix.rst b/news/58.bugfix.rst new file mode 100644 index 0000000..58e42dc --- /dev/null +++ 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 new file mode 100644 index 0000000..c17a3f5 --- /dev/null +++ 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 new file mode 100644 index 0000000..73448fc --- /dev/null +++ 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 new file mode 100644 index 0000000..55adc20 --- /dev/null +++ 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 new file mode 100644 index 0000000..a6dfa44 --- /dev/null +++ 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 index 07bbec4..7e32520 100644 --- 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 index c420f41..d641c6d 100644 --- 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 index 1d31ca9..07a08e5 100644 --- 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/urllib3@1.23#egg=urllib3" ) - ireq = InstallRequirement.from_editable( - "git+https://github.com/urllib3/urllib3@1.23#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"