Skip to content

Use $HOME as CWD for virtualenv creation #4852

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 5, 2018
Merged

Use $HOME as CWD for virtualenv creation #4852

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions readthedocs/doc_builder/python_environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,10 @@ def setup_base(self):
site_packages,
'--no-download',
env_path,
bin_path=None, # Don't use virtualenv bin that doesn't exist yet
cwd=self.checkout_path,
# Don't use virtualenv bin that doesn't exist yet
bin_path=None,
# Don't use the project's root, some config files can interfere
cwd='$HOME',
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should be running this from /tmp or something else that isn't in the checkout. I guess we aren't always sure any other path will exist (unless we make one w/ the tempfile module), so this seems fine. But I do wonder if we'll hit other issues at some point (eg. if we add our own pyproject.toml` file for RTD?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do the docker containers have access to the rtd code? I don't think so. But, yeah, using /tmp sounds better. People running this outside docker could have some problems I think, but I'm less worried about that. Should I go for plain /tmp or create one with the tempfile module?

Copy link
Member

Choose a reason for hiding this comment

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

Ah good point, I forgot about Docker not having access. I think we can just go with $HOME for now, because it feels overkill (and could lead to errors) to create a temp directory.

)

def install_core_requirements(self):
Expand Down