Skip to content

Make additional_dependencies rollforward safe #319

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
asottile merged 1 commit into from
Dec 11, 2015
Merged

Make additional_dependencies rollforward safe #319

Show file tree
Hide file tree
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
63 changes: 50 additions & 13 deletions pre_commit/repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
from __future__ import unicode_literals

import io
import json
import logging
import os
import shutil
from collections import defaultdict

import pkg_resources
from cached_property import cached_property

from pre_commit import five
from pre_commit import git
from pre_commit.clientlib.validate_config import is_local_hooks
from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA
Expand All @@ -23,6 +27,9 @@
pkg_resources.get_distribution('pre-commit').version
)

# Bump when installation changes in a backwards / forwards incompatible way
INSTALLED_STATE_VERSION = '1'


class Repository(object):
def __init__(self, repo_config, repo_path_getter):
Expand Down Expand Up @@ -110,14 +117,45 @@ def require_installed(self):

def install(self):
"""Install the hook repository."""
def state(language_name, language_version):
return {
'additional_dependencies': sorted(
self.additional_dependencies[
language_name
][language_version],
)
}

def state_filename(venv, suffix=''):
return self.cmd_runner.path(
venv, '.install_state_v' + INSTALLED_STATE_VERSION + suffix,
)

def read_state(venv):
if not os.path.exists(state_filename(venv)):
return None
else:
return json.loads(io.open(state_filename(venv)).read())

def write_state(venv, language_name, language_version):
with io.open(
state_filename(venv, suffix='staging'), 'w',
) as state_file:
state_file.write(five.to_text(json.dumps(
state(language_name, language_version),
)))
# Move the file into place atomically to indicate we've installed
os.rename(
state_filename(venv, suffix='staging'),
state_filename(venv),
)

def language_is_installed(language_name, language_version):
language = languages[language_name]
directory = environment_dir(
language.ENVIRONMENT_DIR, language_version,
)
venv = environment_dir(language.ENVIRONMENT_DIR, language_version)
return (
directory is None or
self.cmd_runner.exists(directory, '.installed')
venv is None or
read_state(venv) == state(language_name, language_version)
)

if not all(
Expand All @@ -131,24 +169,23 @@ def language_is_installed(language_name, language_version):
logger.info('This may take a few minutes...')

for language_name, language_version in self.languages:
language = languages[language_name]
if language_is_installed(language_name, language_version):
continue

directory = environment_dir(
language.ENVIRONMENT_DIR, language_version,
)
language = languages[language_name]
venv = environment_dir(language.ENVIRONMENT_DIR, language_version)

# There's potentially incomplete cleanup from previous runs
# Clean it up!
if self.cmd_runner.exists(directory):
shutil.rmtree(self.cmd_runner.path(directory))
if self.cmd_runner.exists(venv):
shutil.rmtree(self.cmd_runner.path(venv))

language.install_environment(
self.cmd_runner, language_version,
self.additional_dependencies[language_name][language_version],
)
# Touch the .installed file (atomic) to indicate we've installed
open(self.cmd_runner.path(directory, '.installed'), 'w').close()
# Write our state to indicate we're installed
write_state(venv, language_name, language_version)

def run_hook(self, hook, file_args):
"""Run a hook.
Expand Down
1 change: 0 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
'nodeenv>=0.11.1',
'ordereddict',
'pyyaml',
'simplejson',
'virtualenv',
],
entry_points={
Expand Down
17 changes: 17 additions & 0 deletions tests/repository_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,23 @@ def test_additional_python_dependencies_installed(tempdir_factory, store):
assert 'mccabe' in output


@pytest.mark.integration
def test_additional_dependencies_roll_forward(tempdir_factory, store):
path = make_repo(tempdir_factory, 'python_hooks_repo')
config = make_config_from_repo(path)
# Run the repo once without additional_dependencies
repo = Repository.create(config, store)
repo.run_hook(repo.hooks[0][1], [])
# Now run it with additional_dependencies
config['hooks'][0]['additional_dependencies'] = ['mccabe']
repo = Repository.create(config, store)
repo.run_hook(repo.hooks[0][1], [])
# We should see our additional dependency installed
with python.in_env(repo.cmd_runner, 'default') as env:
output = env.run('pip freeze -l')[1]
assert 'mccabe' in output


@xfailif_windows_no_ruby
@pytest.mark.integration
def test_additional_ruby_dependencies_installed(
Expand Down