From 23d2859cd6c0a3ad8846d3a429c7dacddd753635 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 12 Aug 2015 14:46:57 -0700 Subject: [PATCH 1/2] Call python scripts as argument to virtualenv python command This fixes #994, where the 127 character limit on shebang line lengths was triggering failures of commands inside virtualenvs that had long names or long branch names. This PR adds some path environment variable handling to allow for shorter commands, and makes all python script call wrapped by a call to the virtual env symlinked python binary. --- readthedocs/doc_builder/backends/mkdocs.py | 7 +++++- readthedocs/doc_builder/backends/sphinx.py | 12 +++++++--- readthedocs/doc_builder/environments.py | 8 ++++++- readthedocs/projects/models.py | 8 +++++++ readthedocs/projects/tasks.py | 26 +++++++++++++++------- readthedocs/rtd_tests/tests/test_builds.py | 3 ++- 6 files changed, 50 insertions(+), 14 deletions(-) diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 869fb0253c3..8644db08f6b 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -130,6 +130,7 @@ def append_conf(self, **kwargs): def build(self, **kwargs): checkout_path = self.project.checkout_path(self.version.slug) build_command = [ + 'python', self.project.venv_bin(version=self.version.slug, bin='mkdocs'), self.builder, '--clean', @@ -137,7 +138,11 @@ def build(self, **kwargs): ] if self.use_theme: build_command.extend(['--theme', 'readthedocs']) - cmd_ret = self.run(*build_command, cwd=checkout_path) + cmd_ret = self.run( + *build_command, + cwd=checkout_path, + bin_path=self.project.venv_bin(version=self.version.slug, bin=None) + ) return cmd_ret.successful diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index 1db47b12877..868d8dd6373 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -126,6 +126,7 @@ def build(self, **kwargs): self.clean() project = self.project build_command = [ + 'python', project.venv_bin(version=self.version.slug, bin='sphinx-build'), '-T' ] @@ -138,8 +139,11 @@ def build(self, **kwargs): '.', self.sphinx_build_dir ]) - cmd_ret = self.run(*build_command, - cwd=project.conf_dir(self.version.slug)) + cmd_ret = self.run( + *build_command, + cwd=project.conf_dir(self.version.slug), + bin_path=project.venv_bin(version=self.version.slug, bin=None) + ) return cmd_ret.successful @@ -232,13 +236,15 @@ def build(self, **kwargs): # Default to this so we can return it always. self.run( + 'python', self.project.venv_bin(version=self.version.slug, bin='sphinx-build'), '-b', 'latex', '-D', 'language={lang}'.format(lang=self.project.language), '-d', '_build/doctrees', '.', '_build/latex', - cwd=cwd + cwd=cwd, + bin_path=self.project.venv_bin(version=self.version.slug, bin=None) ) latex_cwd = os.path.join(cwd, '_build', 'latex') tex_files = glob(os.path.join(latex_cwd, '*.tex')) diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index a341b8bf739..a97935d471c 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -44,7 +44,8 @@ class BuildCommand(object): # TODO add short name here for reporting def __init__(self, command, cwd=None, shell=False, environment=None, - combine_output=True, input_data=None, build_env=None): + combine_output=True, input_data=None, build_env=None, + bin_path=None): self.command = command self.shell = shell if cwd is None: @@ -55,6 +56,7 @@ def __init__(self, command, cwd=None, shell=False, environment=None, self.environment.update(environment) self.combine_output = combine_output self.build_env = build_env + self.bin_path = bin_path self.status = None self.input_data = input_data self.output = None @@ -91,6 +93,10 @@ def run(self): del environment['DJANGO_SETTINGS_MODULE'] if 'PYTHONPATH' in environment: del environment['PYTHONPATH'] + if self.bin_path is not None: + env_paths = environment.get('PATH', '').split(':') + env_paths.insert(0, self.bin_path) + environment['PATH'] = ':'.join(env_paths) try: proc = subprocess.Popen( diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 9dc02450cf5..8a23ad0c5d9 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -485,6 +485,14 @@ def single_version_symlink_path(self): # def venv_bin(self, version=LATEST, bin='python'): + """Return path to the virtualenv bin path, or a specific binary + + By default, return the path to the ``python`` binary in the virtual + environment path. If ``bin`` is :py:data:`None`, then return the path to + the virtual env path. + """ + if bin is None: + bin = '' return os.path.join(self.venv_path(version), 'bin', bin) def full_doc_path(self, version=LATEST): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 2566d34c277..df74861a311 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -267,6 +267,7 @@ def setup_environment(self): ] cmd = [ + 'python', self.project.venv_bin(version=self.version.slug, bin='pip'), 'install', '--use-wheel', @@ -280,7 +281,10 @@ def setup_environment(self): # --system-site-packages is used) cmd.append('-I') cmd.extend(requirements) - self.build_env.run(*cmd) + self.build_env.run( + *cmd, + bin_path=self.project.venv_bin(version=self.version.slug, bin=None) + ) # Handle requirements requirements_file_path = self.project.requirements_file @@ -298,11 +302,14 @@ def setup_environment(self): if requirements_file_path: self.build_env.run( + 'python', self.project.venv_bin(version=self.version.slug, bin='pip'), 'install', '--exists-action=w', '-r{0}'.format(requirements_file_path), - cwd=checkout_path + cwd=checkout_path, + bin_path=self.project.venv_bin(version=self.version.slug, + bin=None) ) # Handle setup.py @@ -311,21 +318,24 @@ def setup_environment(self): if os.path.isfile(setup_path): if getattr(settings, 'USE_PIP_INSTALL', False): self.build_env.run( - self.project.venv_bin(version=self.version.slug, - bin='pip'), + 'python', + self.project.venv_bin(version=self.version.slug, bin='pip'), 'install', '--ignore-installed', '.', - cwd=checkout_path + cwd=checkout_path, + bin_path=self.project.venv_bin(version=self.version.slug, + bin=None) ) else: self.build_env.run( - self.project.venv_bin(version=self.version.slug, - bin='python'), + 'python', 'setup.py', 'install', '--force', - cwd=checkout_path + cwd=checkout_path, + bin_path=self.project.venv_bin(version=self.version.slug, + bin=None) ) def build_docs(self): diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index e7822811196..b47ca877436 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -46,7 +46,8 @@ def test_build(self): # Get command and check first part of command list is a call to sphinx self.assertEqual(self.mocks.popen.call_count, 1) cmd = self.mocks.popen.call_args_list[0][0] - self.assertRegexpMatches(cmd[0][0], r'sphinx-build') + self.assertRegexpMatches(cmd[0][0], r'python') + self.assertRegexpMatches(cmd[0][1], r'sphinx-build') def test_build_respects_pdf_flag(self): '''Build output format control''' From 9dfa0de18ec8c2a0f5a3e2dad0bb3aebf39b73b0 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 12 Aug 2015 16:24:52 -0700 Subject: [PATCH 2/2] Better defaults and some cleanup on Project.venv_bin --- readthedocs/doc_builder/backends/mkdocs.py | 2 +- readthedocs/doc_builder/backends/sphinx.py | 4 ++-- readthedocs/projects/models.py | 15 ++++++++------- readthedocs/projects/tasks.py | 11 ++++------- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 8644db08f6b..dde624ef4b0 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -141,7 +141,7 @@ def build(self, **kwargs): cmd_ret = self.run( *build_command, cwd=checkout_path, - bin_path=self.project.venv_bin(version=self.version.slug, bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) return cmd_ret.successful diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index 868d8dd6373..9814c235433 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -142,7 +142,7 @@ def build(self, **kwargs): cmd_ret = self.run( *build_command, cwd=project.conf_dir(self.version.slug), - bin_path=project.venv_bin(version=self.version.slug, bin=None) + bin_path=project.venv_bin(version=self.version.slug) ) return cmd_ret.successful @@ -244,7 +244,7 @@ def build(self, **kwargs): '.', '_build/latex', cwd=cwd, - bin_path=self.project.venv_bin(version=self.version.slug, bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) latex_cwd = os.path.join(cwd, '_build', 'latex') tex_files = glob(os.path.join(latex_cwd, '*.tex')) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 8a23ad0c5d9..bedb99004f5 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -484,16 +484,17 @@ def single_version_symlink_path(self): # End symlink paths # - def venv_bin(self, version=LATEST, bin='python'): + def venv_bin(self, version=LATEST, bin=None): """Return path to the virtualenv bin path, or a specific binary - By default, return the path to the ``python`` binary in the virtual - environment path. If ``bin`` is :py:data:`None`, then return the path to - the virtual env path. + If ``bin`` is :py:data:`None`, then return the path to the virtual env + path, otherwise, return the path to the executable ``bin`` in the + virtual env ``bin`` path """ - if bin is None: - bin = '' - return os.path.join(self.venv_path(version), 'bin', bin) + parts = [self.venv_path(version), 'bin'] + if bin is not None: + parts.append(bin) + return os.path.join(*parts) def full_doc_path(self, version=LATEST): """ diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index df74861a311..9bc29dc8589 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -283,7 +283,7 @@ def setup_environment(self): cmd.extend(requirements) self.build_env.run( *cmd, - bin_path=self.project.venv_bin(version=self.version.slug, bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) # Handle requirements @@ -308,8 +308,7 @@ def setup_environment(self): '--exists-action=w', '-r{0}'.format(requirements_file_path), cwd=checkout_path, - bin_path=self.project.venv_bin(version=self.version.slug, - bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) # Handle setup.py @@ -324,8 +323,7 @@ def setup_environment(self): '--ignore-installed', '.', cwd=checkout_path, - bin_path=self.project.venv_bin(version=self.version.slug, - bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) else: self.build_env.run( @@ -334,8 +332,7 @@ def setup_environment(self): 'install', '--force', cwd=checkout_path, - bin_path=self.project.venv_bin(version=self.version.slug, - bin=None) + bin_path=self.project.venv_bin(version=self.version.slug) ) def build_docs(self):