Skip to content

Commit

Permalink
Set build_isolation and use_pep517 correctly
Browse files Browse the repository at this point in the history
- Fix how `use_pep517` and `build_isolation` are read from the
  environment -- introduce a new environment helper to detect
  `<PREFIX>_<SETTING>` and `<PREFIX>_NO_<SETTING>`, check for booleans
  and return appropriately boolean, str, or None types
- Check for `False` values when adding `--no-use-pep517` and
  `--no-build-isolation` during resolution rather than falsey values
- Change environment variable name from `PIP_PYTHON_VERSION` to
  `PIPENV_REQUESTED_PYTHON_VERSION` to avoid causing `pip` to fail due
  to accidentally percieving the `python_version` flag as being set --
  this is an artifact from attempting to resolve outside of the
  virtualenv
- Add `pipenv` to the path of patched `notpip.__main__` to accommodate
  updated import fully qualified module names
- Update `pip` and `piptools` patches
- Add test packages for each of two known failure modes: outdated
  `setuptools` with a missing `build-backend` (which `pip` forces to
  `build_meta:__legacy__` & which doesn't exist before `40.8`), and
  `import Cython` statements in `setup.py` in packages with properly
  defined `pyproject.toml` `build-backend` lines.
- Fixes #4231
- Replaces, includes, and closes #4242

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

Add integration tests for #4231

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
  • Loading branch information
techalchemy committed May 9, 2020
1 parent 7059a26 commit 47aa2ac
Show file tree
Hide file tree
Showing 18 changed files with 476 additions and 15 deletions.
1 change: 1 addition & 0 deletions news/4231.bugfix.rst
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Fixed a bug which caused pipenv to prefer source distributions over wheels from ``PyPI`` during the dependency resolution phase.
Fixed an issue which prevented proper build isolation using ``pep517`` based builders during dependency resolution.
30 changes: 30 additions & 0 deletions pipenv/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,36 @@ def _is_env_truthy(name):
return os.environ.get(name).lower() not in ("0", "false", "no", "off")


def get_from_env(arg, prefix="PIPENV", check_for_negation=True):
"""
Check the environment for a variable, returning its truthy or stringified value
For example, setting ``PIPENV_NO_RESOLVE_VCS=1`` would mean that
``get_from_env("RESOLVE_VCS", prefix="PIPENV")`` would return ``False``.
:param str arg: The name of the variable to look for
:param str prefix: The prefix to attach to the variable, defaults to "PIPENV"
:param bool check_for_negation: Whether to check for ``<PREFIX>_NO_<arg>``, defaults
to True
:return: The value from the environment if available
:rtype: Optional[Union[str, bool]]
"""
negative_lookup = "NO_{0}".format(arg)
positive_lookup = arg
if prefix:
positive_lookup = "{0}_{1}".format(prefix, arg)
negative_lookup = "{0}_{1}".format(prefix, negative_lookup)
if positive_lookup in os.environ:
if _is_env_truthy(positive_lookup):
return bool(os.environ[positive_lookup])
return os.environ[positive_lookup]
if negative_lookup in os.environ:
if _is_env_truthy(negative_lookup):
return not bool(os.environ[negative_lookup])
return os.environ[negative_lookup]
return None


PIPENV_IS_CI = bool("CI" in os.environ or "TF_BUILD" in os.environ)

# HACK: Prevent invalid shebangs with Homebrew-installed Python:
Expand Down
2 changes: 2 additions & 0 deletions pipenv/patched/notpip/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
# Resulting path is the name of the wheel itself
# Add that to sys.path so we can import pipenv.patched.notpip
path = os.path.dirname(os.path.dirname(__file__))
pipenv = os.path.dirname(os.path.dirname(path))
sys.path.insert(0, path)
sys.path.insert(0, pipenv)

from pipenv.patched.notpip._internal.cli.main import main as _main # isort:skip # noqa

Expand Down
2 changes: 1 addition & 1 deletion pipenv/patched/piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def simplify_markers(ireq):
def clean_requires_python(candidates):
"""Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes."""
all_candidates = []
py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
py_version = parse_version(os.environ.get('PIPENV_REQUESTED_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
for c in candidates:
if getattr(c, "requires_python", None):
# Old specifications had people setting this to single digits
Expand Down
2 changes: 1 addition & 1 deletion pipenv/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ def resolve(packages, pre, project, sources, clear, system, requirements_dir=Non


def _main(pre, clear, verbose, system, write, requirements_dir, packages, parse_only=False):
os.environ["PIP_PYTHON_VERSION"] = ".".join([str(s) for s in sys.version_info[:3]])
os.environ["PIPENV_REQUESTED_PYTHON_VERSION"] = ".".join([str(s) for s in sys.version_info[:3]])
os.environ["PIP_PYTHON_PATH"] = str(sys.executable)
if parse_only:
parse_packages(
Expand Down
19 changes: 8 additions & 11 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,14 @@ def __init__(self, python_version, python_path):
def __enter__(self):
# Only inject when the value is valid
if self.python_version:
os.environ["PIP_PYTHON_VERSION"] = str(self.python_version)
os.environ["PIPENV_REQUESTED_PYTHON_VERSION"] = str(self.python_version)
if self.python_path:
os.environ["PIP_PYTHON_PATH"] = str(self.python_path)

def __exit__(self, *args):
# Restore original Python version information.
try:
del os.environ["PIP_PYTHON_VERSION"]
del os.environ["PIPENV_REQUESTED_PYTHON_VERSION"]
except KeyError:
pass

Expand Down Expand Up @@ -682,25 +682,21 @@ def pip_command(self):
self._pip_command = self._get_pip_command()
return self._pip_command

def prepare_pip_args(self, use_pep517=True, build_isolation=True):
def prepare_pip_args(self, use_pep517=False, build_isolation=True):
pip_args = []
if self.sources:
pip_args = prepare_pip_source_args(self.sources, pip_args)
if not use_pep517:
if use_pep517 is False:
pip_args.append("--no-use-pep517")
if not build_isolation:
if build_isolation is False:
pip_args.append("--no-build-isolation")
pip_args.extend(["--cache-dir", environments.PIPENV_CACHE_DIR])
return pip_args

@property
def pip_args(self):
use_pep517 = False if (
os.environ.get("PIP_NO_USE_PEP517", None) is not None
) else (True if os.environ.get("PIP_USE_PEP517", None) is not None else None)
build_isolation = False if (
os.environ.get("PIP_NO_BUILD_ISOLATION", None) is not None
) else (True if os.environ.get("PIP_BUILD_ISOLATION", None) is not None else None)
use_pep517 = environments.get_from_env("USE_PEP517", prefix="PIP")
build_isolation = environments.get_from_env("BUILD_ISOLATION", prefix="PIP")
if self._pip_args is None:
self._pip_args = self.prepare_pip_args(
use_pep517=use_pep517, build_isolation=build_isolation
Expand Down Expand Up @@ -790,6 +786,7 @@ def get_resolver(self, clear=False, pre=False):
self._resolver = PiptoolsResolver(
constraints=self.parsed_constraints, repository=self.repository,
cache=DependencyCache(environments.PIPENV_CACHE_DIR), clear_caches=clear,
# TODO: allow users to toggle the 'allow unsafe' flag to resolve setuptools?
prereleases=pre, allow_unsafe=False
)

Expand Down
14 changes: 14 additions & 0 deletions tasks/vendoring/patches/patched/pip20.patch
Original file line number Diff line number Diff line change
Expand Up @@ -589,3 +589,17 @@ index 65e41bc7..9eabf28e 100644


class AdjacentTempDirectory(TempDirectory):
diff --git a/pipenv/patched/pip/__main__.py b/pipenv/patched/pip/__main__.py
index 56f669fa..3c216189 100644
--- a/pipenv/patched/pip/__main__.py
+++ b/pipenv/patched/pip/__main__.py
@@ -11,7 +11,9 @@ if __package__ == '':
# Resulting path is the name of the wheel itself
# Add that to sys.path so we can import pip
path = os.path.dirname(os.path.dirname(__file__))
+ pipenv = os.path.dirname(os.path.dirname(path))
sys.path.insert(0, path)
+ sys.path.insert(0, pipenv)

from pip._internal.cli.main import main as _main # isort:skip # noqa

2 changes: 1 addition & 1 deletion tasks/vendoring/patches/patched/piptools.patch
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ index 7733447..e6f232f 100644
+def clean_requires_python(candidates):
+ """Get a cleaned list of all the candidates with valid specifiers in the `requires_python` attributes."""
+ all_candidates = []
+ py_version = parse_version(os.environ.get('PIP_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
+ py_version = parse_version(os.environ.get('PIPENV_REQUESTED_PYTHON_VERSION', '.'.join(map(str, sys.version_info[:3]))))
+ for c in candidates:
+ if getattr(c, "requires_python", None):
+ # Old specifications had people setting this to single digits
Expand Down
52 changes: 52 additions & 0 deletions tests/fixtures/cython-import-package/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[build-system]
requires = ["setuptools >= 40.6.0", "setuptools-scm", "cython"]
build-backend = "setuptools.build_meta"

[tool.black]
line-length = 90
target_version = ['py27', 'py35', 'py36', 'py37', 'py38']
include = '\.pyi?$'
exclude = '''
/(
\.eggs
| \.git
| \.hg
| \.mypy_cache
| \.tox
| \.pyre_configuration
| \.venv
| _build
| buck-out
| build
| dist
)
'''

[tool.towncrier]
package = 'cython-import-package'
package_dir = 'src'
filename = 'CHANGELOG.rst'
directory = 'news/'
title_format = '{version} ({project_date})'
issue_format = '`#{issue} <https://github.com/sarugaku/cython_import_package/issues/{issue}>`_'
template = 'tasks/CHANGELOG.rst.jinja2'

[[tool.towncrier.type]]
directory = 'feature'
name = 'Features'
showcontent = true

[[tool.towncrier.type]]
directory = 'bugfix'
name = 'Bug Fixes'
showcontent = true

[[tool.towncrier.type]]
directory = 'trivial'
name = 'Trivial Changes'
showcontent = false

[[tool.towncrier.type]]
directory = 'removal'
name = 'Removals and Deprecations'
showcontent = true
58 changes: 58 additions & 0 deletions tests/fixtures/cython-import-package/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
[metadata]
name = cython_import_package
package_name = cython-import-package
description = A fake python package.
url = https://github.com/sarugaku/cython_import_package
author = Dan Ryan
author_email = dan@danryan.co
long_description = file: README.rst
license = ISC License
keywords = fake package test
classifier =
Development Status :: 1 - Planning
License :: OSI Approved :: ISC License (ISCL)
Operating System :: OS Independent
Programming Language :: Python :: 2
Programming Language :: Python :: 2.6
Programming Language :: Python :: 2.7
Programming Language :: Python :: 3
Programming Language :: Python :: 3.4
Programming Language :: Python :: 3.5
Programming Language :: Python :: 3.6
Programming Language :: Python :: 3.7
Topic :: Software Development :: Libraries :: Python Modules

[options.extras_require]
tests =
pytest
pytest-xdist
pytest-cov
pytest-timeout
readme-renderer[md]
twine
dev =
black;python_version>="3.6"
flake8
flake8-bugbear;python_version>="3.5"
invoke
isort
mypy;python_version>="3.5"
parver
pre-commit
rope
wheel

[options]
zip_safe = true
python_requires = >=2.6,!=3.0,!=3.1,!=3.2,!=3.3
install_requires =
attrs
vistir

[bdist_wheel]
universal = 1

[egg_info]
tag_build =
tag_date = 0

43 changes: 43 additions & 0 deletions tests/fixtures/cython-import-package/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import ast
import os

from setuptools import setup, find_packages
from setuptools.command.test import test as TestCommand

# ORDER MATTERS
# Import this after setuptools or it will fail
from Cython.Build import cythonize # noqa: I100
import Cython.Distutils



ROOT = os.path.dirname(__file__)

PACKAGE_NAME = 'cython_import_package'

VERSION = None

with open(os.path.join(ROOT, 'src', PACKAGE_NAME.replace("-", "_"), '__init__.py')) as f:
for line in f:
if line.startswith('__version__ = '):
VERSION = ast.literal_eval(line[len('__version__ = '):].strip())
break
if VERSION is None:
raise EnvironmentError('failed to read version')


# Put everything in setup.cfg, except those that don't actually work?
setup(
# These really don't work.
package_dir={'': 'src'},
packages=find_packages('src'),

# I don't know how to specify an empty key in setup.cfg.
package_data={
'': ['LICENSE*', 'README*'],
},
setup_requires=["setuptools_scm", "cython"],

# I need this to be dynamic.
version=VERSION,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
__version__ = "0.0.1"
51 changes: 51 additions & 0 deletions tests/fixtures/legacy-backend-package/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
[build-system]
requires = ["setuptools>=30.3.0", "wheel", "setuptools_scm>=3.3.1"]

[tool.black]
line-length = 90
target_version = ['py27', 'py35', 'py36', 'py37', 'py38']
include = '\.pyi?$'
exclude = '''
/(
\.eggs
| \.git
| \.hg
| \.mypy_cache
| \.tox
| \.pyre_configuration
| \.venv
| _build
| buck-out
| build
| dist
)
'''

[tool.towncrier]
package = 'legacy-backend-package'
package_dir = 'src'
filename = 'CHANGELOG.rst'
directory = 'news/'
title_format = '{version} ({project_date})'
issue_format = '`#{issue} <https://github.com/sarugaku/legacy_backend_package/issues/{issue}>`_'
template = 'tasks/CHANGELOG.rst.jinja2'

[[tool.towncrier.type]]
directory = 'feature'
name = 'Features'
showcontent = true

[[tool.towncrier.type]]
directory = 'bugfix'
name = 'Bug Fixes'
showcontent = true

[[tool.towncrier.type]]
directory = 'trivial'
name = 'Trivial Changes'
showcontent = false

[[tool.towncrier.type]]
directory = 'removal'
name = 'Removals and Deprecations'
showcontent = true
Loading

0 comments on commit 47aa2ac

Please sign in to comment.