From a84f5443af8a3e6615acd1a4afebf96d1af8002d Mon Sep 17 00:00:00 2001 From: Peter Schutt Date: Thu, 15 Nov 2018 10:17:58 +1100 Subject: [PATCH 1/8] Incorrect ref in VCS example Small changes, but a couple of issues in the `A Note About VCS Dependencies` section of the docs. Running the example `$ pipenv install -e git+https://github.com/requests/requests.git@v2.19#egg=requests` raises the error: `Did not find branch or tag 'v2.19', assuming revision or ref.` I think it should be 'v2.19.1' which is just a typo as 'v2.19.1' is used in the subsequent lines of the example. The example Pipfile in the section excludes the `v` in the version number written to the `ref = ` part. I found it a little confusing that it would strip the 'v' from the tag, but then on testing it I found that it doesn't do that: ``` [packages] requests = {editable = true, ref = "v2.20.1", git = "https://github.com/requests/requests.git"} ``` Also, while I was here I figured I'd update the example to the most recent requests release. Thanks. --- docs/basics.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/basics.rst b/docs/basics.rst index ee2e6621..00f4c7d9 100644 --- a/docs/basics.rst +++ b/docs/basics.rst @@ -367,18 +367,18 @@ The only optional section is the ``@`` section. When using git o Note that it is **strongly recommended** that you install any version-controlled dependencies in editable mode, using ``pipenv install -e``, in order to ensure that dependency resolution can be performed with an up to date copy of the repository each time it is performed, and that it includes all known dependencies. -Below is an example usage which installs the git repository located at ``https://github.com/requests/requests.git`` from tag ``v2.19.1`` as package name ``requests``:: +Below is an example usage which installs the git repository located at ``https://github.com/requests/requests.git`` from tag ``v2.20.1`` as package name ``requests``:: - $ pipenv install -e git+https://github.com/requests/requests.git@v2.19#egg=requests + $ pipenv install -e git+https://github.com/requests/requests.git@v2.20.1#egg=requests Creating a Pipfile for this project... - Installing -e git+https://github.com/requests/requests.git@v2.19.1#egg=requests... + Installing -e git+https://github.com/requests/requests.git@v2.20.1#egg=requests... [...snipped...] - Adding -e git+https://github.com/requests/requests.git@v2.19.1#egg=requests to Pipfile's [packages]... + Adding -e git+https://github.com/requests/requests.git@v2.20.1#egg=requests to Pipfile's [packages]... [...] $ cat Pipfile [packages] - requests = {git = "https://github.com/requests/requests.git", editable = true, ref = "2.19.1"} + requests = {git = "https://github.com/requests/requests.git", editable = true, ref = "v2.20.1"} Valid values for ```` include ``git``, ``bzr``, ``svn``, and ``hg``. Valid values for ```` include ``http``, ``https``, ``ssh``, and ``file``. In specific cases you also have access to other schemes: ``svn`` may be combined with ``svn`` as a scheme, and ``bzr`` can be combined with ``sftp`` and ``lp``. From 262293699e583f39e97f8e95e1215f4b257c528b Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Fri, 16 Nov 2018 00:26:35 -0500 Subject: [PATCH 2/8] Fix race condition when installing 2+ editable non-VCS pkgs at once This regression was recently introduced, and only affects non-vcs packages. The effect of the race is that installations of multiple editable non-VCS sourced packages at once may cause some of them to be un-importable. The fix is to make editable package installs Blocking just like VCS installs are. --- pipenv/core.py | 2 +- tests/integration/test_install_twists.py | 53 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/pipenv/core.py b/pipenv/core.py index f815e5de..f9ffeba3 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -731,7 +731,7 @@ def batch_install(deps_list, procs, failed_deps_queue, ignore_hashes=any([ignore_hashes, dep.editable, dep.is_vcs]), allow_global=allow_global, no_deps=False if is_artifact else no_deps, - block=any([dep.is_vcs, blocking]), + block=any([dep.editable, dep.is_vcs, blocking]), index=index, requirements_dir=requirements_dir, pypi_mirror=pypi_mirror, diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index 055c39b2..6b202d4d 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -8,6 +8,7 @@ from pipenv.utils import mkdir_p, temp_environ import pytest from flaky import flaky +import delegator @pytest.mark.extras @@ -333,3 +334,55 @@ six = {{path = "./artifacts/{}"}} c = p.pipenv("install") assert c.return_code == 0 assert "six" in p.lockfile["default"] + + +@pytest.mark.files +@pytest.mark.needs_internet +@pytest.mark.install +@pytest.mark.run +def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir, testsroot): + """Test for a race condition that can occur when installing multiple 'editable' packages at + once, and which causes some of them to not be importable. + + This issue had been fixed for VCS packages already, but not local 'editable' packages. + + So this test locally installs packages from tarballs that have already been committed in + the local `pypi` dir to avoid using VCS packages. + """ + pkgs = { + "requests-2.19.1": "requests/requests-2.19.1.tar.gz", + "flask-0.12.2": "flask/flask-0.12.2.tar.gz", + "six-1.11.0": "six/six-1.11.0.tar.gz", + "jinja2-2.10": "jinja2/jinja2-2.10.tar.gz", + } + + pipfile_string=""" +[packages] +""" + # Unzip tarballs to known location, and update Pipfile template. + for pkg_name, file_name in pkgs.items(): + source_path = os.path.abspath(os.path.join(testsroot, "pypi", file_name)) + unzip_path = os.path.join(tmpdir, pkg_name) + + import tarfile + + with tarfile.open(source_path, "r:gz") as tgz: + tgz.extractall(path=tmpdir) + + pipfile_string += '"{0}" = {{path = "{1}", editable = true}}\n'.format(pkg_name, unzip_path) + + with PipenvInstance(pypi=pypi, chdir=True) as p: + with open(p.pipfile_path, 'w') as f: + f.write(pipfile_string.strip()) + + c = p.pipenv('install') + assert c.return_code == 0 + + c = p.pipenv('run python -c "import requests"') + assert c.return_code == 0 + c = p.pipenv('run python -c "import flask"') + assert c.return_code == 0 + c = p.pipenv('run python -c "import six"') + assert c.return_code == 0 + c = p.pipenv('run python -c "import jinja2"') + assert c.return_code == 0 From a95ce8bb32efea8c73fa8b7e1c4290319444405a Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Fri, 16 Nov 2018 08:14:21 -0500 Subject: [PATCH 3/8] Remove unnecessary test marks and imports. --- tests/integration/test_install_twists.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index 6b202d4d..ee11a186 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -8,7 +8,6 @@ from pipenv.utils import mkdir_p, temp_environ import pytest from flaky import flaky -import delegator @pytest.mark.extras @@ -337,7 +336,6 @@ six = {{path = "./artifacts/{}"}} @pytest.mark.files -@pytest.mark.needs_internet @pytest.mark.install @pytest.mark.run def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir, testsroot): @@ -351,9 +349,9 @@ def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir """ pkgs = { "requests-2.19.1": "requests/requests-2.19.1.tar.gz", - "flask-0.12.2": "flask/flask-0.12.2.tar.gz", + "Flask-0.12.2": "flask/Flask-0.12.2.tar.gz", "six-1.11.0": "six/six-1.11.0.tar.gz", - "jinja2-2.10": "jinja2/jinja2-2.10.tar.gz", + "Jinja2-2.10": "jinja2/Jinja2-2.10.tar.gz", } pipfile_string=""" From db1eb1bacb9d8fe163f5f6c39304c532b262b5c6 Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Fri, 16 Nov 2018 09:05:58 -0500 Subject: [PATCH 4/8] Use correct Path module to get tests to work. --- tests/integration/test_install_twists.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index ee11a186..cef6c34d 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -360,14 +360,14 @@ def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir # Unzip tarballs to known location, and update Pipfile template. for pkg_name, file_name in pkgs.items(): source_path = os.path.abspath(os.path.join(testsroot, "pypi", file_name)) - unzip_path = os.path.join(tmpdir, pkg_name) + unzip_path = tmpdir.join(pkg_name) import tarfile with tarfile.open(source_path, "r:gz") as tgz: - tgz.extractall(path=tmpdir) + tgz.extractall(path=tmpdir.strpath) - pipfile_string += '"{0}" = {{path = "{1}", editable = true}}\n'.format(pkg_name, unzip_path) + pipfile_string += '"{0}" = {{path = "{1}", editable = true}}\n'.format(pkg_name, unzip_path.strpath) with PipenvInstance(pypi=pypi, chdir=True) as p: with open(p.pipfile_path, 'w') as f: From 65e49f1bffcdd64db697c911c7723df2f6e794f5 Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Fri, 16 Nov 2018 09:45:59 -0500 Subject: [PATCH 5/8] Print generated Pipfile during test to help debug failures. --- tests/integration/test_install_twists.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index cef6c34d..95dc058a 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -370,6 +370,7 @@ def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir pipfile_string += '"{0}" = {{path = "{1}", editable = true}}\n'.format(pkg_name, unzip_path.strpath) with PipenvInstance(pypi=pypi, chdir=True) as p: + print(pipfile_string) with open(p.pipfile_path, 'w') as f: f.write(pipfile_string.strip()) From 70f6bec85e65d7b4f02a5c0299fa52b3d2cf75d2 Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Fri, 16 Nov 2018 11:50:25 -0500 Subject: [PATCH 6/8] Use cross platform Path compatibility module in test. --- tests/integration/test_install_twists.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index 95dc058a..92dedca8 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -359,15 +359,15 @@ def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir """ # Unzip tarballs to known location, and update Pipfile template. for pkg_name, file_name in pkgs.items(): - source_path = os.path.abspath(os.path.join(testsroot, "pypi", file_name)) - unzip_path = tmpdir.join(pkg_name) + source_path = str(Path(testsroot, "pypi", file_name)) + unzip_path = str(Path(tmpdir.strpath, pkg_name)) import tarfile with tarfile.open(source_path, "r:gz") as tgz: tgz.extractall(path=tmpdir.strpath) - pipfile_string += '"{0}" = {{path = "{1}", editable = true}}\n'.format(pkg_name, unzip_path.strpath) + pipfile_string += "'{0}' = {{path = '{1}', editable = true}}\n".format(pkg_name, unzip_path) with PipenvInstance(pypi=pypi, chdir=True) as p: print(pipfile_string) From 905c8c062a872a30a1defb5c1ef97c9fd58974da Mon Sep 17 00:00:00 2001 From: frostming Date: Fri, 16 Nov 2018 10:51:18 +0800 Subject: [PATCH 7/8] Derive missing values of source from existing fields Signed-off-by: frostming --- pipenv/project.py | 22 +++++++++++++--------- tests/integration/test_lock.py | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index 590fafe5..0067348c 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -727,6 +727,18 @@ class Project(object): data[u"requires"] = {"python_version": version[: len("2.7")]} self.write_toml(data) + @classmethod + def populate_source(cls, source): + """Derive missing values of source from the existing fields.""" + # Only URL pararemter is mandatory, let the KeyError be thrown. + if "name" not in source: + source["name"] = get_url_name(source["url"]) + if "verify_ssl" not in source: + source["verify_ssl"] = "https://" in source["url"] + if not isinstance(source["verify_ssl"], bool): + source["verify_ssl"] = source["verify_ssl"].lower() == "true" + return source + def get_or_create_lockfile(self): from pipenv.vendor.requirementslib.models.lockfile import Lockfile as Req_Lockfile lockfile = None @@ -747,15 +759,7 @@ class Project(object): elif not isinstance(sources, list): sources = [sources,] lockfile_dict["_meta"]["sources"] = [ - { - "name": s.get("name", get_url_name(s.get("url"))), - "url": s["url"], - "verify_ssl": ( - s["verify_ssl"] if isinstance(s["verify_ssl"], bool) else ( - True if s["verify_ssl"].lower() == "true" else False - ) - ) - } for s in sources + self.populate_source(s) for s in sources ] _created_lockfile = Req_Lockfile.from_data( path=self.lockfile_location, data=lockfile_dict, meta_from_project=False diff --git a/tests/integration/test_lock.py b/tests/integration/test_lock.py index 2b520808..f2f0ac76 100644 --- a/tests/integration/test_lock.py +++ b/tests/integration/test_lock.py @@ -486,3 +486,20 @@ def test_lockfile_with_empty_dict(PipenvInstance): assert c.return_code == 0 assert 'Pipfile.lock is corrupted' in c.err assert p.lockfile['_meta'] + + +@pytest.mark.lock +@pytest.mark.install +def test_lock_with_incomplete_source(PipenvInstance, pypi): + with PipenvInstance(pypi=pypi, chdir=True) as p: + with open(p.pipfile_path, 'w') as f: + f.write(""" +[[source]] +url = "https://test.pypi.org/simple" + +[packages] +requests = "*" + """) + c = p.pipenv('install') + assert c.return_code == 0 + assert p.lockfile['_meta']['sources'] From cba5a1efc62530aff87558cf0c3aeac7c9896e4f Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Sat, 17 Nov 2018 12:52:12 -0500 Subject: [PATCH 8/8] Remove debug print from test. --- tests/integration/test_install_twists.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_install_twists.py b/tests/integration/test_install_twists.py index 92dedca8..2ad12691 100644 --- a/tests/integration/test_install_twists.py +++ b/tests/integration/test_install_twists.py @@ -370,7 +370,6 @@ def test_multiple_editable_packages_should_not_race(PipenvInstance, pypi, tmpdir pipfile_string += "'{0}' = {{path = '{1}', editable = true}}\n".format(pkg_name, unzip_path) with PipenvInstance(pypi=pypi, chdir=True) as p: - print(pipfile_string) with open(p.pipfile_path, 'w') as f: f.write(pipfile_string.strip())