Skip to content

Commit

Permalink
Build: expose READTHEDOCS_VIRTUALENV_PATH variable (#9971)
Browse files Browse the repository at this point in the history
* Build: expose `READTHEDOCS_VIRTUALENV_PATH` variable

This variable points to where Read the Docs created the virtualenv.

Closes #9629

* Build: use our own variable `$READTHEDOCS_VIRTUALENV_PATH`

This variable contains the path we need to call the Python executable.

* Build: use environment variables for Conda as well

* Tests: add missing variable

* Lint

* Lint again

* Missing :

* Test: update env variables

* Test: make it work locally and CicleCI

* Docs: add link to the builds page

* Refactor: make `venv_bin` more concrete to match venv/conda

* Build: missing return

* API: sanitize build command properly

* Update readthedocs/api/v2/serializers.py

Co-authored-by: Eric Holscher <[email protected]>

---------

Co-authored-by: Eric Holscher <[email protected]>
  • Loading branch information
humitos and ericholscher authored Feb 7, 2023
1 parent 58e38e0 commit ee1eb04
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 18 deletions.
7 changes: 7 additions & 0 deletions docs/user/environment-variables.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ Read the Docs builders set the following environment variables automatically for

:Examples: ``en``, ``it``, ``de_AT``, ``es``, ``pt_BR``

.. envvar:: READTHEDOCS_VIRTUALENV_PATH

Path for the :ref:`virtualenv that was created for this build <builds:Understanding what's going on>`.
Only exists for builds using Virtualenv and not Conda.

:Example: ``/home/docs/checkouts/readthedocs.org/user_builds/project/envs/version``

User-defined environment variables
----------------------------------

Expand Down
11 changes: 10 additions & 1 deletion readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,17 @@ def get_command(self, obj):
docroot = re.sub("/[0-9a-z]+/?$", "", settings.DOCROOT, count=1)
container_hash = "/([0-9a-z]+/)?"

command = obj.command
regex = f"{docroot}{container_hash}{project_slug}/envs/{version_slug}(/bin/)?"
command = re.sub(regex, "", obj.command, count=1)
command = re.sub(regex, "", command, count=1)

# Remove explicit variable names we use to run commands,
# since users don't care about these.
regex = r"^\$READTHEDOCS_VIRTUALENV_PATH/bin/"
command = re.sub(regex, "", command, count=1)

regex = r"^\$CONDA_ENVS_PATH/\\$CONDA_DEFAULT_ENV/bin/"
command = re.sub(regex, "", command, count=1)
return command


Expand Down
4 changes: 4 additions & 0 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ def get_build_env_vars(self):
if self.data.config.conda is not None:
env.update(
{
# NOTE: should these be prefixed with "READTHEDOCS_"?
"CONDA_ENVS_PATH": os.path.join(
self.data.project.doc_path, "conda"
),
Expand All @@ -577,6 +578,9 @@ def get_build_env_vars(self):
self.data.version.slug,
"bin",
),
"READTHEDOCS_VIRTUALENV_PATH": os.path.join(
self.data.project.doc_path, "envs", self.data.version.slug
),
}
)

Expand Down
13 changes: 12 additions & 1 deletion readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,18 @@ def get_wrapped_command(self):

def _escape_command(self, cmd):
r"""Escape the command by prefixing suspicious chars with `\`."""
return self.bash_escape_re.sub(r'\\\1', cmd)
command = self.bash_escape_re.sub(r"\\\1", cmd)

# HACK: avoid escaping variables that we need to use in the commands
not_escape_variables = (
"READTHEDOCS_OUTPUT",
"READTHEDOCS_VIRTUALENV_PATH",
"CONDA_ENVS_PATH",
"CONDA_DEFAULT_ENV",
)
for variable in not_escape_variables:
command = command.replace(f"\\${variable}", f"${variable}")
return command


