From 106bd0d77fbe5f22c9a67f74ca74b3e97513aba9 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Sun, 29 Mar 2020 12:00:14 +0530 Subject: [PATCH 01/12] Raise an exception if revision is empty in git url --- news/7402.bugfix | 1 + src/pip/_internal/vcs/versioncontrol.py | 6 ++++++ tests/unit/test_vcs.py | 15 +++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 news/7402.bugfix diff --git a/news/7402.bugfix b/news/7402.bugfix new file mode 100644 index 00000000000..8c8372914aa --- /dev/null +++ b/news/7402.bugfix @@ -0,0 +1 @@ +Raise an exception if revision part of URL is empty for URL used in VCS support diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index da53827cd46..8bfa1cd5772 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/src/pip/_internal/vcs/versioncontrol.py @@ -436,6 +436,12 @@ def get_url_rev_and_auth(cls, url): rev = None if '@' in path: path, rev = path.rsplit('@', 1) + if not rev: + raise ValueError( + "The URL {!r} has an empty revision (after @) " + "which is not supported. Include a revision after @ " + "or remove @ from the URL.".format(url) + ) url = urllib_parse.urlunsplit((scheme, netloc, path, query, '')) return url, rev, user_pass diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 42fc43d6855..92e1c0e345d 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -292,6 +292,21 @@ def test_version_control__get_url_rev_and_auth__missing_plus(url): assert 'malformed VCS url' in str(excinfo.value) +@pytest.mark.parametrize('url', [ + # Test a URL with revision part as empty. + 'git+https://github.com/MyUser/myProject.git@#egg=py_pkg', +]) +def test_version_control__get_url_rev_and_auth__no_revision(url): + """ + Test passing a URL to VersionControl.get_url_rev_and_auth() with + empty revision + """ + with pytest.raises(ValueError) as excinfo: + VersionControl.get_url_rev_and_auth(url) + + assert 'an empty revision (after @)' in str(excinfo.value) + + @pytest.mark.parametrize('url, expected', [ # Test http. ('bzr+http://bzr.myproject.org/MyProject/trunk/#egg=MyProject', From 0d2ca67729344adb5514ad7d1be7e1850c3d6be6 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Mon, 30 Mar 2020 22:29:40 +0530 Subject: [PATCH 02/12] Changed ValueError to InstallationError --- src/pip/_internal/vcs/versioncontrol.py | 4 ++-- tests/unit/test_vcs.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index 8bfa1cd5772..71b4650a252 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/src/pip/_internal/vcs/versioncontrol.py @@ -11,7 +11,7 @@ from pip._vendor import pkg_resources from pip._vendor.six.moves.urllib import parse as urllib_parse -from pip._internal.exceptions import BadCommand +from pip._internal.exceptions import BadCommand, InstallationError from pip._internal.utils.compat import samefile from pip._internal.utils.misc import ( ask_path_exists, @@ -437,7 +437,7 @@ def get_url_rev_and_auth(cls, url): if '@' in path: path, rev = path.rsplit('@', 1) if not rev: - raise ValueError( + raise InstallationError( "The URL {!r} has an empty revision (after @) " "which is not supported. Include a revision after @ " "or remove @ from the URL.".format(url) diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 92e1c0e345d..590cb5c0b75 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -5,7 +5,7 @@ from mock import patch from pip._vendor.packaging.version import parse as parse_version -from pip._internal.exceptions import BadCommand +from pip._internal.exceptions import BadCommand, InstallationError from pip._internal.utils.misc import hide_url, hide_value from pip._internal.vcs import make_vcs_requirement_url from pip._internal.vcs.bazaar import Bazaar @@ -301,7 +301,7 @@ def test_version_control__get_url_rev_and_auth__no_revision(url): Test passing a URL to VersionControl.get_url_rev_and_auth() with empty revision """ - with pytest.raises(ValueError) as excinfo: + with pytest.raises(InstallationError) as excinfo: VersionControl.get_url_rev_and_auth(url) assert 'an empty revision (after @)' in str(excinfo.value) From 6fbf80a9f9ba031a188737454bf90c1607cce99f Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Mon, 30 Mar 2020 15:54:02 +0530 Subject: [PATCH 03/12] Remove vcs urls pertaining to git protocol from docs --- docs/html/reference/pip_install.rst | 16 +++++++++------- news/1983.doc | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 news/1983.doc diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index bd61151d712..6600693ca94 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -402,28 +402,30 @@ Git ^^^ pip currently supports cloning over ``git``, ``git+http``, ``git+https``, -``git+ssh``, ``git+git`` and ``git+file``: +``git+ssh``, ``git+git`` and ``git+file``, but note that the ``git``, ``git+git``, +and ``git+http`` are not recommended due to their lack of security. +(The former two uses `the Git Protocol.`_) Here are the supported forms:: - [-e] git://git.example.com/MyProject#egg=MyProject [-e] git+http://git.example.com/MyProject#egg=MyProject [-e] git+https://git.example.com/MyProject#egg=MyProject [-e] git+ssh://git.example.com/MyProject#egg=MyProject - [-e] git+git://git.example.com/MyProject#egg=MyProject [-e] git+file:///home/user/projects/MyProject#egg=MyProject Passing a branch name, a commit hash, a tag name or a git ref is possible like so:: - [-e] git://git.example.com/MyProject.git@master#egg=MyProject - [-e] git://git.example.com/MyProject.git@v1.0#egg=MyProject - [-e] git://git.example.com/MyProject.git@da39a3ee5e6b4b0d3255bfef95601890afd80709#egg=MyProject - [-e] git://git.example.com/MyProject.git@refs/pull/123/head#egg=MyProject + [-e] git+https://git.example.com/MyProject.git@master#egg=MyProject + [-e] git+https://git.example.com/MyProject.git@v1.0#egg=MyProject + [-e] git+https://git.example.com/MyProject.git@da39a3ee5e6b4b0d3255bfef95601890afd80709#egg=MyProject + [-e] git+https://git.example.com/MyProject.git@refs/pull/123/head#egg=MyProject When passing a commit hash, specifying a full hash is preferable to a partial hash because a full hash allows pip to operate more efficiently (e.g. by making fewer network calls). +.. _`the Git Protocol.`: https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols + Mercurial ^^^^^^^^^ diff --git a/news/1983.doc b/news/1983.doc new file mode 100644 index 00000000000..58b85ac1fe2 --- /dev/null +++ b/news/1983.doc @@ -0,0 +1 @@ +Remove VCS URLs pertaining to the Git protocol from docs From c5908bd222b6186059c43d2d170307e63636e5ab Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 31 Mar 2020 18:45:05 +0530 Subject: [PATCH 04/12] Use a code block for denoting directory structure --- docs/html/reference/pip_install.rst | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index bd61151d712..8348f9da387 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -384,16 +384,14 @@ where ``setup.py`` is not in the root of project, the "subdirectory" component is used. The value of the "subdirectory" component should be a path starting from the root of the project to where ``setup.py`` is located. -So if your repository layout is: - - - pkg_dir/ - - - setup.py # setup.py for package ``pkg`` - - some_module.py - - other_dir/ - - - some_file - - some_other_file +So if your repository layout is:: + + pkg_dir + ├── setup.py # setup.py for package "pkg" + └── some_module.py + other_dir + └── some_file + some_other_file You'll need to use ``pip install -e "vcs+protocol://repo_url/#egg=pkg&subdirectory=pkg_dir"``. From 59df53690604ebed67f20977d76a9f96ece03918 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Tue, 31 Mar 2020 20:08:47 +0530 Subject: [PATCH 05/12] Update newsfile message --- news/7402.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/7402.bugfix b/news/7402.bugfix index 8c8372914aa..91eb085f5bc 100644 --- a/news/7402.bugfix +++ b/news/7402.bugfix @@ -1 +1 @@ -Raise an exception if revision part of URL is empty for URL used in VCS support +Reject VCS URLs with an empty revision. From 91444f546e0542a2e7414432de00a773a895902a Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 31 Mar 2020 20:13:15 +0530 Subject: [PATCH 06/12] Avoid Sphinx from justify-expanding code block --- docs/html/reference/pip_install.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index 8348f9da387..1bd8e1293c0 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -384,7 +384,7 @@ where ``setup.py`` is not in the root of project, the "subdirectory" component is used. The value of the "subdirectory" component should be a path starting from the root of the project to where ``setup.py`` is located. -So if your repository layout is:: +If your repository layout is:: pkg_dir ├── setup.py # setup.py for package "pkg" @@ -393,7 +393,9 @@ So if your repository layout is:: └── some_file some_other_file -You'll need to use ``pip install -e "vcs+protocol://repo_url/#egg=pkg&subdirectory=pkg_dir"``. +Then, to install from this repository, the syntax would be:: + + $ pip install -e "vcs+protocol://repo_url/#egg=pkg&subdirectory=pkg_dir" Git From 9af42c27e806779db32757533625651bb49a3f63 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Tue, 31 Mar 2020 20:34:21 +0530 Subject: [PATCH 07/12] Clarification on removed urls --- docs/html/reference/pip_install.rst | 12 ++++++++---- news/1983.doc | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index 6600693ca94..7b0cebbacf7 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -402,9 +402,13 @@ Git ^^^ pip currently supports cloning over ``git``, ``git+http``, ``git+https``, -``git+ssh``, ``git+git`` and ``git+file``, but note that the ``git``, ``git+git``, -and ``git+http`` are not recommended due to their lack of security. -(The former two uses `the Git Protocol.`_) +``git+ssh``, ``git+git`` and ``git+file``. + +.. warning:: + + Note that the ``git``, ``git+git``,and ``git+http`` are not recommended. + (The former two use `the Git Protocol`_, which lacks authentication, and HTTP is + insecure due to lack of TLS based encryption) Here are the supported forms:: @@ -424,7 +428,7 @@ When passing a commit hash, specifying a full hash is preferable to a partial hash because a full hash allows pip to operate more efficiently (e.g. by making fewer network calls). -.. _`the Git Protocol.`: https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols +.. _`the Git Protocol`: https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols Mercurial ^^^^^^^^^ diff --git a/news/1983.doc b/news/1983.doc index 58b85ac1fe2..9766ebb571d 100644 --- a/news/1983.doc +++ b/news/1983.doc @@ -1 +1,2 @@ -Remove VCS URLs pertaining to the Git protocol from docs +Emphasize that VCS URLs using git, git+git and git+http are insecure due to +lack of authentication and encryption From 2a8f5705b20102b8c594ccaa7dcba5e976b06a2c Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Tue, 31 Mar 2020 15:35:29 -0500 Subject: [PATCH 08/12] combine yaml test functionality into single module --- tests/functional/test_yaml.py | 56 +++++++++++++++++++++++++++++++---- tests/lib/yaml_helpers.py | 43 --------------------------- 2 files changed, 50 insertions(+), 49 deletions(-) delete mode 100644 tests/lib/yaml_helpers.py diff --git a/tests/functional/test_yaml.py b/tests/functional/test_yaml.py index eab11e2a42a..2a1b93f16db 100644 --- a/tests/functional/test_yaml.py +++ b/tests/functional/test_yaml.py @@ -1,15 +1,17 @@ -"""Tests for the resolver +""" +Tests for the resolver """ import os import re import pytest +import yaml from tests.lib import DATA_DIR, create_basic_wheel_for_package, path_to_url -from tests.lib.yaml_helpers import generate_yaml_tests, id_func -_conflict_finder_re = re.compile( + +conflict_finder_re = re.compile( # Conflicting Requirements: \ # A 1.0.0 requires B == 2.0.0, C 1.0.0 requires B == 1.0.0. r""" @@ -24,7 +26,49 @@ ) -def _convert_to_dict(string): +def generate_yaml_tests(directory): + """ + Generate yaml test cases from the yaml files in the given directory + """ + for yml_file in directory.glob("*/*.yml"): + data = yaml.safe_load(yml_file.read_text()) + assert "cases" in data, "A fixture needs cases to be used in testing" + + # Strip the parts of the directory to only get a name without + # extension and resolver directory + base_name = str(yml_file)[len(str(directory)) + 1:-4] + + base = data.get("base", {}) + cases = data["cases"] + + for i, case_template in enumerate(cases): + case = base.copy() + case.update(case_template) + + case[":name:"] = base_name + if len(cases) > 1: + case[":name:"] += "-" + str(i) + + if case.pop("skip", False): + case = pytest.param(case, marks=pytest.mark.xfail) + + yield case + + +def id_func(param): + """ + Give a nice parameter name to the generated function parameters + """ + if isinstance(param, dict) and ":name:" in param: + return param[":name:"] + + retval = str(param) + if len(retval) > 25: + retval = retval[:20] + "..." + retval[-2:] + return retval + + +def convert_to_dict(string): def stripping_split(my_str, splitwith, count=None): if count is None: @@ -89,7 +133,7 @@ def handle_install_request(script, requirement): message = result.stderr.rsplit("\n", 1)[-1] # XXX: There might be a better way than parsing the message - for match in re.finditer(message, _conflict_finder_re): + for match in re.finditer(message, conflict_finder_re): di = match.groupdict() retval["conflicting"].append( { @@ -119,7 +163,7 @@ def test_yaml_based(script, case): # XXX: This doesn't work because this isn't making an index of files. for package in available: if isinstance(package, str): - package = _convert_to_dict(package) + package = convert_to_dict(package) assert isinstance(package, dict), "Needs to be a dictionary" diff --git a/tests/lib/yaml_helpers.py b/tests/lib/yaml_helpers.py deleted file mode 100644 index 73770632099..00000000000 --- a/tests/lib/yaml_helpers.py +++ /dev/null @@ -1,43 +0,0 @@ -""" -""" - -import pytest -import yaml - - -def generate_yaml_tests(directory): - for yml_file in directory.glob("*/*.yml"): - data = yaml.safe_load(yml_file.read_text()) - assert "cases" in data, "A fixture needs cases to be used in testing" - - # Strip the parts of the directory to only get a name without - # extension and resolver directory - base_name = str(yml_file)[len(str(directory)) + 1:-4] - - base = data.get("base", {}) - cases = data["cases"] - - for i, case_template in enumerate(cases): - case = base.copy() - case.update(case_template) - - case[":name:"] = base_name - if len(cases) > 1: - case[":name:"] += "-" + str(i) - - if case.pop("skip", False): - case = pytest.param(case, marks=pytest.mark.xfail) - - yield case - - -def id_func(param): - """Give a nice parameter name to the generated function parameters - """ - if isinstance(param, dict) and ":name:" in param: - return param[":name:"] - - retval = str(param) - if len(retval) > 25: - retval = retval[:20] + "..." + retval[-2:] - return retval From 9de02c2b41a53c0eb7d0bf1ca4293e33b9c74b77 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Tue, 31 Mar 2020 15:47:43 -0500 Subject: [PATCH 09/12] better naming of constant --- tests/functional/test_yaml.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_yaml.py b/tests/functional/test_yaml.py index 2a1b93f16db..b2fc4539a63 100644 --- a/tests/functional/test_yaml.py +++ b/tests/functional/test_yaml.py @@ -11,7 +11,7 @@ from tests.lib import DATA_DIR, create_basic_wheel_for_package, path_to_url -conflict_finder_re = re.compile( +_conflict_finder_pat = re.compile( # Conflicting Requirements: \ # A 1.0.0 requires B == 2.0.0, C 1.0.0 requires B == 1.0.0. r""" @@ -133,7 +133,7 @@ def handle_install_request(script, requirement): message = result.stderr.rsplit("\n", 1)[-1] # XXX: There might be a better way than parsing the message - for match in re.finditer(message, conflict_finder_re): + for match in re.finditer(message, _conflict_finder_pat): di = match.groupdict() retval["conflicting"].append( { From 83ba23989de94820166a6a330b3efce882dc15e5 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Tue, 31 Mar 2020 15:55:16 -0500 Subject: [PATCH 10/12] remove extra whitespace --- tests/functional/test_yaml.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/test_yaml.py b/tests/functional/test_yaml.py index b2fc4539a63..cef76d2ca69 100644 --- a/tests/functional/test_yaml.py +++ b/tests/functional/test_yaml.py @@ -10,7 +10,6 @@ from tests.lib import DATA_DIR, create_basic_wheel_for_package, path_to_url - _conflict_finder_pat = re.compile( # Conflicting Requirements: \ # A 1.0.0 requires B == 2.0.0, C 1.0.0 requires B == 1.0.0. From 8f0dbec5734c5197c3b7070987814b584e3f31a6 Mon Sep 17 00:00:00 2001 From: Devesh Kumar Singh Date: Wed, 1 Apr 2020 19:38:41 +0530 Subject: [PATCH 11/12] Updated clarification on unsafe urls --- docs/html/reference/pip_install.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/html/reference/pip_install.rst b/docs/html/reference/pip_install.rst index 71755730328..08149a76773 100644 --- a/docs/html/reference/pip_install.rst +++ b/docs/html/reference/pip_install.rst @@ -406,9 +406,9 @@ pip currently supports cloning over ``git``, ``git+http``, ``git+https``, .. warning:: - Note that the ``git``, ``git+git``,and ``git+http`` are not recommended. - (The former two use `the Git Protocol`_, which lacks authentication, and HTTP is - insecure due to lack of TLS based encryption) + Note that the use of ``git``, ``git+git``, and ``git+http`` is discouraged. + The former two use `the Git Protocol`_, which lacks authentication, and HTTP is + insecure due to lack of TLS based encryption. Here are the supported forms:: From 209c74f6902e32c89977555f59adc883e80a36ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Sun, 29 Mar 2020 17:19:34 +0700 Subject: [PATCH 12/12] Use better temporary files mechanism --- news/7699.bugfix | 2 + src/pip/_internal/operations/install/wheel.py | 54 +++++++++---------- src/pip/_internal/utils/filesystem.py | 16 ++++-- tests/unit/test_wheel.py | 2 +- 4 files changed, 40 insertions(+), 34 deletions(-) create mode 100644 news/7699.bugfix diff --git a/news/7699.bugfix b/news/7699.bugfix new file mode 100644 index 00000000000..51dbef88fda --- /dev/null +++ b/news/7699.bugfix @@ -0,0 +1,2 @@ +Use better mechanism for handling temporary files, when recording metadata +about installed files (RECORD) and the installer (INSTALLER). diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 329d90c67e6..e66d12b4bf0 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -27,6 +27,7 @@ from pip._internal.exceptions import InstallationError from pip._internal.locations import get_major_minor_version +from pip._internal.utils.filesystem import adjacent_tmp_file, replace from pip._internal.utils.misc import captured_stdout, ensure_dir, hash_file from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -36,7 +37,7 @@ if MYPY_CHECK_RUNNING: from email.message import Message from typing import ( - Dict, List, Optional, Sequence, Tuple, IO, Text, Any, + Dict, List, Optional, Sequence, Tuple, Any, Iterable, Callable, Set, ) @@ -64,15 +65,15 @@ def rehash(path, blocksize=1 << 20): return (digest, str(length)) # type: ignore -def open_for_csv(name, mode): - # type: (str, Text) -> IO[Any] - if sys.version_info[0] < 3: - nl = {} # type: Dict[str, Any] - bin = 'b' +def csv_io_kwargs(mode): + # type: (str) -> Dict[str, Any] + """Return keyword arguments to properly open a CSV file + in the given mode. + """ + if sys.version_info.major < 3: + return {'mode': '{}b'.format(mode)} else: - nl = {'newline': ''} # type: Dict[str, Any] - bin = '' - return open(name, mode + bin, **nl) + return {'mode': mode, 'newline': ''} def fix_script(path): @@ -563,28 +564,25 @@ def is_entrypoint_wrapper(name): logger.warning(msg) # Record pip as the installer - installer = os.path.join(dest_info_dir, 'INSTALLER') - temp_installer = os.path.join(dest_info_dir, 'INSTALLER.pip') - with open(temp_installer, 'wb') as installer_file: + installer_path = os.path.join(dest_info_dir, 'INSTALLER') + with adjacent_tmp_file(installer_path) as installer_file: installer_file.write(b'pip\n') - shutil.move(temp_installer, installer) - generated.append(installer) + replace(installer_file.name, installer_path) + generated.append(installer_path) # Record details of all files installed - record = os.path.join(dest_info_dir, 'RECORD') - temp_record = os.path.join(dest_info_dir, 'RECORD.pip') - with open_for_csv(record, 'r') as record_in: - with open_for_csv(temp_record, 'w+') as record_out: - reader = csv.reader(record_in) - outrows = get_csv_rows_for_installed( - reader, installed=installed, changed=changed, - generated=generated, lib_dir=lib_dir, - ) - writer = csv.writer(record_out) - # Sort to simplify testing. - for row in sorted_outrows(outrows): - writer.writerow(row) - shutil.move(temp_record, record) + record_path = os.path.join(dest_info_dir, 'RECORD') + with open(record_path, **csv_io_kwargs('r')) as record_file: + rows = get_csv_rows_for_installed( + csv.reader(record_file), + installed=installed, + changed=changed, + generated=generated, + lib_dir=lib_dir) + with adjacent_tmp_file(record_path, **csv_io_kwargs('w')) as record_file: + writer = csv.writer(record_file) + writer.writerows(sorted_outrows(rows)) # sort to simplify testing + replace(record_file.name, record_path) def install_wheel( diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index 161c450aafd..36578fb6244 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -17,7 +17,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING, cast if MYPY_CHECK_RUNNING: - from typing import BinaryIO, Iterator + from typing import Any, BinaryIO, Iterator class NamedTemporaryFileResult(BinaryIO): @property @@ -85,16 +85,22 @@ def is_socket(path): @contextmanager -def adjacent_tmp_file(path): - # type: (str) -> Iterator[NamedTemporaryFileResult] - """Given a path to a file, open a temp file next to it securely and ensure - it is written to disk after the context reaches its end. +def adjacent_tmp_file(path, **kwargs): + # type: (str, **Any) -> Iterator[NamedTemporaryFileResult] + """Return a file-like object pointing to a tmp file next to path. + + The file is created securely and is ensured to be written to disk + after the context reaches its end. + + kwargs will be passed to tempfile.NamedTemporaryFile to control + the way the temporary file will be opened. """ with NamedTemporaryFile( delete=False, dir=os.path.dirname(path), prefix=os.path.basename(path), suffix='.tmp', + **kwargs ) as f: result = cast('NamedTemporaryFileResult', f) try: diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 05300b96438..9328f6fb8bc 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -141,7 +141,7 @@ def call_get_csv_rows_for_installed(tmpdir, text): generated = [] lib_dir = '/lib/dir' - with wheel.open_for_csv(path, 'r') as f: + with open(path, **wheel.csv_io_kwargs('r')) as f: reader = csv.reader(f) outrows = wheel.get_csv_rows_for_installed( reader, installed=installed, changed=changed,