diff --git a/setup.py b/setup.py index 35d98b9..fccda9d 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,9 @@ setup( }, install_requires=[ # https://github.com/mitsuhiko/click/issues/200 - 'click>=3.1', + 'click>=5.0', + 'click-log', + 'click-threading', 'requests', 'lxml>=3.0', # https://github.com/sigmavirus24/requests-toolbelt/pull/28 diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index faef69f..3ab3ed5 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -90,6 +90,7 @@ def test_empty_storage(tmpdir, runner): tmpdir.join('path_a/haha.txt').write('UID:haha') result = runner.invoke(['sync']) + assert not result.exception tmpdir.join('path_b/haha.txt').remove() result = runner.invoke(['sync']) lines = result.output.splitlines() diff --git a/tests/conftest.py b/tests/conftest.py index 9d34156..c494932 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,15 +2,14 @@ ''' General-purpose fixtures for vdirsyncer's testsuite. ''' -import pytest +import click_log -import vdirsyncer.log +import pytest @pytest.fixture(autouse=True) def setup_logging(): - vdirsyncer.log.set_level(vdirsyncer.log.logging.DEBUG) - vdirsyncer.log.add_handler(vdirsyncer.log.stdout_handler) + click_log.basic_config('vdirsyncer') try: diff --git a/tests/test_doubleclick.py b/tests/test_doubleclick.py deleted file mode 100644 index 98eef96..0000000 --- a/tests/test_doubleclick.py +++ /dev/null @@ -1,22 +0,0 @@ -# -*- coding: utf-8 -*- - -from click.testing import CliRunner - -from vdirsyncer.doubleclick import _ctx_stack, click, ctx as global_ctx - - -def test_simple(): - @click.command() - @click.pass_context - def cli(ctx): - assert global_ctx - assert ctx.obj is global_ctx.obj - assert _ctx_stack.top is ctx - click.echo('hello') - - assert not global_ctx - runner = CliRunner() - result = runner.invoke(cli, catch_exceptions=False) - assert not global_ctx - assert not result.exception - assert result.output == 'hello\n' diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index e65a21b..3143f6b 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import logging import os import platform import stat @@ -7,11 +8,14 @@ import stat import click from click.testing import CliRunner +import click_log + import pytest import requests -from vdirsyncer import doubleclick, log, utils +from vdirsyncer import utils +from vdirsyncer.cli import pass_context # These modules might be uninitialized and unavailable if not explicitly # imported @@ -44,11 +48,12 @@ def empty_password_storages(monkeypatch): @pytest.fixture(autouse=True) def no_debug_output(request): - old = log._level - log.set_level(log.logging.WARNING) + logger = click_log.basic_config('vdirsyncer') + logger.setLevel(logging.WARNING) + old = logger.level def teardown(): - log.set_level(old) + logger.setLevel(old) request.addfinalizer(teardown) @@ -111,10 +116,10 @@ def test_get_password_from_command(tmpdir): st = os.stat(filepath) os.chmod(filepath, st.st_mode | stat.S_IEXEC) - @doubleclick.click.command() - @doubleclick.click.pass_context + @click.command() + @pass_context def fake_app(ctx): - ctx.obj = {'config': ({'password_command': filepath}, {}, {})} + ctx.config = {'password_command': filepath}, {}, {} _password = utils.password.get_password(username, resource) assert _password == password @@ -158,10 +163,9 @@ def test_set_keyring_password(monkeypatch): monkeypatch.setattr(utils.password, 'keyring', KeyringMock()) - @doubleclick.click.command() - @doubleclick.click.pass_context + @click.command() + @pass_context def fake_app(ctx): - ctx.obj = {} x = utils.password.get_password('foouser', 'http://example.com/a/b') click.echo('password is ' + x) @@ -179,15 +183,14 @@ def test_get_password_from_cache(monkeypatch): user = 'my_user' resource = 'http://example.com' - @doubleclick.click.command() - @doubleclick.click.pass_context + @click.command() + @pass_context def fake_app(ctx): - ctx.obj = {} x = utils.password.get_password(user, resource) click.echo('Password is {}'.format(x)) - monkeypatch.setattr(doubleclick.click, 'prompt', blow_up) + monkeypatch.setattr(click, 'prompt', blow_up) - assert (user, 'example.com') in ctx.obj['passwords'] + assert (user, 'example.com') in ctx.passwords x = utils.password.get_password(user, resource) click.echo('Password is {}'.format(x)) diff --git a/vdirsyncer/cli/__init__.py b/vdirsyncer/cli/__init__.py index 7a03927..f1f1cad 100644 --- a/vdirsyncer/cli/__init__.py +++ b/vdirsyncer/cli/__init__.py @@ -3,13 +3,26 @@ import functools import sys +import click + +import click_log + from .. import __version__, log -from ..doubleclick import click, ctx cli_logger = log.get(__name__) +class AppContext(object): + def __init__(self): + self.config = None + self.passwords = {} + self.logger = None + + +pass_context = click.make_pass_decorator(AppContext, ensure=True) + + class CliError(RuntimeError): def __init__(self, msg, problems=None): self.msg = msg @@ -52,32 +65,25 @@ def validate_verbosity(ctx, param, value): @click.group() -@click.option('--verbosity', '-v', default='INFO', - callback=validate_verbosity, - help='Either CRITICAL, ERROR, WARNING, INFO or DEBUG') +@click_log.init('vdirsyncer') +@click_log.simple_verbosity_option() @click.version_option(version=__version__) +@pass_context @catch_errors -def app(verbosity): +def app(ctx): ''' vdirsyncer -- synchronize calendars and contacts ''' from .utils import load_config - log.add_handler(log.stdout_handler) - log.set_level(verbosity) - if ctx.obj is None: - ctx.obj = {} - - ctx.obj['verbosity'] = verbosity - - if 'config' not in ctx.obj: - ctx.obj['config'] = load_config() + if not ctx.config: + ctx.config = load_config() main = app def max_workers_callback(ctx, param, value): - if value == 0 and ctx.obj['verbosity'] == log.logging.DEBUG: + if value == 0 and click_log.get_level() == log.logging.DEBUG: value = 1 cli_logger.debug('Using {} maximal workers.'.format(value)) @@ -99,8 +105,9 @@ max_workers_option = click.option( help=('Do/Don\'t abort synchronization when all items are about ' 'to be deleted from both sides.')) @max_workers_option +@pass_context @catch_errors -def sync(pairs, force_delete, max_workers): +def sync(ctx, pairs, force_delete, max_workers): ''' Synchronize the given pairs. If no arguments are given, all will be synchronized. @@ -119,27 +126,27 @@ def sync(pairs, force_delete, max_workers): ''' from .tasks import prepare_pair, sync_collection from .utils import parse_pairs_args, WorkerQueue - general, all_pairs, all_storages = ctx.obj['config'] + general, all_pairs, all_storages = ctx.config wq = WorkerQueue(max_workers) - for pair_name, collections in parse_pairs_args(pairs, all_pairs): - wq.put(functools.partial(prepare_pair, pair_name=pair_name, - collections=collections, - general=general, all_pairs=all_pairs, - all_storages=all_storages, - force_delete=force_delete, - callback=sync_collection)) - wq.spawn_worker() - - wq.join() + with wq.join(): + for pair_name, collections in parse_pairs_args(pairs, all_pairs): + wq.put(functools.partial(prepare_pair, pair_name=pair_name, + collections=collections, + general=general, all_pairs=all_pairs, + all_storages=all_storages, + force_delete=force_delete, + callback=sync_collection)) + wq.spawn_worker() @app.command() @click.argument('pairs', nargs=-1) @max_workers_option +@pass_context @catch_errors -def metasync(pairs, max_workers): +def metasync(ctx, pairs, max_workers): ''' Synchronize metadata of the given pairs. @@ -147,58 +154,57 @@ def metasync(pairs, max_workers): ''' from .tasks import prepare_pair, metasync_collection from .utils import parse_pairs_args, WorkerQueue - general, all_pairs, all_storages = ctx.obj['config'] + general, all_pairs, all_storages = ctx.config wq = WorkerQueue(max_workers) - for pair_name, collections in parse_pairs_args(pairs, all_pairs): - wq.put(functools.partial(prepare_pair, pair_name=pair_name, - collections=collections, - general=general, all_pairs=all_pairs, - all_storages=all_storages, - callback=metasync_collection)) - wq.spawn_worker() - - wq.join() + with wq.join(): + for pair_name, collections in parse_pairs_args(pairs, all_pairs): + wq.put(functools.partial(prepare_pair, pair_name=pair_name, + collections=collections, + general=general, all_pairs=all_pairs, + all_storages=all_storages, + callback=metasync_collection)) + wq.spawn_worker() @app.command() @click.argument('pairs', nargs=-1) @max_workers_option +@pass_context @catch_errors -def discover(pairs, max_workers): +def discover(ctx, pairs, max_workers): ''' Refresh collection cache for the given pairs. ''' from .tasks import discover_collections from .utils import WorkerQueue - general, all_pairs, all_storages = ctx.obj['config'] + general, all_pairs, all_storages = ctx.config wq = WorkerQueue(max_workers) - for pair in (pairs or all_pairs): - try: - name_a, name_b, pair_options = all_pairs[pair] - except KeyError: - raise CliError('Pair not found: {}\n' - 'These are the pairs found: {}' - .format(pair, list(all_pairs))) + with wq.join(): + for pair in (pairs or all_pairs): + try: + name_a, name_b, pair_options = all_pairs[pair] + except KeyError: + raise CliError('Pair not found: {}\n' + 'These are the pairs found: {}' + .format(pair, list(all_pairs))) - wq.put(functools.partial( - discover_collections, - status_path=general['status_path'], name_a=name_a, name_b=name_b, - pair_name=pair, config_a=all_storages[name_a], - config_b=all_storages[name_b], pair_options=pair_options, - skip_cache=True - )) - wq.spawn_worker() - - wq.join() + wq.put(functools.partial( + discover_collections, status_path=general['status_path'], + name_a=name_a, name_b=name_b, pair_name=pair, + config_a=all_storages[name_a], config_b=all_storages[name_b], + pair_options=pair_options, skip_cache=True + )) + wq.spawn_worker() @app.command() @click.argument('collection') +@pass_context @catch_errors -def repair(collection): +def repair(ctx, collection): ''' Repair a given collection. @@ -211,7 +217,7 @@ def repair(collection): collection of the `calendars_local` storage. ''' from .tasks import repair_collection - general, all_pairs, all_storages = ctx.obj['config'] + general, all_pairs, all_storages = ctx.config cli_logger.warning('This operation will take a very long time.') cli_logger.warning('It\'s recommended to turn off other client\'s ' diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index ce30fcf..3464068 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import contextlib import errno import hashlib import importlib @@ -7,14 +8,16 @@ import json import os import string import sys -import threading from itertools import chain from atomicwrites import atomic_write +import click + +import click_threading + from . import CliError, cli_logger from .. import DOCS_HOME, PROJECT_HOME, exceptions -from ..doubleclick import click from ..sync import IdentConflict, StorageEmpty, SyncConflict from ..utils import expand_path, get_class_init_args from ..utils.compat import text_type @@ -560,46 +563,52 @@ class WorkerQueue(object): self._workers = [] self._exceptions = [] self._max_workers = max_workers + self._shutdown_handlers = [] + + def shutdown(self): + if not self._queue.unfinished_tasks: + for handler in self._shutdown_handlers: + try: + handler() + except Exception: + pass def _worker(self): - # This is a daemon thread. Since the global namespace is going to - # vanish on interpreter shutdown, redefine everything from the global - # namespace here. - _TypeError = TypeError - _Empty = queue.Empty - _handle_cli_error = handle_cli_error - while True: try: func = self._queue.get(False) - except (_TypeError, _Empty): - # Any kind of error might be raised if vdirsyncer just finished - # processing all items and the interpreter is shutting down, - # yet the workers try to get new tasks. - # https://github.com/untitaker/vdirsyncer/issues/167 - # http://bugs.python.org/issue14623 + except queue.Empty: break try: func(wq=self) except Exception as e: - _handle_cli_error() + handle_cli_error() self._exceptions.append(e) finally: self._queue.task_done() + if not self._queue.unfinished_tasks: + self.shutdown() def spawn_worker(self): if self._max_workers and len(self._workers) >= self._max_workers: return - t = threading.Thread(target=self._worker) + t = click_threading.Thread(target=self._worker) t.daemon = True t.start() self._workers.append(t) + @contextlib.contextmanager def join(self): - assert self._workers or self._queue.empty() - self._queue.join() + assert self._workers or not self._queue.unfinished_tasks + ui_worker = click_threading.UiWorker() + self._shutdown_handlers.append(ui_worker.shutdown) + with ui_worker.patch_click(): + yield + ui_worker.run() + self._queue.join() + if self._exceptions: sys.exit(1) diff --git a/vdirsyncer/doubleclick.py b/vdirsyncer/doubleclick.py deleted file mode 100644 index e58d946..0000000 --- a/vdirsyncer/doubleclick.py +++ /dev/null @@ -1,144 +0,0 @@ -# -*- coding: utf-8 -*- -''' -Utilities for writing threaded applications with click: - -- There is a global ``ctx`` object to be used. - -- The ``click`` object's attributes are supposed to be used instead of the - click package's content. - - - It wraps some UI functions such that they don't produce overlapping - output or prompt the user at the same time. - - - It wraps BaseCommand subclasses such that their invocation changes the - ctx global, and also changes the shortcut decorators to use the new - classes. -''' - -import functools -import threading - - -class _ClickProxy(object): - def __init__(self, wrappers, click=None): - if click is None: - import click - self._click = click - self._cache = {} - self._wrappers = dict(wrappers) - - def __getattr__(self, name): - if name not in self._cache: - f = getattr(self._click, name) - f = self._wrappers.get(name, lambda x: x)(f) - self._cache[name] = f - - return self._cache[name] - - -_ui_lock = threading.Lock() - - -def _ui_function(f): - @functools.wraps(f) - def inner(*a, **kw): - with _ui_lock: - rv = f(*a, **kw) - return rv - return inner - - -class _Stack(object): - def __init__(self): - self._stack = [] - - @property - def top(self): - return self._stack[-1] - - def push(self, value): - self._stack.append(value) - - def pop(self): - return self._stack.pop() - - -class _StackProxy(object): - def __init__(self, stack): - object.__setattr__(self, '_doubleclick_stack', stack) - - def __bool__(self): - try: - self._doubleclick_stack.top - except IndexError: - return False - else: - return True - - __nonzero__ = __bool__ - - __getattr__ = lambda s, n: getattr(s._doubleclick_stack.top, n) - __setattr__ = lambda s, n, v: setattr(s._doubleclick_stack.top, n, v) - - -_ctx_stack = _Stack() -ctx = _StackProxy(_ctx_stack) - - -def _ctx_pushing_class(cls): - class ContextPusher(cls): - def command(self, *args, **kwargs): - # Also wrap commands created with @group.command() - def decorator(f): - cmd = click.command(*args, **kwargs)(f) - self.add_command(cmd) - return cmd - return decorator - - def invoke(self, ctx): - _ctx_stack.push(ctx) - try: - cls.invoke(self, ctx) - finally: - new_ctx = _ctx_stack.pop() - if new_ctx is not ctx: - raise RuntimeError( - 'While doubleclick is supposed to make writing ' - 'threaded applications easier, it removes thread ' - 'safety from click. It is therefore not recommended ' - 'to launch more than one doubleclick application per ' - 'process.' - ) - - return ContextPusher - - -def _command_class_wrapper(cls_name): - def inner(f): - def wrapper(name=None, **attrs): - attrs.setdefault('cls', getattr(click, cls_name)) - return f(name, **attrs) - return wrapper - return inner - - -WRAPPERS = { - 'echo': _ui_function, - 'echo_via_pager': _ui_function, - 'prompt': _ui_function, - 'confirm': _ui_function, - 'clear': _ui_function, - 'edit': _ui_function, - 'launch': _ui_function, - 'getchar': _ui_function, - 'pause': _ui_function, - 'BaseCommand': _ctx_pushing_class, - 'Command': _ctx_pushing_class, - 'MultiCommand': _ctx_pushing_class, - 'Group': _ctx_pushing_class, - 'CommandCollection': _ctx_pushing_class, - 'command': _command_class_wrapper('Command'), - 'group': _command_class_wrapper('Group') -} - -click = _ClickProxy(WRAPPERS) diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 68110a9..f4ca350 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -1,63 +1,5 @@ # -*- coding: utf-8 -*- import logging -import sys -from .doubleclick import click - - -class ColorFormatter(logging.Formatter): - colors = { - 'error': dict(fg='red'), - 'exception': dict(fg='red'), - 'critical': dict(fg='red'), - 'debug': dict(fg='blue'), - 'warning': dict(fg='yellow') - } - - def format(self, record): - if not record.exc_info: - level = record.levelname.lower() - if level in self.colors: - prefix = click.style('{}: '.format(level), - **self.colors[level]) - record.msg = '\n'.join(prefix + x - for x in str(record.msg).splitlines()) - - return logging.Formatter.format(self, record) - - -class ClickStream(object): - def write(self, string): - click.echo(string, file=sys.stderr, nl=False) - - -stdout_handler = logging.StreamHandler(ClickStream()) -stdout_handler.formatter = ColorFormatter() - -_level = logging.INFO -_handlers = [] - -_loggers = {} - - -def get(name): - assert name.startswith('vdirsyncer.') - if name not in _loggers: - _loggers[name] = x = logging.getLogger(name) - x.handlers = _handlers - x.setLevel(_level) - - return _loggers[name] - - -def add_handler(handler): - if handler not in _handlers: - _handlers.append(handler) - - -def set_level(level): - global _level - _level = level - for logger in _loggers.values(): - logger.setLevel(_level) +get = logging.getLogger diff --git a/vdirsyncer/utils/password.py b/vdirsyncer/utils/password.py index e13e26c..3d3f3f0 100644 --- a/vdirsyncer/utils/password.py +++ b/vdirsyncer/utils/password.py @@ -2,10 +2,12 @@ import threading +import click + from . import expand_path from .compat import urlparse from .. import exceptions, log -from ..doubleclick import click, ctx +from ..cli import AppContext logger = log.get(__name__) password_key_prefix = 'vdirsyncer:' @@ -40,10 +42,15 @@ def get_password(username, resource, _lock=threading.Lock()): """ - if ctx: - password_cache = ctx.obj.setdefault('passwords', {}) - else: - password_cache = {} # discard passwords + # If no app is running, Click will automatically create an empty cache for + # us and discard it. + try: + ctx = click.get_current_context().find_object(AppContext) + if ctx is None: + raise RuntimeError() + password_cache = ctx.passwords + except RuntimeError: + password_cache = {} def _password_from_cache(username, host): '''internal cache''' @@ -101,11 +108,17 @@ def _password_from_command(username, host): '''command''' import subprocess - if not ctx: + try: + ctx = click.get_current_context() + except RuntimeError: + return None + + ctx = ctx.find_object(AppContext) + if ctx is None or not ctx.config: return None try: - general, _, _ = ctx.obj['config'] + general, _, _ = ctx.config command = general['password_command'].split() except KeyError: return None