Don't give child deps of vcs deps auto-precedence

- Stop preferring resolution of VCS dependencies in all cases
- Resolve vcs dependencies together with non-vcs dependencies
- Clarify blocking and no-deps logic
- Add artifacts and tests
- Add vendoring task for artifacts
- Clean up release tasks
- Fixes #3296

Signed-off-by: Dan Ryan <dan@danryan.co>

.gitmodules

@@ -22,3 +22,6 @@
 [submodule "tests/test_artifacts/git/flask"]
 	path = tests/test_artifacts/git/flask
 	url = https://github.com/pallets/flask.git
+[submodule "tests/test_artifacts/git/requests-2.18.4"]
+	path = tests/test_artifacts/git/requests-2.18.4
+	url = https://github.com/requests/requests

news/3296.bugfix.rst

@@ -0,0 +1 @@
+Pipenv will now respect top-level pins over VCS dependency locks.

pipenv/core.py

@@ -734,25 +734,33 @@ def batch_install(deps_list, procs, failed_deps_queue,
                 os.environ["PIP_USER"] = vistir.compat.fs_str("0")
                 if "PYTHONHOME" in os.environ:
                     del os.environ["PYTHONHOME"]
+            if no_deps:
+                link = getattr(dep.req, "link", None)
+                is_wheel = False
+                if link:
+                    is_wheel = link.is_wheel
+                is_non_editable_vcs = (dep.is_vcs and not dep.editable)
+                no_deps = not (dep.is_file_or_url and not (is_wheel or dep.editable))
+            block = any([dep.editable, dep.is_vcs, blocking])
             c = pip_install(
                 dep,
                 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.editable, dep.is_vcs, blocking]),
+                no_deps=no_deps,
+                block=block,
                 index=index,
                 requirements_dir=requirements_dir,
                 pypi_mirror=pypi_mirror,
                 trusted_hosts=trusted_hosts,
                 extra_indexes=extra_indexes
             )
-            if dep.is_vcs:
+            if dep.is_vcs or block:
                 c.block()
             if procs.qsize() < nprocs:
                 c.dep = dep
                 procs.put(c)
 
-            if procs.full() or procs.qsize() == len(deps_list):
+            if procs.full() or procs.qsize() == len(deps_list) or block:
                 _cleanup_procs(procs, not blocking, failed_deps_queue, retry=retry)
 
 

pipenv/utils.py

@@ -540,7 +540,10 @@ def resolve(cmd, sp):
     return c
 
 