class BaseEnvironment:
Expand Down
37 changes: 22 additions & 15 deletions readthedocs/doc_builder/python_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ def install_package(self, install):
:param install: A install object from the config module.
:type install: readthedocs.config.models.PythonInstall
"""
# NOTE: `venv_bin` requires `prefixes`.
# However, it's overwritten in the subclasses and
# it forces passing the `prefixes=` attribute.
# I'm not sure how to solve this, so I'm skipping this check for now.
# pylint: disable=no-value-for-parameter

if install.method == PIP:
# Prefix ./ so pip installs from a local path rather than pypi
local_path = (
Expand Down Expand Up @@ -89,17 +95,17 @@ def install_package(self, install):
bin_path=self.venv_bin(),
)

def venv_bin(self, filename=None):
def venv_bin(self, prefixes, filename=None):
"""
Return path to the virtualenv bin path, or a specific binary.
:param filename: If specified, add this filename to the path return
:param prefixes: List of path prefixes to include in the resulting path
:returns: Path to virtualenv bin or filename in virtualenv bin
"""
parts = [self.venv_path(), 'bin']
if filename is not None:
parts.append(filename)
return os.path.join(*parts)
prefixes.append(filename)
return os.path.join(*prefixes)


class Virtualenv(PythonEnvironment):
Expand All @@ -110,8 +116,10 @@ class Virtualenv(PythonEnvironment):
.. _virtualenv: https://virtualenv.pypa.io/
"""

def venv_path(self):
return os.path.join(self.project.doc_path, 'envs', self.version.slug)
# pylint: disable=arguments-differ
def venv_bin(self, filename=None):
prefixes = ["$READTHEDOCS_VIRTUALENV_PATH", "bin"]
return super().venv_bin(prefixes, filename=filename)

def setup_base(self):
"""
Expand All @@ -133,9 +141,7 @@ def setup_base(self):
cli_args.append('--system-site-packages')

# Append the positional destination argument
cli_args.append(
self.venv_path(),
)
cli_args.append("$READTHEDOCS_VIRTUALENV_PATH")

self.build_env.run(
self.config.python_interpreter,
Expand Down Expand Up @@ -167,7 +173,9 @@ def install_core_requirements(self):
)
cmd = pip_install_cmd + [pip_version, 'setuptools<58.3.0']
self.build_env.run(
*cmd, bin_path=self.venv_bin(), cwd=self.checkout_path
*cmd,
bin_path=self.venv_bin(),
cwd=self.checkout_path,
)

requirements = []
Expand Down Expand Up @@ -318,8 +326,10 @@ class Conda(PythonEnvironment):
.. _Conda: https://conda.io/docs/
"""

def venv_path(self):
return os.path.join(self.project.doc_path, 'conda', self.version.slug)
# pylint: disable=arguments-differ
def venv_bin(self, filename=None):
prefixes = ["$CONDA_ENVS_PATH", "$CONDA_DEFAULT_ENV", "bin"]
return super().venv_bin(prefixes, filename=filename)

def conda_bin_name(self):
"""
Expand Down Expand Up @@ -354,9 +364,6 @@ def _update_conda_startup(self):
)

def setup_base(self):
conda_env_path = os.path.join(self.project.doc_path, 'conda')
os.path.join(conda_env_path, self.version.slug)

if self.project.has_feature(Feature.UPDATE_CONDA_STARTUP):
self._update_conda_startup()

Expand Down
3 changes: 3 additions & 0 deletions readthedocs/projects/tests/test_build_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@ def test_get_env_vars(self, load_yaml_config, build_environment, config, externa
"bin",
),
PUBLIC_TOKEN="a1b2c3",
# Local and Circle are different values.
# We only check it's present, but not its value.
READTHEDOCS_VIRTUALENV_PATH=mock.ANY,
)
if not external:
expected_build_env_vars["PRIVATE_TOKEN"] = "a1b2c3"
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/rtd_tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def test_response_finished_and_success(self):
buildcommandresult = get(
BuildCommandResult,
build=build,
command="/home/docs/checkouts/readthedocs.org/user_builds/myproject/envs/myversion/bin/python -m pip install --upgrade --no-cache-dir pip setuptools<58.3.0",
command="python -m pip install --upgrade --no-cache-dir pip setuptools<58.3.0",
exit_code=0,
)
resp = client.get('/api/v2/build/{build}/'.format(build=build.pk))
Expand Down

0 comments on commit ee1eb04

Please sign in to comment.