Merge de33fad483 into da7e85c851

pre_commit/clientlib/validate_base.py

@@ -9,9 +9,9 @@ import sys
 import jsonschema
 import jsonschema.exceptions
 import pkg_resources
-import yaml
 
 from pre_commit.jsonschema_extensions import apply_defaults
+from pre_commit.util import yaml_load
 
 
 def is_regex_valid(regex):
@@ -36,14 +36,14 @@ def get_validator(
             the object read from the file.  The function should either raise
             exception_type on failure.
     """
-    def validate(filename, load_strategy=yaml.load):
+    def validate(filename):
         if not os.path.exists(filename):
             raise exception_type('File {0} does not exist'.format(filename))
 
         file_contents = open(filename, 'r').read()
 
         try:
-            obj = load_strategy(file_contents)
+            obj = yaml_load(file_contents)
         except Exception as e:
             raise exception_type(
                 'Invalid yaml: {0}\n{1}'.format(os.path.relpath(filename), e),

pre_commit/commands/autoupdate.py

@@ -3,18 +3,14 @@ from __future__ import unicode_literals
 
 import sys
 
-from aspy.yaml import ordered_dump
-from aspy.yaml import ordered_load
-
-import pre_commit.constants as C
 from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
 from pre_commit.clientlib.validate_config import is_local_hooks
 from pre_commit.clientlib.validate_config import load_config
 from pre_commit.jsonschema_extensions import remove_defaults
-from pre_commit.ordereddict import OrderedDict
 from pre_commit.repository import Repository
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
+from pre_commit.util import yaml_dump
 
 
 class RepositoryCannotBeUpdatedError(RuntimeError):
@@ -37,10 +33,10 @@ def _update_repository(repo_config, runner):
 
     # Don't bother trying to update if our sha is the same
     if head_sha == repo_config['sha']:
-        return repo_config
+        return head_sha
 
-    # Construct a new config with the head sha
-    new_config = OrderedDict(repo_config)
+    # Modify the sha to be the head sha
+    new_config = dict(repo_config)
     new_config['sha'] = head_sha
     new_repo = Repository.create(new_config, runner.store)
 
@@ -53,53 +49,40 @@ def _update_repository(repo_config, runner):
             '{0}'.format(', '.join(sorted(hooks_missing)))
         )
 
-    return new_config
+    return head_sha
 
 
 def autoupdate(runner):
     """Auto-update the pre-commit config to the latest versions of repos."""
     retv = 0
-    output_configs = []
     changed = False
 
-    input_configs = load_config(
-        runner.config_file_path,
-        load_strategy=ordered_load,
-    )
+    configs = load_config(runner.config_file_path)
 
-    for repo_config in input_configs:
+    for repo_config in configs:
         if is_local_hooks(repo_config):
-            output_configs.append(repo_config)
             continue
         sys.stdout.write('Updating {0}...'.format(repo_config['repo']))
         sys.stdout.flush()
+        original_sha = repo_config['sha']
         try:
-            new_repo_config = _update_repository(repo_config, runner)
+            new_sha = _update_repository(repo_config, runner)
         except RepositoryCannotBeUpdatedError as error:
             print(error.args[0])
-            output_configs.append(repo_config)
             retv = 1
             continue
 
-        if new_repo_config['sha'] != repo_config['sha']:
+        if new_sha != original_sha:
             changed = True
-            print(
-                'updating {0} -> {1}.'.format(
-                    repo_config['sha'], new_repo_config['sha'],
-                )
-            )
-            output_configs.append(new_repo_config)
+            print('updating {0} -> {1}.'.format(original_sha, new_sha))
+            repo_config['sha'] = new_sha
         else:
             print('already up to date.')
-            output_configs.append(repo_config)
 
     if changed:
         with open(runner.config_file_path, 'w') as config_file:
             config_file.write(
-                ordered_dump(
-                    remove_defaults(output_configs, CONFIG_JSON_SCHEMA),
-                    **C.YAML_DUMP_KWARGS
-                )
+                yaml_dump(remove_defaults(configs, CONFIG_JSON_SCHEMA))
             )
 
     return retv

pre_commit/constants.py

@@ -4,10 +4,3 @@ from __future__ import unicode_literals
 CONFIG_FILE = '.pre-commit-config.yaml'
 
 MANIFEST_FILE = 'hooks.yaml'
-
-YAML_DUMP_KWARGS = {
-    'default_flow_style': False,
-    # Use unicode
-    'encoding': None,
-    'indent': 4,
-}

pre_commit/ordereddict.py

@@ -1,7 +0,0 @@
-from __future__ import absolute_import
-from __future__ import unicode_literals
-
-try:
-    from collections import OrderedDict  # noqa
-except ImportError:  # pragma: no cover (PY26)
-    from ordereddict import OrderedDict  # noqa

pre_commit/util.py

@@ -12,6 +12,7 @@ import tarfile
 import tempfile
 
 import pkg_resources
+import ruamel.yaml
 
 from pre_commit import five
 from pre_commit import parse_shebang
@@ -213,3 +214,11 @@ def rmtree(path):
         else:
             raise
     shutil.rmtree(path, ignore_errors=False, onerror=handle_remove_readonly)
+
+
+yaml_load = functools.partial(
+    ruamel.yaml.load, Loader=ruamel.yaml.RoundTripLoader,
+)
+yaml_dump = functools.partial(
+    ruamel.yaml.dump, Dumper=ruamel.yaml.RoundTripDumper, encoding=None,
+)

setup.py

@@ -39,12 +39,11 @@ setup(
         ]
     },
     install_requires=[
-        'aspy.yaml',
         'cached-property',
         'jsonschema',
         'nodeenv>=0.11.1',
         'pyterminalsize',
-        'pyyaml',
+        'ruamel.yaml',
         'virtualenv',
     ],
     extras_require={

testing/fixtures.py

@@ -5,17 +5,17 @@ import contextlib
 import io
 import os.path
 
-from aspy.yaml import ordered_dump
-from aspy.yaml import ordered_load
+from ruamel.yaml.comments import CommentedMap
 
 import pre_commit.constants as C
 from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
 from pre_commit.clientlib.validate_config import validate_config_extra
 from pre_commit.clientlib.validate_manifest import load_manifest
 from pre_commit.jsonschema_extensions import apply_defaults
-from pre_commit.ordereddict import OrderedDict
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
+from pre_commit.util import yaml_dump
+from pre_commit.util import yaml_load
 from testing.util import copy_tree_to_path
 from testing.util import get_head_sha
 from testing.util import get_resource_path
@@ -41,10 +41,10 @@ def make_repo(tempdir_factory, repo_source):
 def modify_manifest(path):
     """Modify the manifest yielded by this context to write to hooks.yaml."""
     manifest_path = os.path.join(path, C.MANIFEST_FILE)
-    manifest = ordered_load(io.open(manifest_path).read())
+    manifest = yaml_load(io.open(manifest_path).read())
     yield manifest
     with io.open(manifest_path, 'w') as manifest_file:
-        manifest_file.write(ordered_dump(manifest, **C.YAML_DUMP_KWARGS))
+        manifest_file.write(yaml_dump(manifest))
     cmd_output('git', 'commit', '-am', 'update hooks.yaml', cwd=path)
 
 
@@ -54,18 +54,18 @@ def modify_config(path='.', commit=True):
     .pre-commit-config.yaml
     """
     config_path = os.path.join(path, C.CONFIG_FILE)
-    config = ordered_load(io.open(config_path).read())
+    config = yaml_load(io.open(config_path).read())
     yield config
     with io.open(config_path, 'w', encoding='UTF-8') as config_file:
-        config_file.write(ordered_dump(config, **C.YAML_DUMP_KWARGS))
+        config_file.write(yaml_dump(config))
     if commit:
         cmd_output('git', 'commit', '-am', 'update config', cwd=path)
 
 
 def config_with_local_hooks():
-    return OrderedDict((
+    return CommentedMap((
         ('repo', 'local'),
-        ('hooks', [OrderedDict((
+        ('hooks', [CommentedMap((
             ('id', 'do_not_commit'),
             ('name', 'Block if "DO NOT COMMIT" is found'),
             ('entry', 'DO NOT COMMIT'),
@@ -77,13 +77,10 @@ def config_with_local_hooks():
 
 def make_config_from_repo(repo_path, sha=None, hooks=None, check=True):
     manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE))
-    config = OrderedDict((
-        ('repo', repo_path),
+    config = CommentedMap((
         ('sha', sha or get_head_sha(repo_path)),
-        (
-            'hooks',
-            hooks or [OrderedDict((('id', hook['id']),)) for hook in manifest],
-        ),
+        ('repo', repo_path),
+        ('hooks', hooks or [{'id': hook['id']} for hook in manifest]),
     ))
 
     if check:
@@ -96,10 +93,10 @@ def make_config_from_repo(repo_path, sha=None, hooks=None, check=True):
 
 def write_config(directory, config):
     if type(config) is not list:
-        assert type(config) is OrderedDict
+        assert type(config) is CommentedMap
         config = [config]
     with io.open(os.path.join(directory, C.CONFIG_FILE), 'w') as config_file:
-        config_file.write(ordered_dump(config, **C.YAML_DUMP_KWARGS))
+        config_file.write(yaml_dump(config))
 
 
 def add_config_to_repo(git_path, config):

tests/clientlib/validate_base_test.py

@@ -1,10 +1,8 @@
 from __future__ import unicode_literals
 
 import pytest
-from aspy.yaml import ordered_load
 
 from pre_commit.clientlib.validate_base import get_validator
-from pre_commit.ordereddict import OrderedDict
 from testing.util import get_resource_path
 
 
@@ -63,11 +61,3 @@ def test_raises_when_additional_validation_fails(additional_validator):
 def test_returns_object_after_validating(noop_validator):
     ret = noop_validator(get_resource_path('array_yaml_file.yaml'))
     assert ret == ['foo', 'bar']
-
-
-def test_load_strategy(noop_validator):
-    ret = noop_validator(
-        get_resource_path('ordering_data_test.yaml'),
-        load_strategy=ordered_load,
-    )
-    assert type(ret) is OrderedDict

tests/commands/autoupdate_test.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import io
 import shutil
 
 import pytest
@@ -9,7 +10,6 @@ from pre_commit.clientlib.validate_config import load_config
 from pre_commit.commands.autoupdate import _update_repository
 from pre_commit.commands.autoupdate import autoupdate
 from pre_commit.commands.autoupdate import RepositoryCannotBeUpdatedError
-from pre_commit.ordereddict import OrderedDict
 from pre_commit.runner import Runner
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
@@ -33,7 +33,7 @@ def test_up_to_date_repo(up_to_date_repo, runner_with_mocked_store):
     config = make_config_from_repo(up_to_date_repo)
     input_sha = config['sha']
     ret = _update_repository(config, runner_with_mocked_store)
-    assert ret['sha'] == input_sha
+    assert ret == input_sha
 
 
 def test_autoupdate_up_to_date_repo(
@@ -72,12 +72,12 @@ def test_out_of_date_repo(out_of_date_repo, runner_with_mocked_store):
         out_of_date_repo.path, sha=out_of_date_repo.original_sha,
     )
     ret = _update_repository(config, runner_with_mocked_store)
-    assert ret['sha'] != out_of_date_repo.original_sha
-    assert ret['sha'] == out_of_date_repo.head_sha
+    assert ret != out_of_date_repo.original_sha
+    assert ret == out_of_date_repo.head_sha
 
 
 def test_autoupdate_out_of_date_repo(
-        out_of_date_repo, in_tmpdir, mock_out_store_directory
+        out_of_date_repo, in_tmpdir, mock_out_store_directory,
 ):
     # Write out the config
     config = make_config_from_repo(
@@ -85,10 +85,10 @@ def test_autoupdate_out_of_date_repo(
     )
     write_config('.', config)
 
-    before = open(C.CONFIG_FILE).read()
+    before = io.open(C.CONFIG_FILE).read()
     runner = Runner('.')
     ret = autoupdate(runner)
-    after = open(C.CONFIG_FILE).read()
+    after = io.open(C.CONFIG_FILE).read()
     assert ret == 0
     assert before != after
     # Make sure we don't add defaults
@@ -96,6 +96,21 @@ def test_autoupdate_out_of_date_repo(
     assert out_of_date_repo.head_sha in after
 
 
+def test_autoupdate_preserves_comments(
+        out_of_date_repo, in_tmpdir, mock_out_store_directory,
+):
+    config = make_config_from_repo(
+        out_of_date_repo.path, sha=out_of_date_repo.original_sha, check=False,
+    )
+    write_config('.', config)
+    with io.open(C.CONFIG_FILE, 'a') as config_file:
+        config_file.write("# I'm a comment!\n")
+    runner = Runner('.')
+    autoupdate(runner)
+    after = io.open(C.CONFIG_FILE).read()
+    assert after.endswith("# I'm a comment!\n")
+
+
 @pytest.yield_fixture
 def hook_disappearing_repo(tempdir_factory):
     path = make_repo(tempdir_factory, 'python_hooks_repo')
@@ -118,7 +133,7 @@ def test_hook_disppearing_repo_raises(
     config = make_config_from_repo(
         hook_disappearing_repo.path,
         sha=hook_disappearing_repo.original_sha,
-        hooks=[OrderedDict((('id', 'foo'),))],
+        hooks=[{'id': 'foo'}],
     )
     with pytest.raises(RepositoryCannotBeUpdatedError):
         _update_repository(config, runner_with_mocked_store)
@@ -130,7 +145,7 @@ def test_autoupdate_hook_disappearing_repo(
     config = make_config_from_repo(
         hook_disappearing_repo.path,
         sha=hook_disappearing_repo.original_sha,
-        hooks=[OrderedDict((('id', 'foo'),))],
+        hooks=[{'id': 'foo'}],
         check=False,
     )
     write_config('.', config)

tests/commands/run_test.py

@@ -10,6 +10,7 @@ import sys
 
 import mock
 import pytest
+from ruamel.yaml.comments import CommentedMap
 
 import pre_commit.constants as C
 from pre_commit.commands.install_uninstall import install
@@ -17,7 +18,6 @@ from pre_commit.commands.run import _get_skips
 from pre_commit.commands.run import _has_unmerged_paths
 from pre_commit.commands.run import get_changed_files
 from pre_commit.commands.run import run
-from pre_commit.ordereddict import OrderedDict
 from pre_commit.output import sys_stdout_write_wrapper
 from pre_commit.runner import Runner
 from pre_commit.util import cmd_output
@@ -454,16 +454,16 @@ def test_local_hook_for_stages(
         hook_stage,
         expected_output
 ):
-    config = OrderedDict((
+    config = CommentedMap((
         ('repo', 'local'),
-        ('hooks', (OrderedDict((
+        ('hooks', (CommentedMap((
             ('id', 'pylint'),
             ('name', 'hook 1'),
             ('entry', 'python -m pylint.__main__'),
             ('language', 'system'),
             ('files', r'\.py$'),
             ('stages', stage_for_first_hook)
-        )), OrderedDict((
+        )), CommentedMap((
             ('id', 'do_not_commit'),
             ('name', 'hook 2'),
             ('entry', 'DO NOT COMMIT'),
@@ -488,15 +488,15 @@ def test_local_hook_for_stages(
 
 
 def test_local_hook_passes(repo_with_passing_hook, mock_out_store_directory):
-    config = OrderedDict((
+    config = CommentedMap((
         ('repo', 'local'),
-        ('hooks', (OrderedDict((
+        ('hooks', (CommentedMap((
             ('id', 'pylint'),
             ('name', 'PyLint'),
             ('entry', 'python -m pylint.__main__'),
             ('language', 'system'),
             ('files', r'\.py$'),
-        )), OrderedDict((
+        )), CommentedMap((
             ('id', 'do_not_commit'),
             ('name', 'Block if "DO NOT COMMIT" is found'),
             ('entry', 'DO NOT COMMIT'),
@@ -520,9 +520,9 @@ def test_local_hook_passes(repo_with_passing_hook, mock_out_store_directory):
 
 
 def test_local_hook_fails(repo_with_passing_hook, mock_out_store_directory):
-    config = OrderedDict((
+    config = CommentedMap((
         ('repo', 'local'),
-        ('hooks', [OrderedDict((
+        ('hooks', [CommentedMap((
             ('id', 'no-todo'),
             ('name', 'No TODO'),
             ('entry', 'sh -c "! grep -iI todo $@" --'),

tests/runner_test.py

@@ -4,8 +4,9 @@ from __future__ import unicode_literals
 import os
 import os.path
 
+from ruamel.yaml.comments import CommentedMap
+
 import pre_commit.constants as C
-from pre_commit.ordereddict import OrderedDict
 from pre_commit.runner import Runner
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
@@ -56,16 +57,16 @@ def test_repositories(tempdir_factory, mock_out_store_directory):
 
 
 def test_local_hooks(tempdir_factory, mock_out_store_directory):
-    config = OrderedDict((
+    config = CommentedMap((
         ('repo', 'local'),
-        ('hooks', (OrderedDict((
+        ('hooks', (CommentedMap((
             ('id', 'arg-per-line'),
             ('name', 'Args per line hook'),
             ('entry', 'bin/hook.sh'),
             ('language', 'script'),
             ('files', ''),
             ('args', ['hello', 'world']),
-        )), OrderedDict((
+        )), CommentedMap((
             ('id', 'do_not_commit'),
             ('name', 'Block if "DO NOT COMMIT" is found'),
             ('entry', 'DO NOT COMMIT'),