-def get_locked_dep(dep, pipfile_section):
+def get_locked_dep(dep, pipfile_section, prefer_pipfile=False):
+    # the prefer pipfile flag is not used yet, but we are introducing
+    # it now for development purposes
+    # TODO: Is this implementation clear? How can it be improved?
     entry = None
     cleaner_kwargs = {
         "is_top_level": False,
@@ -554,6 +557,14 @@ def get_locked_dep(dep, pipfile_section):
     if entry:
         cleaner_kwargs.update({"is_top_level": True, "pipfile_entry": entry})
     lockfile_entry = clean_resolved_dep(dep, **cleaner_kwargs)
+    if entry and isinstance(entry, Mapping):
+        version = entry.get("version", "") if entry else ""
+    else:
+        version = entry if entry else ""
+    lockfile_version = lockfile_entry.get("version", "")
+    # Keep pins from the lockfile
+    if prefer_pipfile and lockfile_version != version and version.startswith("=="):
+        lockfile_version = version
     return lockfile_entry
 
 
@@ -592,9 +603,17 @@ def venv_resolve_deps(
     pipfile_section = "dev_packages" if dev else "packages"
     lockfile_section = "develop" if dev else "default"
     vcs_section = "vcs_{0}".format(pipfile_section)
-    vcs_deps = getattr(project, vcs_section, [])
-    if not deps and not vcs_deps:
+    editable_section = "editable_{0}".format(pipfile_section)
+    vcs_deps = getattr(project, vcs_section, {})
+    editable_deps = {
+        k: v for k, v in getattr(project, editable_section, {}).items()
+        if k not in vcs_deps
+    }
+    if not deps and not vcs_deps and not editable_deps:
         return {}
+    editable_deps = convert_deps_to_pip(
+        editable_deps, project, r=False, include_index=True
+    )
 
     if not pipfile:
         pipfile = getattr(project, pipfile_section, None)
@@ -612,6 +631,7 @@ def venv_resolve_deps(
                 dev=dev,
             )
             vcs_deps = [req.as_line() for req in vcs_reqs if req.editable]
+    deps = set(deps) | set(vcs_deps) | set(editable_deps)
     cmd = [
         which("python", allow_global=allow_global),
         Path(resolver.__file__.rstrip("co")).as_posix()
@@ -633,36 +653,17 @@ def venv_resolve_deps(
         with create_spinner(text=fs_str("Locking...")) as sp:
             c = resolve(cmd, sp)
             results = c.out
-            if vcs_deps:
-                with temp_environ():
-                    os.environ["PIPENV_PACKAGES"] = str("\n".join(vcs_deps))
-                    sp.text = to_native_string("Locking VCS Dependencies...")
-                    vcs_c = resolve(cmd, sp)
-                    vcs_results, vcs_err = vcs_c.out, vcs_c.err
-            else:
-                vcs_results, vcs_err = "", ""
             sp.green.ok(environments.PIPENV_SPINNER_OK_TEXT.format("Success!"))
-    outputs = [results, vcs_results]
     if environments.is_verbose():
-        for output in outputs:
-            click_echo(output.split("RESULTS:")[0], err=True)
+        click_echo(results.split("RESULTS:")[0], err=True)
     try:
         results = json.loads(results.split("RESULTS:")[1].strip())
-        if vcs_results:
-            # For vcs dependencies, treat the initial pass at locking (i.e. checkout)
-            # as the pipfile entry because it gets us an actual ref to use
-            vcs_results = json.loads(vcs_results.split("RESULTS:")[1].strip())
-            vcs_lockfile = prepare_lockfile(vcs_results, vcs_lockfile.copy(), vcs_lockfile)
-        else:
-            vcs_results = []
 
     except (IndexError, JSONDecodeError):
-        for out, err in [(c.out, c.err), (vcs_results, vcs_err)]:
-            click_echo(out.strip(), err=True)
-            click_echo(err.strip(), err=True)
+        click_echo(out.strip(), err=True)
+        click_echo(err.strip(), err=True)
         raise RuntimeError("There was a problem with locking.")
     lockfile[lockfile_section] = prepare_lockfile(results, pipfile, lockfile[lockfile_section])
-    lockfile[lockfile_section].update(vcs_lockfile)
 
 
 def resolve_deps(

tasks/__init__.py

@@ -11,12 +11,4 @@ from pathlib import Path
 ROOT = Path(".").parent.parent.absolute()
 
 
-@invoke.task
-def clean_mdchangelog(ctx):
-    changelog = ROOT / "CHANGELOG.md"
-    content = changelog.read_text()
-    content = re.sub(r"([^\n]+)\n?\s+\[[\\]+(#\d+)\]\(https://github\.com/pypa/[\w\-]+/issues/\d+\)", r"\1 \2", content, flags=re.MULTILINE)
-    changelog.write_text(content)
-
-
-ns = invoke.Collection(vendoring, release, clean_mdchangelog, vendor_passa.vendor_passa)
+ns = invoke.Collection(vendoring, release, release.clean_mdchangelog, vendor_passa.vendor_passa)

tasks/release.py

@@ -5,9 +5,16 @@ import sys
 from pipenv.__version__ import __version__
 from parver import Version
 from .vendoring import _get_git_root, drop_dir
+import pathlib
+from towncrier._builder import (
+    find_fragments, render_fragments, split_fragments,
+)
+from towncrier._settings import load_config
 
 
 VERSION_FILE = 'pipenv/__version__.py'
+ROOT = pathlib.Path(".").parent.parent.absolute()
+PACKAGE_NAME = "pipenv"
 
 
 def log(msg):
@@ -18,6 +25,15 @@ def get_version_file(ctx):
     return _get_git_root(ctx).joinpath(VERSION_FILE)
 
 
+def find_version(ctx):
+    version_file = get_version_file(ctx).read_text()
+    version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]",
+                              version_file, re.M)
+    if version_match:
+        return version_match.group(1)
+    raise RuntimeError("Unable to find version string.")
+
+
 def get_history_file(ctx):
     return _get_git_root(ctx).joinpath('HISTORY.txt')
 
@@ -30,6 +46,66 @@ def get_build_dir(ctx):
     return _get_git_root(ctx) / 'build'
 
 
+def _render_log():
+    """Totally tap into Towncrier internals to get an in-memory result.
+    """
+    config = load_config(ROOT)
+    definitions = config['types']
+    fragments, fragment_filenames = find_fragments(
+        pathlib.Path(config['directory']).absolute(),
+        config['sections'],
+        None,
+        definitions,
+    )
+    rendered = render_fragments(
+        pathlib.Path(config['template']).read_text(encoding='utf-8'),
+        config['issue_format'],
+        split_fragments(fragments, definitions),
+        definitions,
+        config['underlines'][1:],
+        False,  # Don't add newlines to wrapped text.
+    )
+    return rendered
+
+
+@invoke.task
+def release(ctx, dry_run=False):
+    drop_dist_dirs(ctx)
+    bump_version(ctx)
+    version = find_version(ctx)
+    tag_content = _render_log()
+    if dry_run:
+        ctx.run('towncrier --draft')
+        log('would remove: news/*')
+        log('would remove: CHANGELOG.draft.rst')
+        log(f'Would commit with message: "Release v{version}"')
+    else:
+        ctx.run('towncrier')
+        ctx.run("git add CHANGELOG.rst news/")
+        ctx.run("git rm CHANGELOG.draft.rst")
+        ctx.run(f'git commit -m "Release v{version}"')
+
+    tag_content = tag_content.replace('"', '\\"')
+    if dry_run:
+        log("Generated tag content: f{tag_content}")
+        markdown = ctx.run("towncrier --draft | pandoc -f rst -t markdown -o CHANGELOG.md", hide=True).stdout.strip()
+        content = clean_mdchangelog(ctx, markdown)
+        log(f"would generate markdown: {content}")
+    else:
+        generate_markdown(ctx)
+        clean_mdchangelog(ctx)
+        ctx.run(f'git tag -a v{version} -m "Version v{version}\n\n{tag_content}"')
+    build_dists(ctx)
+    if dry_run:
+        dist_pattern = f'{PACKAGE_NAME.replace("-", "[-_]")}-*'
+        artifacts = list(ROOT.joinpath('dist').glob(dist_pattern))
+        filename_display = '\n'.join(f'  {a}' for a in artifacts)
+        log(f"Would upload dists: {filename_display}")
+    else:
+        upload_dists(ctx)
+        bump_version(ctx, dev=True)
+
+
 def drop_dist_dirs(ctx):
     log('Dropping Dist dir...')
     drop_dir(get_dist_dir(ctx))
@@ -41,19 +117,32 @@ def drop_dist_dirs(ctx):
 def build_dists(ctx):
     drop_dist_dirs(ctx)
     log('Building sdist using %s ....' % sys.executable)
-    for py_version in ['2.7', '3.6', '3.7']:
+    for py_version in ['3.6', '2.7']:
         env = {'PIPENV_PYTHON': py_version}
         ctx.run('pipenv install --dev', env=env)
+        ctx.run('pipenv run pip install -e . --upgrade --upgrade-strategy=eager', env=env)
         if py_version == '3.6':
-            ctx.run('pipenv run python setup.py sdist', env=env)
+            ctx.run('pipenv run python setup.py sdist bdist_wheel', env=env)
+        else:
+            ctx.run('pipenv run python setup.py bdist_wheel', env=env)
         log('Building wheel using python %s ....' % py_version)
-        ctx.run('pipenv run python setup.py bdist_wheel', env=env)
 
 
 @invoke.task(build_dists)
-def upload_dists(ctx):
-    log('Uploading distributions to pypi...')
-    ctx.run('twine upload dist/*')
+def upload_dists(ctx, repo="pypi"):
+    dist_pattern = f'{PACKAGE_NAME.replace("-", "[-_]")}-*'
+    artifacts = list(ROOT.joinpath('dist').glob(dist_pattern))
+    filename_display = '\n'.join(f'  {a}' for a in artifacts)
+    print(f'[release] Will upload:\n{filename_display}')
+    try:
+        input('[release] Release ready. ENTER to upload, CTRL-C to abort: ')
+    except KeyboardInterrupt:
+        print('\nAborted!')
+        return
+
+    arg_display = ' '.join(f'"{n}"' for n in artifacts)
+    ctx.run(f'twine upload --repository="{repo}" {arg_display}')
+
 
 
 @invoke.task
@@ -69,67 +158,68 @@ def generate_changelog(ctx, commit=False, draft=False):
         commit = False
         log('Writing draft to file...')
         ctx.run('towncrier --draft > CHANGELOG.draft.rst')
-    if commit:
+    else:
         ctx.run('towncrier')
+    if commit:
         log('Committing...')
         ctx.run('git add CHANGELOG.rst')
         ctx.run('git rm CHANGELOG.draft.rst')
         ctx.run('git commit -m "Update changelog."')
 
 
+@invoke.task
+def clean_mdchangelog(ctx, content=None):
+    changelog = None
+    if not content:
+        changelog = _get_git_root(ctx) / "CHANGELOG.md"
+        content = changelog.read_text()
+    content = re.sub(r"([^\n]+)\n?\s+\[[\\]+(#\d+)\]\(https://github\.com/pypa/[\w\-]+/issues/\d+\)", r"\1 \2", content, flags=re.MULTILINE)
+    if changelog:
+        changelog.write_text(content)
+    else:
+        return content
+
+
 @invoke.task
 def tag_version(ctx, push=False):
-    version = Version.parse(__version__)
-    log('Tagging revision: v%s' % version)
-    ctx.run('git tag v%s' % version)
+    version = find_version(ctx)
+    version = Version.parse(version)
+    log('Tagging revision: v%s' % version.normalize())
+    ctx.run('git tag v%s' % version.normalize())
     if push:
         log('Pushing tags...')
+        ctx.run('git push origin master')
         ctx.run('git push --tags')
 
 
 @invoke.task
-def bump_version(ctx, dry_run=False, increment=True, release=False, dev=False, pre=False, tag=None, clear=False, commit=False,):
+def bump_version(ctx, dry_run=False, dev=False, pre=False, tag=None, commit=False):
     current_version = Version.parse(__version__)
     today = datetime.date.today()
-    next_month_number = today.month + 1 if today.month != 12 else 1
-    next_year_number = today.year if next_month_number != 1 else today.year+1
-    next_month = (next_year_number, next_month_number, 0)
+    tomorrow = today + datetime.timedelta(days=1)
+    next_month = today + datetime.timedelta(months=1)
+    next_year = today + datetime.timedelta(years=1)
     if pre and not tag:
         print('Using "pre" requires a corresponding tag.')
         return
-    if release and not dev and not pre and increment:
+    if not (dev or pre or tag):
         new_version = current_version.replace(release=today.timetuple()[:3]).clear(pre=True, dev=True)
-    elif release and (dev or pre):
-        if increment:
-            new_version = current_version.replace(release=today.timetuple()[:3])
-        else:
-            new_version = current_version
+    if pre and dev:
+        raise RuntimeError("Can't use 'pre' and 'dev' together!")
+    if dev or pre:
+        new_version = current_version.replace(release=tomorrow.timetuple()[:3]).clear(pre=True, dev=True)
         if dev:
             new_version = new_version.bump_dev()
-        elif pre:
-            new_version = new_version.bump_pre(tag=tag)
-    else:
-        if not release:
-            increment = False
-        if increment:
-            new_version = current_version.replace(release=next_month)
         else:
-            new_version = current_version
-        if dev:
-            new_version = new_version.bump_dev()
-        elif pre:
             new_version = new_version.bump_pre(tag=tag)
-    if clear:
-        new_version = new_version.clear(dev=True, pre=True, post=True)
     log('Updating version to %s' % new_version.normalize())
-    version_file = get_version_file(ctx)
-    file_contents = version_file.read_text()
-    log('Found current version: %s' % __version__)
+    version = find_version(ctx)
+    log('Found current version: %s' % version)
     if dry_run:
         log('Would update to: %s' % new_version.normalize())
     else:
         log('Updating to: %s' % new_version.normalize())
-        version_file.write_text(file_contents.replace(__version__, str(new_version.normalize())))
+        version_file.write_text(file_contents.replace(version, str(new_version.normalize())))
         if commit:
             ctx.run('git add {0}'.format(version_file))
             log('Committing...')

tasks/vendoring/__init__.py

@@ -2,9 +2,13 @@
 """"Vendoring script, python 3.5 needed"""
 # Taken from pip
 # see https://github.com/pypa/pip/blob/95bcf8c5f6394298035a7332c441868f3b0169f4/tasks/vendoring/__init__.py
-from pipenv._compat import NamedTemporaryFile, TemporaryDirectory
+from pipenv.vendor.vistir.compat import NamedTemporaryFile, TemporaryDirectory
+from pipenv.vendor.vistir.contextmanagers import open_file
 from pathlib import Path
 from pipenv.utils import mkdir_p
+import io
+from urllib3.util import parse_url as urllib3_parse
+import bs4
 # from tempfile import TemporaryDirectory
 import tarfile
 import zipfile
@@ -640,3 +644,22 @@ def main(ctx, package=None):
     # vendor_passa(ctx)
     # update_safety(ctx)
     log('Revendoring complete')
+
+
+@invoke.task
+def vendor_artifact(ctx, package, version=None):
+    simple = requests.get("https://pypi.org/simple/{0}/".format(package))
+    pkg_str = "{0}-{1}".format(package, version)
+    soup = bs4.BeautifulSoup(simple.content)
+    links = [
+        a.attrs["href"] for a in soup.find_all("a") if a.getText().startswith(pkg_str)
+    ]
+    for link in links:
+        dest_dir = _get_git_root(ctx) / "tests" / "pypi" / package
+        if not dest_dir.exists():
+            dest_dir.mkdir()
+        _, _, dest_path = urllib3_parse(link).path.rpartition("/")
+        dest_file = dest_dir / dest_path
+        with io.open(dest_file.as_posix(), "wb") as target_handle:
+            with open_file(link) as fp:
+                shutil.copyfileobj(fp, target_handle)

tests/integration/test_lock.py

@@ -540,3 +540,23 @@ def test_lock_missing_cache_entries_gets_all_hashes(monkeypatch, PipenvInstance,
             assert "scandir" in p.lockfile["default"]
             assert isinstance(p.lockfile["default"]["scandir"]["hashes"], list)
             assert len(p.lockfile["default"]["scandir"]["hashes"]) > 1
+
+
+@pytest.mark.lock
+@pytest.mark.vcs
+def test_vcs_lock_respects_top_level_pins(PipenvInstance, pypi):
+    """Test that locking VCS dependencies respects top level packages pinned in Pipfiles"""
+
+    with PipenvInstance(pypi=pypi, chdir=True) as p:
+        requests_uri = p._pipfile.get_fixture_path("git/requests").as_uri()
+        p._pipfile.add("requests", {
+            "editable": True, "git": "{0}".format(requests_uri),
+            "ref": "v2.18.4"
+        })
+        p._pipfile.add("urllib3", "==1.21.1")
+        c = p.pipenv("install")
+        assert c.return_code == 0
+        assert "requests" in p.lockfile["default"]
+        assert "git" in p.lockfile["default"]["requests"]
+        assert "urllib3" in p.lockfile["default"]
+        assert p.lockfile["default"]["urllib3"]["version"] == "==1.21.1"