From ee1eb043c260e5d3b691ed4ecb9512c400e138e1 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 7 Feb 2023 11:02:27 +0100 Subject: [PATCH] Build: expose `READTHEDOCS_VIRTUALENV_PATH` variable (#9971) * 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 <25510+ericholscher@users.noreply.github.com> --------- Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com> --- docs/user/environment-variables.rst | 7 ++++ readthedocs/api/v2/serializers.py | 11 +++++- readthedocs/doc_builder/director.py | 4 ++ readthedocs/doc_builder/environments.py | 13 ++++++- .../doc_builder/python_environments.py | 37 +++++++++++-------- .../projects/tests/test_build_tasks.py | 3 ++ readthedocs/rtd_tests/tests/test_api.py | 2 +- 7 files changed, 59 insertions(+), 18 deletions(-) diff --git a/docs/user/environment-variables.rst b/docs/user/environment-variables.rst index 83db28d5762..7833c6a0ad8 100644 --- a/docs/user/environment-variables.rst +++ b/docs/user/environment-variables.rst @@ -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 `. + Only exists for builds using Virtualenv and not Conda. + + :Example: ``/home/docs/checkouts/readthedocs.org/user_builds/project/envs/version`` + User-defined environment variables ---------------------------------- diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index 37c58e06c6d..0e14572c372 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -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 diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index fedd6ed7f63..0485169ba05 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -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" ), @@ -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 + ), } ) diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 979fc18dda8..63198f9d403 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -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: diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 193b9494d53..15713ae4b94 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -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 = ( @@ -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): @@ -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): """ @@ -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, @@ -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 = [] @@ -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): """ @@ -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() diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index e1b2d28e973..9eb7f4e5915 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -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" diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index de34a19f104..33d28050048 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -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))