Set build_isolation and use_pep517 correctly #4248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- `ignore_compatibility` is meant to resolve hashes into the lockfile after resolution happens - We still want compatible items to be the ones we actually tell pip to install - Fixes #4231 Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
techalchemy
added
Category: Dependency Resolution
- 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>
frostming
reviewed
May 9, 2020
pipenv/environments.py
Outdated
| if positive_lookup in os.environ: | ||
| if _is_env_truthy(positive_lookup): | ||
| return bool(os.environ[positive_lookup]) | ||
| return os.environ[positive_lookup] |
There was a problem hiding this comment.
It seems wrong to me. Think of ENABLE_SOMETHING=0, does it mean to enable or disable?
There was a problem hiding this comment.
so the answer to that is it should be disabled -- the real answer is that I was not at all thinking about how _is_env_truthy works and it really can't be used here to achieve this
There was a problem hiding this comment.
Yeah, you are probably seeking for _is_env_boolean_like.
There was a problem hiding this comment.
oh gosh is that a thing, I couldn't think of the name of it and probably just reinvented it
There was a problem hiding this comment.
It is not implemented, I just make up the name that indicates how the function should work.
Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
use_pep517andbuild_isolationare read from theenvironment -- introduce a new environment helper to detect
<PREFIX>_<SETTING>and<PREFIX>_NO_<SETTING>, check for booleansand return appropriately boolean, str, or None types
Falsevalues when adding--no-use-pep517and--no-build-isolationduring resolution rather than falsey valuesPIP_PYTHON_VERSIONtoPIPENV_REQUESTED_PYTHON_VERSIONto avoid causingpipto fail dueto accidentally percieving the
python_versionflag as being set --this is an artifact from attempting to resolve outside of the
virtualenv
pipenvto the path of patchednotpip.__main__to accommodateupdated import fully qualified module names
pipandpiptoolspatchessetuptoolswith a missingbuild-backend(whichpipforces tobuild_meta:__legacy__& which doesn't exist before40.8), andimport Cythonstatements insetup.pyin packages with properlydefined
pyproject.tomlbuild-backendlines.