Build: expose READTHEDOCS_VIRTUALENV_PATH variable

[humitos]

The variable $READTHEDOCS_VIRTUALENV_PATH points to where Read the Docs created the virtualenv.

While working on this I found that we can use this variable to run our own commands. This way, we will be avoiding mistakes and potentially having differences path between "the real path" and the content of this variable. I changed our code to use this variable when running /bin/python and others.

Besides, I did the same for Conda environments and the variables CONDA_ENV_PATHS and CONDA_DEFAULT_ENV that were already defined.

This makes the code cleaner and simpler, while more standardized as well. Users can use these variables in the same way Read the Docs does and be sure the content they are expecting is going to be there.

Closes #9629

---

📚 Documentation previews 📚

• User's documentation (docs): https://docs--9971.org.readthedocs.build/en/9971/

• Developer's documentation (dev): https://dev--9971.org.readthedocs.build/en/9971/

[ericholscher]

I have a couple concerns here, but I agree with the overall approach. Using this env definitely simplifies things and allows us to make sure we don't break them for the user, as well.

I think the hacky part and possible user confusion is my biggest worry.

[ericholscher]

This seems like an awkward place to introduce conda. Previously this function didn't know about conda at all, and I'm wondering if it's the right place for this logic? Or perhaps we need to rename this function?

[humitos]

I don't have a strong preference here. It's the same code -- The only difference is the prefix (virtualenv or condaenv variable). I put it in the super-class only to avoid repetition of the same logic.

[humitos]

I updated this to make the code more explicit: 66fe0a7. Let me know if that looks better 👍🏼

[ericholscher]

That is definitely nicer. Though I'd argue the venv in the name is misleading for conda envs :) Perhaps it should just be called env_bin or similar? Not a huge deal tho.

[humitos]

This is ready for another round of review. I think I addressed all the feedback provided.

[ericholscher]

I'm on board with this as is.

A small nitpick is the venv_bin implementation is still a bit hacky how it is different in signature. Now that we've removed some so much logic from it, I wonder if it's even still useful?

[humitos]

I added a note in PythonEnvironment.install_package about the venv_bin and how we are managing the prefixes= attribute since I'm skipping that linter check for now. I'm not sure how to solve it without re-structuring the code or duplicating the logic from venv_bin (maybe that's the simplest thing to do, tho).

We can come back to this later if we want to. For now, I want to deploy the feature and unblock this to move forward and deploy these changes.

[Bilokin]

Hello everyone,

I have a build failure with an error for a project which uses conda:

python -mvirtualenv $READTHEDOCS_VIRTUALENV_PATH
SystemExit: 2
usage: virtualenv [--version] [--with-traceback] [-v | -q] [--read-only-app-data] [--app-data APP_DATA] [--reset-app-data] [--upgrade-embed-wheels] [--discovery {builtin}] [-p py] [--try-first-with py_exe]
                  [--creator {builtin,cpython3-posix,venv}] [--seeder {app-data,pip}] [--no-seed] [--activators comma_sep_list] [--clear] [--no-vcs-ignore] [--system-site-packages] [--symlinks | --copies] [--no-download | --download]
                  [--extra-search-dir d [d ...]] [--pip version] [--setuptools version] [--wheel version] [--no-pip] [--no-setuptools] [--no-wheel] [--no-periodic-update] [--symlink-app-data] [--prompt prompt] [-h]
                  dest
virtualenv: error: the following arguments are required: dest

I think this error is related to this PR.
Is there workaround for this issue?
Or do I need to set READTHEDOCS_VIRTUALENV_PATH environment variable value from now on?

[humitos]

@Bilokin please open a new issue filling the template and linking back to this issue if you think it's related. Thanks 👍