From 2699026908c75a17437b60a568d54dcf3ec76dee Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Mar 2014 20:06:03 -0700 Subject: [PATCH 1/5] Add ordered yaml load/dump --- pre_commit/yaml_extensions.py | 28 ++++++++++++++++++++++++++++ tests/yaml_extensions_test.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 pre_commit/yaml_extensions.py create mode 100644 tests/yaml_extensions_test.py diff --git a/pre_commit/yaml_extensions.py b/pre_commit/yaml_extensions.py new file mode 100644 index 00000000..70bcec72 --- /dev/null +++ b/pre_commit/yaml_extensions.py @@ -0,0 +1,28 @@ + +import yaml + +from pre_commit.ordereddict import OrderedDict + + +# Adapted from http://stackoverflow.com/a/21912744/812183 + +def ordered_load(s): + class OrderedLoader(yaml.loader.Loader): pass + def constructor(loader, node): + return OrderedDict(loader.construct_pairs(node)) + OrderedLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, + constructor, + ) + return yaml.load(s, Loader=OrderedLoader) + + +def ordered_dump(s, **kwargs): + class OrderedDumper(yaml.dumper.Dumper): pass + def dict_representer(dumper, data): + return dumper.represent_mapping( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, + data.items(), + ) + OrderedDumper.add_representer(OrderedDict, dict_representer) + return yaml.dump(s, Dumper=OrderedDumper, **kwargs) diff --git a/tests/yaml_extensions_test.py b/tests/yaml_extensions_test.py new file mode 100644 index 00000000..c60c1aef --- /dev/null +++ b/tests/yaml_extensions_test.py @@ -0,0 +1,35 @@ + +from pre_commit.ordereddict import OrderedDict +from pre_commit.yaml_extensions import ordered_dump +from pre_commit.yaml_extensions import ordered_load + + +def test_ordered_load(): + ret = ordered_load( + 'a: herp\n' + 'c: derp\n' + 'd: darp\n' + 'b: harp\n' + ) + # Original behavior + assert ret == {'a': 'herp', 'b': 'harp', 'c': 'derp', 'd': 'darp'} + # Ordered behavior + assert ( + ret.items() == + [('a', 'herp'), ('c', 'derp'), ('d', 'darp'), ('b', 'harp')] + ) + + +def test_ordered_dump(): + ret = ordered_dump( + OrderedDict( + (('a', 'herp'), ('c', 'derp'), ('b', 'harp'), ('d', 'darp')) + ), + default_flow_style=False, + ) + assert ret == ( + 'a: herp\n' + 'c: derp\n' + 'b: harp\n' + 'd: darp\n' + ) From d8f8f5e1f4f34f455e6c146c985b8cc52afa080e Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Mar 2014 20:19:09 -0700 Subject: [PATCH 2/5] Extend validate_base to take a load_strategy --- pre_commit/clientlib/validate_base.py | 4 ++-- testing/resources/ordering_data_test.yaml | 2 ++ tests/clientlib/validate_base_test.py | 10 ++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 testing/resources/ordering_data_test.yaml diff --git a/pre_commit/clientlib/validate_base.py b/pre_commit/clientlib/validate_base.py index 37180873..b15e9d77 100644 --- a/pre_commit/clientlib/validate_base.py +++ b/pre_commit/clientlib/validate_base.py @@ -24,7 +24,7 @@ def get_validator( exception_type on failure. """ - def validate(filename=None): + def validate(filename=None, load_strategy=yaml.load): filename = filename or os.path.join(git.get_root(), default_filename) if not os.path.exists(filename): @@ -33,7 +33,7 @@ def get_validator( file_contents = open(filename, 'r').read() try: - obj = yaml.load(file_contents) + obj = load_strategy(file_contents) except Exception as e: raise exception_type( 'File {0} is not a valid yaml file'.format(filename), e, diff --git a/testing/resources/ordering_data_test.yaml b/testing/resources/ordering_data_test.yaml new file mode 100644 index 00000000..3d606686 --- /dev/null +++ b/testing/resources/ordering_data_test.yaml @@ -0,0 +1,2 @@ +foo: bar +bar: baz diff --git a/tests/clientlib/validate_base_test.py b/tests/clientlib/validate_base_test.py index b74fba31..4d8851a9 100644 --- a/tests/clientlib/validate_base_test.py +++ b/tests/clientlib/validate_base_test.py @@ -7,6 +7,8 @@ import pytest from pre_commit import git from pre_commit.clientlib.validate_base import get_validator +from pre_commit.ordereddict import OrderedDict +from pre_commit.yaml_extensions import ordered_load from testing.util import get_resource_path @@ -71,3 +73,11 @@ 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 From 49da1ba72af4cb0da741bbec490a557a46e066a6 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Mar 2014 21:35:35 -0700 Subject: [PATCH 3/5] Added the useful thinking bit of the autoupdate and tests. --- pre_commit/commands.py | 47 ++++++++++++ pre_commit/run.py | 11 +-- testing/auto_namedtuple.py | 11 +++ testing/resources/manifest_without_foo.yaml | 4 ++ tests/commands_test.py | 79 +++++++++++++++++++++ 5 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 testing/auto_namedtuple.py create mode 100644 testing/resources/manifest_without_foo.yaml diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 2ace2606..7353c8e3 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -4,6 +4,10 @@ from __future__ import print_function import os import pkg_resources import stat +from plumbum import local + +from pre_commit.ordereddict import OrderedDict +from pre_commit.repository import Repository def install(runner): @@ -29,3 +33,46 @@ def uninstall(runner): os.remove(runner.pre_commit_path) print('pre-commit uninstalled') return 0 + + +class RepositoryCannotBeUpdatedError(RuntimeError): pass + + +def _update_repository(repo_config): + """Updates a repository to the tip of `master`. If the repository cannot + be updated because a hook that is configured does not exist in `master`, + this raises a RepositoryCannotBeUpdatedError + + Args: + repo_config - A config for a repository + """ + repo = Repository(repo_config) + + with repo.in_checkout(): + local['git']['fetch']() + head_sha = local['git']['rev-parse', 'origin/master']().strip() + + # Don't bother trying to update if our sha is the same + if head_sha == repo_config['sha']: + return repo_config + + # Construct a new config with the head sha + new_config = OrderedDict(repo_config) + new_config['sha'] = head_sha + new_repo = Repository(new_config) + + # See if any of our hooks were deleted with the new commits + hooks = set(repo.hooks.keys()) + hooks_missing = hooks - (hooks & set(new_repo.manifest.keys())) + if hooks_missing: + raise RepositoryCannotBeUpdatedError( + 'Cannot update because the tip of master is missing these hooks:\n' + '{0}'.format(', '.join(sorted(hooks_missing))) + ) + + return new_config + + +def autoupdate(runner): + """Auto-update the pre-commit config to the latest versions of repos.""" + pass diff --git a/pre_commit/run.py b/pre_commit/run.py index 2826c44a..f83d629f 100644 --- a/pre_commit/run.py +++ b/pre_commit/run.py @@ -100,14 +100,7 @@ def run(argv): subparsers.add_parser('uninstall', help='Uninstall the pre-commit script.') - execute_hook = subparsers.add_parser( - 'execute-hook', help='Run a single hook.' - ) - execute_hook.add_argument('hook', help='The hook-id to run.') - execute_hook.add_argument( - '--all-files', '-a', action='store_true', default=False, - help='Run on all the files in the repo.', - ) + subparsers.add_parser('autoupdate', help='Auto-update hooks config.') run = subparsers.add_parser('run', help='Run hooks.') run.add_argument('hook', nargs='?', help='A single hook-id to run'), @@ -130,6 +123,8 @@ def run(argv): return commands.install(runner) elif args.command == 'uninstall': return commands.uninstall(runner) + elif args.command == 'autoupdate': + return commands.autoupdate(runner) elif args.command == 'run': if args.hook: return run_single_hook(runner, args.hook, all_files=args.all_files) diff --git a/testing/auto_namedtuple.py b/testing/auto_namedtuple.py new file mode 100644 index 00000000..cd0c30a6 --- /dev/null +++ b/testing/auto_namedtuple.py @@ -0,0 +1,11 @@ + +import collections + +def auto_namedtuple(classname='auto_namedtuple', **kwargs): + """Returns an automatic namedtuple object. + + Args: + classname - The class name for the returned object. + **kwargs - Properties to give the returned object. + """ + return (collections.namedtuple(classname, kwargs.keys())(**kwargs)) diff --git a/testing/resources/manifest_without_foo.yaml b/testing/resources/manifest_without_foo.yaml new file mode 100644 index 00000000..220eedeb --- /dev/null +++ b/testing/resources/manifest_without_foo.yaml @@ -0,0 +1,4 @@ +- id: bar + name: Bar + entry: bar + language: python diff --git a/tests/commands_test.py b/tests/commands_test.py index d99a5ab3..201c83dc 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -1,12 +1,24 @@ +import jsonschema import os import os.path import pkg_resources +import pytest +import shutil import stat +from plumbum import local +from pre_commit import git +from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA +from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.commands import install +from pre_commit.commands import RepositoryCannotBeUpdatedError from pre_commit.commands import uninstall +from pre_commit.commands import _update_repository +from pre_commit.ordereddict import OrderedDict from pre_commit.runner import Runner +from testing.auto_namedtuple import auto_namedtuple +from testing.util import get_resource_path def test_install_pre_commit(empty_git_dir): @@ -35,3 +47,70 @@ def test_uninstall(empty_git_dir): assert os.path.exists(runner.pre_commit_path) uninstall(runner) assert not os.path.exists(runner.pre_commit_path) + + +@pytest.yield_fixture +def up_to_date_repo(python_hooks_repo): + config = OrderedDict(( + ('repo', python_hooks_repo), + ('sha', git.get_head_sha(python_hooks_repo)), + ('hooks', [{'id': 'foo', 'files': ''}]), + )) + jsonschema.validate([config], CONFIG_JSON_SCHEMA) + validate_config_extra([config]) + yield auto_namedtuple( + repo_config=config, + python_hooks_repo=python_hooks_repo, + ) + + +def test_up_to_date_repo(up_to_date_repo): + input_sha = up_to_date_repo.repo_config['sha'] + ret = _update_repository(up_to_date_repo.repo_config) + assert ret['sha'] == input_sha + + +@pytest.yield_fixture +def out_of_date_repo(python_hooks_repo): + config = OrderedDict(( + ('repo', python_hooks_repo), + ('sha', git.get_head_sha(python_hooks_repo)), + ('hooks', [{'id': 'foo', 'files': ''}]), + )) + jsonschema.validate([config], CONFIG_JSON_SCHEMA) + validate_config_extra([config]) + local['git']['commit', '--allow-empty', '-m', 'foo']() + head_sha = git.get_head_sha(python_hooks_repo) + yield auto_namedtuple( + repo_config=config, + head_sha=head_sha, + python_hooks_repo=python_hooks_repo, + ) + + +def test_out_of_date_repo(out_of_date_repo): + ret = _update_repository(out_of_date_repo.repo_config) + assert ret['sha'] == out_of_date_repo.head_sha + + +@pytest.yield_fixture +def hook_disappearing_repo(python_hooks_repo): + config = OrderedDict(( + ('repo', python_hooks_repo), + ('sha', git.get_head_sha(python_hooks_repo)), + ('hooks', [{'id': 'foo', 'files': ''}]), + )) + jsonschema.validate([config], CONFIG_JSON_SCHEMA) + validate_config_extra([config]) + shutil.copy(get_resource_path('manifest_without_foo.yaml'), 'manifest.yaml') + local['git']['add', '.']() + local['git']['commit', '-m', 'Remove foo']() + yield auto_namedtuple( + repo_config=config, + python_hooks_repo=python_hooks_repo, + ) + + +def test_hook_disppearing_repo_raises(hook_disappearing_repo): + with pytest.raises(RepositoryCannotBeUpdatedError): + _update_repository(hook_disappearing_repo.repo_config) From 26d563ee2056c873489ee8dca9682336993aec1a Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Mar 2014 22:50:01 -0700 Subject: [PATCH 4/5] Autoupdate works. Closes #44. --- pre_commit/commands.py | 43 +++++++++++++++++++++++++- pre_commit/constants.py | 6 ++++ pre_commit/yaml_extensions.py | 2 +- tests/commands_test.py | 57 ++++++++++++++++++++++++++++++++--- tests/yaml_extensions_test.py | 3 +- 5 files changed, 104 insertions(+), 7 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 7353c8e3..456aa9b0 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -6,8 +6,12 @@ import pkg_resources import stat from plumbum import local +import pre_commit.constants as C +from pre_commit.clientlib.validate_config import load_config from pre_commit.ordereddict import OrderedDict from pre_commit.repository import Repository +from pre_commit.yaml_extensions import ordered_dump +from pre_commit.yaml_extensions import ordered_load def install(runner): @@ -75,4 +79,41 @@ def _update_repository(repo_config): def autoupdate(runner): """Auto-update the pre-commit config to the latest versions of repos.""" - pass + retv = 0 + output_configs = [] + changed = False + + input_configs = load_config( + runner.config_file_path, + load_strategy=ordered_load, + ) + + for repo_config in input_configs: + print('Updating {0}...'.format(repo_config['repo']), end='') + try: + new_repo_config = _update_repository(repo_config) + except RepositoryCannotBeUpdatedError as e: + print(e.args[0]) + output_configs.append(repo_config) + retv = 1 + continue + + if new_repo_config['sha'] != repo_config['sha']: + changed = True + print( + 'updating {0} -> {1}.'.format( + repo_config['sha'], new_repo_config['sha'], + ) + ) + output_configs.append(new_repo_config) + 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(output_configs, **C.YAML_DUMP_KWARGS) + ) + + return retv diff --git a/pre_commit/constants.py b/pre_commit/constants.py index dd4eed2c..c5a81eb5 100644 --- a/pre_commit/constants.py +++ b/pre_commit/constants.py @@ -10,3 +10,9 @@ SUPPORTED_LANGUAGES = set([ 'ruby', 'node', ]) + + +YAML_DUMP_KWARGS = { + 'default_flow_style': False, + 'indent': 4, +} diff --git a/pre_commit/yaml_extensions.py b/pre_commit/yaml_extensions.py index 70bcec72..af19a646 100644 --- a/pre_commit/yaml_extensions.py +++ b/pre_commit/yaml_extensions.py @@ -18,7 +18,7 @@ def ordered_load(s): def ordered_dump(s, **kwargs): - class OrderedDumper(yaml.dumper.Dumper): pass + class OrderedDumper(yaml.dumper.SafeDumper): pass def dict_representer(dumper, data): return dumper.represent_mapping( yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, diff --git a/tests/commands_test.py b/tests/commands_test.py index 201c83dc..7cb54d76 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -8,15 +8,18 @@ import shutil import stat from plumbum import local +import pre_commit.constants as C from pre_commit import git from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra +from pre_commit.commands import autoupdate from pre_commit.commands import install from pre_commit.commands import RepositoryCannotBeUpdatedError from pre_commit.commands import uninstall from pre_commit.commands import _update_repository from pre_commit.ordereddict import OrderedDict from pre_commit.runner import Runner +from pre_commit.yaml_extensions import ordered_dump from testing.auto_namedtuple import auto_namedtuple from testing.util import get_resource_path @@ -54,10 +57,16 @@ def up_to_date_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), ('sha', git.get_head_sha(python_hooks_repo)), - ('hooks', [{'id': 'foo', 'files': ''}]), + ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) jsonschema.validate([config], CONFIG_JSON_SCHEMA) validate_config_extra([config]) + + with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: + file_obj.write( + ordered_dump([config], **C.YAML_DUMP_KWARGS) + ) + yield auto_namedtuple( repo_config=config, python_hooks_repo=python_hooks_repo, @@ -70,17 +79,32 @@ def test_up_to_date_repo(up_to_date_repo): assert ret['sha'] == input_sha +def test_autoupdate_up_to_date_repo(up_to_date_repo): + before = open(C.CONFIG_FILE).read() + runner = Runner(up_to_date_repo.python_hooks_repo) + ret = autoupdate(runner) + after = open(C.CONFIG_FILE).read() + assert ret == 0 + assert before == after + + @pytest.yield_fixture def out_of_date_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), ('sha', git.get_head_sha(python_hooks_repo)), - ('hooks', [{'id': 'foo', 'files': ''}]), + ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) jsonschema.validate([config], CONFIG_JSON_SCHEMA) validate_config_extra([config]) local['git']['commit', '--allow-empty', '-m', 'foo']() head_sha = git.get_head_sha(python_hooks_repo) + + with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: + file_obj.write( + ordered_dump([config], **C.YAML_DUMP_KWARGS) + ) + yield auto_namedtuple( repo_config=config, head_sha=head_sha, @@ -93,18 +117,34 @@ def test_out_of_date_repo(out_of_date_repo): assert ret['sha'] == out_of_date_repo.head_sha +def test_autoupdate_out_of_date_repo(out_of_date_repo): + before = open(C.CONFIG_FILE).read() + runner = Runner(out_of_date_repo.python_hooks_repo) + ret = autoupdate(runner) + after = open(C.CONFIG_FILE).read() + assert ret == 0 + assert before != after + assert out_of_date_repo.head_sha in after + + @pytest.yield_fixture def hook_disappearing_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), ('sha', git.get_head_sha(python_hooks_repo)), - ('hooks', [{'id': 'foo', 'files': ''}]), + ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) jsonschema.validate([config], CONFIG_JSON_SCHEMA) validate_config_extra([config]) - shutil.copy(get_resource_path('manifest_without_foo.yaml'), 'manifest.yaml') + shutil.copy(get_resource_path('manifest_without_foo.yaml'), C.MANIFEST_FILE) local['git']['add', '.']() local['git']['commit', '-m', 'Remove foo']() + + with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: + file_obj.write( + ordered_dump([config], **C.YAML_DUMP_KWARGS) + ) + yield auto_namedtuple( repo_config=config, python_hooks_repo=python_hooks_repo, @@ -114,3 +154,12 @@ def hook_disappearing_repo(python_hooks_repo): def test_hook_disppearing_repo_raises(hook_disappearing_repo): with pytest.raises(RepositoryCannotBeUpdatedError): _update_repository(hook_disappearing_repo.repo_config) + + +def test_autoupdate_hook_disappearing_repo(hook_disappearing_repo): + before = open(C.CONFIG_FILE).read() + runner = Runner(hook_disappearing_repo.python_hooks_repo) + ret = autoupdate(runner) + after = open(C.CONFIG_FILE).read() + assert ret == 1 + assert before == after diff --git a/tests/yaml_extensions_test.py b/tests/yaml_extensions_test.py index c60c1aef..098d9300 100644 --- a/tests/yaml_extensions_test.py +++ b/tests/yaml_extensions_test.py @@ -1,4 +1,5 @@ +import pre_commit.constants as C from pre_commit.ordereddict import OrderedDict from pre_commit.yaml_extensions import ordered_dump from pre_commit.yaml_extensions import ordered_load @@ -25,7 +26,7 @@ def test_ordered_dump(): OrderedDict( (('a', 'herp'), ('c', 'derp'), ('b', 'harp'), ('d', 'darp')) ), - default_flow_style=False, + **C.YAML_DUMP_KWARGS ) assert ret == ( 'a: herp\n' From 2127945b6a3e0d6eff39d153c2a6663755897944 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Mar 2014 22:59:43 -0700 Subject: [PATCH 5/5] Autoupdate pre-commit config. --- .pre-commit-config.yaml | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5d709431..3e5dc5f5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,22 +1,20 @@ - - repo: git@github.com:pre-commit/pre-commit-hooks - sha: 76739902911688e8d7b13241409f9facc0e534e4 + sha: 8aa14d218d0fe5a2c486498269b3e37d3ba5aad2 hooks: - - id: pyflakes - files: '\.py$' - - id: debug-statements - files: '\.py$' - - id: trailing-whitespace - files: '\.(py|sh|yaml)$' - - id: name-tests-test - files: 'tests/.+\.py$' - - id: end-of-file-fixer - files: '\.(py|sh|yaml)$' - + - id: pyflakes + files: \.py$ + - id: debug-statements + files: \.py$ + - id: trailing-whitespace + files: \.(py|sh|yaml)$ + - id: name-tests-test + files: tests/.+\.py$ + - id: end-of-file-fixer + files: \.(py|sh|yaml)$ - repo: git@github.com:pre-commit/pre-commit - sha: 47b7ca44ed1fcaa83464ed00cef72049ae22c33d + sha: c695ee9a9a78ac73439c52e0085bafec8037bc2d hooks: - - id: validate_manifest - files: '^manifest.yaml$' - - id: validate_config - files: '^\.pre-commit-config.yaml$' + - id: validate_manifest + files: ^manifest.yaml$ + - id: validate_config + files: ^\.pre-commit-config.yaml$