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.

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

Jump to bottom

Install latest version of pip #4938

Merged
merged 5 commits into from
Dec 4, 2018
Merged

Install latest version of pip #4938

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
718aa8e
Install latest version of pip
stsewd Nov 29, 2018
f4815c0
Copy list explicitly
stsewd Dec 3, 2018
7f5ac89
Merge branch 'master' into install-latest-pip
stsewd Dec 3, 2018
e98083a
Unpin pip
stsewd Dec 3, 2018
2adec8c
Python2 again
stsewd Dec 4, 2018
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
31 changes: 21 additions & 10 deletions readthedocs/doc_builder/python_environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,24 @@ def setup_base(self):

def install_core_requirements(self):
"""Install basic Read the Docs requirements into the virtualenv."""
pip_install_cmd = [
'python',
self.venv_bin(filename='pip'),
'install',
'--upgrade',
'--cache-dir',
self.project.pip_cache_path,
]

# Install latest pip first,
# so it is used when installing the other requirements.
cmd = pip_install_cmd + ['pip==18.1']
Copy link
Member

Choose a reason for hiding this comment

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

Do we really expect to update this regularly? Feels like we should probably just call it with --upgrade, but I also worry about being exposed to pip bugs on release :/

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the release history looks good https://pypi.org/project/pip/#history. Not sure, I'm also worry about not having control of the version in case a bug comes up from pip.

Copy link
Member

Choose a reason for hiding this comment

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

pip is a really mature piece of software with active development. All the python ecosystem relies on it. I think it will be better to just unpin it and rely on them.

The case would be different if we are talking about a different package.

self.build_env.run(
*cmd,
bin_path=self.venv_bin(),
cwd=self.checkout_path
)

requirements = [
'Pygments==2.2.0',
# Assume semver for setuptools version, support up to next backwards
Expand Down Expand Up @@ -255,21 +273,14 @@ def install_core_requirements(self):
'readthedocs-sphinx-ext<0.6'
])

cmd = [
'python',
self.venv_bin(filename='pip'),
'install',
'--upgrade',
'--cache-dir',
self.project.pip_cache_path,
]
cmd = pip_install_cmd
stsewd marked this conversation as resolved.
Show resolved Hide resolved
if self.config.python.use_system_site_packages:
# Other code expects sphinx-build to be installed inside the
# virtualenv. Using the -I option makes sure it gets installed
# even if it is already installed system-wide (and
# --system-site-packages is used)
cmd.append('-I')
cmd.extend(requirements)
cmd += ['-I']
cmd += requirements
self.build_env.run(
*cmd,
bin_path=self.venv_bin(),
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/rtd_tests/tests/test_doc_building.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ def test_install_core_requirements_sphinx(self, checkout_path):
]
requirements = self.base_requirements + requirements_sphinx
args = self.pip_install_args + requirements
self.build_env_mock.run.assert_called_once()
self.assertEqual(self.build_env_mock.run.call_count, 2)
self.assertArgsStartsWith(args, self.build_env_mock.run)

@patch('readthedocs.projects.models.Project.checkout_path')
Expand All @@ -1212,7 +1212,7 @@ def test_install_core_requirements_mkdocs(self, checkout_path):
]
requirements = self.base_requirements + requirements_mkdocs
args = self.pip_install_args + requirements
self.build_env_mock.run.assert_called_once()
self.assertEqual(self.build_env_mock.run.call_count, 2)
self.assertArgsStartsWith(args, self.build_env_mock.run)

@patch('readthedocs.projects.models.Project.checkout_path')
Expand Down