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

Add pyenv shims to PATH #1263

Merged
merged 3 commits into from
May 10, 2021

Conversation

danielmitterdorfer
Copy link
Member

Recently the pyenv init command does not add pyenv shims to the PATH
anymore. This needs to be done now explicitly with a new command. We add
the command in build scripts and our documentation to make sure shims
are available.

Recently the pyenv init command does not add pyenv shims to the `PATH`
anymore. This needs to be done now explicitly with a new command. We add
the command in build scripts and our documentation to make sure shims
are available.
@danielmitterdorfer danielmitterdorfer added the bug Something's wrong label May 10, 2021
@danielmitterdorfer danielmitterdorfer added this to the 2.2.1 milestone May 10, 2021
@danielmitterdorfer danielmitterdorfer self-assigned this May 10, 2021
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM, let's get our CI green again.

Makefile Outdated
@@ -29,8 +29,7 @@ VENV_ACTIVATE = . $(VENV_ACTIVATE_FILE)
VEPYTHON = $(VENV_NAME)/bin/$(PY_BIN)
VEPYLINT = $(VENV_NAME)/bin/pylint
PYENV_ERROR = "\033[0;31mIMPORTANT\033[0m: Please install pyenv.\n"
PYENV_PATH_ERROR = "\033[0;31mIMPORTANT\033[0m: Please add $(HOME)/$(PYENV_REGEX) to your PATH env.\n"
PYENV_PREREQ_HELP = "\033[0;31mIMPORTANT\033[0m: please add \033[0;31meval \"\$$(pyenv init -)\"\033[0m to your bash profile and restart your terminal before proceeding any further.\n"
PYENV_PREREQ_HELP = "\033[0;31mIMPORTANT\033[0m: please add \033[0;31meval \"\$$(pyenv init -)\"\033[0m and \033[0;31meval \"\$$(pyenv init --path)\"\033[0m to your bash profile and restart your terminal before proceeding any further.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed offline maybe we just reference the instructions; the most authoritative form I think is by executing pyenv init.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in ffb25ba.

@@ -6,7 +6,7 @@ Prerequisites

Install the following software packages:

* Pyenv installed and ``eval "$(pyenv init -)"`` is added to the shell configuration file. For more details please refer to the PyEnv `installation instructions <https://github.com/pyenv/pyenv#installation>`_.
* Pyenv installed, ``eval "$(pyenv init -)"`` and ``eval "$(pyenv init --path)"`` is added to the shell configuration file. For more details please refer to the PyEnv `installation instructions <https://github.com/pyenv/pyenv#installation>`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise here, let's reference pyenv init for the prerequisites.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in ffb25ba.

@@ -44,6 +44,7 @@ function build {
export LC_ALL=en_US.UTF-8
update_pyenv
eval "$(pyenv init -)"
eval "$(pyenv init --path)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should leave a breadcrumb where this comes from i.e. either pyenv/pyenv#1906 (comment) or the issue itself pyenv/pyenv#1906

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in ffb25ba.

@danielmitterdorfer danielmitterdorfer merged commit 9608870 into elastic:master May 10, 2021
@danielmitterdorfer danielmitterdorfer deleted the pyenv-path branch May 10, 2021 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants