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

Allows overriding PIP_EXISTS_ACTION environment variable (it was forced to w) #3738

Merged
merged 8 commits into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
PIPENV_CACHE_DIR, PIPENV_COLORBLIND, PIPENV_DEFAULT_PYTHON_VERSION,
PIPENV_DONT_USE_PYENV, PIPENV_HIDE_EMOJIS, PIPENV_MAX_SUBPROCESS,
PIPENV_PYUP_API_KEY, PIPENV_SHELL_FANCY, PIPENV_SKIP_VALIDATION,
PIPENV_YES, SESSION_IS_INTERACTIVE
PIPENV_YES, SESSION_IS_INTERACTIVE, PIP_EXISTS_ACTION
)
from .project import Project, SourceNotFound
from .utils import (
Expand Down Expand Up @@ -1503,7 +1503,7 @@ def pip_install(
"PIP_DESTINATION_DIR": vistir.misc.fs_str(
cache_dir.joinpath("pkgs").as_posix()
),
"PIP_EXISTS_ACTION": vistir.misc.fs_str("w"),
"PIP_EXISTS_ACTION": vistir.misc.fs_str(PIP_EXISTS_ACTION or "w"),
Copy link
Member

@uranusjr uranusjr May 19, 2019

Choose a reason for hiding this comment

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

Is this a good idea? The line in environments.py sets w when the variable is missing, so maybe we should respect the user explicitly setting it to an empty string.

Copy link
Member

Choose a reason for hiding this comment

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

That would just mean using the pip defaults which seems fine to me, I don't think it matters much -- probably better to be consistent and do less hidden assumptions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@uranusjr may be right, we should leave it to PIP, in case it gets overriden by empty string.
However, explicit is better than implicit.
Your choice.

Copy link
Member

Choose a reason for hiding this comment

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

Honestly I doubt whether anyone would care about this extreme edge case, so in the end it’s only a theoretical preference.

Copy link
Member

Choose a reason for hiding this comment

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

I'll be happy to merge this either way, planning to do so once CI passes

"PATH": vistir.misc.fs_str(os.environ.get("PATH")),
}
if src:
Expand Down
6 changes: 6 additions & 0 deletions pipenv/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,12 @@ def _is_env_truthy(name):
NOTE: This only affects the ``install`` and ``uninstall`` commands.
"""

PIP_EXISTS_ACTION = os.environ.get("PIP_EXISTS_ACTION", "w")
"""Specifies the value for pip's --exists-action option

Defaullts to (w)ipe
"""

PIPENV_PYUP_API_KEY = os.environ.get(
"PIPENV_PYUP_API_KEY", "1ab8d58f-5122e025-83674263-bc1e79e0"
)
Expand Down