Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix environment #3243

Merged
merged 14 commits into from
Nov 19, 2018
1 change: 1 addition & 0 deletions news/3178.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Environment variables are expanded correctly before running scripts on POSIX.
1 change: 1 addition & 0 deletions news/3231.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correctly detect the virtualenv location inside an activated virtualenv.
1 change: 1 addition & 0 deletions news/3240.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug that editable pacakges can't be uninstalled correctly.
9 changes: 4 additions & 5 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1567,10 +1567,7 @@ def gen(out):

def warn_in_virtualenv():
# Only warn if pipenv isn't already active.
pipenv_active = os.environ.get("PIPENV_ACTIVE")
if (environments.PIPENV_USE_SYSTEM or environments.PIPENV_VIRTUALENV) and not (
pipenv_active or environments.is_quiet()
):
if environments.is_in_virtualenv() and not environments.is_quiet():
click.echo(
"{0}: Pipenv found itself running within a virtual environment, "
"so it will automatically use that environment, instead of "
Expand Down Expand Up @@ -2290,7 +2287,9 @@ def do_run_posix(script, command):
err=True,
)
sys.exit(1)
os.execl(command_path, command_path, *script.args)
os.execl(
command_path, command_path, *[os.path.expandvars(arg) for arg in script.args]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point I think os.execv (which takes a sequence directly) is better than execl.

)


def do_run(command, args, three=None, python=False, pypi_mirror=None):
Expand Down
8 changes: 3 additions & 5 deletions pipenv/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def __init__(self, prefix=None, is_venv=False, base_working_set=None, pipfile=No
self._modules = {'pkg_resources': pkg_resources, 'pipenv': pipenv}
self.base_working_set = base_working_set if base_working_set else BASE_WORKING_SET
prefix = normalize_path(prefix)
self.is_venv = not prefix == normalize_path(sys.prefix)
self.is_venv = is_venv or prefix != normalize_path(sys.prefix)
if not sources:
sources = []
self.project = project
Expand Down Expand Up @@ -246,7 +246,7 @@ def get_distributions(self):

def find_egg(self, egg_dist):
"""Find an egg by name in the given environment"""
site_packages = get_python_lib()
site_packages = self.libdir[1]
search_filename = "{0}.egg-link".format(egg_dist.project_name)
try:
user_site = site.getusersitepackages()
Expand All @@ -264,8 +264,7 @@ def locate_dist(self, dist):
If the egg - link doesn 't exist, return the supplied distribution."""

location = self.find_egg(dist)
if not location:
return dist.location
return location or dist.location

def dist_is_in_project(self, dist):
"""Determine whether the supplied distribution is in the environment."""
Expand Down Expand Up @@ -483,7 +482,6 @@ def activated(self, include_extras=True, extra_dists=None):
])
os.environ["PYTHONIOENCODING"] = vistir.compat.fs_str("utf-8")
os.environ["PYTHONDONTWRITEBYTECODE"] = vistir.compat.fs_str("1")
os.environ["PATH"] = self.base_paths["PATH"]
os.environ["PYTHONPATH"] = self.base_paths["PYTHONPATH"]
if self.is_venv:
os.environ["VIRTUAL_ENV"] = vistir.compat.fs_str(prefix)
Expand Down
6 changes: 6 additions & 0 deletions pipenv/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@ def is_quiet(threshold=-1):
return PIPENV_VERBOSITY <= threshold


def is_in_virtualenv():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't like using globals everywhere to figure this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the global environment variable is good and convenient but I need a way to get the right status when the environment is modified in runtime.(see the test case)

It isn't a good idea to change the logic only for testing though, because in real case the environment is always decided before any command is launched.

If you are not for this, I can revert these bits and consider using patching in test case.

pipenv_active = os.environ.get("PIPENV_ACTIVE")
virtual_env = os.environ.get("VIRTUAL_ENV")
return (PIPENV_USE_SYSTEM or virtual_env) and not pipenv_active


PIPENV_SPINNER_FAIL_TEXT = fix_utf8(u"✘ {0}") if not PIPENV_HIDE_EMOJIS else ("{0}")

PIPENV_SPINNER_OK_TEXT = fix_utf8(u"✔ {0}") if not PIPENV_HIDE_EMOJIS else ("{0}")
13 changes: 8 additions & 5 deletions pipenv/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@
PIPENV_MAX_DEPTH,
PIPENV_PIPFILE,
PIPENV_VENV_IN_PROJECT,
PIPENV_VIRTUALENV,
PIPENV_TEST_INDEX,
PIPENV_PYTHON,
PIPENV_DEFAULT_PYTHON_VERSION,
PIPENV_IGNORE_VIRTUALENVS,
is_in_virtualenv
)


