diff --git a/pre_commit/repository.py b/pre_commit/repository.py index df93ac02..cbe0535c 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import logging import shutil from cached_property import cached_property @@ -9,6 +10,9 @@ from pre_commit.manifest import Manifest from pre_commit.prefixed_command_runner import PrefixedCommandRunner +logger = logging.getLogger('pre_commit') + + class Repository(object): def __init__(self, repo_config, repo_path_getter): self.repo_config = repo_config @@ -62,14 +66,28 @@ class Repository(object): def install(self): """Install the hook repository.""" - for language_name, language_version in self.languages: + def language_is_installed(language_name): language = languages[language_name] - if ( + return ( language.ENVIRONMENT_DIR is None or self.cmd_runner.exists(language.ENVIRONMENT_DIR, '.installed') - ): - # The language is already installed + ) + + if not all( + language_is_installed(language_name) + for language_name, _ in self.languages + ): + logger.info( + 'Installing environment for {0}.'.format(self.repo_url) + ) + logger.info('Once installed this environment will be reused.') + 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): continue + # There's potentially incomplete cleanup from previous runs # Clean it up! if self.cmd_runner.exists(language.ENVIRONMENT_DIR): diff --git a/pre_commit/store.py b/pre_commit/store.py index 30962576..71e339ae 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -81,9 +81,7 @@ class Store(object): if os.path.exists(sha_path): return os.readlink(sha_path) - logger.info('Installing environment for {0}.'.format(url)) - logger.info('Once installed this environment will be reused.') - logger.info('This may take a few minutes...') + logger.info('Initializing environment for {0}.'.format(url)) dir = tempfile.mkdtemp(prefix='repo', dir=self.directory) with clean_path_on_failure(dir): diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index d83d6ec3..aa2da657 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -128,9 +128,7 @@ FILES_CHANGED = ( NORMAL_PRE_COMMIT_RUN = re.compile( - r'^\[INFO\] Installing environment for .+\.\n' - r'\[INFO\] Once installed this environment will be reused\.\n' - r'\[INFO\] This may take a few minutes\.\.\.\n' + r'^\[INFO\] Initializing environment for .+\.\n' r'Bash hook\.+Passed\n' r'\[master [a-f0-9]{7}\] Commit!\n' + FILES_CHANGED + @@ -180,9 +178,7 @@ def test_environment_not_sourced(tmpdir_factory): FAILING_PRE_COMMIT_RUN = re.compile( - r'^\[INFO\] Installing environment for .+\.\n' - r'\[INFO\] Once installed this environment will be reused\.\n' - r'\[INFO\] This may take a few minutes\.\.\.\n' + r'^\[INFO\] Initializing environment for .+\.\n' r'Failing hook\.+Failed\n' r'hookid: failing_hook\n' r'\n' @@ -258,9 +254,7 @@ def test_install_existing_hook_no_overwrite_idempotent(tmpdir_factory): FAIL_OLD_HOOK = re.compile( r'fail!\n' - r'\[INFO\] Installing environment for .+\.\n' - r'\[INFO\] Once installed this environment will be reused\.\n' - r'\[INFO\] This may take a few minutes\.\.\.\n' + r'\[INFO\] Initializing environment for .+\.\n' r'Bash hook\.+Passed\n' ) diff --git a/tests/conftest.py b/tests/conftest.py index 8c74c684..5f5dcacf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ from __future__ import absolute_import from __future__ import unicode_literals import io +import logging import os import os.path @@ -113,3 +114,9 @@ def cmd_runner(tmpdir_factory): @pytest.yield_fixture def runner_with_mocked_store(mock_out_store_directory): yield Runner('/') + + +@pytest.yield_fixture +def log_info_mock(): + with mock.patch.object(logging.getLogger('pre_commit'), 'info') as mck: + yield mck diff --git a/tests/repository_test.py b/tests/repository_test.py index 469274b3..2b2fcef9 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -254,18 +254,21 @@ def test_languages(tmpdir_factory, store): assert repo.languages == set([('python', 'default')]) -def test_reinstall(tmpdir_factory, store): +def test_reinstall(tmpdir_factory, store, log_info_mock): path = make_repo(tmpdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) repo = Repository.create(config, store) repo.require_installed() + # We print some logging during clone (1) + install (3) + assert log_info_mock.call_count == 4 + log_info_mock.reset_mock() # Reinstall with same repo should not trigger another install - # TODO: how to assert this? repo.require_installed() + assert log_info_mock.call_count == 0 # Reinstall on another run should not trigger another install - # TODO: how to assert this? repo = Repository.create(config, store) repo.require_installed() + assert log_info_mock.call_count == 0 def test_control_c_control_c_on_install(tmpdir_factory, store): diff --git a/tests/store_test.py b/tests/store_test.py index 5045f33c..deac0e1b 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -11,7 +11,6 @@ import pytest from pre_commit import five from pre_commit.store import _get_default_directory -from pre_commit.store import logger from pre_commit.store import Store from pre_commit.util import cmd_output from pre_commit.util import cwd @@ -80,12 +79,6 @@ def test_does_not_recreate_if_directory_already_exists(store): assert not os.path.exists(os.path.join(store.directory, 'README')) -@pytest.yield_fixture -def log_info_mock(): - with mock.patch.object(logger, 'info', autospec=True) as info_mock: - yield info_mock - - def test_clone(store, tmpdir_factory, log_info_mock): path = git_dir(tmpdir_factory) with cwd(path): @@ -95,7 +88,9 @@ def test_clone(store, tmpdir_factory, log_info_mock): ret = store.clone(path, sha) # Should have printed some stuff - log_info_mock.assert_called_with('This may take a few minutes...') + assert log_info_mock.call_args_list[0][0][0].startswith( + 'Initializing environment for ' + ) # Should return a directory inside of the store assert os.path.exists(ret)