diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bf311b8f..4b245686a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,33 @@ +## v1.0.3.dev + +#### Changed: + +- Both "-h" and "--help" can now be used to print help output for a command. +- Show both the daemon and GUI version in the GUI settings window. +- The command line tool bundled with the macOS app now provides proper help output. +- Significantly reduced CPU usage of the GUI on macOS. +- The macOS app now uses a hardened runtime and is properly signed and notarized. + +#### Fixed: + +- Fixes an issue which could lead to the local Dropbox folder being moved before syncing + has been paused. +- Fixes an issue where download errors would show a rev number instead of the Dropbox + path. +- Fixes a race condition when two processes try to start a sync daemon at the same time. +- Fixes an issue in the macOS GUI where updating the displayed sync issues could fail. +- Fixes truncated text in the macOS setup dialog. +- Fixes an issue on fresh macOS installs where creating autostart entries could fail if + /Library/LaunchAgents does not yet exist. +- Fixes an issue in the macOS app bundle where installing the command line could tool + would fail if /usr/local/bin is owned by root (as is default on a fresh install). Now, + the user is asked for permission instead. + +#### Dependencies: + +- Removed `lockfile` dependency. +- Added `fasteners` dependency. + ## v1.0.2 This release fixes bugs in the command line interface. diff --git a/ROADMAP.md b/ROADMAP.md index 98e5b32da..76329502b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,15 +1,12 @@ #### Short term: -* Switch from implicit grant to PKCE OAuth flow as soon as Dropbox supports it (DONE). * Snap package once core20 is released and kde-neon works on core20. * CLI autocomplete for paths once there is better support from upstream `click`. -* Update macOS app bundle to Python 3.8 and Qt 5.14. +* Update macOS app bundle to Python 3.8. #### Long term: * deb and rpm packages: either with Pyinstaller executable or as Python package. * GUI support for multiple Dropbox accounts. -* Option to install command line scripts from macOS app bundle (DONE). -* Work with upstream `toga` to fix remaining issues for native macOS GUI, - notably memory leak in `rubicon.objc` (DONE). +* Consider using sqlite for index and state. diff --git a/docs/api/sync.rst b/docs/api/sync.rst index add7bac6a..a1cba43b7 100644 --- a/docs/api/sync.rst +++ b/docs/api/sync.rst @@ -4,4 +4,5 @@ Sync module .. automodule:: sync :members: + :private-members: :show-inheritance: diff --git a/docs/api/utils/backend.rst b/docs/api/utils/backend.rst deleted file mode 100644 index 960995ae9..000000000 --- a/docs/api/utils/backend.rst +++ /dev/null @@ -1,7 +0,0 @@ - -Backend functions -================= - -.. automodule:: utils.backend - :members: - :show-inheritance: diff --git a/docs/api/utils/index.rst b/docs/api/utils/index.rst index e891be541..393537e3c 100644 --- a/docs/api/utils/index.rst +++ b/docs/api/utils/index.rst @@ -7,10 +7,8 @@ maestral.utils utils.appdirs utils.autostart - utils.backend utils.housekeeping utils.notify - utils.oauth_implicit utils.path utils.serializer utils.updates diff --git a/docs/api/utils/oauth_implicit.rst b/docs/api/utils/oauth_implicit.rst deleted file mode 100644 index 80ec4630b..000000000 --- a/docs/api/utils/oauth_implicit.rst +++ /dev/null @@ -1,7 +0,0 @@ - -OAuth implicit flow -=================== - -.. automodule:: utils.oauth_implicit - :members: - :show-inheritance: diff --git a/docs/config_files.rst b/docs/config_files.rst new file mode 100644 index 000000000..5626b4c09 --- /dev/null +++ b/docs/config_files.rst @@ -0,0 +1,68 @@ + +Config files +============ + +The config files are located at ``$XDG_CONFIG_HOME/maestral`` on Linux (typically +``~/.config/maestral``) and ``~/Library/Application Support/maestral`` on macOS. Each +configuration will get its own INI file with the settings documented below. + +Config values in the sections ``main`` and ``account`` should not be edited manually but +rather through the corresponding CLI commands or GUI options. This is because changes of +these settings require Maestral to perform accompanying actions, e.g., download items +which have been removed from the excluded list or move the local Dropbox directory. +Those will not be performed if the user edits the options manually. + +Changes to the other sections may be performed manually but will only take effect once +Maestral is restarted. Maestral will overwrite the entire config file if any change is +made to one of the options through the ``maestral.config`` module. + +.. code-block:: ini + + [main] + + # The current Dropbox directory + path = /Users/samschott/Dropbox (Maestral) + + # Default directory name for the config + default_dir_name = Dropbox (Maestral) + + # List of excluded files and folders + excluded_items = ['/test_folder', '/sub/folder'] + + # Config file version (not the Maestral version!) + version = 12.0.0 + + [account] + + # Unique Dropbox account ID. The account's email + # address may change and is therefore not stored here. + account_id = dbid:AABP7CC5bpYd8cGHqIColDFrMoc9SdhACA4 + + [app] + + # Level for desktop notifications: + # 15 = FILECHAANGE + # 30 = SYNCISSUE + # 40 = ERROR + # 100 = NONE + notification_level = 15 + + # Level for log messages: + # 10 = DEBUG + # 20 = INFO + # 30 = WARNING + # 40 = ERR0R + log_level = 20 + + # Interval in sec to check for updates + update_notification_interval = 604800 + + # Enable or disable automatic error reports + analytics = False + + [sync] + + # Interval in sec to perform a full reindexing + reindex_interval = 604800 + # Maximum CPU usage per core + max_cpu_percent = 20.0 diff --git a/docs/index.rst b/docs/index.rst index 3a277ab73..312aea908 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -10,16 +10,16 @@ refer to the `wiki `_. The following APIs should remain stable for frontends such as the GUI or CLI: * maestral.main.Maestral -* maestral.constants * maestral.daemon +* maestral.constants * maestral.errors -* maestral.utils.appdirs -* maestral.utils.autostart .. toctree:: :caption: Table of Contents :maxdepth: 2 + config_files + state_files api/index .. toctree:: diff --git a/docs/requirements.txt b/docs/requirements.txt index e61f40a41..d5b5d4805 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -2,10 +2,10 @@ atomicwrites bugsnag click dropbox +fasteners importlib_metadata keyring keyrings.alt -lockfile packaging pathspec Pyro5 diff --git a/docs/state_files.rst b/docs/state_files.rst new file mode 100644 index 000000000..84a9ad5f3 --- /dev/null +++ b/docs/state_files.rst @@ -0,0 +1,65 @@ + +State files +=========== + +Maestral saves its persistent state in two files: "{config_name}.index" for the file +index and "{config_name}.state" for anything else. Both files are located at +$XDG_DATA_DIR/maestral on Linux (typically ~/.local/share/maestral) and +~/Library/Application Support/maestral on macOS. Each configuration will get its own +state file. + + +Index file +********** + +The index file contains all the tracked files and folders with their lower-case path +relative to the Dropbox folder and their "rev". Each line contains a single entry, written +as a dictionary ``{path: rev}`` in json format, for example: + +.. code-block:: python + + {"/test folder/subfolder/file.txt": "015a4ae1f15853400000001695a6c40"} + +If there are multiple entries (lines) which refer to the same path, the last entry +overwrites any previous entries. This allows rapidly updating the rev for a file or folder +by appending a new line to the index file without needing to write an entire file. + +An entry with ``rev == None`` means that any previous entries for this path and its +children should be discarded. + +After a sync cycle has completed, the file is cleaned up and all duplicate or empty +entries are removed. + + +State file +********** + +The state file has the following sections: + +.. code-block:: ini + + [account] + email = foo@bar.com + display_name = Foo Bar + abbreviated_name = FB + type = business + usage = 39.2% of 1312.8TB used + usage_type = team + + [app] + update_notification_last = 0.0 + latest_release = 1.0.3 + + [sync] + cursor = ... + lastsync = 1589979736.623609 + last_reindex = 1589577566.8533309 + download_errors = [] + pending_downloads = [] + recent_changes = [] + + [main] + version = 12.0.0 + +Notably, account info which can be changed by the user such as the email address is saved +in the state file while only the fixed Dropbox ID is saved in the config file. diff --git a/maestral/__init__.py b/maestral/__init__.py index 69acef49a..ff0afcf60 100644 --- a/maestral/__init__.py +++ b/maestral/__init__.py @@ -16,6 +16,6 @@ """ -__version__ = '1.0.2' +__version__ = '1.0.3' __author__ = 'Sam Schott' __url__ = 'https://github.com/SamSchott/maestral' diff --git a/maestral/cli.py b/maestral/cli.py index f8b039764..766b9f598 100755 --- a/maestral/cli.py +++ b/maestral/cli.py @@ -24,6 +24,7 @@ import Pyro5.errors # local imports +from maestral.daemon import freeze_support from maestral.config import MaestralConfig, MaestralState, list_configs from maestral.utils.housekeeping import remove_configuration @@ -362,33 +363,13 @@ def _run_daemon(ctx, param, value): ) -hidden_config_option = click.option( - '-c', '--config-name', - default='maestral', - is_eager=True, - expose_value=False, - help='For internal use only.', - hidden=True, -) +CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help']) -frozen_daemon_option = click.option( - '--frozen-daemon', - is_flag=True, - default=False, - is_eager=True, - expose_value=False, - callback=_run_daemon, - hidden=True, - help='For internal use only.' -) - - -@click.group(cls=SpecialHelpOrder) -@frozen_daemon_option -@hidden_config_option +@click.group(cls=SpecialHelpOrder, context_settings=CONTEXT_SETTINGS) def main(): - """Maestral Dropbox Client for Linux and macOS.""" + """Maestral Dropbox client for Linux and macOS.""" + freeze_support() check_for_updates() @@ -445,16 +426,10 @@ def gui(config_name): def start(config_name: str, foreground: bool, verbose: bool): """Starts the Maestral daemon.""" - from maestral.daemon import get_maestral_pid, get_maestral_proxy + from maestral.daemon import get_maestral_proxy from maestral.daemon import (start_maestral_daemon_thread, threads, start_maestral_daemon_process, Start) - # do nothing if already running - if get_maestral_pid(config_name): - click.echo('Maestral daemon is already running.') - return - - # start daemon click.echo('Starting Maestral...', nl=False) if foreground: @@ -464,6 +439,9 @@ def start(config_name: str, foreground: bool, verbose: bool): if res == Start.Ok: click.echo('\rStarting Maestral... ' + OK) + elif res == Start.AlreadyRunning: + click.echo('\rStarting Maestral... Already running.') + return else: click.echo('\rStarting Maestral... ' + FAILED) click.echo('Please check logs for more information.') @@ -841,14 +819,14 @@ def rebuild_index(config_name: str): @main.command(help_priority=16) def configs(): """Lists all configured Dropbox accounts.""" - from maestral.daemon import get_maestral_pid + from maestral.daemon import is_running # clean up stale configs config_names = list_configs() for name in config_names: dbid = MaestralConfig(name).get('account', 'account_id') - if dbid == '' and not get_maestral_pid(name): + if dbid == '' and not is_running(name): remove_configuration(name) # display remaining configs @@ -868,8 +846,8 @@ def analytics(config_name: str, yes: bool, no: bool): """ Enables or disables sharing error reports. - Sharing is disabled by default. If enbled, error reports are shared with bugsnag and - no personal infortmation will typically be collected. Shared tracebacks may however + Sharing is disabled by default. If enabled, error reports are shared with bugsnag and + no personal information will typically be collected. Shared tracebacks may however include file names, depending on the error. """ @@ -1132,4 +1110,4 @@ def notify_snooze(config_name: str, minutes: int): click.echo(f'Notifications snoozed for {minutes} min. ' 'Set snooze to 0 to reset.') else: - click.echo(f'Notifications enabled.') + click.echo('Notifications enabled.') diff --git a/maestral/client.py b/maestral/client.py index 0a737511e..ed1e14622 100644 --- a/maestral/client.py +++ b/maestral/client.py @@ -118,7 +118,7 @@ def wrapper(*args, **kwargs): return decorator -class MaestralApiClient: +class DropboxClient: """Client for the Dropbox SDK. This client defines basic methods to wrap Dropbox Python SDK calls, such as creating, @@ -251,18 +251,18 @@ def list_revisions(self, dbx_path, mode='path', limit=10): return self.dbx.files_list_revisions(dbx_path, mode=mode, limit=limit) @to_maestral_error(dbx_path_arg=1) - def download(self, dbx_path, dst_path, **kwargs): + def download(self, dbx_path, local_path, **kwargs): """ Downloads file from Dropbox to our local folder. - :param str dbx_path: Path to file on Dropbox. - :param str dst_path: Path to local download destination. + :param str dbx_path: Path to file on Dropbox or rev number. + :param str local_path: Path to local download destination. :param kwargs: Keyword arguments for Dropbox SDK files_download_to_file. :returns: Metadata of downloaded item. :rtype: :class:`dropbox.files.FileMetadata` """ # create local directory if not present - dst_path_directory = osp.dirname(dst_path) + dst_path_directory = osp.dirname(local_path) try: os.makedirs(dst_path_directory) except FileExistsError: @@ -275,7 +275,7 @@ def download(self, dbx_path, dst_path, **kwargs): downloaded = 0 - with open(dst_path, 'wb') as f: + with open(local_path, 'wb') as f: with contextlib.closing(http_resp): for c in http_resp.iter_content(chunksize): if md.size > 5 * 10 ** 6: # 5 MB @@ -285,7 +285,7 @@ def download(self, dbx_path, dst_path, **kwargs): return md - @to_maestral_error(dbx_path_arg=2) + @to_maestral_error(local_path_arg=1, dbx_path_arg=2) def upload(self, local_path, dbx_path, chunk_size_mb=5, **kwargs): """ Uploads local file to Dropbox. diff --git a/maestral/config/main.py b/maestral/config/main.py index 218e3e005..fe913cd76 100644 --- a/maestral/config/main.py +++ b/maestral/config/main.py @@ -11,6 +11,7 @@ import copy import logging +import threading from .base import get_conf_path, get_data_path from .user import UserConfig @@ -95,8 +96,11 @@ # Factories # ============================================================================= -_config_instances = {} -_state_instances = {} +_config_instances = dict() +_state_instances = dict() + +_config_lock = threading.Lock() +_state_lock = threading.Lock() def MaestralConfig(config_name): @@ -111,30 +115,32 @@ def MaestralConfig(config_name): global _config_instances - if config_name in _config_instances: - return _config_instances[config_name] - else: - defaults = copy.deepcopy(DEFAULTS) - # set default dir name according to config - for sec, options in defaults: - if sec == 'main': - options['default_dir_name'] = f'Dropbox ({config_name.title()})' - - config_path = get_conf_path(CONFIG_DIR_NAME) + with _config_lock: try: - conf = UserConfig( - config_path, config_name, defaults=defaults, version=CONF_VERSION, - load=True, backup=True, raw_mode=True, remove_obsolete=True - ) - except OSError: - conf = UserConfig( - config_path, config_name, defaults=defaults, version=CONF_VERSION, - load=False, backup=True, raw_mode=True, remove_obsolete=True - ) - - _config_instances[config_name] = conf - return conf + return _config_instances[config_name] + except KeyError: + defaults = copy.deepcopy(DEFAULTS) + # set default dir name according to config + for sec, options in defaults: + if sec == 'main': + options['default_dir_name'] = f'Dropbox ({config_name.title()})' + + config_path = get_conf_path(CONFIG_DIR_NAME) + + try: + conf = UserConfig( + config_path, config_name, defaults=defaults, version=CONF_VERSION, + load=True, backup=True, raw_mode=True, remove_obsolete=True + ) + except OSError: + conf = UserConfig( + config_path, config_name, defaults=defaults, version=CONF_VERSION, + load=False, backup=True, raw_mode=True, remove_obsolete=True + ) + + _config_instances[config_name] = conf + return conf def MaestralState(config_name): @@ -149,23 +155,25 @@ def MaestralState(config_name): global _state_instances - if config_name in _state_instances: - return _state_instances[config_name] - else: - state_path = get_data_path(CONFIG_DIR_NAME) + with _state_lock: try: - state = UserConfig( - state_path, config_name, defaults=DEFAULTS_STATE, - version=CONF_VERSION, load=True, backup=True, raw_mode=True, - remove_obsolete=True, suffix='.state' - ) - except OSError: - state = UserConfig( - state_path, config_name, defaults=DEFAULTS_STATE, - version=CONF_VERSION, load=False, backup=True, raw_mode=True, - remove_obsolete=True, suffix='.state' - ) - - _state_instances[config_name] = state - return state + return _state_instances[config_name] + except KeyError: + state_path = get_data_path(CONFIG_DIR_NAME) + + try: + state = UserConfig( + state_path, config_name, defaults=DEFAULTS_STATE, + version=CONF_VERSION, load=True, backup=True, raw_mode=True, + remove_obsolete=True, suffix='.state' + ) + except OSError: + state = UserConfig( + state_path, config_name, defaults=DEFAULTS_STATE, + version=CONF_VERSION, load=False, backup=True, raw_mode=True, + remove_obsolete=True, suffix='.state' + ) + + _state_instances[config_name] = state + return state diff --git a/maestral/daemon.py b/maestral/daemon.py index 36ab0398b..0873ebb4f 100644 --- a/maestral/daemon.py +++ b/maestral/daemon.py @@ -16,22 +16,50 @@ import signal import traceback import enum +import subprocess +from shlex import quote +import threading +import fcntl +import struct +import tempfile # external imports import Pyro5.errors from Pyro5.api import Daemon, Proxy, expose, oneway from Pyro5.serializers import SerpentSerializer -from lockfile.pidlockfile import PIDLockFile, AlreadyLocked +from fasteners import InterProcessLock # local imports from maestral.errors import SYNC_ERRORS, FATAL_ERRORS -from maestral.constants import IS_FROZEN +from maestral.constants import IS_FROZEN, IS_MACOS +from maestral.utils.appdirs import get_runtime_path threads = dict() URI = 'PYRO:maestral.{0}@{1}' +def freeze_support(): + """Freeze support for multiprocessing and daemon startup. This works by checking for + '--multiprocessing-fork' and '--frozen-daemon' command line arguments. Call this + function at the entry point of the executable, as soon as possible and ideally before + any heavy imports.""" + + import argparse + import multiprocessing as mp + + mp.freeze_support() + + parser = argparse.ArgumentParser(add_help=False) + parser.add_argument('-c', '--config-name', default='maestral') + parser.add_argument('--frozen-daemon', action='store_true') + parsed_args, remaining = parser.parse_known_args() + + if parsed_args.frozen_daemon: + start_maestral_daemon(parsed_args.config_name) + sys.exit() + + class Exit(enum.Enum): """Enumeration of daemon exit results.""" Ok = 0 @@ -76,10 +104,165 @@ def serpent_deserialize_api_error(class_name, d): ) +# ==== interprocess locking ============================================================== + +def _get_lockdata(): + + try: + os.O_LARGEFILE + except AttributeError: + start_len = 'll' + else: + start_len = 'qq' + + if (sys.platform.startswith(('netbsd', 'freebsd', 'openbsd')) + or sys.platform == 'darwin'): + if struct.calcsize('l') == 8: + off_t = 'l' + pid_t = 'i' + else: + off_t = 'lxxxx' + pid_t = 'l' + + fmt = off_t + off_t + pid_t + 'hh' + pid_index = 2 + lockdata = struct.pack(fmt, 0, 0, 0, fcntl.F_WRLCK, 0) + # elif sys.platform.startswith('gnukfreebsd'): + # fmt = 'qqihhi' + # pid_index = 2 + # lockdata = struct.pack(fmt, 0, 0, 0, fcntl.F_WRLCK, 0, 0) + # elif sys.platform in ('hp-uxB', 'unixware7'): + # fmt = 'hhlllii' + # pid_index = 2 + # lockdata = struct.pack(fmt, fcntl.F_WRLCK, 0, 0, 0, 0, 0, 0) + elif sys.platform.startswith('linux'): + fmt = 'hh' + start_len + 'ih' + pid_index = 4 + lockdata = struct.pack(fmt, fcntl.F_WRLCK, 0, 0, 0, 0, 0) + else: + raise RuntimeError(f'Unsupported platform {sys.platform}') + + return lockdata, fmt, pid_index + + +class Lock: + """ + A inter-process and inter-thread lock. This reuses uses code from oslo.concurrency + but provides non-blocking acquire. Use the :meth:`singleton` class method to retrieve + an existing instance for thread-safe usage. + """ + + _instances = dict() + _singleton_lock = threading.Lock() + + @classmethod + def singleton(cls, name, lock_path=None): + """ + Retrieve an existing lock object with a given 'name' or create a new one. Use this + method for thread-safe locks. + + :param str name: Name of lock file. + :param str lock_path: Directory for lock files. Defaults to the temporary + directory returned by :func:`tempfile.gettempdir()` if not given. + """ + + with cls._singleton_lock: + try: + instance = cls._instances[name] + except KeyError: + instance = cls(name, lock_path) + cls._instances[name] = instance + + return instance + + def __init__(self, name, lock_path=None): + + self.name = name + dirname = lock_path or tempfile.gettempdir() + lock_path = os.path.join(dirname, name) + + self._internal_lock = threading.Semaphore() + self._external_lock = InterProcessLock(lock_path) + + self._lock = threading.RLock() + + def acquire(self): + """ + Attempts to acquire the given lock. + + :returns: Whether or not the acquisition succeeded. + :rtype: bool + """ + + with self._lock: + locked_internal = self._internal_lock.acquire(blocking=False) + + if not locked_internal: + return False + + try: + locked_external = self._external_lock.acquire(blocking=False) + except Exception: + self._internal_lock.release() + raise + else: + + if locked_external: + return True + else: + self._internal_lock.release() + return False + + def release(self): + """Release the previously acquired lock.""" + with self._lock: + self._external_lock.release() + self._internal_lock.release() + + def locked(self): + """Checks if the lock is currently held by any thread or process.""" + with self._lock: + gotten = self.acquire() + if gotten: + self.release() + return not gotten + + def locking_pid(self): + """ + Returns the PID of the process which currently holds the lock or ``None``. This + should work on macOS, OpenBSD and Linux but may fail on some platforms. Always use + :meth:`locked` to check if the lock is held by any process. + + :returns: The PID of the process which currently holds the lock or ``None``. + :rtype: int + """ + + with self._lock: + + if self._external_lock.acquired: + return os.getpid() + + try: + # don't close again in case we are the locking process + self._external_lock._do_open() + lockdata, fmt, pid_index = _get_lockdata() + lockdata = fcntl.fcntl(self._external_lock.lockfile, + fcntl.F_GETLK, lockdata) + + lockdata_list = struct.unpack(fmt, lockdata) + pid = lockdata_list[pid_index] + + if pid > 0: + return pid + + except OSError: + pass + + # ==== helpers for daemon management ===================================================== def _escape_spaces(string): - return string.replace(" ", "_") + return string.replace(' ', '_') def _sigterm_handler(signal_number, frame): @@ -93,12 +276,19 @@ def _send_term(pid): pass -def _process_exists(pid): - try: - os.kill(pid, signal.SIG_DFL) - return True - except ProcessLookupError: - return False +class MaestralLock: + """ + An inter-process and inter-thread lock for Maestral. This is a wrapper around + :class:`Lock` which fills out the appropriate lockfile name and directory for the + given config name. + + :param str config_name: The name of the Maestral configuration. + """ + + def __new__(cls, config_name): + name = f'{config_name}.lock' + path = get_runtime_path('maestral') + return Lock.singleton(name, path) def sockpath_for_config(config_name): @@ -106,72 +296,44 @@ def sockpath_for_config(config_name): Returns the unix socket location to be used for the config. This should default to the apps runtime directory + '/maestral/CONFIG_NAME.sock'. """ - from maestral.utils.appdirs import get_runtime_path - return get_runtime_path('maestral', config_name + '.sock') + return get_runtime_path('maestral', f'{config_name}.sock') -def pidpath_for_config(config_name): - from maestral.utils.appdirs import get_runtime_path - return get_runtime_path('maestral', config_name + '.pid') - - -def is_pidfile_stale(pidfile): - """ - Determine whether a PID file is stale. Returns ``True`` if the PID file is stale, - ``False`` otherwise. The PID file is stale if its contents are valid but do not - match the PID of a currently-running process. - """ - result = False - - pid = pidfile.read_pid() - if pid: - return not _process_exists(pid) - else: - return result +def lockpath_for_config(config_name): + return get_runtime_path('maestral', f'{config_name}.lock') def get_maestral_pid(config_name): """ Returns Maestral's PID if the daemon is running, ``None`` otherwise. - :param str config_name: The name of the Maestral configuration to use. + :param str config_name: The name of the Maestral configuration. :returns: The daemon's PID. :rtype: int """ - lockfile = PIDLockFile(pidpath_for_config(config_name)) - pid = lockfile.read_pid() - - if pid and not is_pidfile_stale(lockfile): - return pid - else: - lockfile.break_lock() + return MaestralLock(config_name).locking_pid() -def _wait_for_startup(config_name, timeout=8): - """Waits for the daemon to start and verifies Pyro communication. Returns ``Start.Ok`` - if startup and communication succeeds within timeout, ``Start.Failed`` otherwise.""" - t0 = time.time() - pid = None +def is_running(config_name): + """ + Checks if a daemon is currently running. - while not pid and time.time() - t0 < timeout / 2: - pid = get_maestral_pid(config_name) - time.sleep(0.2) + :param str config_name: The name of the Maestral configuration. + :returns: Whether the daemon is running. + :rtype: bool + """ - if pid: - return _check_pyro_communication(config_name, timeout=int(timeout / 2)) - else: - return Start.Failed + return MaestralLock(config_name).locked() -def _check_pyro_communication(config_name, timeout=4): +def _wait_for_startup(config_name, timeout=8): """Checks if we can communicate with the maestral daemon. Returns ``Start.Ok`` if communication succeeds within timeout, ``Start.Failed`` otherwise.""" sock_name = sockpath_for_config(config_name) maestral_daemon = Proxy(URI.format(_escape_spaces(config_name), './u:' + sock_name)) - # wait until we can communicate with daemon, timeout after :param:`timeout` while timeout > 0: try: maestral_daemon._pyroBind() @@ -189,36 +351,30 @@ def _check_pyro_communication(config_name, timeout=4): def start_maestral_daemon(config_name='maestral', log_to_stdout=False): """ + Starts the Maestral daemon with event loop in the current thread. Startup is race + free: there will never be two daemons running for the same config. + Wraps :class:`main.Maestral` as Pyro daemon object, creates a new instance and starts Pyro's event loop to listen for requests on a unix domain socket. This call will block until the event loop shuts down. - This command will return silently if the daemon is already running. - :param str config_name: The name of the Maestral configuration to use. :param bool log_to_stdout: If ``True``, write logs to stdout. Defaults to ``False``. + :raises: :class:`RuntimeError` if a daemon for the given ``config_name`` is already + running. """ import threading from maestral.main import Maestral - sock_name = sockpath_for_config(config_name) - pid_name = pidpath_for_config(config_name) - - lockfile = PIDLockFile(pid_name) + sockpath = sockpath_for_config(config_name) if threading.current_thread() is threading.main_thread(): signal.signal(signal.SIGTERM, _sigterm_handler) # acquire PID lock file - - try: - lockfile.acquire() - except AlreadyLocked: - if is_pidfile_stale(lockfile): - lockfile.break_lock() - lockfile.acquire() - else: - return + lock = MaestralLock(config_name) + if not lock.acquire(): + raise RuntimeError('Maestral daemon is already running') # Nice ourselves to give other processes priority. We will likely only # have significant CPU usage in case of many concurrent downloads. @@ -227,7 +383,7 @@ def start_maestral_daemon(config_name='maestral', log_to_stdout=False): try: # clean up old socket try: - os.remove(sock_name) + os.remove(sockpath) except FileNotFoundError: pass @@ -243,7 +399,7 @@ def start_maestral_daemon(config_name='maestral', log_to_stdout=False): m = ExposedMaestral(config_name, log_to_stdout=log_to_stdout) - with Daemon(unixsocket=sock_name) as daemon: + with Daemon(unixsocket=sockpath) as daemon: daemon.register(m, f'maestral.{_escape_spaces(config_name)}') daemon.requestLoop(loopCondition=m._loop_condition) @@ -252,19 +408,23 @@ def start_maestral_daemon(config_name='maestral', log_to_stdout=False): except (KeyboardInterrupt, SystemExit): sys.exit(0) finally: - lockfile.release() + lock.release() def start_maestral_daemon_thread(config_name='maestral', log_to_stdout=False): """ - Starts the Maestral daemon in a thread (by calling :func:`start_maestral_daemon`). + Starts the Maestral daemon in a new thread by calling :func:`start_maestral_daemon`. + Startup is race free: there will never be two daemons running for the same config. :param str config_name: The name of the Maestral configuration to use. :param bool log_to_stdout: If ``True``, write logs to stdout. Defaults to ``False``. :returns: ``Start.Ok`` if successful, ``Start.AlreadyRunning`` if the daemon was - already running or ``Start.Failed`` if startup failed. + already running or ``Start.Failed`` if startup failed. It is possible that + Start.Ok is returned instead of Start.AlreadyRunning in case of a race. """ - import threading + + if is_running(config_name): + return Start.AlreadyRunning t = threading.Thread( target=start_maestral_daemon, @@ -282,57 +442,57 @@ def start_maestral_daemon_thread(config_name='maestral', log_to_stdout=False): return _wait_for_startup(config_name) -def start_maestral_daemon_process(config_name='maestral', log_to_stdout=False): +def _subprocess_launcher(config_name, log_to_stdout): + + if IS_FROZEN: + subprocess.Popen([sys.executable, '--frozen-daemon', '-c', config_name], + start_new_session=True) + else: + cc = quote(config_name).strip("'") # protect against injection + std_log = bool(log_to_stdout) + + cmd = (f'import maestral.daemon; ' + f'maestral.daemon.start_maestral_daemon("{cc}", {std_log})') + + subprocess.Popen([sys.executable, '-c', cmd], start_new_session=True) + + +def start_maestral_daemon_process(config_name='maestral', log_to_stdout=False, detach=True): """ - Starts the Maestral daemon in a separate process by calling - :func:`start_maestral_daemon`. - - This function assumes that ``sys.executable`` points to the Python executable. In - case of a frozen app, the executable must take the command line argument - ``--frozen-daemon to start`` a daemon process which is *not syncing*, .i.e., just run - :meth:`start_maestral_daemon`. This is currently supported through the - constole_script entry points of both `maestral` and `maestral_qt`. - - Starting a detached daemon process is difficult from a standalone executable since - the typical double-fork magic may fail on macOS and we do not have acccess to a - standalone Python interpreter to spawn a subprocess. Our approach mimics the "freeze - support" implemented by the multiprocessing module but fully detaches the spawned - process. + Starts the Maestral daemon in a new process by calling :func:`start_maestral_daemon`. + Startup is race free: there will never be two daemons running for the same config. + + This function assumes that ``sys.executable`` points to the Python executable or a + frozen executable. In case of a frozen executable, the executable must take the + command line argument '--frozen-daemon' to start a daemon process which is *not + syncing* by calling :func:`start_maestral_daemon`. This is currently supported through + :func:`freeze_support` which should be called from the main entry point, as soon as + possible after startup. :param str config_name: The name of the Maestral configuration to use. :param bool log_to_stdout: If ``True``, write logs to stdout. Defaults to ``False``. + :param bool detach: If ``True``, the daemon process will be detached by double-forking. :returns: ``Start.Ok`` if successful, ``Start.AlreadyRunning`` if the daemon was - already running or ``Start.Failed`` if startup failed. + already running or ``Start.Failed`` if startup failed. It is possible that + Start.Ok is returned instead of Start.AlreadyRunning in case of a race. """ - import subprocess - from shlex import quote - import multiprocessing as mp - - # use nested Popen and multiprocessing.Process to effectively create double fork - # see Unix 'double-fork magic' - if IS_FROZEN: + if is_running(config_name): + return Start.AlreadyRunning - def target(): - subprocess.Popen([sys.executable, '--frozen-daemon', '-c', config_name]) + if detach: + _subprocess_launcher(config_name, log_to_stdout) else: + import multiprocessing as mp + ctx = mp.get_context('spawn' if IS_MACOS else 'fork') - def target(): - # protect against injection - cc = quote(config_name).strip("'") - std_log = bool(log_to_stdout) - - cmd = (f'import maestral.daemon; ' - f'maestral.daemon.start_maestral_daemon("{cc}", {std_log})') - - subprocess.Popen([sys.executable, '-c', cmd]) - - mp.Process( - target=target, - name='maestral-daemon-launcher', - daemon=True, - ).start() + ctx.Process( + target=start_maestral_daemon, + args=(config_name, log_to_stdout), + name='maestral-daemon', + daemon=True, + ).start() return _wait_for_startup(config_name) @@ -340,48 +500,49 @@ def target(): def stop_maestral_daemon_process(config_name='maestral', timeout=10): """Stops a maestral daemon process by finding its PID and shutting it down. - This function first tries to shut down Maestral gracefully. If this fails, it will - send SIGTERM. If that fails as well, it will send SIGKILL to the process. + This function first tries to shut down Maestral gracefully. If this fails and we know + its PID, it will send SIGTERM. If that fails as well, it will send SIGKILL to the + process. :param str config_name: The name of the Maestral configuration to use. :param float timeout: Number of sec to wait for daemon to shut down before killing it. - :returns: ``Exit.Ok`` if successful, ``Exit.Killed`` if killed and ``Exit.NotRunning`` - if the daemon was not running. + :returns: ``Exit.Ok`` if successful, ``Exit.Killed`` if killed, ``Exit.NotRunning`` if + the daemon was not running and ``Exit.Failed`` if killing the process failed + because we could not retrieve its PID. """ - lockfile = PIDLockFile(pidpath_for_config(config_name)) - pid = lockfile.read_pid() + if not is_running(config_name): + return Exit.NotRunning - try: - if not pid or not _process_exists(pid): - return Exit.NotRunning + pid = get_maestral_pid(config_name) - try: - with MaestralProxy(config_name) as m: - m.stop_sync() - m.shutdown_pyro_daemon() - except Pyro5.errors.CommunicationError: + try: + with MaestralProxy(config_name) as m: + m.stop_sync() + m.shutdown_pyro_daemon() + except Pyro5.errors.CommunicationError: + if pid: _send_term(pid) - finally: - while timeout > 0: - if not _process_exists(pid): - return Exit.Ok - else: - time.sleep(0.2) - timeout -= 0.2 + finally: + while timeout > 0: + if not is_running(config_name): + return Exit.Ok + else: + time.sleep(0.2) + timeout -= 0.2 - # send SIGTERM after timeout and delete PID file - _send_term(pid) + # send SIGTERM after timeout and delete PID file + _send_term(pid) - time.sleep(1) + time.sleep(1) - if not _process_exists(pid): - return Exit.Ok - else: - os.kill(pid, signal.SIGKILL) - return Exit.Killed - finally: - lockfile.break_lock() + if not is_running(config_name): + return Exit.Ok + elif pid: + os.kill(pid, signal.SIGKILL) + return Exit.Killed + else: + return Exit.Failed def stop_maestral_daemon_thread(config_name='maestral', timeout=10): @@ -393,11 +554,7 @@ def stop_maestral_daemon_thread(config_name='maestral', timeout=10): ``Exit.Failed`` if it could not be stopped within timeout. """ - lockfile = PIDLockFile(pidpath_for_config(config_name)) - t = threads[config_name] - - if not t.is_alive(): - lockfile.break_lock() + if not is_running(config_name): return Exit.NotRunning # tell maestral daemon to shut down @@ -409,6 +566,7 @@ def stop_maestral_daemon_thread(config_name='maestral', timeout=10): return Exit.Failed # wait for maestral to carry out shutdown + t = threads.get(config_name) t.join(timeout=timeout) if t.is_alive(): return Exit.Failed @@ -428,9 +586,7 @@ def get_maestral_proxy(config_name='maestral', fallback=False): ``fallback`` is ``False``. """ - pid = get_maestral_pid(config_name) - - if pid: + if is_running(config_name): sock_name = sockpath_for_config(config_name) sys.excepthook = Pyro5.errors.excepthook diff --git a/maestral/errors.py b/maestral/errors.py index 2b35c0f8d..da5a5438d 100644 --- a/maestral/errors.py +++ b/maestral/errors.py @@ -638,8 +638,7 @@ def _get_lookup_error_msg(lookup_error): err_cls = SyncError if lookup_error.is_malformed_path(): - text = ('The destination path is invalid. Paths may not end with a slash or ' - 'whitespace.') + text = 'The path is invalid. Paths may not end with a slash or whitespace.' err_cls = PathError elif lookup_error.is_not_file(): text = 'The given path refers to a folder.' diff --git a/maestral/main.py b/maestral/main.py index 475ed927f..781adf38e 100644 --- a/maestral/main.py +++ b/maestral/main.py @@ -37,8 +37,8 @@ # local imports from maestral import __version__ from maestral.oauth import OAuth2Session -from maestral.client import MaestralApiClient, to_maestral_error -from maestral.sync import MaestralMonitor +from maestral.client import DropboxClient, to_maestral_error +from maestral.sync import SyncMonitor from maestral.errors import ( MaestralApiError, NotLinkedError, NoDropboxDirError, NotFoundError, PathError @@ -241,8 +241,8 @@ def __init__(self, config_name='maestral', log_to_stdout=False): self._setup_logging() self._auth = OAuth2Session(config_name) - self.client = MaestralApiClient(config_name=self.config_name, access_token='none') - self.monitor = MaestralMonitor(self.client) + self.client = DropboxClient(config_name=self.config_name, access_token='none') + self.monitor = SyncMonitor(self.client) self.sync = self.monitor.sync # periodically check for updates and refresh account info @@ -604,7 +604,7 @@ def syncing(self): else: return (self.monitor.syncing.is_set() or self.monitor.startup.is_set() - or self.sync.lock.locked()) + or self.sync.busy()) @property def paused(self): @@ -614,7 +614,7 @@ def paused(self): if self.pending_link: return False else: - return self.monitor.paused_by_user.is_set() and not self.sync.lock.locked() + return self.monitor.paused_by_user.is_set() and not self.sync.busy() @property def running(self): @@ -626,7 +626,7 @@ def running(self): if self.pending_link: return False else: - return self.monitor.running.is_set() or self.sync.lock.locked() + return self.monitor.running.is_set() or self.sync.busy() @property def connected(self): @@ -705,7 +705,7 @@ def get_file_status(self, local_path): if not self.syncing: return FileStatus.Unwatched.value - local_path = osp.abspath(local_path) + local_path = osp.realpath(local_path) try: dbx_path = self.sync.to_dbx_path(local_path) @@ -821,7 +821,7 @@ def get_profile_pic(self): def list_folder(self, dbx_path, **kwargs): """ List all items inside the folder given by ``dbx_path``. Keyword arguments are - passed on the the Dropbox API call :meth:`client.MaestralApiClient.list_folder`. + passed on the the Dropbox API call :meth:`client.DropboxClient.list_folder`. :param dbx_path: Path to folder on Dropbox. :returns: List of Dropbox item metadata as dicts or ``False`` if listing failed @@ -942,7 +942,7 @@ def exclude_item(self, dbx_path): raise NotFoundError('Cannot exlcude item', f'"{dbx_path}" does not exist on Dropbox') - dbx_path = dbx_path.lower().rstrip(osp.sep) + dbx_path = dbx_path.lower().rstrip('/') # add the path to excluded list if self.sync.is_excluded_by_user(dbx_path): @@ -1004,7 +1004,7 @@ def include_item(self, dbx_path): raise NotFoundError('Cannot include item', f'"{dbx_path}" does not exist on Dropbox') - dbx_path = dbx_path.lower().rstrip(osp.sep) + dbx_path = dbx_path.lower().rstrip('/') old_excluded_items = self.sync.excluded_items @@ -1086,7 +1086,7 @@ def excluded_status(self, dbx_path): :raises: :class:`errors.NotLinkedError` if no Dropbox account is linked. """ - dbx_path = dbx_path.lower().rstrip(osp.sep) + dbx_path = dbx_path.lower().rstrip('/') excluded_items = self._conf.get('main', 'excluded_items') @@ -1113,7 +1113,7 @@ def move_dropbox_directory(self, new_path): """ old_path = self.sync.dropbox_path - new_path = osp.abspath(osp.expanduser(new_path)) + new_path = osp.realpath(osp.expanduser(new_path)) try: if osp.samefile(old_path, new_path): @@ -1145,7 +1145,7 @@ def create_dropbox_directory(self, path): :raises: :class:`errors.NotLinkedError` if no Dropbox account is linked. """ - path = osp.abspath(osp.expanduser(path)) + path = osp.realpath(osp.expanduser(path)) self.monitor.reset_sync_state() diff --git a/maestral/sync.py b/maestral/sync.py index 783e247e0..05f81f928 100644 --- a/maestral/sync.py +++ b/maestral/sync.py @@ -19,7 +19,7 @@ import tempfile import random import json -from threading import Thread, Event, Lock, RLock, current_thread +from threading import Thread, Event, RLock, current_thread from concurrent.futures import ThreadPoolExecutor, as_completed from queue import Queue, Empty from collections import abc @@ -35,23 +35,28 @@ import dropbox from dropbox.files import Metadata, DeletedMetadata, FileMetadata, FolderMetadata from watchdog.events import FileSystemEventHandler -from watchdog.events import (EVENT_TYPE_CREATED, EVENT_TYPE_DELETED, - EVENT_TYPE_MODIFIED, EVENT_TYPE_MOVED) -from watchdog.events import (DirModifiedEvent, FileModifiedEvent, DirCreatedEvent, - FileCreatedEvent, DirDeletedEvent, FileDeletedEvent, - DirMovedEvent, FileMovedEvent) +from watchdog.events import ( + EVENT_TYPE_CREATED, EVENT_TYPE_DELETED, EVENT_TYPE_MODIFIED, EVENT_TYPE_MOVED +) +from watchdog.events import ( + DirModifiedEvent, FileModifiedEvent, DirCreatedEvent, FileCreatedEvent, + DirDeletedEvent, FileDeletedEvent, DirMovedEvent, FileMovedEvent +) from watchdog.utils.dirsnapshot import DirectorySnapshot from atomicwrites import atomic_write # local imports from maestral.config import MaestralConfig, MaestralState from maestral.fsevents import Observer -from maestral.constants import (IDLE, SYNCING, PAUSED, STOPPED, DISCONNECTED, - EXCLUDED_FILE_NAMES, EXCLUDED_DIR_NAMES, - MIGNORE_FILE, FILE_CACHE, IS_FS_CASE_SENSITIVE) -from maestral.errors import (RevFileError, NoDropboxDirError, CacheDirError, SyncError, - PathError, NotFoundError, FileConflictError, FolderConflictError, - fswatch_to_maestral_error, os_to_maestral_error) +from maestral.constants import ( + IDLE, SYNCING, PAUSED, STOPPED, DISCONNECTED, EXCLUDED_FILE_NAMES, EXCLUDED_DIR_NAMES, + MIGNORE_FILE, FILE_CACHE, IS_FS_CASE_SENSITIVE +) +from maestral.errors import ( + MaestralApiError, SyncError, RevFileError, NoDropboxDirError, CacheDirError, + PathError, NotFoundError, FileConflictError, FolderConflictError, + fswatch_to_maestral_error, os_to_maestral_error +) from maestral.utils.content_hasher import DropboxContentHasher from maestral.utils.notify import MaestralDesktopNotifier, FILECHANGE from maestral.utils.path import ( @@ -109,13 +114,13 @@ def __exit__(self, err_type, err_value, err_traceback): class FSEventHandler(FileSystemEventHandler): """ - Handles captured file events and adds them to :class:`UpDownSync`'s file event queue + Handles captured file events and adds them to :class:`SyncEngine`'s file event queue to be uploaded by :meth:`upload_worker`. This acts as a translation layer between - :class:`watchdog.Observer` and :class:`UpDownSync`. + :class:`watchdog.Observer` and :class:`SyncEngine`. :param Event syncing: Set when syncing is running. :param Event startup: Set when startup is running. - :param UpDownSync sync: UpDownSync instance. + :param SyncEngine sync: SyncEngine instance. :cvar int ignore_timeout: Timeout in seconds after which ignored paths will be discarded. @@ -130,8 +135,8 @@ def __init__(self, syncing, startup, sync): self.sync = sync self.sync.fs_events = self - self._ignored_paths = list() - self._mutex = Lock() + self._ignored_events = [] + self._ignored_events_mutex = RLock() self.local_file_event_queue = Queue() @@ -149,13 +154,13 @@ def ignore(self, *events, recursive=True): itself, for instance during a download or when moving a conflict. :param iterable events: Local events to ignore. - :param bool recursive: If ``True``, all child events of a dirctory event will be - ignored as well. + :param bool recursive: If ``True``, all child events of a directory event will be + ignored as well. This parameter will be ignored for file events. """ - with self._mutex: + with self._ignored_events_mutex: now = time.time() - new_ignores = list() + new_ignores = [] for e in events: new_ignores.append( dict( @@ -165,59 +170,62 @@ def ignore(self, *events, recursive=True): recursive=recursive and e.is_directory, ) ) - self._ignored_paths.extend(new_ignores) + self._ignored_events.extend(new_ignores) try: yield finally: - with self._mutex: + with self._ignored_events_mutex: for ignore in new_ignores: ignore['ttl'] = time.time() + self.ignore_timeout - def _expire_ignored_paths(self): + def _expire_ignored_events(self): """Removes all expired ignore entries.""" - with self._mutex: + with self._ignored_events_mutex: + now = time.time() - for ignore in self._ignored_paths.copy(): + for ignore in self._ignored_events.copy(): ttl = ignore['ttl'] if ttl and ttl < now: - self._ignored_paths.remove(ignore) + self._ignored_events.remove(ignore) - def _discrad_ignored(self, event): + def _is_ignored(self, event): """ - Checks if a file system event should been explicitly ignored because it was likely - triggered by Maestral. Split moved events if necessary and returns the event to - keep (if any) + Checks if a file system event should been explicitly ignored because it was + triggered by Maestral itself. :param FileSystemEvent event: Local file system event. - :returns: Event to keep or ``None``. - :rtype: :class:`watchdog.FileSystemEvent` + :returns: ``True`` if the event should be ignored, ``False`` otherwise. + :rtype: bool """ - self._expire_ignored_paths() + with self._ignored_events_mutex: + + self._expire_ignored_events() - for ignore in self._ignored_paths: - ignore_event = ignore['event'] - recursive = ignore['recursive'] + for ignore in self._ignored_events: + ignore_event = ignore['event'] + recursive = ignore['recursive'] - if event == ignore_event: + if event == ignore_event: - if not recursive: - self._ignored_paths.remove(ignore) + if not recursive: + self._ignored_events.remove(ignore) - return None + return True - elif recursive: + elif recursive: - type_match = event.event_type == ignore_event.event_type - src_match = is_equal_or_child(event.src_path, ignore_event.src_path) - dest_match = is_equal_or_child(get_dest_path(event), get_dest_path(ignore_event)) + type_match = event.event_type == ignore_event.event_type + src_match = is_equal_or_child(event.src_path, ignore_event.src_path) + dest_match = is_equal_or_child(get_dest_path(event), + get_dest_path(ignore_event)) - if type_match and src_match and dest_match: - return None + if type_match and src_match and dest_match: + return True - return event + return False def on_any_event(self, event): """ @@ -236,21 +244,22 @@ def on_any_event(self, event): if isinstance(event, DirModifiedEvent): return - # check for ignored paths, split moved events if necessary - event = self._discrad_ignored(event) + # check if event should be ignored + if self._is_ignored(event): + return - if event: - self.local_file_event_queue.put(event) + self.local_file_event_queue.put(event) -class MaestralStateWrapper(abc.MutableSet): +class PersistentStateMutableSet(abc.MutableSet): """ - A wrapper for a list in the saved state that implements a MutableSet interface. All - given paths are stored in lower-case, reflecting Dropbox's insensitive file system. + A wrapper for a list of strings in the saved state that implements a MutableSet + interface. All strings are stored as lower-case, reflecting Dropbox's case-insensitive + file system. - :param str config_name: Name of config. - :param str section: Section name in state. - :param str option: Option name in state. + :param str config_name: Name of config (determines name of state file). + :param str section: Section name in state file. + :param str option: Option name in state file. """ _lock = RLock() @@ -275,7 +284,7 @@ def __len__(self): return len(self._state.get(self.section, self.option)) def discard(self, dbx_path): - dbx_path = dbx_path.lower().rstrip(osp.sep) + dbx_path = dbx_path.lower().rstrip('/') with self._lock: state_list = self._state.get(self.section, self.option) state_list = set(state_list) @@ -283,7 +292,7 @@ def discard(self, dbx_path): self._state.set(self.section, self.option, list(state_list)) def add(self, dbx_path): - dbx_path = dbx_path.lower().rstrip(osp.sep) + dbx_path = dbx_path.lower().rstrip('/') with self._lock: state_list = self._state.get(self.section, self.option) state_list = set(state_list) @@ -303,7 +312,10 @@ def __repr__(self): def catch_sync_issues(download=False): """ Returns a decorator that catches all SyncErrors and logs them. - Should only be used for methods of UpDownSync. + Should only be used to decorate methods of :class:`SyncEngine`. + + :param bool download: If ``True``, sync issues will be added to the `download_errors` + list to retry later. """ def decorator(func): @@ -347,86 +359,81 @@ def wrapper(self, *args, **kwargs): return decorator -class UpDownSync: +class SyncEngine: """ Class that contains methods to sync local file events with Dropbox and vice versa. Notes on event processing: - Remote events come in three types, DeletedMetadata, FolderMetadata and FileMetadata. + Remote events come in three types: DeletedMetadata, FolderMetadata and FileMetadata. The Dropbox API does not differentiate between created, moved or modified events. - Maestral processes the events as follows: + Maestral processes remote events as follows: - 1) ``_clean_remote_changes``: Combine multiple events per file path into one. This - is rarely necessary, Dropbox typically already provides a only single event per - path but this is not guaranteed and may change. One exception is sharing a + 1) :meth:`wait_for_remote_changes` blocks until remote changes are available. + 2) :meth:`get_remote_changes` lists all remote changes since the last sync. + 3) :meth:`_clean_remote_changes`: Combines multiple events per file path into one. + This is rarely necessary, Dropbox typically already provides only a single event + per path but this is not guaranteed and may change. One exception is sharing a folder: This is done by removing the folder from Dropbox and re-mounting it as a shared folder and produces at least one DeletedMetadata and one FolderMetadata event. If querying for changes *during* this process, multiple DeletedMetadata - events may be returned. If a File / Folder event implies a type changes, e.g., + events may be returned. If a file / folder event implies a type changes, e.g., replacing a folder with a file, we explicitly generate the necessary DeletedMetadata here to simplify conflict resolution. - 2) ``_filter_excluded_changes_remote``: Filters out events that occurred for files - or folders excluded by selective sync as well as hard-coded file names which are - always excluded (e.g., `.DS_Store`). - 3) ``apply_remote_changes``: Sorts all events hierarchically, with top-level events - coming first. Deleted and folder events are processed in order, file events in - parallel with up to 6 worker threads. - 4) ``create_local_entry``: Checks for sync conflicts by comparing the file version, - as determined from its rev number, with our locally saved rev. We assign folders - a rev of 'folder' and deleted / non-existent items a rev of None. If revs are - equal, the local item is the same or newer as an Dropbox, no download / deletion - occurs. If revs are different, we compare content hashes. Folders are assigned a - hash of 'folder'. If hashes are equal, no download occurs. Finally we check if - the local item has been modified since the last download sync. In case of a - folder, we take newest change of any of its children. If the local item has not - been modified since the last sync, it will be overridden. Otherwise, we create a + 2) :meth:`_filter_excluded_changes_remote`: Filters out events that occurred for + entries that are excluded by selective sync and hard-coded file names which are + always excluded (e.g., '.DS_Store'). + 3) :meth:`apply_remote_changes`: Sorts all events hierarchically, with top-level + events coming first. Deleted and folder events are processed in order, file + events in parallel with up to 6 worker threads. The actual download is carried + out by :meth:`_create_local_entry`. + 4) :meth:`_create_local_entry`: Checks for sync conflicts by comparing the file + "rev" with our locally saved rev. We assign folders a rev of ``'folder'`` and + deleted / non-existent items a rev of ``None``. If revs are equal, the local item + is the same or newer as on Dropbox and no download / deletion occurs. If revs are + different, we compare content hashes. Folders are assigned a hash of 'folder'. If + hashes are equal, no download occurs. If they are different, we check if the + local item has been modified since the last download sync. In case of a folder, + we take the newest change of any of its children. If the local entry has not been + modified since the last sync, it will be replaced. Otherwise, we create a conflicting copy. + 5) :meth:`notify_user`: Shows a desktop notification for the remote changes. Local file events come in eight types: For both files and folders we collect created, moved, modified and deleted events. They are processed as follows: - 1) ``FSEventHandler``: Our file system event handler tries to discard any events - that originate from Maestral itself, e.g., from downloads. In case of a moved - event, if only one of the two paths should be ignored at this stage, the event - will be split into a deleted event (old path) and a created event (new path) and - one of the two will be ignored. - 2) We wait until no new changes happen for at least 1.0 sec. - 3) ``_filter_excluded_changes_local``: Filters out events ignored by a `.mignore` - pattern as well as hard-coded file names which are always excluded. - 4) ``_clean_local_events``: Cleans up local events in two stages. First, multiple - events per path are combined into a single event to reproduce the file changes. - The only exceptions is when the item type changes from file to folder or vice - versa: in this case, both deleted and created events are kept. Second, when a - whole folder is moved or deleted, we discard the moved and deleted events of its - children. - 4) ``apply_local_changes``: Sort local changes hierarchically and apply events in - the order of deleted, folders and files. File uploads will be carrier out in - parallel with up to 6 threads. Conflict resolution and upload / move / deletion - will be handled by ``create_remote_entry`` as follows: - 5) Conflict resolution: For created and moved events, we check if the new path has - been excluded by the user with selective sync but still exists on Dropbox. If - yes, it will be renamed by appending "(selective sync conflict)". On case- - sensitive file systems, we check if the new path differs only in casing from an - existing path. If yes, it will be renamed by appending "(case conflict)". If a - file has been replaced with a folder or vice versa, check if any un-synced - changes will be lost replacing the remote item. Create a conflicting copy if - necessary. Dropbox does not handle conflict resolution for us in this case. - 7) For created or modified files, check if the local content hash equals the remote - content hash. If yes, we don't upload but update our rev number. - 8) Upload the changes, specify the rev which we want to replace / delete. If the - remote item is newer (different rev), Dropbox will handle conflict resolution. - 9) Confirm the successful upload and check if Dropbox has renamed the item to a - conflicting copy. In the latter case, apply those changes locally. - 10) Update local revs with the new revs assigned by Dropbox. - - :param MaestralApiClient client: Dropbox API client instance. - """ - - lock = Lock() + 1) :meth:`wait_for_local_changes`: Blocks until local changes were registered by + :class:`FSEventHandler` and returns those changes. + 2) :meth:`_filter_excluded_changes_local`: Filters out events ignored by a + "mignore" pattern as well as hard-coded file names and changes in our cache path. + 3) :meth:`_clean_local_events`: Cleans up local events in two stages. First, + multiple events per path are combined into a single event which reproduces the + file changes. The only exception is when the entry type changes from file to + folder or vice versa: in this case, both deleted and created events are kept. + Second, when a whole folder is moved or deleted, we discard the moved and deleted + events of its children. + 4) :meth:`apply_local_changes`: Sorts local changes hierarchically and applies + events in the order of deleted, folders and files. Deletions and creations + will be carried out in parallel with up to 6 threads. Conflict resolution and + the actual upload will be handled by :meth:`_create_remote_entry` as follows: + 5) :meth:`_create_remote_entry`: For created and moved events, we check if the new + path has been excluded by the user with selective sync but still exists on + Dropbox. If yes, it will be renamed by appending "(selective sync conflict)". On + case-sensitive file systems, we check if the new path differs only in casing from + an existing path. If yes, it will be renamed by appending "(case conflict)". If a + file has been replaced with a folder or vice versa, we check if any un-synced + changes will be lost by replacing the remote item and create a conflicting copy + if necessary. Dropbox does not handle conflict resolution for us in this case. + For created or modified files, check if the local content hash equals the remote + content hash. If yes, we don't upload but update our rev number. If no, we upload + the changes and specify the rev which we want to replace or delete. If the remote + item is newer (different rev), Dropbox will handle conflict resolution for us. We + finally confirm the successful upload and check if Dropbox has renamed the item + to a conflicting copy. In the latter case, we apply those changes locally. + + :param DropboxClient client: Dropbox API client instance. - _rev_lock = RLock() - _last_sync_lock = RLock() + """ _max_history = 30 _num_threads = min(32, os.cpu_count() * 3) @@ -438,14 +445,17 @@ def __init__(self, client): self.cancel_pending = Event() self.fs_events = None + self.sync_lock = RLock() + self._rev_lock = RLock() + self._conf = MaestralConfig(self.config_name) self._state = MaestralState(self.config_name) self._notifier = MaestralDesktopNotifier.for_config(self.config_name) - self.download_errors = MaestralStateWrapper( + self.download_errors = PersistentStateMutableSet( self.config_name, section='sync', option='download_errors' ) - self.pending_downloads = MaestralStateWrapper( + self.pending_downloads = PersistentStateMutableSet( self.config_name, section='sync', option='pending_downloads' ) @@ -461,7 +471,7 @@ def __init__(self, client): self.queue_downloading = Queue() # load cached properties - self._dropbox_path = self._conf.get('main', 'path') + self._dropbox_path = osp.realpath(self._conf.get('main', 'path')) self._mignore_path = osp.join(self._dropbox_path, MIGNORE_FILE) self._file_cache_path = osp.join(self._dropbox_path, FILE_CACHE) self._rev_file_path = get_data_path('maestral', f'{self.config_name}.index') @@ -491,10 +501,14 @@ def dropbox_path(self): @dropbox_path.setter def dropbox_path(self, path): """Setter: dropbox_path""" - self._dropbox_path = path - self._mignore_path = osp.join(self._dropbox_path, MIGNORE_FILE) - self._file_cache_path = osp.join(self._dropbox_path, FILE_CACHE) - self._conf.set('main', 'path', path) + + path = osp.realpath(path) + + with self.sync_lock: + self._dropbox_path = path + self._mignore_path = osp.join(self._dropbox_path, MIGNORE_FILE) + self._file_cache_path = osp.join(self._dropbox_path, FILE_CACHE) + self._conf.set('main', 'path', path) @property def rev_file_path(self): @@ -503,7 +517,9 @@ def rev_file_path(self): @property def file_cache_path(self): - """Path to sync index with rev numbers (read only).""" + """Path to cache folder for temporary files (read only). The cache folder + '.maestral.cache' is located inside the local Dropbox folder to prevent + file transfer between different partitions or drives during sync.""" return self._file_cache_path @property @@ -518,9 +534,11 @@ def excluded_items(self): @excluded_items.setter def excluded_items(self, folder_list): """Setter: excluded_items""" - clean_list = self.clean_excluded_items_list(folder_list) - self._excluded_items = clean_list - self._conf.set('main', 'excluded_items', clean_list) + + with self.sync_lock: + clean_list = self.clean_excluded_items_list(folder_list) + self._excluded_items = clean_list + self._conf.set('main', 'excluded_items', clean_list) @staticmethod def clean_excluded_items_list(folder_list): @@ -534,7 +552,7 @@ def clean_excluded_items_list(folder_list): """ # remove duplicate entries by creating set, strip trailing '/' - folder_list = set(f.lower().rstrip(osp.sep) for f in folder_list) + folder_list = set(f.lower().rstrip('/') for f in folder_list) # remove all children of excluded folders clean_list = list(folder_list) @@ -546,8 +564,8 @@ def clean_excluded_items_list(folder_list): @property def max_cpu_percent(self): """Maximum CPU usage for parallel downloads or uploads in percent of the total - available CPU time. Individual workers in a thread pool will pause until the - usage drops below this value. Tasks in the main thread such as indexing file + available CPU time per core. Individual workers in a thread pool will pause until + the usage drops below this value. Tasks in the main thread such as indexing file changes may still use more CPU time. Setting this to 100% means that no limits on CPU usage will be applied.""" return self._max_cpu_percent @@ -569,8 +587,9 @@ def last_cursor(self): @last_cursor.setter def last_cursor(self, cursor): """Setter: last_cursor""" - self._state.set('sync', 'cursor', cursor) - logger.debug('Remote cursor saved: %s', cursor) + with self.sync_lock: + self._state.set('sync', 'cursor', cursor) + logger.debug('Remote cursor saved: %s', cursor) @property def last_sync(self): @@ -581,8 +600,9 @@ def last_sync(self): @last_sync.setter def last_sync(self, last_sync): """Setter: last_sync""" - logger.debug('Local cursor saved: %s', last_sync) - self._state.set('sync', 'lastsync', last_sync) + with self.sync_lock: + logger.debug('Local cursor saved: %s', last_sync) + self._state.set('sync', 'lastsync', last_sync) @property def last_reindex(self): @@ -603,9 +623,8 @@ def get_last_sync_for_path(self, dbx_path): :returns: Time of last sync. :rtype: float """ - with self._last_sync_lock: - dbx_path = dbx_path.lower() - return max(self._last_sync_for_path.get(dbx_path, 0.0), self.last_sync) + dbx_path = dbx_path.lower() + return max(self._last_sync_for_path.get(dbx_path, 0.0), self.last_sync) def set_last_sync_for_path(self, dbx_path, last_sync): """ @@ -614,15 +633,14 @@ def set_last_sync_for_path(self, dbx_path, last_sync): :param str dbx_path: Path relative to Dropbox folder. :param float last_sync: Time of last sync. """ - with self._last_sync_lock: - dbx_path = dbx_path.lower() - if last_sync == 0.0: - try: - del self._last_sync_for_path[dbx_path] - except KeyError: - pass - else: - self._last_sync_for_path[dbx_path] = last_sync + dbx_path = dbx_path.lower() + if last_sync == 0.0: + try: + del self._last_sync_for_path[dbx_path] + except KeyError: + pass + else: + self._last_sync_for_path[dbx_path] = last_sync # ==== rev file management =========================================================== @@ -658,7 +676,7 @@ def set_local_rev(self, dbx_path, rev): entry for the file is removed. :param str dbx_path: Path relative to Dropbox folder. - :param str, None rev: Revision number as string or ``None``. + :param str rev: Revision number as string or ``None``. """ with self._rev_lock: dbx_path = dbx_path.lower() @@ -687,7 +705,8 @@ def set_local_rev(self, dbx_path, rev): def _clean_and_save_rev_file(self): """Cleans the revision index from duplicate entries and keeps only the last entry for any individual path. Then saves the index to the drive.""" - self._save_rev_dict_to_file() + with self._rev_lock: + self._save_rev_dict_to_file() def clear_rev_index(self): """Clears the revision index.""" @@ -750,8 +769,8 @@ def _handle_rev_write_exceptions(self, raise_exception=False): def _load_rev_dict_from_file(self, raise_exception=False): """ Loads Maestral's rev index from ``rev_file_path``. Every line contains the rev - number for a single path, saved in a json format. Only the last entry for every - path is kept since it is the newest. + number for a single path, saved in a json format. Only the last entry for each + path is kept, overriding possible (older) previous entries. :param bool raise_exception: If ``True``, raises an exception when loading fails. If ``False``, an error message is logged instead. @@ -793,13 +812,16 @@ def _save_rev_dict_to_file(self, raise_exception=False): def _append_rev_to_file(self, path, rev, raise_exception=False): """ - Appends a new line with '{path}: {rev}\n' to the rev file. This is quicker than + Appends a new line with a rev entry to the rev file. This is quicker than saving the entire rev index. When loading the rev file, older entries will be - overwritten with newer ones and all entries with rev == None will be discarded. + overwritten with newer ones and all entries with ``rev == None`` will be + discarded. :param str path: Path for rev. - :param str, None rev: Dropbox rev number. - :raises: RevFileError if ``raise_exception`` is ``True``. + :param str rev: Dropbox rev or ``None``. + :param bool raise_exception: If ``True``, raises an exception when saving fails. + If ``False``, an error message is logged instead. + :raises: :class:`errors.RevFileError` """ with self._rev_lock: @@ -816,17 +838,20 @@ def mignore_path(self): @property def mignore_rules(self): - """List of mignore rules following git wildmatch syntax.""" - if self.get_ctime(self.mignore_path) != self._mignore_ctime_loaded: + """List of mignore rules following git wildmatch syntax (read only).""" + if self._get_ctime(self.mignore_path) != self._mignore_ctime_loaded: self._mignore_rules = self._load_mignore_rules_form_file() return self._mignore_rules def _load_mignore_rules_form_file(self): - self._mignore_ctime_loaded = self.get_ctime(self.mignore_path) + """Loads rules from mignore file. No rules are loaded if the file does + not exist or cannot be read.""" + self._mignore_ctime_loaded = self._get_ctime(self.mignore_path) try: with open(self.mignore_path) as f: spec = f.read() - except FileNotFoundError: + except OSError as exc: + logger.debug(f'Could not load mignore rules from {self.mignore_path}: {exc}') spec = '' return pathspec.PathSpec.from_lines('gitwildmatch', spec.splitlines()) @@ -863,11 +888,14 @@ def _ensure_cache_dir_present(self): f'{self.file_cache_path}.') def clean_cache_dir(self): - err = delete(self._file_cache_path) - if err and not isinstance(err, FileNotFoundError): - raise CacheDirError(f'Cannot create cache directory (errno {err.errno})', - 'Please check if you have write permissions for ' - f'{self._file_cache_path}.') + """Removes all items in the cache directory.""" + + with self.sync_lock: + err = delete(self._file_cache_path) + if err and not isinstance(err, FileNotFoundError): + raise CacheDirError(f'Cannot create cache directory (errno {err.errno})', + 'Please check if you have write permissions for ' + f'{self._file_cache_path}.') def _new_tmp_file(self): self._ensure_cache_dir_present() @@ -890,10 +918,9 @@ def to_dbx_path(self, local_path): :raises: :class:`ValueError` the path lies outside of the local Dropbox folder. """ - if local_path == self.dropbox_path: # path corresponds to dropbox_path - return '/' - elif is_child(local_path, self.dropbox_path): - return local_path.replace(self.dropbox_path, '', 1) + if is_equal_or_child(local_path, self.dropbox_path): + dbx_path = osp.sep + local_path.replace(self.dropbox_path, '', 1).lstrip(osp.sep) + return dbx_path.replace(osp.sep, '/') else: raise ValueError(f'Specified path "{local_path}" is outside of Dropbox ' f'directory "{self.dropbox_path}"') @@ -904,7 +931,7 @@ def to_local_path(self, dbx_path): The ``path_display`` attribute returned by the Dropbox API only guarantees correct casing of the basename and not of the full path. This is because Dropbox itself is - not case sensitive and stores all paths in lowercase internally. To the extend + not case sensitive and stores all paths in lowercase internally. To the extent where parent directories of ``dbx_path`` exist on the local drive, their casing will be used. Otherwise, the casing from ``dbx_path`` is used. This aims to preserve the correct casing of file and folder names and prevents the creation of @@ -921,7 +948,7 @@ def to_local_path(self, dbx_path): local_parent = to_cased_path(dbx_path_parent, root=self.dropbox_path) if local_parent == '': - return osp.join(self.dropbox_path, dbx_path.lstrip(osp.sep)) + return osp.join(self.dropbox_path, dbx_path.lstrip('/')) else: return osp.join(local_parent, dbx_path_basename) @@ -974,7 +1001,7 @@ def is_excluded(path): :returns: ``True`` if excluded, ``False`` otherwise. :rtype: bool """ - path = path.lower() + path = path.lower().replace(osp.sep, '/') # is root folder? if path in ('/', ''): @@ -1049,6 +1076,20 @@ def _slow_down(self): while cpu_usage > self._max_cpu_percent: cpu_usage = cpu_usage_percent(0.5 + 2 * random.random()) + def busy(self): + """ + Checks if we are currently syncing. + + :returns: ``True`` if :attr:`sync_lock` cannot be aquired, ``False`` otherwise. + :rtype: bool + """ + + idle = self.sync_lock.acquire(blocking=False) + if idle: + self.sync_lock.release() + + return not idle + # ==== Upload sync =================================================================== def upload_local_changes_while_inactive(self): @@ -1057,22 +1098,24 @@ def upload_local_changes_while_inactive(self): Call this method when resuming sync. """ - logger.info('Indexing local changes...') + with self.sync_lock: - try: - events, local_cursor = self._get_local_changes_while_inactive() - logger.debug('Retrieved local changes:\n%s', pprint.pformat(events)) - events = self._clean_local_events(events) - except FileNotFoundError: - self.ensure_dropbox_folder_present() - return + logger.info('Indexing local changes...') - if len(events) > 0: - self.apply_local_changes(events, local_cursor) - logger.debug('Uploaded local changes while inactive') - else: - self.last_sync = local_cursor - logger.debug('No local changes while inactive') + try: + events, local_cursor = self._get_local_changes_while_inactive() + logger.debug('Retrieved local changes:\n%s', pprint.pformat(events)) + events = self._clean_local_events(events) + except FileNotFoundError: + self.ensure_dropbox_folder_present() + return + + if len(events) > 0: + self.apply_local_changes(events, local_cursor) + logger.debug('Uploaded local changes while inactive') + else: + self.last_sync = local_cursor + logger.debug('No local changes while inactive') def _get_local_changes_while_inactive(self): @@ -1173,6 +1216,82 @@ def wait_for_local_changes(self, timeout=5, delay=1): return self._clean_local_events(events), local_cursor + def apply_local_changes(self, events, local_cursor): + """ + Applies locally detected changes to the remote Dropbox. + + :param iterable events: List of local file system events. + :param float local_cursor: Time stamp of last event in ``events``. + """ + + with self.sync_lock: + + events, _ = self._filter_excluded_changes_local(events) + + sorted_events = dict(deleted=[], dir_created=[], dir_moved=[], file=[]) + for e in events: + if e.event_type == EVENT_TYPE_DELETED: + sorted_events['deleted'].append(e) + elif e.is_directory and e.event_type == EVENT_TYPE_CREATED: + sorted_events['dir_created'].append(e) + elif e.is_directory and e.event_type == EVENT_TYPE_MOVED: + sorted_events['dir_moved'].append(e) + elif not e.is_directory and e.event_type != EVENT_TYPE_DELETED: + sorted_events['file'].append(e) + + # update queues + for e in itertools.chain(*sorted_events.values()): + self.queued_for_upload.put(get_dest_path(e)) + + success = [] + + # apply deleted events first, folder moved events second + # neither event type requires an actual upload + if sorted_events['deleted']: + logger.info('Uploading deletions...') + + last_emit = time.time() + with ThreadPoolExecutor(max_workers=self._num_threads, + thread_name_prefix='maestral-upload-pool') as executor: + fs = (executor.submit(self._create_remote_entry, e) + for e in sorted_events['deleted']) + n_files = len(sorted_events['deleted']) + for f, n in zip(as_completed(fs), range(1, n_files + 1)): + if time.time() - last_emit > 1 or n in (1, n_files): + # emit message at maximum every second + logger.info(f'Deleting {n}/{n_files}...') + last_emit = time.time() + success.append(f.result()) + + if sorted_events['dir_moved']: + logger.info('Moving folders...') + + for event in sorted_events['dir_moved']: + logger.info(f'Moving {event.src_path}...') + res = self._create_remote_entry(event) + success.append(res) + + # apply file created events in parallel since order does not matter + last_emit = time.time() + with ThreadPoolExecutor(max_workers=self._num_threads, + thread_name_prefix='maestral-upload-pool') as executor: + fs = (executor.submit(self._create_remote_entry, e) for e in + itertools.chain(sorted_events['dir_created'], sorted_events['file'])) + n_files = len(sorted_events['dir_created']) + len(sorted_events['file']) + for f, n in zip(as_completed(fs), range(1, n_files + 1)): + if time.time() - last_emit > 1 or n in (1, n_files): + # emit message at maximum every second + logger.info(f'Uploading {n}/{n_files}...') + last_emit = time.time() + success.append(f.result()) + + # bookkeeping + + if all(success): + self.last_sync = local_cursor + + self._clean_and_save_rev_file() + def _filter_excluded_changes_local(self, events): """ Checks for and removes file events referring to items which are excluded from @@ -1432,8 +1551,8 @@ def _handle_case_conflict(self, event): def _handle_selective_sync_conflict(self, event): """ - Checks for other items in the same directory with same name but a different - case. Only needed for case sensitive file systems. + Checks for items in the local directory with same path as an item which is + excluded by selective sync. Renames items if necessary. :param FileSystemEvent event: Created or moved event. :returns: ``True`` or ``False``. @@ -1464,82 +1583,8 @@ def _handle_selective_sync_conflict(self, event): else: return False - def apply_local_changes(self, events, local_cursor): - """ - Applies locally detected changes to the remote Dropbox. - - :param iterable events: List of local file system events. - :param float local_cursor: Time stamp of last event in ``events``. - """ - - events, _ = self._filter_excluded_changes_local(events) - - sorted_events = dict(deleted=[], dir_created=[], dir_moved=[], file=[]) - for e in events: - if e.event_type == EVENT_TYPE_DELETED: - sorted_events['deleted'].append(e) - elif e.is_directory and e.event_type == EVENT_TYPE_CREATED: - sorted_events['dir_created'].append(e) - elif e.is_directory and e.event_type == EVENT_TYPE_MOVED: - sorted_events['dir_moved'].append(e) - elif not e.is_directory and e.event_type != EVENT_TYPE_DELETED: - sorted_events['file'].append(e) - - # update queues - for e in itertools.chain(*sorted_events.values()): - self.queued_for_upload.put(get_dest_path(e)) - - success = [] - - # apply deleted events first, folder moved events second - # neither event type requires an actual upload - if sorted_events['deleted']: - logger.info('Uploading deletions...') - - last_emit = time.time() - with ThreadPoolExecutor(max_workers=self._num_threads, - thread_name_prefix='maestral-upload-pool') as executor: - fs = (executor.submit(self.create_remote_entry, e) - for e in sorted_events['deleted']) - n_files = len(sorted_events['deleted']) - for f, n in zip(as_completed(fs), range(1, n_files + 1)): - if time.time() - last_emit > 1 or n in (1, n_files): - # emit message at maximum every second - logger.info(f'Deleting {n}/{n_files}...') - last_emit = time.time() - success.append(f.result()) - - if sorted_events['dir_moved']: - logger.info('Moving folders...') - - for event in sorted_events['dir_moved']: - logger.info(f'Moving {event.src_path}...') - res = self.create_remote_entry(event) - success.append(res) - - # apply file created events in parallel since order does not matter - last_emit = time.time() - with ThreadPoolExecutor(max_workers=self._num_threads, - thread_name_prefix='maestral-upload-pool') as executor: - fs = (executor.submit(self.create_remote_entry, e) for e in - itertools.chain(sorted_events['dir_created'], sorted_events['file'])) - n_files = len(sorted_events['dir_created']) + len(sorted_events['file']) - for f, n in zip(as_completed(fs), range(1, n_files + 1)): - if time.time() - last_emit > 1 or n in (1, n_files): - # emit message at maximum every second - logger.info(f'Uploading {n}/{n_files}...') - last_emit = time.time() - success.append(f.result()) - - # bookkeeping - - if all(success): - self.last_sync = local_cursor - - self._clean_and_save_rev_file() - @catch_sync_issues(download=False) - def create_remote_entry(self, event): + def _create_remote_entry(self, event): """ Applies a local file system event to the remote Dropbox and clears any existing sync errors belonging to that path. Any :class:`errors.MaestralApiError` will be @@ -1927,42 +1972,45 @@ def get_remote_folder(self, dbx_path='/', ignore_excluded=True): :rtype: bool """ - dbx_path = dbx_path or '/' - is_dbx_root = dbx_path == '/' - success = [] + with self.sync_lock: - if is_dbx_root: - logger.info('Downloading your Dropbox') - else: - logger.info('Downloading %s', dbx_path) + dbx_path = dbx_path or '/' + is_dbx_root = dbx_path == '/' + success = [] - if not any(is_child(folder, dbx_path) for folder in self.excluded_items): - # if there are no excluded subfolders, index and download all at once - ignore_excluded = False + if is_dbx_root: + logger.info('Downloading your Dropbox') + else: + logger.info('Downloading %s', dbx_path) - # get a cursor for the folder - cursor = self.client.get_latest_cursor(dbx_path) + if not any(is_child(folder, dbx_path) for folder in self.excluded_items): + # if there are no excluded subfolders, index and download all at once + ignore_excluded = False - root_result = self.client.list_folder(dbx_path, recursive=(not ignore_excluded), - include_deleted=False) + # get a cursor for the folder + cursor = self.client.get_latest_cursor(dbx_path) - # download top-level folders / files first - logger.info(SYNCING) - _, s = self.apply_remote_changes(root_result, save_cursor=False) - success.append(s) + root_result = self.client.list_folder(dbx_path, + recursive=(not ignore_excluded), + include_deleted=False) - if ignore_excluded: - # download sub-folders if not excluded - for entry in root_result.entries: - if isinstance(entry, FolderMetadata) and not self.is_excluded_by_user( - entry.path_display): - success.append(self.get_remote_folder(entry.path_display)) + # download top-level folders / files first + logger.info(SYNCING) + _, s = self.apply_remote_changes(root_result, save_cursor=False) + success.append(s) - if is_dbx_root: - self.last_cursor = cursor - self.last_reindex = time.time() + if ignore_excluded: + # download sub-folders if not excluded + for entry in root_result.entries: + if isinstance(entry, FolderMetadata) and not self.is_excluded_by_user( + entry.path_display): + success.append(self.get_remote_folder(entry.path_display)) - return all(success) + if is_dbx_root: + self.last_cursor = cursor + self.last_reindex = time.time() + + return all(success) def get_remote_item(self, dbx_path): """ @@ -1972,28 +2020,31 @@ def get_remote_item(self, dbx_path): so that they will be resumed in case Maestral is terminated during the download. If ``dbx_path`` refers to a folder, the download will be handled by :meth:`get_remote_folder`. If it refers to a single file, the download will be - performed by :meth:`create_local_entry`. + performed by :meth:`_create_local_entry`. This method can be used to fetch individual items outside of the regular sync - cycle, for instance when including a new file or folder. + cycle, for instance when including a previously excluded file or folder. :param str dbx_path: Path relative to Dropbox folder. :returns: ``True`` on success, ``False`` otherwise. :rtype: bool """ - self.pending_downloads.add(dbx_path) - md = self.client.get_metadata(dbx_path, include_deleted=True) - if isinstance(md, FolderMetadata): - res = self.get_remote_folder(dbx_path) - else: # FileMetadata or DeletedMetadata - with InQueue(self.queue_downloading, md.path_display): - res = self.create_local_entry(md) + with self.sync_lock: - if res is True: - self.pending_downloads.discard(dbx_path) + self.pending_downloads.add(dbx_path) + md = self.client.get_metadata(dbx_path, include_deleted=True) - return res + if isinstance(md, FolderMetadata): + res = self.get_remote_folder(dbx_path) + elif md: # FileMetadata or DeletedMetadata + with InQueue(self.queue_downloading, md.path_display): + res = self._create_local_entry(md) + + if res is True: + self.pending_downloads.discard(dbx_path) + + return res def wait_for_remote_changes(self, last_cursor, timeout=40, delay=2): """ @@ -2026,29 +2077,6 @@ def list_remote_changes(self, last_cursor): logger.debug('Cleaned remote changes:\n%s', entries_to_str(clean_changes.entries)) return clean_changes - def _filter_excluded_changes_remote(self, changes): - """Removes all excluded items from the given list of changes. - - :param changes: :class:`dropbox.files.ListFolderResult` instance. - :returns: (``changes_filtered``, ``changes_discarded``) - :rtype: tuple[:class:`dropbox.files.ListFolderResult`] - """ - entries_filtered = [] - entries_discarded = [] - - for e in changes.entries: - if self.is_excluded_by_user(e.path_lower) or self.is_excluded(e.path_lower): - entries_discarded.append(e) - else: - entries_filtered.append(e) - - changes_filtered = dropbox.files.ListFolderResult( - entries=entries_filtered, cursor=changes.cursor, has_more=False) - changes_discarded = dropbox.files.ListFolderResult( - entries=entries_discarded, cursor=changes.cursor, has_more=False) - - return changes_filtered, changes_discarded - def apply_remote_changes(self, changes, save_cursor=True): """ Applies remote changes to local folder. Call this on the result of @@ -2056,11 +2084,10 @@ def apply_remote_changes(self, changes, save_cursor=True): has been successfully applied. Entries in the local index are created after successful completion. - :param changes: :class:`dropbox.files.ListFolderResult` instance or ``False`` if - requests failed. - :param bool save_cursor: If True, :attr:`last_cursor` will be updated from the - last applied changes. Take care to only save a cursors which represent the - state of the entire Dropbox + :param changes: :class:`dropbox.files.ListFolderResult` instance. + :param bool save_cursor: If ``True``, :attr:`last_cursor` will be updated from + the last applied changes. Take care to only save a cursors which represent + the state of the entire Dropbox. :returns: List of changes that were made to local files and bool indicating if all download syncs were successful. :rtype: (list, bool) @@ -2097,14 +2124,14 @@ def apply_remote_changes(self, changes, save_cursor=True): if deleted: logger.info('Applying deletions...') for item in deleted: - res = self.create_local_entry(item) + res = self._create_local_entry(item) downloaded.append(res) # create local folders, start with top-level and work your way down if folders: logger.info('Creating folders...') for folder in folders: - res = self.create_local_entry(folder) + res = self._create_local_entry(folder) downloaded.append(res) # apply created files @@ -2112,7 +2139,7 @@ def apply_remote_changes(self, changes, save_cursor=True): last_emit = time.time() with ThreadPoolExecutor(max_workers=self._num_threads, thread_name_prefix='maestral-download-pool') as executor: - fs = (executor.submit(self.create_local_entry, file) for file in files) + fs = (executor.submit(self._create_local_entry, file) for file in files) for f, n in zip(as_completed(fs), range(1, n_files + 1)): if time.time() - last_emit > 1 or n in (1, n_files): # emit messages at maximum every second @@ -2129,7 +2156,90 @@ def apply_remote_changes(self, changes, save_cursor=True): return [entry for entry in downloaded if isinstance(entry, Metadata)], success - def check_download_conflict(self, md): + def notify_user(self, changes): + """ + Sends system notification for file changes. + + :param list changes: List of Dropbox metadata which has been applied locally. + """ + + # get number of remote changes + n_changed = len(changes) + + if n_changed == 0: + return + + user_name = None + change_type = 'changed' + + # find out who changed the item(s), get the user name if its only a single user + dbid_list = set(self._get_modified_by_dbid(md) for md in changes) + if len(dbid_list) == 1: + # all files have been modified by the same user + dbid = dbid_list.pop() + if dbid == self._conf.get('account', 'account_id'): + user_name = 'You' + else: + account_info = self.client.get_account_info(dbid) + user_name = account_info.name.display_name + + if n_changed == 1: + # display user name, file name, and type of change + md = changes[0] + file_name = osp.basename(md.path_display) + + if isinstance(md, DeletedMetadata): + change_type = 'removed' + elif isinstance(md, FileMetadata): + revs = self.client.list_revisions(md.path_lower, limit=2) + is_new_file = len(revs.entries) == 1 + change_type = 'added' if is_new_file else 'changed' + elif isinstance(md, FolderMetadata): + change_type = 'added' + + else: + # display user name if unique, number of files, and type of change + file_name = f'{n_changed} items' + + if all(isinstance(x, DeletedMetadata) for x in changes): + change_type = 'removed' + elif all(isinstance(x, FolderMetadata) for x in changes): + change_type = 'added' + file_name = f'{n_changed} folders' + elif all(isinstance(x, FileMetadata) for x in changes): + file_name = f'{n_changed} files' + + if user_name: + msg = f'{user_name} {change_type} {file_name}' + else: + msg = f'{file_name} {change_type}' + + self._notifier.notify(msg, level=FILECHANGE) + + def _filter_excluded_changes_remote(self, changes): + """Removes all excluded items from the given list of changes. + + :param changes: :class:`dropbox.files.ListFolderResult` instance. + :returns: (``changes_filtered``, ``changes_discarded``) + :rtype: tuple[:class:`dropbox.files.ListFolderResult`] + """ + entries_filtered = [] + entries_discarded = [] + + for e in changes.entries: + if self.is_excluded_by_user(e.path_lower) or self.is_excluded(e.path_lower): + entries_discarded.append(e) + else: + entries_filtered.append(e) + + changes_filtered = dropbox.files.ListFolderResult( + entries=entries_filtered, cursor=changes.cursor, has_more=False) + changes_discarded = dropbox.files.ListFolderResult( + entries=entries_discarded, cursor=changes.cursor, has_more=False) + + return changes_filtered, changes_discarded + + def _check_download_conflict(self, md): """ Check if a local item is conflicting with remote change. The equivalent check when uploading and a change will be carried out by Dropbox itself. @@ -2182,7 +2292,7 @@ def check_download_conflict(self, md): logger.debug('Equal content hashes for "%s": no conflict', dbx_path) self.set_local_rev(dbx_path, remote_rev) return Conflict.Identical - elif self.get_ctime(local_path) <= self.get_last_sync_for_path(dbx_path): + elif self._get_ctime(local_path) <= self.get_last_sync_for_path(dbx_path): logger.debug('Ctime is older than last sync for "%s": remote item ' 'is newer', dbx_path) return Conflict.RemoteNewer @@ -2194,10 +2304,10 @@ def check_download_conflict(self, md): logger.debug('Ctime is newer than last sync for "%s": conflict', dbx_path) return Conflict.Conflict - def get_ctime(self, local_path, ignore_excluded=True): + def _get_ctime(self, local_path, ignore_excluded=True): """ Returns the ctime of a local item or -1.0 if there is nothing at the path. If - the item is a directory, return the largest ctime of itself and its children. + the item is a directory, return the largest ctime of it and its children. :param str local_path: Absolute path on local drive. :param bool ignore_excluded: If ``True``, the ctimes of children for which @@ -2222,66 +2332,6 @@ def get_ctime(self, local_path, ignore_excluded=True): except FileNotFoundError: return -1.0 - def notify_user(self, changes): - """ - Sends system notification for file changes. - - :param list changes: List of Dropbox metadata which has been applied locally. - """ - - # get number of remote changes - n_changed = len(changes) - - if n_changed == 0: - return - - user_name = None - change_type = 'changed' - - # find out who changed the item(s), get the user name if its only a single user - dbid_list = set(self._get_modified_by_dbid(md) for md in changes) - if len(dbid_list) == 1: - # all files have been modified by the same user - dbid = dbid_list.pop() - if dbid == self._conf.get('account', 'account_id'): - user_name = 'You' - else: - account_info = self.client.get_account_info(dbid) - user_name = account_info.name.display_name - - if n_changed == 1: - # display user name, file name, and type of change - md = changes[0] - file_name = os.path.basename(md.path_display) - - if isinstance(md, DeletedMetadata): - change_type = 'removed' - elif isinstance(md, FileMetadata): - revs = self.client.list_revisions(md.path_lower, limit=2) - is_new_file = len(revs.entries) == 1 - change_type = 'added' if is_new_file else 'changed' - elif isinstance(md, FolderMetadata): - change_type = 'added' - - else: - # display user name if unique, number of files, and type of change - file_name = f'{n_changed} items' - - if all(isinstance(x, DeletedMetadata) for x in changes): - change_type = 'removed' - elif all(isinstance(x, FolderMetadata) for x in changes): - change_type = 'added' - file_name = f'{n_changed} folders' - elif all(isinstance(x, FileMetadata) for x in changes): - file_name = f'{n_changed} files' - - if user_name: - msg = f'{user_name} {change_type} {file_name}' - else: - msg = f'{file_name} {change_type}' - - self._notifier.notify(msg, level=FILECHANGE) - def _get_modified_by_dbid(self, md): """ Returns the Dropbox ID of the user who modified a shared item or our own ID if the @@ -2377,7 +2427,7 @@ def _clean_remote_changes(self, changes): return changes @catch_sync_issues(download=True) - def create_local_entry(self, entry): + def _create_local_entry(self, entry): """ Applies a file change from Dropbox servers to the local Dropbox folder. Any :class:`errors.MaestralApiError` will be caught and logged as appropriate. @@ -2404,7 +2454,7 @@ def create_local_entry(self, entry): with InQueue(self.queue_downloading, entry.path_display): - conflict_check = self.check_download_conflict(entry) + conflict_check = self._check_download_conflict(entry) applied = None @@ -2419,11 +2469,17 @@ def create_local_entry(self, entry): # we download to a temporary file first (this may take some time) tmp_fname = self._new_tmp_file() - md = self.client.download(f'rev:{entry.rev}', tmp_fname) + + try: + md = self.client.download(f'rev:{entry.rev}', tmp_fname) + except MaestralApiError as err: + # replace rev number with path + err.dbx_path = entry.path_display + raise err # re-check for conflict and move the conflict # out of the way if anything has changed - if self.check_download_conflict(entry) == Conflict.Conflict: + if self._check_download_conflict(entry) == Conflict.Conflict: new_local_path = generate_cc_name(local_path) event_cls = DirMovedEvent if osp.isdir(local_path) else FileMovedEvent with self.fs_events.ignore(event_cls(local_path, new_local_path)): @@ -2448,7 +2504,7 @@ def create_local_entry(self, entry): raise os_to_maestral_error(exc, dbx_path=entry.path_display, local_path=local_path) - self.set_last_sync_for_path(entry.path_lower, self.get_ctime(local_path)) + self.set_last_sync_for_path(entry.path_lower, self._get_ctime(local_path)) self.set_local_rev(entry.path_lower, md.rev) logger.debug('Created local file "%s"', entry.path_display) @@ -2486,7 +2542,7 @@ def create_local_entry(self, entry): raise os_to_maestral_error(exc, dbx_path=entry.path_display, local_path=local_path) - self.set_last_sync_for_path(entry.path_lower, self.get_ctime(local_path)) + self.set_last_sync_for_path(entry.path_lower, self._get_ctime(local_path)) self.set_local_rev(entry.path_lower, 'folder') logger.debug('Created local folder "%s"', entry.path_display) @@ -2549,7 +2605,7 @@ def helper(mm): and syncing has not been paused by the user. 3) Triggers weekly reindexing. - :param MaestralMonitor mm: MaestralMonitor instance. + :param SyncMonitor mm: MaestralMonitor instance. """ while mm.running.is_set(): @@ -2577,7 +2633,7 @@ def download_worker(sync, syncing, running, connected): """ Worker to sync changes of remote Dropbox with local folder. - :param UpDownSync sync: Instance of :class:`UpDownSync`. + :param SyncEngine sync: Instance of :class:`SyncEngine`. :param Event syncing: Event that indicates if workers are running or paused. :param Event running: Event to shutdown local file event handler and worker threads. :param Event connected: Event that indicates if we can connect to Dropbox. @@ -2590,11 +2646,12 @@ def download_worker(sync, syncing, running, connected): try: has_changes = sync.wait_for_remote_changes(sync.last_cursor) - if not (running.is_set() and syncing.is_set()): - continue + with sync.sync_lock: + + if not (running.is_set() and syncing.is_set()): + continue - if has_changes: - with sync.lock: + if has_changes: logger.info(SYNCING) changes = sync.list_remote_changes(sync.last_cursor) @@ -2603,9 +2660,9 @@ def download_worker(sync, syncing, running, connected): logger.info(IDLE) - sync.client.get_space_usage() + sync.client.get_space_usage() - gc.collect() + gc.collect() except ConnectionError: syncing.clear() @@ -2622,7 +2679,7 @@ def download_worker_added_item(sync, syncing, running, connected): """ Worker to download items which have been newly included in sync. - :param UpDownSync sync: Instance of :class:`UpDownSync`. + :param SyncEngine sync: Instance of :class:`SyncEngine`. :param Event syncing: Event that indicates if workers are running or paused. :param Event running: Event to shutdown local file event handler and worker threads. :param Event connected: Event that indicates if we can connect to Dropbox. @@ -2632,17 +2689,19 @@ def download_worker_added_item(sync, syncing, running, connected): syncing.wait() - dbx_path = sync.queued_newly_included_downloads.get() + try: + dbx_path = sync.queued_newly_included_downloads.get() - if not (running.is_set() and syncing.is_set()): - sync.pending_downloads.add(dbx_path) - continue + with sync.sync_lock: + if not (running.is_set() and syncing.is_set()): + sync.pending_downloads.add(dbx_path) + continue - try: - with sync.lock: sync.get_remote_item(dbx_path) + logger.info(IDLE) + gc.collect() - logger.info(IDLE) + except ConnectionError: syncing.clear() connected.clear() @@ -2658,7 +2717,7 @@ def upload_worker(sync, syncing, running, connected): """ Worker to sync local changes to remote Dropbox. - :param UpDownSync sync: Instance of :class:`UpDownSync`. + :param SyncEngine sync: Instance of :class:`SyncEngine`. :param Event syncing: Event that indicates if workers are running or paused. :param Event running: Event to shutdown local file event handler and worker threads. :param Event connected: Event that indicates if we can connect to Dropbox. @@ -2671,18 +2730,18 @@ def upload_worker(sync, syncing, running, connected): try: events, local_cursor = sync.wait_for_local_changes() - if not (running.is_set() and syncing.is_set()): - continue + with sync.sync_lock: + if not (running.is_set() and syncing.is_set()): + continue - if len(events) > 0: - with sync.lock: + if len(events) > 0: logger.info(SYNCING) sync.apply_local_changes(events, local_cursor) logger.info(IDLE) - gc.collect() - else: - sync.last_sync = local_cursor + gc.collect() + else: + sync.last_sync = local_cursor except ConnectionError: syncing.clear() @@ -2699,7 +2758,7 @@ def startup_worker(sync, syncing, running, connected, startup, paused_by_user): """ Worker to sync local changes to remote Dropbox. - :param UpDownSync sync: Instance of :class:`UpDownSync`. + :param SyncEngine sync: Instance of :class:`SyncEngine`. :param Event syncing: Event that indicates if workers are running or paused. :param Event running: Event to shutdown local file event handler and worker threads. :param Event connected: Event that indicates if we can connect to Dropbox. @@ -2712,7 +2771,7 @@ def startup_worker(sync, syncing, running, connected, startup, paused_by_user): startup.wait() try: - with sync.lock: + with sync.sync_lock: # run / resume initial download # local changes during this download will be registered # by the local FileSystemObserver but only uploaded after @@ -2753,12 +2812,12 @@ def startup_worker(sync, syncing, running, connected, startup, paused_by_user): gc.collect() - if not paused_by_user.is_set(): - syncing.set() + if not paused_by_user.is_set(): + syncing.set() - startup.clear() + startup.clear() - logger.info(IDLE) + logger.info(IDLE) except ConnectionError: syncing.clear() @@ -2778,15 +2837,15 @@ def startup_worker(sync, syncing, running, connected, startup, paused_by_user): # Main Monitor class to start, stop and coordinate threads # ======================================================================================== -class MaestralMonitor: +class SyncMonitor: """ - Class to sync changes between Dropbox and a local folder. It creates four - threads: `observer` to catch local file events, `upload_thread` to upload - caught changes to Dropbox, `download_thread` to query for and download - remote changes, and `connection_thread` which periodically checks the - connection to Dropbox servers. + Class to sync changes between Dropbox and a local folder. It creates five threads: + `observer` to retrieve local file system events, `startup_thread` to carry out any + startup jobs such as initial syncs, `upload_thread` to upload local changes to + Dropbox, `download_thread` to query for and download remote changes, and + `helper_thread` which periodically checks the connection to Dropbox servers. - :param MaestralApiClient client: The Dropbox API client, a wrapper around the Dropbox + :param DropboxClient client: The Dropbox API client, a wrapper around the Dropbox Python SDK. """ @@ -2796,7 +2855,7 @@ def __init__(self, client): self.client = client self.config_name = self.client.config_name - self.sync = UpDownSync(self.client) + self.sync = SyncEngine(self.client) self.connected = Event() self.syncing = Event() @@ -2975,8 +3034,9 @@ def idle_time(self): return 0.0 def reset_sync_state(self): + """Resets all saved sync state.""" - if self.syncing.is_set() or self.startup.is_set() or self.sync.lock.locked(): + if self.syncing.is_set() or self.startup.is_set() or self.sync.busy(): raise RuntimeError('Cannot reset sync state while syncing.') self.sync.last_cursor = '' @@ -3008,8 +3068,9 @@ def rebuild_index(self): self.resume() def _wait_for_idle(self): - self.sync.lock.acquire() - self.sync.lock.release() + + self.sync.sync_lock.acquire() + self.sync.sync_lock.release() def _threads_alive(self): """Returns ``True`` if all threads are alive, ``False`` otherwise.""" diff --git a/maestral/utils/autostart.py b/maestral/utils/autostart.py index 37879286a..57fadf8f0 100644 --- a/maestral/utils/autostart.py +++ b/maestral/utils/autostart.py @@ -4,8 +4,12 @@ (c) Sam Schott; This work is licensed under the MIT licence. -This module handles starting for Maestral on user login and supports multiple backends, -depending on the platform and if we want to start the daemon or GUI. +This module handles starting Maestral on user login and supports multiple platform +specific backends such as launchd or systemd. Additionally, this module also provides +support for GUIs via launchd or xdg-desktop entries by passing the ``gui`` option to the +``maestral`` command or executable. Therefore, only GUIs which are explicitly supported by +the CLI with the `maestral gui` command or frozen executables which provide their own GUI +are supported. """ @@ -216,7 +220,8 @@ def __init__(self, config_name, gui): with open(osp.join(_resources, 'com.samschott.maestral.plist')) as f: plist_template = f.read() - self.destination = osp.join(get_home_dir(), 'Library', 'LaunchAgents', filename) + self.path = osp.join(get_home_dir(), 'Library', 'LaunchAgents') + self.destination = osp.join(self.path, filename) arguments = [f'\t\t{arg}' for arg in self.start_cmd] @@ -226,6 +231,8 @@ def __init__(self, config_name, gui): ) def _enable(self): + os.makedirs(self.path, exist_ok=True) + with open(self.destination, 'w+') as f: f.write(self.contents) diff --git a/maestral/utils/notify.py b/maestral/utils/notify.py index 9cec79c38..a69cd9b23 100644 --- a/maestral/utils/notify.py +++ b/maestral/utils/notify.py @@ -27,6 +27,7 @@ import pkg_resources import logging from collections import deque +import threading # external imports import click @@ -359,7 +360,8 @@ class MaestralDesktopNotifier(logging.Handler): ``notify_level`` will be applied in addition to the log level. """ - _instances = {} + _instances = dict() + _lock = threading.Lock() @classmethod def for_config(cls, config_name): @@ -370,12 +372,13 @@ def for_config(cls, config_name): :param str config_name: Name of maestral config. """ - if config_name in cls._instances: - return cls._instances[config_name] - else: - instance = cls(config_name) - cls._instances[config_name] = instance - return instance + with cls._lock: + try: + return cls._instances[config_name] + except KeyError: + instance = cls(config_name) + cls._instances[config_name] = instance + return instance def __init__(self, config_name): super().__init__() diff --git a/maestral/utils/path.py b/maestral/utils/path.py index 973675eb1..2f7019429 100644 --- a/maestral/utils/path.py +++ b/maestral/utils/path.py @@ -39,8 +39,8 @@ def is_equal_or_child(path, parent): :param str path: Item path. :param str parent: Parent path. - :returns: ``True`` if ``path`` semantically lies inside ``parent``, - ``False`` otherwise (including ``path == parent``). + :returns: ``True`` if ``path`` semantically lies inside ``parent`` or + ``path == parent``. :rtype: bool """ diff --git a/package/bin/maestral_cli b/package/bin/maestral_cli index 46885bf7b..49de7f1b4 100755 --- a/package/bin/maestral_cli +++ b/package/bin/maestral_cli @@ -2,9 +2,4 @@ SCRIPTPATH="$(dirname "$(readlink "$0")")" -if [ "$1" = "gui" ] -then - open -n -a Maestral.app --args "$@" -else - "$SCRIPTPATH/main" --cli "$@" -fi +"$SCRIPTPATH/main" --cli "$@" diff --git a/package/build_linux.sh b/package/build-linux.sh similarity index 86% rename from package/build_linux.sh rename to package/build-linux.sh index cec18d574..3c113e66d 100755 --- a/package/build_linux.sh +++ b/package/build-linux.sh @@ -6,8 +6,8 @@ echo "**** INSTALLING DEPENDENCIES ****************************" pip install -U pyinstaller -git clone https://github.com/samschott/maestral-dropbox build/maestral-dropbox -cd build/maestral-dropbox +git clone https://github.com/samschott/maestral build/maestral +cd build/maestral git checkout develop git pull pip install . diff --git a/package/build-macos.sh b/package/build-macos.sh new file mode 100755 index 000000000..71bc48cc4 --- /dev/null +++ b/package/build-macos.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash + +SPEC_FILE=maestral_macos.spec +BUILD_NO=$(grep -E -o "[0-9]*" bundle_version_macos.txt) + +export MACOSX_DEPLOYMENT_TARGET=10.13 +export CFLAGS=-mmacosx-version-min=10.13 +export CPPFLAGS=-mmacosx-version-min=10.13 +export LDFLAGS=-mmacosx-version-min=10.13 +export LINKFLAGS=-mmacosx-version-min=10.13 + +echo "**** INSTALLING DEPENDENCIES ***************************" + +git clone https://github.com/pyinstaller/pyinstaller.git build/pyinstaller +cd build/pyinstaller +git checkout master +git pull +cd bootloader +python3 ./waf all +cd .. +pip3 install . +cd ../.. + +git clone https://github.com/samschott/maestral build/maestral +cd build/maestral +git checkout develop +git pull +pip3 install . +cd ../.. + +git clone https://github.com/samschott/maestral-cocoa build/maestral-cocoa +cd build/maestral-cocoa +git checkout develop +git pull +pip3 install . +cd ../.. + +echo "**** BUILD NUMBER $BUILD_NO ****************************" + +python3 -m PyInstaller -y --clean -w $SPEC_FILE + +echo "**** COPY CLI ENTRY POINT ******************************" + +cp bin/maestral_cli dist/Maestral.app/Contents/MacOS/maestral_cli + +echo "**** SIGNING APP ***************************************" + +echo "removing xattr" +xattr -cr dist/Maestral.app + +echo "signing app" +codesign -s "Developer ID Application: Sam Schott" \ + --entitlements entitlements.plist --deep -o runtime dist/Maestral.app + +echo "**** CREATING DMG **************************************" + +test -f dist/Maestral.dmg && rm dist/Maestral.dmg + +create-dmg \ + --volname "Maestral" \ + --window-size 300 150 \ + --icon-size 64 \ + --text-size 11 \ + --icon "Maestral.app" 75 75 \ + --app-drop-link 225 75 \ + "dist/Maestral.dmg" \ + "dist/Maestral.app" + +echo "signing dmg" +codesign --verify --sign "Developer ID Application: Sam Schott" dist/Maestral.dmg + +echo "**** NOTARISING DMG ************************************" + +./macos-notarize-dmg.sh dist/Maestral.dmg + +echo "**** DONE **********************************************" diff --git a/package/build_macos_cocoa.sh b/package/build_macos_cocoa.sh deleted file mode 100755 index 462bdace0..000000000 --- a/package/build_macos_cocoa.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env bash - -SPEC_FILE=maestral_macos_cocoa.spec -BUILD_NO=$(grep -E -o "[0-9]*" bundle_version_macos.txt) - -echo "**** INSTALLING DEPENDENCIES ****************************" - -git clone https://github.com/pyinstaller/pyinstaller.git build/pyinstaller -cd build/pyinstaller -git checkout master -git pull -cd bootloader -export MACOSX_DEPLOYMENT_TARGET=10.13 -export CFLAGS=-mmacosx-version-min=10.13 -export CPPFLAGS=-mmacosx-version-min=10.13 -export LDFLAGS=-mmacosx-version-min=10.13 -export LINKFLAGS=-mmacosx-version-min=10.13 -python ./waf all -cd .. -pip install . -cd ../.. - -git clone https://github.com/samschott/maestral-dropbox build/maestral-dropbox -cd build/maestral-dropbox -git checkout develop -git pull -pip install . -cd ../.. - -git clone https://github.com/samschott/maestral-cocoa build/maestral-cocoa -cd build/maestral-cocoa -git checkout develop -git pull -pip install . -cd ../.. - -echo "**** BUILD NUMBER $BUILD_NO ****************************" - -python3 -m PyInstaller -y --clean -w $SPEC_FILE - -echo "**** COPY ENTRY POINT **********************************" - -cp bin/maestral_cli dist/Maestral.app/Contents/MacOS/maestral_cli - -echo "**** RUNNING POST-BUILD SCRIPTS ************************" - -# pass - -echo "**** SIGNING ******************************************" - -codesign -s "Apple Development: sam.schott@outlook.com (FJNXBRUVWL)" --deep dist/Maestral.app - -echo "**** DONE *********************************************" diff --git a/package/build_macos_qt.sh b/package/build_macos_qt.sh deleted file mode 100755 index 1b7ede3c2..000000000 --- a/package/build_macos_qt.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env bash - -SPEC_FILE=maestral_macos_qt.spec -BUILD_NO=$(grep -E -o "[0-9]*" bundle_version_macos.txt) - -echo "**** INSTALLING DEPENDENCIES ****************************" - -git clone https://github.com/pyinstaller/pyinstaller.git build/pyinstaller -cd build/pyinstaller -git checkout master -git pull -cd bootloader -export MACOSX_DEPLOYMENT_TARGET=10.13 -export CFLAGS=-mmacosx-version-min=10.13 -export CPPFLAGS=-mmacosx-version-min=10.13 -export LDFLAGS=-mmacosx-version-min=10.13 -export LINKFLAGS=-mmacosx-version-min=10.13 -python ./waf all -cd .. -pip install . -cd ../.. - -git clone https://github.com/samschott/maestral-dropbox build/maestral-dropbox -cd build/maestral-dropbox -git checkout develop -git pull -pip install . -cd ../.. - -git clone https://github.com/samschott/maestral-cocoa build/maestral-cocoa -cd build/maestral-cocoa -git checkout develop -git pull -pip install . -cd ../.. - -echo "**** BUILD NUMBER $BUILD_NO ****************************" - -python3 -m PyInstaller -y --clean -w $SPEC_FILE - -echo "**** COPY ENTRY POINT **********************************" - -cp bin/maestral_cli dist/Maestral.app/Contents/MacOS/maestral_cli - -echo "**** RUNNING POST-BUILD SCRIPTS ************************" - -python3 post_build_macos_qt.py - -echo "**** SIGNING ******************************************" - -codesign -s "Apple Development: sam.schott@outlook.com (FJNXBRUVWL)" --deep dist/Maestral.app - -echo "**** DONE *********************************************" diff --git a/package/entitlements.plist b/package/entitlements.plist new file mode 100644 index 000000000..a1c430a57 --- /dev/null +++ b/package/entitlements.plist @@ -0,0 +1,8 @@ + + + + + com.apple.security.cs.allow-unsigned-executable-memory + + + diff --git a/package/macos-notarize-app.sh b/package/macos-notarize-app.sh new file mode 100755 index 000000000..abf027e74 --- /dev/null +++ b/package/macos-notarize-app.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash + +if [ -z "$1" ]; then + echo "Specify app bundle as first parameter" + exit 1 +fi + +if [ -z "$APPLE_ID_USER" ] || [ -z "$APPLE_ID_PASSWORD" ]; then + echo "You need to set your Apple ID credentials with \$APPLE_ID_USER and \$APPLE_ID_PASSWORD." + exit 1 +fi + +APP_BUNDLE=$(basename "$1") +APP_BUNDLE_DIR=$(dirname "$1") + +cd "$APP_BUNDLE_DIR" || exit 1 + +# Package app for submission +echo "Generating ZIP archive ${APP_BUNDLE}.zip..." +ditto -c -k --rsrc --keepParent "$APP_BUNDLE" "${APP_BUNDLE}.zip" + +# Submit for notarization +echo "Submitting $APP_BUNDLE for notarization..." +RESULT=$(xcrun altool --notarize-app --type osx \ + --file "${APP_BUNDLE}.zip" \ + --primary-bundle-id com.samschott.maestral \ + --username $APPLE_ID_USER \ + --password @env:APPLE_ID_PASSWORD \ + --output-format xml) + +if [ $? -ne 0 ]; then + echo "Submitting $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 +fi + +REQUEST_UUID=$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'RequestUUID']/following-sibling::string[1]/text()" 2> /dev/null) + +if [ -z "$REQUEST_UUID" ]; then + echo "Submitting $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 +fi + +echo "$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'success-message']/following-sibling::string[1]/text()" 2> /dev/null)" + +# Poll for notarization status +echo "Submitted notarization request $REQUEST_UUID, waiting for response..." +sleep 60 +while : +do + RESULT=$(xcrun altool --notarization-info "$REQUEST_UUID" \ + --username "$APPLE_ID_USER" \ + --password @env:APPLE_ID_PASSWORD \ + --output-format xml) + STATUS=$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'Status']/following-sibling::string[1]/text()" 2> /dev/null) + + if [ "$STATUS" = "success" ]; then + echo "Notarization of $APP_BUNDLE succeeded!" + break + elif [ "$STATUS" = "in progress" ]; then + echo "Notarization in progress..." + sleep 20 + else + echo "Notarization of $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 + fi +done + +# Staple the notary ticket +xcrun stapler staple "$APP_BUNDLE" diff --git a/package/macos-notarize-dmg.sh b/package/macos-notarize-dmg.sh new file mode 100755 index 000000000..bb85e7c67 --- /dev/null +++ b/package/macos-notarize-dmg.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash + +if [ -z "$1" ]; then + echo "Specify dmg as first parameter" + exit 1 +fi + +if [ -z "$APPLE_ID_USER" ] || [ -z "$APPLE_ID_PASSWORD" ]; then + echo "You need to set your Apple ID credentials with \$APPLE_ID_USER and \$APPLE_ID_PASSWORD." + exit 1 +fi + +APP_BUNDLE=$(basename "$1") +APP_BUNDLE_DIR=$(dirname "$1") + +cd "$APP_BUNDLE_DIR" || exit 1 + +# Submit for notarization +echo "Submitting $APP_BUNDLE for notarization..." +RESULT=$(xcrun altool --notarize-app --type osx \ + --file "${APP_BUNDLE}" \ + --primary-bundle-id com.samschott.maestral \ + --username $APPLE_ID_USER \ + --password @env:APPLE_ID_PASSWORD \ + --output-format xml) + +if [ $? -ne 0 ]; then + echo "Submitting $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 +fi + +REQUEST_UUID=$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'RequestUUID']/following-sibling::string[1]/text()" 2> /dev/null) + +if [ -z "$REQUEST_UUID" ]; then + echo "Submitting $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 +fi + +echo "$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'success-message']/following-sibling::string[1]/text()" 2> /dev/null)" + +# Poll for notarization status +echo "Submitted notarization request $REQUEST_UUID, waiting for response..." +sleep 60 +while : +do + RESULT=$(xcrun altool --notarization-info "$REQUEST_UUID" \ + --username "$APPLE_ID_USER" \ + --password @env:APPLE_ID_PASSWORD \ + --output-format xml) + STATUS=$(echo "$RESULT" | xpath \ + "//key[normalize-space(text()) = 'Status']/following-sibling::string[1]/text()" 2> /dev/null) + + if [ "$STATUS" = "success" ]; then + echo "Notarization of $APP_BUNDLE succeeded!" + break + elif [ "$STATUS" = "in progress" ]; then + echo "Notarization in progress..." + sleep 20 + else + echo "Notarization of $APP_BUNDLE failed:" + echo "$RESULT" + exit 1 + fi +done + +# Staple the notary ticket +xcrun stapler staple "$APP_BUNDLE" diff --git a/package/maestral_macos_cocoa.spec b/package/maestral_macos.spec similarity index 100% rename from package/maestral_macos_cocoa.spec rename to package/maestral_macos.spec diff --git a/package/maestral_macos_qt.spec b/package/maestral_macos_qt.spec deleted file mode 100644 index f5f9191e4..000000000 --- a/package/maestral_macos_qt.spec +++ /dev/null @@ -1,110 +0,0 @@ -# -*- mode: python ; coding: utf-8 -*- - -block_cipher = None - - -import time -import pkg_resources as pkgr -from maestral import __version__, __author__ - - -try: - with open('bundle_version_macos.txt', 'r') as f: - bundle_version = str(int(f.read()) + 1) -except FileNotFoundError: - bundle_version = '1' - -with open('bundle_version_macos.txt', 'w') as f: - f.write(bundle_version) - - -def Entrypoint(dist, group, name, **kwargs): - - packages = [] - - kwargs.setdefault('pathex', []) - # get the entry point - ep = pkgr.get_entry_info(dist, group, name) - # insert path of the egg at the verify front of the search path - kwargs['pathex'] = [ep.dist.location] + kwargs['pathex'] - # script name must not be a valid module name to avoid name clashes on import - script_path = os.path.join(workpath, name + '-script.py') - print("creating script for entry point", dist, group, name) - with open(script_path, 'w') as fh: - print("import", ep.module_name, file=fh) - print("%s.%s()" % (ep.module_name, '.'.join(ep.attrs)), file=fh) - for package in packages: - print("import", package, file=fh) - - return Analysis( - [script_path] + kwargs.get('scripts', []), - **kwargs - ) - - -a = Entrypoint( - 'maestral_qt', 'console_scripts', 'maestral_qt', - binaries=None, - datas= [ - (pkgr.resource_filename('maestral_qt', 'resources/tray-icons-svg/*.svg'), 'maestral_qt/resources/tray-icons-svg'), - (pkgr.resource_filename('maestral_qt', 'resources/maestral.png'), 'maestral_qt/resources'), - (pkgr.resource_filename('maestral_qt', 'resources/faceholder.png'), 'maestral_qt/resources'), - (pkgr.resource_filename('maestral_qt', 'resources/*.ui'), 'maestral_qt/resources'), - (pkgr.resource_filename('maestral', 'resources/*.plist'), 'maestral/resources'), - (pkgr.resource_filename('maestral', 'resources/*.desktop'), 'maestral/resources'), - (pkgr.resource_filename('maestral', 'resources/*.png'), 'maestral/resources'), - (pkgr.resource_filename('maestral', 'resources/*.service'), 'maestral/resources'), - ], - hiddenimports=['pkg_resources.py2_warn'], - hookspath=['hooks'], - runtime_hooks=[], - excludes=['_tkinter'], - win_no_prefer_redirects=False, - win_private_assemblies=False, - cipher=block_cipher, - noarchive=False -) - -pyz = PYZ( - a.pure, a.zipped_data, - cipher=block_cipher -) - -exe = EXE( - pyz, - a.scripts, - [], - exclude_binaries=True, - name='main', - debug=False, - bootloader_ignore_signals=False, - strip=False, - upx=True, - console=False -) - -coll = COLLECT( - exe, - a.binaries, - a.zipfiles, - a.datas, - strip=False, - upx=True, - upx_exclude=[], - name='main' -) - -app = BUNDLE( - coll, - name='Maestral.app', - icon=pkgr.resource_filename('maestral_qt', 'resources/maestral.icns'), - bundle_identifier='com.samschott.maestral', - info_plist={ - 'NSHighResolutionCapable': 'True', - 'LSUIElement': '1', - 'CFBundleVersion': bundle_version, - 'CFBundleShortVersionString': __version__, - 'NSHumanReadableCopyright': 'Copyright © {} {}. All rights reserved.'.format(time.strftime('%Y'), __author__), - 'LSMinimumSystemVersion': '10.13.0', - }, -) diff --git a/package/post_build_macos_qt.py b/package/post_build_macos_qt.py deleted file mode 100644 index e0a67d50f..000000000 --- a/package/post_build_macos_qt.py +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/local/bin/python3 - -import shutil -from pathlib import Path - - -bundle_path = Path(__file__).parent / Path('dist/Maestral.app/Contents/macOS') - -items_to_remove = [ - 'QtQml', - 'QtQuick', - 'QtNetwork', - 'QtWebSockets', - 'QtQmlModels', - 'PyQt5/Qt/translations', - 'PyQt5/Qt/plugins/imageformats/libqgif.dylib', - 'PyQt5/Qt/plugins/imageformats/libqtiff.dylib', - 'PyQt5/Qt/plugins/imageformats/libqwebp.dylib', - 'PyQt5/Qt/plugins/platforms/libqwebgl.dylib', - 'PyQt5/Qt/plugins/platforms/libqoffscreen.dylib', - 'PyQt5/Qt/plugins/platforms/libqminimal.dylib', - 'libsqlite3.0.dylib', -] - -print("Removing unneeded Qt modules...") - -for path in items_to_remove: - lib_path = bundle_path / path - if lib_path.is_file(): - lib_path.unlink() - elif lib_path.is_dir(): - shutil.rmtree(lib_path) - -print("Done.") diff --git a/setup.py b/setup.py index 1fcb20ced..fe52c5e87 100644 --- a/setup.py +++ b/setup.py @@ -1,36 +1,11 @@ # -*- coding: utf-8 -*- # system imports -import sys -import os.path as osp from setuptools import setup, find_packages import importlib.util # local imports (must not depend on 3rd party packages) from maestral import __version__, __author__, __url__ -from maestral.utils.appdirs import get_runtime_path, get_old_runtime_path -from maestral.config.base import list_configs - - -# abort install if there are running daemons -running_daemons = [] - -for config in list_configs(): - pid_file = get_runtime_path('maestral', config + '.pid') - old_pid_file = get_old_runtime_path('maestral', config + '.pid') - if osp.exists(pid_file) or osp.exists(old_pid_file): - running_daemons.append(config) - -if running_daemons: - sys.stderr.write(f""" -Maestral daemons with the following configs are running: - -{', '.join(running_daemons)} - -Please stop the daemons before updating to ensure a clean upgrade -of config files and compatibility been the CLI and daemon. - """) - sys.exit(1) # proceed with actual install @@ -39,11 +14,11 @@ 'bugsnag>=3.4.0', 'click>=7.1.1', 'dropbox>=10.0.0', + 'fasteners', 'importlib_metadata;python_version<"3.8"', 'jeepney;sys_platform=="linux"', 'keyring>=19.0.0', 'keyrings.alt>=3.1.0', - 'lockfile>=0.12.0', 'packaging', 'pathspec>=0.5.8', 'Pyro5>=5.7', @@ -55,8 +30,8 @@ ] gui_requires = [ - 'maestral_qt>=1.0.0;sys_platform=="linux"', - 'maestral_cocoa>=1.0.0;sys_platform=="darwin"', + 'maestral_qt>=1.0.3;sys_platform=="linux"', + 'maestral_cocoa>=1.0.3;sys_platform=="darwin"', ] syslog_requires = ['systemd-python'] diff --git a/tests/resources/dmca.gif b/tests/resources/dmca.gif new file mode 100644 index 000000000..23c7b6689 Binary files /dev/null and b/tests/resources/dmca.gif differ diff --git a/tests/test_main.py b/tests/test_main.py index 5c114bd6c..473fa96c8 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -36,7 +36,7 @@ def setUpClass(cls): cls.test_folder_dbx = cls.TEST_FOLDER_PATH cls.test_folder_local = cls.m.dropbox_path + cls.TEST_FOLDER_PATH - # aquire test lock + # acquire test lock while True: try: cls.m.client.make_dir(cls.TEST_LOCK_PATH) @@ -60,6 +60,11 @@ def tearDownClass(cls): except NotFoundError: pass + try: + cls.m.client.remove('/.mignore') + except NotFoundError: + pass + # release test lock try: @@ -91,7 +96,7 @@ def wait_for_idle(self, minimum=4): t0 = time.time() while time.time() - t0 < minimum: - if self.m.sync.lock.locked(): + if self.m.sync.busy(): self.m.monitor._wait_for_idle() t0 = time.time() else: @@ -104,12 +109,17 @@ def clean_remote(self): except NotFoundError: pass + try: + self.m.client.remove('/.mignore') + except NotFoundError: + pass + self.m.client.make_dir(self.test_folder_dbx) # test functions def test_selective_sync(self): - """Test `Maetsral.exclude_item` and Maetsral.include_item`.""" + """Test `Maestral.exclude_item` and Maestral.include_item`.""" test_path_local = self.test_folder_local + '/selective_sync_test_folder' test_path_dbx = self.test_folder_dbx + '/selective_sync_test_folder' @@ -148,7 +158,7 @@ def test_selective_sync(self): self.assertNotIn(test_path_dbx, self.m.excluded_items, 'deleted item is still in "excluded_items" list') - # test exluding a non-existant folder + # test excluding a non-existent folder with self.assertRaises(NotFoundError): self.m.exclude_item(test_path_dbx) @@ -156,6 +166,7 @@ def test_upload_sync_issues(self): # paths with backslash are not allowed on Dropbox test_path_local = self.test_folder_local + '/folder\\' + test_path_dbx = self.test_folder_dbx + '/folder\\' n_errors_initial = len(self.m.sync_errors) @@ -163,23 +174,59 @@ def test_upload_sync_issues(self): self.wait_for_idle() self.assertEqual(len(self.m.sync_errors), n_errors_initial + 1) - self.assertTrue(any(e['local_path'] == test_path_local for e in self.m.sync_errors)) + self.assertEqual(self.m.sync_errors[-1]['local_path'], test_path_local) + self.assertEqual(self.m.sync_errors[-1]['dbx_path'], test_path_dbx) + self.assertEqual(self.m.sync_errors[-1]['type'], 'PathError') delete(test_path_local) self.wait_for_idle() self.assertEqual(len(self.m.sync_errors), n_errors_initial) - self.assertFalse(any(e['local_path'] == test_path_local for e in self.m.sync_errors)) + self.assertTrue(all(e['local_path'] != test_path_local for e in self.m.sync_errors)) + self.assertTrue(all(e['dbx_path'] != test_path_dbx for e in self.m.sync_errors)) def test_download_sync_issues(self): - # TODO: find a file with reproducible download error - pass - - def test_mignore(self): - # TODO: - # 1) test that new files are excluded - # 2) test that tracked files are unaffected - pass + test_path_local = self.test_folder_local + '/dmca.gif' + test_path_dbx = self.test_folder_dbx + '/dmca.gif' + + self.wait_for_idle() + + n_errors_initial = len(self.m.sync_errors) + + self.m.client.upload(self.resources + '/dmca.gif', test_path_dbx) + + self.wait_for_idle() + + # 1) Check that the sync issue is logged + + self.assertEqual(len(self.m.sync_errors), n_errors_initial + 1) + self.assertEqual(self.m.sync_errors[-1]['local_path'], test_path_local) + self.assertEqual(self.m.sync_errors[-1]['dbx_path'], test_path_dbx) + self.assertEqual(self.m.sync_errors[-1]['type'], 'RestrictedContentError') + self.assertIn(test_path_dbx, self.m.sync.download_errors) + + # 2) Check that the sync is retried after pause / resume + + self.m.pause_sync() + self.m.resume_sync() + + self.wait_for_idle() + + self.assertEqual(len(self.m.sync_errors), n_errors_initial + 1) + self.assertEqual(self.m.sync_errors[-1]['local_path'], test_path_local) + self.assertEqual(self.m.sync_errors[-1]['dbx_path'], test_path_dbx) + self.assertEqual(self.m.sync_errors[-1]['type'], 'RestrictedContentError') + self.assertIn(test_path_dbx, self.m.sync.download_errors) + + # 3) Check that the error is cleared when the file is deleted + + self.m.client.remove(test_path_dbx) + self.wait_for_idle() + + self.assertEqual(len(self.m.sync_errors), n_errors_initial) + self.assertTrue(all(e['local_path'] != test_path_local for e in self.m.sync_errors)) + self.assertTrue(all(e['dbx_path'] != test_path_dbx for e in self.m.sync_errors)) + self.assertNotIn(test_path_dbx, self.m.sync.download_errors) if __name__ == '__main__': diff --git a/tests/test_sync.py b/tests/test_sync.py index 39cfe49a5..512543e2f 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -20,7 +20,7 @@ from maestral.sync import delete, move from maestral.sync import is_child from maestral.sync import get_local_hash, DirectorySnapshot -from maestral.sync import UpDownSync, Observer, FSEventHandler +from maestral.sync import SyncEngine, Observer, FSEventHandler from maestral.errors import NotFoundError, FolderConflictError from maestral.main import Maestral from maestral.main import get_log_path @@ -30,11 +30,7 @@ from unittest import TestCase -# run tests in decleration order and not alphabetically -unittest.TestLoader.sortTestMethodsUsing = None - - -class DummyUpDownSync(UpDownSync): +class DummySyncEngine(SyncEngine): def __init__(self, dropbox_path=''): self._dropbox_path = dropbox_path @@ -54,7 +50,7 @@ def path(i): class TestCleanLocalEvents(TestCase): def setUp(self): - self.sync = DummyUpDownSync() + self.sync = DummySyncEngine() def test_single_file_events(self): @@ -290,7 +286,7 @@ def test_performance(self): class TestIgnoreLocalEvents(TestCase): def setUp(self): - self.sync = DummyUpDownSync() + self.sync = DummySyncEngine() self.dummy_dir = Path(os.getcwd()).parent / 'dropbox_dir' delete(self.dummy_dir) @@ -300,14 +296,14 @@ def setUp(self): startup = Event() syncing.set() - self.sync = DummyUpDownSync(self.dummy_dir) + self.sync = DummySyncEngine(self.dummy_dir) self.fs_event_handler = FSEventHandler(syncing, startup, self.sync) self.observer = Observer() self.observer.schedule(self.fs_event_handler, str(self.dummy_dir), recursive=True) self.observer.start() - def test_recieveing_events(self): + def test_receiving_events(self): new_dir = self.dummy_dir / 'parent' new_dir.mkdir() @@ -396,7 +392,7 @@ def setUpClass(cls): cls.test_folder_dbx = cls.TEST_FOLDER_PATH cls.test_folder_local = cls.m.dropbox_path + cls.TEST_FOLDER_PATH - # aquire test lock + # acquire test lock while True: try: cls.m.client.make_dir(cls.TEST_LOCK_PATH) @@ -420,6 +416,11 @@ def tearDownClass(cls): except NotFoundError: pass + try: + cls.m.client.remove('/.mignore') + except NotFoundError: + pass + # release test lock try: @@ -451,21 +452,32 @@ def wait_for_idle(self, minimum=4): t0 = time.time() while time.time() - t0 < minimum: - if self.m.sync.lock.locked(): + if self.m.sync.busy(): self.m.monitor._wait_for_idle() t0 = time.time() else: time.sleep(0.1) def clean_remote(self): - """Recreates a fresh test folder.""" + """Recreates a fresh test folder on remote Dropbox.""" try: self.m.client.remove(self.test_folder_dbx) except NotFoundError: pass + try: + self.m.client.remove('/.mignore') + except NotFoundError: + pass + self.m.client.make_dir(self.test_folder_dbx) + def clean_local(self): + """Recreates a fresh test folder locally.""" + delete(self.m.dropbox_path + '/.mignore') + delete(self.test_folder_local) + os.mkdir(self.test_folder_local) + def assert_synced(self, local_folder, remote_folder): """Asserts that the `local_folder` and `remote_folder` are synced.""" remote_items = self.m.list_folder(remote_folder, recursive=True) @@ -504,7 +516,7 @@ def _count_conflicts(entries, name): candidates = list(e for e in entries if e['name'].startswith(basename)) ccs = list(e for e in candidates if '(1)' in e['name'] # created by Dropbox for add conflict - or 'conflicted copy' in e['name'] # created by Dropbox for update confict + or 'conflicted copy' in e['name'] # created by Dropbox for update conflict or 'conflicting copy' in e['name']) # created by us return len(ccs) @@ -984,6 +996,47 @@ def test_case_conflict(self): self.assert_synced(self.test_folder_local, self.test_folder_dbx) + def test_mignore(self): + + # 1) test that tracked items are unaffected + + os.mkdir(self.test_folder_local + '/bar') + self.wait_for_idle() + + with open(self.m.sync.mignore_path, 'w') as f: + f.write('foo/\n') # ignore folder "foo" + f.write('bar\n') # ignore file or folder "bar" + f.write('build\n') # ignore file or folder "build" + + self.wait_for_idle() + + self.assert_synced(self.test_folder_local, self.test_folder_dbx) + self.assert_exists(self.test_folder_dbx, 'bar') + + # 2) test that new items are excluded + + os.mkdir(self.test_folder_local + '/foo') + self.wait_for_idle() + + self.assertIsNone(self.m.client.get_metadata(self.test_folder_dbx + '/foo')) + + # 3) test that renaming an item excludes it + + move(self.test_folder_local + '/bar', self.test_folder_local + '/build') + self.wait_for_idle() + + self.assertIsNone(self.m.client.get_metadata(self.test_folder_dbx + '/build')) + + # 4) test that renaming an item includes it + + move(self.test_folder_local + '/build', self.test_folder_local + '/folder') + self.wait_for_idle() + + self.assert_exists(self.test_folder_dbx, 'folder') + + self.clean_local() + self.wait_for_idle() + if __name__ == '__main__': unittest.main() diff --git a/tests/utils/test_appdirs.py b/tests/utils/test_appdirs.py index 406cc5ef5..3bc267609 100644 --- a/tests/utils/test_appdirs.py +++ b/tests/utils/test_appdirs.py @@ -7,10 +7,9 @@ """ import os import platform -import tempfile from maestral.utils.appdirs import ( - get_home_dir, get_runtime_path, get_old_runtime_path, get_conf_path, - get_log_path, get_cache_path, get_data_path, get_autostart_path, + get_home_dir, get_runtime_path, get_conf_path, get_log_path, get_cache_path, + get_data_path, get_autostart_path, ) @@ -21,7 +20,6 @@ def test_macos_dirs(): assert get_cache_path(create=False) == get_conf_path(create=False) assert get_data_path(create=False) == get_conf_path(create=False) assert get_runtime_path(create=False) == get_conf_path(create=False) - assert get_old_runtime_path(create=False) == tempfile.gettempdir() assert get_log_path(create=False) == get_home_dir() + '/Library/Logs' assert get_autostart_path(create=False) == get_home_dir() + '/Library/LaunchAgents' @@ -38,7 +36,6 @@ def test_linux_dirs(): assert get_cache_path(create=False) == '/xdg_cache_home' assert get_data_path(create=False) == '/xdg_data_dir' assert get_runtime_path(create=False) == '/xdg_runtime_dir' - assert get_old_runtime_path(create=False) == '/xdg_runtime_dir' assert get_log_path(create=False) == '/xdg_cache_home' assert get_autostart_path(create=False) == '/xdg_config_home/autostart' @@ -51,6 +48,5 @@ def test_linux_dirs(): assert get_cache_path(create=False) == get_home_dir() + '/.cache' assert get_data_path(create=False) == get_home_dir() + '/.local/share' assert get_runtime_path(create=False) == get_home_dir() + '/.cache' - assert get_old_runtime_path(create=False) == get_home_dir() + '/.cache' assert get_log_path(create=False) == get_home_dir() + '/.cache' assert get_autostart_path(create=False) == get_home_dir() + '/.config/autostart'