Expand Down Expand Up @@ -345,8 +346,8 @@ def pipfile_package_names(self):
@property
def environment(self):
if not self._environment:
prefix = self.get_location_for_virtualenv()
is_venv = prefix == sys.prefix
prefix = self.virtualenv_location
is_venv = is_in_virtualenv()
sources = self.sources if self.sources else [DEFAULT_SOURCE,]
self._environment = Environment(
prefix=prefix, is_venv=is_venv, sources=sources, pipfile=self.parsed_pipfile,
Expand Down Expand Up @@ -426,8 +427,10 @@ def virtualenv_name(self):
@property
def virtualenv_location(self):
# if VIRTUAL_ENV is set, use that.
if PIPENV_VIRTUALENV:
return PIPENV_VIRTUALENV
virtualenv_env = os.getenv("VIRTUAL_ENV")
if ("PIPENV_ACTIVE" not in os.environ and
not PIPENV_IGNORE_VIRTUALENVS and virtualenv_env):
return virtualenv_env

if not self._virtualenv_location: # Use cached version, if available.
assert self.project_directory, "project not created"
Expand Down
24 changes: 24 additions & 0 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import pytest

from pipenv._compat import TemporaryDirectory, Path
from pipenv.exceptions import VirtualenvActivationException
from pipenv.utils import temp_environ
from pipenv.vendor import delegator
from pipenv.vendor import requests
from pipenv.vendor import toml
Expand Down Expand Up @@ -111,6 +113,9 @@ def isolate(pathlib_tmpdir):
os.environ["GIT_AUTHOR_EMAIL"] = fs_str("[email protected]")
mkdir_p(os.path.join(home_dir, ".virtualenvs"))
os.environ["WORKON_HOME"] = fs_str(os.path.join(home_dir, ".virtualenvs"))
# Ignore PIPENV_ACTIVE so that it works as under a bare environment.
os.environ.pop("PIPENV_ACTIVE", None)
os.environ.pop("VIRTUAL_ENV", None)
global WE_HAVE_GITHUB_SSH_KEYS
WE_HAVE_GITHUB_SSH_KEYS = check_github_ssh()

Expand Down Expand Up @@ -239,3 +244,22 @@ def finalize():
@pytest.fixture()
def testsroot():
return TESTS_ROOT


@pytest.fixture()
def virtualenv(pathlib_tmpdir):
virtualenv_path = pathlib_tmpdir / "venv"
with temp_environ():
c = delegator.run("virtualenv {}".format(virtualenv_path), block=True)
assert c.return_code == 0
for name in ("bin", "Scripts"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative fixture here is to use pipenv.environment.Environment() which provides a contextmanager called activated:

from pipenv.environment import Environment

@pytest.fixture()
def virtualenv(pathlib_tmpdir, PipenvInstance):
    virtualenv_path = pathlib_tmpdir / "venv"
    with temp_environ():
        c = delegator.run("virtualenv {}".format(virtualenv_path), block=True)
        assert c.return_code == 0
        environment = Environment(prefix=virtualenv_path.as_posix(), is_venv=True, pipfile=PipenvInstance.pipfile)
        with environment.activated():
            yield virtualenv_path

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.s. I have no idea if this works

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense for me.

activate_this = virtualenv_path / name / "activate_this.py"
if activate_this.exists():
with open(str(activate_this)) as f:
code = compile(f.read(), str(activate_this), "exec")
exec(code, dict(__file__=str(activate_this)))
break
else:
raise VirtualenvActivationException("Can't find the activate_this.py script.")
os.environ["VIRTUAL_ENV"] = str(virtualenv_path)
yield virtualenv_path
36 changes: 36 additions & 0 deletions tests/integration/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import io
import pytest
import os
import tarfile
from pipenv.project import Project
from pipenv.utils import temp_environ
from pipenv.patched import pipfile
Expand Down Expand Up @@ -161,3 +162,38 @@ def test_rewrite_outline_table(PipenvInstance, pypi):
contents = f.read()
assert "[packages.requests]" not in contents
assert 'requests = {version = "*"}' in contents


@pytest.mark.install
@pytest.mark.project
def test_include_editable_packages(PipenvInstance, pypi, testsroot, pathlib_tmpdir):
file_name = "requests-2.19.1.tar.gz"
package = pathlib_tmpdir.joinpath("requests-2.19.1")
source_path = os.path.abspath(os.path.join(testsroot, "test_artifacts", file_name))
with PipenvInstance(chdir=True, pypi=pypi) as p:
with tarfile.open(source_path, "r:gz") as tarinfo:
tarinfo.extractall(path=str(pathlib_tmpdir))
c = p.pipenv('install -e {}'.format(package))
assert c.return_code == 0
project = Project()
assert "requests" in [
package.project_name
for package in project.environment.get_installed_packages()
]


@pytest.mark.project
def test_run_in_virtualenv(PipenvInstance, pypi, virtualenv):
with PipenvInstance(chdir=True, pypi=pypi) as p:
os.environ.pop("PIPENV_IGNORE_VIRTUALENVS", None)
project = Project()
assert project.virtualenv_location == str(virtualenv)
c = p.pipenv("run pip install click")
assert c.return_code == 0
assert "Courtesy Notice" in c.err
c = p.pipenv('run python -c "import click;print(click.__file__)"')
assert c.return_code == 0
assert c.out.strip().startswith(str(virtualenv))
c = p.pipenv("clean --dry-run")
assert c.return_code == 0
assert "click" in c.out
12 changes: 12 additions & 0 deletions tests/integration/test_run.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os

from pipenv.project import Project
from pipenv.utils import temp_environ

import pytest

Expand Down Expand Up @@ -28,6 +29,10 @@ def test_scripts(PipenvInstance):
appendscript = "cmd arg1"
multicommand = "bash -c \"cd docs && make html\""
""")
if os.name == "nt":
f.write('scriptwithenv = "echo %HELLO%"\n')
else:
f.write('scriptwithenv = "echo $HELLO"\n')
c = p.pipenv('install')
assert c.return_code == 0

Expand All @@ -52,3 +57,10 @@ def test_scripts(PipenvInstance):
script = project.build_script('appendscript', ['a', 'b'])
assert script.command == 'cmd'
assert script.args == ['arg1', 'a', 'b']

with temp_environ():
os.environ['HELLO'] = 'WORLD'
c = p.pipenv("run scriptwithenv")
assert c.ok
if os.name != "nt": # This doesn't work on CI windows.
assert c.out.strip() == "WORLD"