From 064dab5e50535920f345f03d0c0cbf4d6866e5c9 Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Sun, 27 Jan 2019 19:01:18 +0200 Subject: [PATCH 1/9] add a tar adapter for gnu, bsd and busybox tar --- file_replicator/tar_adapter.py | 150 +++++++++++++++++++++++++++++++ tests/test_tar_adapter.py | 158 +++++++++++++++++++++++++++++++++ 2 files changed, 308 insertions(+) create mode 100644 file_replicator/tar_adapter.py create mode 100644 tests/test_tar_adapter.py diff --git a/file_replicator/tar_adapter.py b/file_replicator/tar_adapter.py new file mode 100644 index 0000000..8e88092 --- /dev/null +++ b/file_replicator/tar_adapter.py @@ -0,0 +1,150 @@ +from abc import ABCMeta, abstractmethod +import subprocess + + +__all__ = [ + "GnuTarAdapter", + "BsdTarAdapter", + "BusyBoxTarAdapter", + "detect_local_tar", + "detect_remote_tar", +] + + +class AbstractTarAdapter(metaclass=ABCMeta): + @abstractmethod + def __str__(self): + raise NotImplementedError + + def __repr__(self): + cls = type(self) + return ( + f"<{cls.__module__}.{cls.__name__} ({str(self)}) object at 0x{id(self):x}>" + ) + + @property + def cmd(self): + return "tar" + + @abstractmethod + def receiver_options(self): + raise NotImplementedError + + @abstractmethod + def sender_options(self, src_file): + raise NotImplementedError + + def receiver_cmd(self): + return [self.cmd] + self.receiver_options() + + def receiver_cmd_str(self): + return " ".join(self.receiver_cmd()) + + def sender_cmd(self, src_file): + return [self.cmd] + self.sender_options(src_file) + + def sender_cmd_str(self, src_file): + return " ".join(self.sender_cmd(src_file)) + + @property + def version_option(self): + return "--version" + + @abstractmethod + def match_flavor_output(self, output): + raise NotImplementedError + + +class PrefixedTarAdapter(AbstractTarAdapter): + def __init__(self, prefix=""): + self._prefix = prefix + super().__init__() + + @property + def cmd(self): + return f"{self._prefix}tar" + + +class GnuTarAdapter(PrefixedTarAdapter): + def __str__(self): + return f"Gnu Tar [{self.cmd}]" + + def receiver_options(self): + return ["--no-same-owner", "--extract", "--verbose"] + + def sender_options(self, src_file): + return ["--create", src_file, "--to-stdout", "--ignore-failed-read"] + + def match_flavor_output(self, output): + return "GNU tar" in output + + +class BsdTarAdapter(AbstractTarAdapter): + def __str__(self): + return "BSD Tar" + + def receiver_options(self): + return ["-o", "-x", "-v"] + + def sender_options(self, src_file): + return ["-c", "-f", "-", src_file] + + def match_flavor_output(self, output): + return "bsdtar" in output + + +class BusyBoxTarAdapter(AbstractTarAdapter): + def __str__(self): + return "BusyBox Tar" + + def receiver_options(self): + return ["x", "-v"] + + def sender_options(self, src_file): + return ["c", "-f", "-", src_file] + + def match_flavor_output(self, output): + return "busybox" in output + + +def detect_local_tar(acceptable=None): + """Determine, if any, a suitable sender tar""" + if acceptable is None: + acceptable = [ + GnuTarAdapter(), + GnuTarAdapter(prefix="g"), + BsdTarAdapter(), + BusyBoxTarAdapter(), + ] + for tar_flavor in acceptable: + try: + result = subprocess.run( + [tar_flavor.cmd, tar_flavor.version_option], + stdout=subprocess.PIPE, + check=True, + universal_newlines=True, + ) + except (FileNotFoundError, subprocess.CalledProcessError) as e: + continue + if tar_flavor.match_flavor_output(result.stdout): + return tar_flavor + return None + + +def detect_remote_tar(connection_command, acceptable=None): + """Determine, if any, a suitable receiver tar""" + if acceptable is None: + acceptable = [GnuTarAdapter(), GnuTarAdapter(prefix="g")] + for tar_flavor in acceptable: + try: + result = subprocess.run( + connection_command, + input=f"{tar_flavor.cmd} {tar_flavor.version_option}", + stdout=subprocess.PIPE, + universal_newlines=True, + ) + except (FileNotFoundError, subprocess.CalledProcessError) as e: + raise RuntimeError(f"Error using connection command: {e}") + if tar_flavor.match_flavor_output(result.stdout): + return tar_flavor + return None diff --git a/tests/test_tar_adapter.py b/tests/test_tar_adapter.py new file mode 100644 index 0000000..097eda7 --- /dev/null +++ b/tests/test_tar_adapter.py @@ -0,0 +1,158 @@ +from abc import ABCMeta, abstractmethod +import os +from pathlib import Path + +import pytest + +from file_replicator.tar_adapter import ( + GnuTarAdapter, + BsdTarAdapter, + BusyBoxTarAdapter, + detect_local_tar, + detect_remote_tar, +) + + +@pytest.mark.parametrize("prefix", ["", "g"]) +def test_gnu_tar_adapter(prefix): + tar = GnuTarAdapter(prefix=prefix) + assert tar.cmd == f"{prefix}tar" + assert tar.receiver_cmd() == [ + f"{prefix}tar", + "--no-same-owner", + "--extract", + "--verbose", + ] + assert tar.receiver_cmd_str() == " ".join(tar.receiver_cmd()) + assert tar.sender_cmd("foo") == [ + f"{prefix}tar", + "--create", + "foo", + "--to-stdout", + "--ignore-failed-read", + ] + assert tar.sender_cmd_str("foo") == " ".join(tar.sender_cmd("foo")) + + +def test_bsd_tar_adapter(): + tar = BsdTarAdapter() + assert tar.cmd == "tar" + assert tar.receiver_cmd() == ["tar", "-o", "-x", "-v"] + assert tar.sender_cmd("foo") == [f"tar", "-c", "-f", "-", "foo"] + + +# not so useful, but here we go +def test_detect_real_local_tar(): + tar = detect_local_tar() + assert isinstance(tar, (GnuTarAdapter, BsdTarAdapter, BusyBoxTarAdapter, None)) + + +class AbstractMockTar(metaclass=ABCMeta): + @property + @abstractmethod + def version(): + raise NotImplementedError + + def create_mock_tar(self): + return f"""#!/bin/sh +/bin/cat << EOF +{self.version} +EOF +""" + + +class MockGnuTar(AbstractMockTar): + @property + def version(self): + return """ +tar (GNU tar) 1.31 +Copyright (C) 2019 Free Software Foundation, Inc. +License GPLv3+: GNU GPL version 3 or later . +This is free software: you are free to change and redistribute it. +There is NO WARRANTY, to the extent permitted by law. + +Written by John Gilmore and Jay Fenlason. +""" + + +class MockBsdTar(AbstractMockTar): + @property + def version(self): + return "bsdtar 2.8.3 - libarchive 2.8.3" + + +class MockBusyBoxTar(AbstractMockTar): + @property + def version(self): + return "tar (busybox) 1.28.4" + + +class MockUnknownTar(AbstractMockTar): + @property + def version(self): + return "Some Unknown tar v.1.0" + + +@pytest.fixture +def temp_dir_as_path(tmp_path, monkeypatch): + with monkeypatch.context() as m: + m.setenv("PATH", str(tmp_path)) + yield tmp_path + + +@pytest.fixture +def mock_tar(request, temp_dir_as_path): + mock_tar, tar_name = request.param + tar_cmd = temp_dir_as_path / tar_name + tar_cmd.write_text(mock_tar.create_mock_tar()) + tar_cmd.chmod(0o755) + + +def which(command): + try: + path = next( + p + for p in map(Path, os.environ["PATH"].split(":")) + if p.is_dir() and any(f.name == "bash" for f in p.iterdir()) + ) + return path / command + except StopIteration: + return None + + +@pytest.fixture(scope="module") +def bash(): + return str(which("bash")) + + +ALL_TARS = [ + GnuTarAdapter(), + GnuTarAdapter(prefix="g"), + BsdTarAdapter(), + BusyBoxTarAdapter(), +] + + +@pytest.mark.parametrize( + "mock_tar,expect", + [ + ((MockGnuTar(), "tar"), GnuTarAdapter), + ((MockGnuTar(), "gtar"), GnuTarAdapter), + ((MockBsdTar(), "tar"), BsdTarAdapter), + ((MockBusyBoxTar(), "tar"), BusyBoxTarAdapter), + ((MockUnknownTar(), "tar"), type(None)), + ], + indirect=["mock_tar"], +) +def test_detect_tar(mock_tar, expect, bash): + tar = detect_local_tar(acceptable=ALL_TARS) + assert isinstance(tar, expect) + remote_tar = detect_remote_tar(bash, acceptable=ALL_TARS) + assert isinstance(remote_tar, expect) + + +def test_no_tar_cmd(temp_dir_as_path, bash): + tar = detect_local_tar(acceptable=ALL_TARS) + assert tar is None + remote_tar = detect_remote_tar(bash, acceptable=ALL_TARS) + assert remote_tar is None From 9b8e40acdd8a9da66881b6a2ce90bb94b926d6fc Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 14:57:54 +0200 Subject: [PATCH 2/9] replace inotify with watchdog and employ tar_adapters --- file_replicator/cli.py | 63 ++++++++++++++++++++- file_replicator/lib.py | 126 ++++++++++++++++++++++++++++------------- tests/test_lib.py | 59 +++++++++++++------ 3 files changed, 193 insertions(+), 55 deletions(-) diff --git a/file_replicator/cli.py b/file_replicator/cli.py index b21bc25..b7dd828 100644 --- a/file_replicator/cli.py +++ b/file_replicator/cli.py @@ -5,6 +5,7 @@ import file_replicator from .lib import make_file_replicator, replicate_all_files, replicate_files_on_change +from .tar_adapter import * @click.command() @@ -35,6 +36,50 @@ @click.option( "--debugging", is_flag=True, default=False, help="Print debugging information." ) +@click.option( + "--local-tar-gnu", + "local_tar_fn", + flag_value=GnuTarAdapter, + help="Local tar is gnu tar.", +) +@click.option( + "--local-tar-bsd", + "local_tar_fn", + flag_value=BsdTarAdapter, + help="Local tar is bsd tar.", +) +@click.option( + "--local-tar-gnu-prefix", + "local_tar_fn", + flag_value=lambda: GnuTarAdapter(prefix="g"), + help="Use gtar as gnu tar locally.", +) +@click.option( + "--local-tar-detect", + "local_tar_fn", + flag_value=detect_local_tar, + default=True, + help="Attempt to detect local tar flavor.", +) +@click.option( + "--remote-tar-gnu", + "remote_tar_fn", + flag_value=lambda cmd: GnuTarAdapter(), + default=True, + help="Remote tar is gnu tar", +) +@click.option( + "--remote-tar-gnu-prefix", + "remote_tar_fn", + flag_value=lambda cmd: GnuTarAdapter(prefix="g"), + help="Use gtar as gnu tar remotely.", +) +@click.option( + "--remote-tar-detect", + "remote_tar_fn", + flag_value=lambda cmd: detect_remote_tar(cmd), + help="Attempt to detect remote tar flavor.", +) @click.version_option(version=file_replicator.__version__) def main( src_dir, @@ -45,6 +90,8 @@ def main( replicate_on_change, gitignore, debugging, + local_tar_fn, + remote_tar_fn, ): """Replicate files to another computer e.g. for remote development. @@ -92,13 +139,27 @@ def main( if not os.path.exists(src_dir) or not os.path.isdir(src_dir): raise click.UsageError("The source destination must exist and be a directory.") + local_tar = local_tar_fn() + remote_tar = remote_tar_fn(connection_command) + if debugging: + print(f"Local tar: {local_tar}") + print(f"Remote tar: {remote_tar}") + if not isinstance(remote_tar, GnuTarAdapter): + click.UsageError("Cannot use non-gnu remote tar!") + if clean_out_first: click.secho( "Clearing out all destination files first!", fg="green", bold="true" ) with make_file_replicator( - src_dir, dest_parent_dir, connection_command, clean_out_first=clean_out_first, debugging=debugging + local_tar, + remote_tar, + src_dir, + dest_parent_dir, + connection_command, + clean_out_first=clean_out_first, + debugging=debugging, ) as copy_file: if with_initial_replication: replicate_all_files( diff --git a/file_replicator/lib.py b/file_replicator/lib.py index 745d6fc..e5e5860 100644 --- a/file_replicator/lib.py +++ b/file_replicator/lib.py @@ -1,11 +1,13 @@ import contextlib import os.path -import shutil import subprocess import time -import inotify.adapters import pathspec +from watchdog.events import FileSystemEventHandler +from watchdog.observers import Observer +from watchdog.utils import has_attribute, unicode_paths + __all__ = ["make_file_replicator", "replicate_all_files", "replicate_files_on_change"] @@ -21,13 +23,15 @@ mkdir -p {dest_dir} cd {dest_dir} while true; do - tar --no-same-owner --extract --verbose -done + {receiver_tar} +done 2>/dev/null """ @contextlib.contextmanager def make_file_replicator( + local_tar, + remote_tar, src_dir, dest_parent_dir, bash_connection_command, @@ -50,7 +54,9 @@ def make_file_replicator( # Get the remote end up and running waiting for tar files. receiver_code = RECEIVER_CODE.format( - dest_dir=dest_dir, clean_out_first=str(clean_out_first).lower() + dest_dir=dest_dir, + clean_out_first=str(clean_out_first).lower(), + receiver_tar=remote_tar.receiver_cmd_str(), ) p.stdin.write(receiver_code.encode()) p.stdin.flush() @@ -61,13 +67,7 @@ def copy_file(src_filename): if debugging: print(f"Sending {src_filename}...") result = subprocess.run( - [ - "tar", - "--create", - rel_src_filename, - "--to-stdout", - "--ignore-failed-read", - ], + local_tar.sender_cmd(rel_src_filename), cwd=src_dir, check=True, stdout=p.stdin, @@ -106,37 +106,87 @@ def replicate_all_files(src_dir, copy_file, use_gitignore=True, debugging=False) copy_file(os.path.join(src_dir, filename)) +class CopyFileEventHandler(FileSystemEventHandler): + """A watchdog.FileSystemEventHandler that copies files using copy_file().""" + + def __init__(self, copy_file, debugging=False): + self.copy_file = copy_file + self.debugging = debugging + self.last_event_timestamp = time.time() + + def on_any_event(self, event): + self.last_event_timestamp = time.time() + if self.debugging: + print(f"Detected change: {event.key}") + + if event.event_type == "deleted": + return + if event.is_directory and event.event_type == "modified": + return + if event.event_type == "moved": + self.copy_file(event.dest_path) + else: + self.copy_file(event.src_path) + + +class GitIgnoreCopyFileEventHandler(CopyFileEventHandler): + def __init__(self, copy_file, ignore_spec, debugging=False): + super().__init__(copy_file, debugging) + self.spec = ignore_spec + + def dispatch(self, event): + if event.src_path and self.spec.match_file( + unicode_paths.decode(event.src_path) + ): + if self.debugging: + print(f"Ignoring source change on {event.src_path}") + return + if has_attribute(event, "dest_path") and self.spec.match_file( + unicode_paths.decode(event.dest_path) + ): + if self.debugging: + print(f"Ignoring destination change on {event.dest_path}") + return + super().dispatch(event) + + +class NoChangeTimeoutError(Exception): + pass + + +def raise_if_timeout(last_change, timeout): + elapsed = time.time() - last_change + if elapsed > timeout: + raise NoChangeTimeoutError(f"No changes detected for {elapsed} seconds.") + + def replicate_files_on_change( src_dir, copy_file, timeout=None, use_gitignore=True, debugging=False ): """Wait for changes to files in src_dir and copy with copy_file(). If provided, the timeout indicates when to return after that many seconds of no change. - - This is an imperfect solution because there are seemingly unavoidable race conditions - when watching for file changes or additions and new directories are involved. - - Returns True to indicate that new directories have been added and the function should - be called again. Otherwise returns False. - """ - please_call_me_again = False - i = inotify.adapters.InotifyTree(src_dir) - spec = get_pathspec(src_dir, use_gitignore) - for event in i.event_gen(yield_nones=False, timeout_s=timeout): - (_, type_names, path, filename) = event - full_path = os.path.abspath(os.path.join(path, filename)) - rel_to_src_dir_path = os.path.relpath(full_path, src_dir) + src_dir = os.path.abspath(src_dir) + if use_gitignore: + spec = get_pathspec(src_dir, use_gitignore) + event_handler = GitIgnoreCopyFileEventHandler( + copy_file, spec, debugging=debugging + ) + else: + event_handler = CopyFileEventHandler(copy_file, debugging=debugging) + observer = Observer() + observer.schedule(event_handler, src_dir, recursive=True) + if debugging: + print("Starting observer") + observer.start() + try: + while True: + if timeout: + raise_if_timeout(event_handler.last_event_timestamp, timeout) + time.sleep(0.5) + except (KeyboardInterrupt, NoChangeTimeoutError) as e: + observer.stop() if debugging: - print(f"Detected change: {full_path} {type_names}") - if not spec.match_file(rel_to_src_dir_path): - if "IN_CREATE" in type_names and "IN_ISDIR" in type_names: - # Race condition danger because a new directory was created (see warning on https://pypi.org/project/inotify). - # Wait a short while for things to settle, then replicate the new directory, then start the watchers again. - time.sleep(0.5) - replicate_all_files(full_path, copy_file) - please_call_me_again = True - break - elif "IN_CLOSE_WRITE" in type_names: - copy_file(full_path) - return please_call_me_again + print("Exitting on {e}") + observer.join() diff --git a/tests/test_lib.py b/tests/test_lib.py index 4a90c71..18bf9f7 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -5,7 +5,18 @@ import tempfile import threading +import pytest + from file_replicator.lib import * +from file_replicator.tar_adapter import detect_local_tar, GnuTarAdapter + + +@pytest.fixture +def local_tar(): + acceptable = (GnuTarAdapter(), GnuTarAdapter(prefix="g")) + tar = detect_local_tar(acceptable=acceptable) + assert tar is not None + return tar @contextlib.contextmanager @@ -32,20 +43,24 @@ def assert_file_contains(filename, text): assert f.read() == text -def test_empty_directories_are_copied(): +def test_empty_directories_are_copied(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") os.makedirs(src_dir) - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: pass assert list(os.listdir(src_dir)) == [] assert list(os.listdir(dest_parent_dir)) == ["test"] -def test_copy_one_file(): +def test_copy_one_file(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: make_test_file(src_dir, "test_file.txt", "hello") assert_file_contains(os.path.join(src_dir, "test_file.txt"), "hello") copy_file(os.path.join(src_dir, "test_file.txt")) @@ -54,10 +69,12 @@ def test_copy_one_file(): ) -def test_copy_file_with_unusual_characters_in_name(): +def test_copy_file_with_unusual_characters_in_name(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: make_test_file(src_dir, "test ~$@%-file.txt", "hello") assert_file_contains(os.path.join(src_dir, "test ~$@%-file.txt"), "hello") copy_file(os.path.join(src_dir, "test ~$@%-file.txt")) @@ -66,10 +83,12 @@ def test_copy_file_with_unusual_characters_in_name(): ) -def test_make_missing_parent_directories(): +def test_make_missing_parent_directories(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: make_test_file(src_dir, "a/b/c/test_file.txt", "hello") assert_file_contains(os.path.join(src_dir, "a/b/c/test_file.txt"), "hello") copy_file(os.path.join(src_dir, "a/b/c/test_file.txt")) @@ -78,18 +97,20 @@ def test_make_missing_parent_directories(): ) -def test_replicate_all_files(): +def test_replicate_all_files(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") make_test_file(src_dir, "a.txt", "hello") make_test_file(src_dir, "b/c.txt", "goodbye") - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: replicate_all_files(src_dir, copy_file) assert_file_contains(os.path.join(src_dir, "a.txt"), "hello") assert_file_contains(os.path.join(src_dir, "b/c.txt"), "goodbye") -def test_detect_and_copy_new_file(): +def test_detect_and_copy_new_file(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -108,7 +129,9 @@ def test_detect_and_copy_new_file(): # Watch for changes and copy files, and stop after short while of inactvitiy. # The second file (see above) should be created during this internval. - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: while timer.is_alive(): while replicate_files_on_change(src_dir, copy_file, timeout=0.2): pass @@ -123,7 +146,7 @@ def test_detect_and_copy_new_file(): assert_file_contains(os.path.join(dest_parent_dir, "test/b.txt"), "goodbye") -def test_detect_and_copy_modified_file(): +def test_detect_and_copy_modified_file(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -142,7 +165,9 @@ def test_detect_and_copy_modified_file(): # Watch for changes and copy files, and stop after short while of inactvitiy. # The second file (see above) should be created during this internval. - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: while timer.is_alive(): while replicate_files_on_change(src_dir, copy_file, timeout=0.2): pass @@ -155,7 +180,7 @@ def test_detect_and_copy_modified_file(): assert_file_contains(os.path.join(dest_parent_dir, "test/a.txt"), "hello again") -def test_detect_and_copy_new_file_in_new_directories(): +def test_detect_and_copy_new_file_in_new_directories(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -176,7 +201,9 @@ def test_detect_and_copy_new_file_in_new_directories(): # Watch for changes and copy files, and stop after short while of inactvitiy. # The second file (see above) should be created during this internval. - with make_file_replicator(src_dir, dest_parent_dir, ("bash",)) as copy_file: + with make_file_replicator( + local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) + ) as copy_file: while timer.is_alive(): while replicate_files_on_change(src_dir, copy_file, timeout=0.2): pass From 8acfbfaba05dd730ac1a3c8b31e96c263a4480dd Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 15:08:14 +0200 Subject: [PATCH 3/9] code style --- file_replicator/lib.py | 1 - file_replicator/tar_adapter.py | 3 +-- tests/test_lib.py | 2 +- tests/test_tar_adapter.py | 4 ++-- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/file_replicator/lib.py b/file_replicator/lib.py index e5e5860..d5037b2 100644 --- a/file_replicator/lib.py +++ b/file_replicator/lib.py @@ -8,7 +8,6 @@ from watchdog.observers import Observer from watchdog.utils import has_attribute, unicode_paths - __all__ = ["make_file_replicator", "replicate_all_files", "replicate_files_on_change"] diff --git a/file_replicator/tar_adapter.py b/file_replicator/tar_adapter.py index 8e88092..1163977 100644 --- a/file_replicator/tar_adapter.py +++ b/file_replicator/tar_adapter.py @@ -1,6 +1,5 @@ -from abc import ABCMeta, abstractmethod import subprocess - +from abc import ABCMeta, abstractmethod __all__ = [ "GnuTarAdapter", diff --git a/tests/test_lib.py b/tests/test_lib.py index 18bf9f7..ee46e3d 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -8,7 +8,7 @@ import pytest from file_replicator.lib import * -from file_replicator.tar_adapter import detect_local_tar, GnuTarAdapter +from file_replicator.tar_adapter import GnuTarAdapter, detect_local_tar @pytest.fixture diff --git a/tests/test_tar_adapter.py b/tests/test_tar_adapter.py index 097eda7..80c9fbf 100644 --- a/tests/test_tar_adapter.py +++ b/tests/test_tar_adapter.py @@ -1,13 +1,13 @@ -from abc import ABCMeta, abstractmethod import os +from abc import ABCMeta, abstractmethod from pathlib import Path import pytest from file_replicator.tar_adapter import ( - GnuTarAdapter, BsdTarAdapter, BusyBoxTarAdapter, + GnuTarAdapter, detect_local_tar, detect_remote_tar, ) From aa708f5bb659c72c1f449ef1acaa95347ad3b07f Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 15:11:19 +0200 Subject: [PATCH 4/9] updated version, dependencies and README -- linux tests not updated --- README.md | 57 ++++++++++++++++++++++++++++++++++++++++---------- pyproject.toml | 4 ++-- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index fa9227f..091597a 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Tested and known to work between two Linux machines. Support for developing on m Dependencies are: * Python 3 and some Python packages on the development machine. * Ability to run a shell (bash or bash-like) on the remote machine with connected `stdin`. -* The tar utility (the full version, not the busybox version) on both machines. +* The gnu tar utility (the full version, not the busybox version) on both machines. Note that nothing is installed remotely, there are no ports to open, and the remote user only needs the ability to create the files and directories at the specified location. @@ -71,7 +71,7 @@ See help with `file-replicate --help`: The CONNECTION_COMMAND must result in a running instance of bash ready to receive commands on stdin. - Example CONNECTION_COMMANDS include: + Example CONNECTION_COMMANDs include: ssh some.host.com bash @@ -107,10 +107,16 @@ See help with `file-replicate --help`: replicate cycle. --gitignore / --no-gitignore Use .gitignore (or not) to filter files. --debugging Print debugging information. + --local-tar-gnu Local tar is gnu tar. + --local-tar-bsd Local tar is bsd tar. + --local-tar-gnu-prefix Use gtar as gnu tar locally. + --local-tar-detect Attempt to detect local tar flavor. + --remote-tar-gnu Remote tar is gnu tar + --remote-tar-gnu-prefix Use gtar as gnu tar remotely. + --remote-tar-detect Attempt to detect remote tar flavor. --version Show the version and exit. --help Show this message and exit. - For example, to replicate files from local directory `my_project_dir` to directory `/home/code/my_project_dir` on remote machine called `my.server.com`: @@ -133,16 +139,10 @@ but replicates into the local `/tmp/my_project_dir`: The unit tests use this degenerate approach to test the tool. -# Limitations - -Due to limitations with inotify (race conditions around watching for changes in newly created directories), it -is possible that the watching-for-changes phase becomes out of step. In which case, just restart the whole program. -The tool includes some self-restarting behaviour, but ultimately a full restart may sometimes be needed. - -Information printed to stdout indicates when this happens. - # Tests +## Linux + ============================= test session starts ============================== platform linux -- Python 3.6.7, pytest-3.10.1, py-1.7.0, pluggy-0.8.0 -- /home/tcorbettclark/.cache/pypoetry/virtualenvs/file-replicator-py3.6/bin/python cachedir: .pytest_cache @@ -158,6 +158,41 @@ Information printed to stdout indicates when this happens. tests/test_lib.py::test_detect_and_copy_new_file_in_new_directories PASSED [100%] =========================== 8 passed in 3.95 seconds =========================== +## MacOS (darwin) + + ================================================ test session starts ================================================= + platform darwin -- Python 3.7.2, pytest-3.10.1, py-1.7.0, pluggy-0.8.0 -- /Users/peter/.local/share/virtualenvs/fr/bin/python + cachedir: .pytest_cache + rootdir: /Users/peter/devel/file-replicator, inifile: + collected 18 items + + tests/test_lib.py::test_empty_directories_are_copied PASSED [ 5%] + tests/test_lib.py::test_copy_one_file PASSED [ 11%] + tests/test_lib.py::test_copy_file_with_unusual_characters_in_name PASSED [ 16%] + tests/test_lib.py::test_make_missing_parent_directories PASSED [ 22%] + tests/test_lib.py::test_replicate_all_files PASSED [ 27%] + tests/test_lib.py::test_detect_and_copy_new_file PASSED [ 33%] + tests/test_lib.py::test_detect_and_copy_modified_file PASSED [ 38%] + tests/test_lib.py::test_detect_and_copy_new_file_in_new_directories PASSED [ 44%] + tests/test_tar_adapter.py::test_gnu_tar_adapter[] PASSED [ 50%] + tests/test_tar_adapter.py::test_gnu_tar_adapter[g] PASSED [ 55%] + tests/test_tar_adapter.py::test_bsd_tar_adapter PASSED [ 61%] + tests/test_tar_adapter.py::test_detect_real_local_tar PASSED [ 66%] + tests/test_tar_adapter.py::test_detect_tar[mock_tar0-GnuTarAdapter] PASSED [ 72%] + tests/test_tar_adapter.py::test_detect_tar[mock_tar1-GnuTarAdapter] PASSED [ 77%] + tests/test_tar_adapter.py::test_detect_tar[mock_tar2-BsdTarAdapter] PASSED [ 83%] + tests/test_tar_adapter.py::test_detect_tar[mock_tar3-BusyBoxTarAdapter] PASSED [ 88%] + tests/test_tar_adapter.py::test_detect_tar[mock_tar4-NoneType] PASSED [ 94%] + tests/test_tar_adapter.py::test_no_tar_cmd PASSED [100%] + + ================================================== warnings summary ================================================== + /Users/peter/.local/share/virtualenvs/fr/lib/python3.7/site-packages/watchdog/utils/bricks.py:175 + /Users/peter/.local/share/virtualenvs/fr/lib/python3.7/site-packages/watchdog/utils/bricks.py:175: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working + class OrderedSet(collections.MutableSet): + + -- Docs: https://docs.pytest.org/en/latest/warnings.html + ======================================= 18 passed, 1 warnings in 4.03 seconds ======================================== + # Contributions Pull-requests are welcome! Please consider including tests and updating docs at the same time. diff --git a/pyproject.toml b/pyproject.toml index 7184391..0420f8a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "file-replicator" -version = "0.1.10" +version = "0.2.0" license = "MIT" authors = ["Timothy Corbett-Clark "] description = "Replicate files to another computer for remote development" @@ -11,9 +11,9 @@ homepage = "https://github.com/tcorbettclark/file-replicator" [tool.poetry.dependencies] python = "^3.6" -inotify = "^0.2.10" click = "^7.0" pathspec = "^0.5.9" +watchdog = "^0.9.0" [tool.poetry.dev-dependencies] pytest = "^3.0" From e5eca36bf5e3af7ae2026965f49fac942827e7c7 Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 16:47:40 +0200 Subject: [PATCH 5/9] updated package version and poetry dependencies --- README.md | 2 ++ file_replicator/__init__.py | 2 +- poetry.lock | 60 ++++++++++++++++++++++++------------- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 091597a..cadd86d 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,8 @@ The unit tests use this degenerate approach to test the tool. ## MacOS (darwin) +Currently tests expect a working Gnu tar (or gtar) available and will use that as both receiver and sender. + ================================================ test session starts ================================================= platform darwin -- Python 3.7.2, pytest-3.10.1, py-1.7.0, pluggy-0.8.0 -- /Users/peter/.local/share/virtualenvs/fr/bin/python cachedir: .pytest_cache diff --git a/file_replicator/__init__.py b/file_replicator/__init__.py index 569b121..d3ec452 100644 --- a/file_replicator/__init__.py +++ b/file_replicator/__init__.py @@ -1 +1 @@ -__version__ = "0.1.10" +__version__ = "0.2.0" diff --git a/poetry.lock b/poetry.lock index 28ba2d9..e0f9bed 100644 --- a/poetry.lock +++ b/poetry.lock @@ -6,6 +6,14 @@ optional = false python-versions = "*" version = "1.4.3" +[[package]] +category = "main" +description = "An unobtrusive argparse wrapper with natural syntax" +name = "argh" +optional = false +python-versions = "*" +version = "0.26.2" + [[package]] category = "dev" description = "Atomic file writes." @@ -53,17 +61,6 @@ optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" version = "0.4.1" -[[package]] -category = "main" -description = "An adapter to Linux kernel support for inotify directory-watching." -name = "inotify" -optional = false -python-versions = "*" -version = "0.2.10" - -[package.dependencies] -nose = "*" - [[package]] category = "dev" description = "A Python utility / library to sort Python imports." @@ -85,19 +82,19 @@ six = ">=1.0.0,<2.0.0" [[package]] category = "main" -description = "nose extends unittest to make testing easier" -name = "nose" +description = "Utility library for gitignore style pattern matching of file paths." +name = "pathspec" optional = false python-versions = "*" -version = "1.3.7" +version = "0.5.9" [[package]] category = "main" -description = "Utility library for gitignore style pattern matching of file paths." -name = "pathspec" +description = "File system general utilities" +name = "pathtools" optional = false python-versions = "*" -version = "0.5.9" +version = "0.1.2" [[package]] category = "dev" @@ -133,6 +130,14 @@ py = ">=1.5.0" setuptools = "*" six = ">=1.10.0" +[[package]] +category = "main" +description = "YAML parser and emitter for Python" +name = "pyyaml" +optional = false +python-versions = "*" +version = "3.13" + [[package]] category = "dev" description = "Python 2 and 3 compatibility utilities" @@ -149,24 +154,39 @@ optional = false python-versions = "*" version = "0.10.0" +[[package]] +category = "main" +description = "Filesystem events monitoring" +name = "watchdog" +optional = false +python-versions = "*" +version = "0.9.0" + +[package.dependencies] +PyYAML = ">=3.10" +argh = ">=0.24.1" +pathtools = ">=0.1.1" + [metadata] -content-hash = "ee2dd6fed97284aa0abea7f43a3c7a2c857a877e70055753dd7bbca36ac07478" +content-hash = "b28fb208cb3caaa7daff6aa5139aae128b47bd3be6a4924bf1a929b9f5aac6ef" python-versions = "^3.6" [metadata.hashes] appdirs = ["9e5896d1372858f8dd3344faf4e5014d21849c756c8d5701f78f8a103b372d92", "d8b24664561d0d34ddfaec54636d502d7cea6e29c3eaf68f3df6180863e2166e"] +argh = ["a9b3aaa1904eeb78e32394cd46c6f37ac0fb4af6dc488daa58971bdc7d7fcaf3", "e9535b8c84dc9571a48999094fda7f33e63c3f1b74f3e5f3ac0105a58405bb65"] atomicwrites = ["0312ad34fcad8fac3704d441f7b317e50af620823353ec657a53e981f92920c0", "ec9ae8adaae229e4f8446952d204a3e4b5fdd2d099f9be3aaf556120135fb3ee"] attrs = ["10cbf6e27dbce8c30807caf056c8eb50917e0eaafe86347671b57254006c3e69", "ca4be454458f9dec299268d472aaa5a11f67a4ff70093396e1ceae9c76cf4bbb"] black = ["817243426042db1d36617910df579a54f1afd659adb96fc5032fcf4b36209739", "e030a9a28f542debc08acceb273f228ac422798e5215ba2a791a6ddeaaca22a5"] click = ["2335065e6395b9e67ca716de5f7526736bfa6ceead690adf616d925bdc622b13", "5b94b49521f6456670fdb30cd82a4eca9412788a93fa6dd6df72c94d5a8ff2d7"] colorama = ["05eed71e2e327246ad6b38c540c4a3117230b19679b875190486ddd2d721422d", "f8ac84de7840f5b9c4e3347b3c1eaa50f7e49c2b07596221daec5edaabbd7c48"] -inotify = ["397f8785450e41f606fe4eb6f5e8e0a1c70b354b56495225fc6c6fe7e07db0c9", "974a623a338482b62e16d4eb705fb863ed33ec178680fc3e96ccdf0df6c02a07"] isort = ["1153601da39a25b14ddc54955dbbacbb6b2d19135386699e2ad58517953b34af", "b9c40e9750f3d77e6e4d441d8b0266cf555e7cdabdcff33c4fd06366ca761ef8", "ec9ef8f4a9bc6f71eec99e1806bfa2de401650d996c59330782b89a5555c1497"] more-itertools = ["38a936c0a6d98a38bcc2d03fdaaedaba9f412879461dd2ceff8d37564d6522e4", "c0a5785b1109a6bd7fac76d6837fd1feca158e54e521ccd2ae8bfe393cc9d4fc", "fe7a7cae1ccb57d33952113ff4fa1bc5f879963600ed74918f1236e212ee50b9"] -nose = ["9ff7c6cc443f8c51994b34a667bbcf45afd6d945be7477b52e97516fd17c53ac", "dadcddc0aefbf99eea214e0f1232b94f2fa9bd98fa8353711dacb112bfcbbb2a", "f1bffef9cbc82628f6e7d7b40d7e255aefaa1adb6a1b1d26c69a8b79e6208a98"] pathspec = ["54a5eab895d89f342b52ba2bffe70930ef9f8d96e398cccf530d21fa0516a873"] +pathtools = ["7c35c5421a39bb82e58018febd90e3b6e5db34c5443aaaf742b3f33d4655f1c0"] pluggy = ["447ba94990e8014ee25ec853339faf7b0fc8050cdc3289d4d71f7f410fb90095", "bde19360a8ec4dfd8a20dcb811780a30998101f078fc7ded6162f0076f50508f"] py = ["bf92637198836372b520efcba9e020c330123be8ce527e535d185ed4b6f45694", "e76826342cefe3c3d5f7e8ee4316b80d1dd8a300781612ddbc765c17ba25a6c6"] pytest = ["3f193df1cfe1d1609d4c583838bea3d532b18d6160fd3f55c9447fdca30848ec", "e246cf173c01169b9617fc07264b7b1316e78d7a650055235d6d897bc80d9660"] +pyyaml = ["3d7da3009c0f3e783b2c873687652d83b1bbfd5c88e9813fb7e5b03c0dd3108b", "3ef3092145e9b70e3ddd2c7ad59bdd0252a94dfe3949721633e41344de00a6bf", "40c71b8e076d0550b2e6380bada1f1cd1017b882f7e16f09a65be98e017f211a", "558dd60b890ba8fd982e05941927a3911dc409a63dcb8b634feaa0cda69330d3", "a7c28b45d9f99102fa092bb213aa12e0aaf9a6a1f5e395d36166639c1f96c3a1", "aa7dd4a6a427aed7df6fb7f08a580d68d9b118d90310374716ae90b710280af1", "bc558586e6045763782014934bfaf39d48b8ae85a2713117d16c39864085c613", "d46d7982b62e0729ad0175a9bc7e10a566fc07b224d2c79fafb5e032727eaa04", "d5eef459e30b09f5a098b9cea68bebfeb268697f78d647bd255a085371ac7f3f", "e01d3203230e1786cd91ccfdc8f8454c8069c91bee3962ad93b87a4b2860f537", "e170a9e6fcfd19021dd29845af83bb79236068bf5fd4df3327c1be18182b2531"] six = ["3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c", "d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73"] toml = ["229f81c57791a41d65e399fc06bf0848bab550a9dfd5ed66df18ce5f05e73d5c", "235682dd292d5899d361a811df37e04a8828a5b1da3115886b73cf81ebc9100e", "f1db651f9657708513243e61e6cc67d101a39bad662eaa9b5546f789338e07a3"] +watchdog = ["965f658d0732de3188211932aeb0bb457587f04f63ab4c1e33eab878e9de961d"] From f9f968da5e55007683d22a42c9b4213458395cdd Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 22:36:47 +0200 Subject: [PATCH 6/9] fix invocation to replicate_files_on_change --- file_replicator/cli.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/file_replicator/cli.py b/file_replicator/cli.py index b7dd828..a502b63 100644 --- a/file_replicator/cli.py +++ b/file_replicator/cli.py @@ -166,11 +166,6 @@ def main( src_dir, copy_file, use_gitignore=gitignore, debugging=debugging ) if replicate_on_change: - while replicate_files_on_change( + replicate_files_on_change( src_dir, copy_file, use_gitignore=gitignore, debugging=debugging - ): - click.secho( - "Restarting watchers after detecting a new directory. Consider restarting!", - fg="red", - bold="true", - ) + ) From 2231a858fb20ba0e923124b9ba586b94bc2d5e17 Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 22:38:31 +0200 Subject: [PATCH 7/9] add a callback upon observer setup for testability, and update tests --- file_replicator/lib.py | 9 ++++++++- tests/test_lib.py | 23 +++++++++++------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/file_replicator/lib.py b/file_replicator/lib.py index d5037b2..f15666d 100644 --- a/file_replicator/lib.py +++ b/file_replicator/lib.py @@ -160,7 +160,12 @@ def raise_if_timeout(last_change, timeout): def replicate_files_on_change( - src_dir, copy_file, timeout=None, use_gitignore=True, debugging=False + src_dir, + copy_file, + timeout=None, + use_gitignore=True, + debugging=False, + notify_observer_up=None, ): """Wait for changes to files in src_dir and copy with copy_file(). @@ -179,6 +184,8 @@ def replicate_files_on_change( if debugging: print("Starting observer") observer.start() + if notify_observer_up is not None: + notify_observer_up() try: while True: if timeout: diff --git a/tests/test_lib.py b/tests/test_lib.py index ee46e3d..42c6588 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -119,7 +119,6 @@ def test_detect_and_copy_new_file(local_tar): # Make another file in a short while (after the watcher has started). timer = threading.Timer(0.1, make_test_file, args=(src_dir, "b.txt", "goodbye")) - timer.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -132,9 +131,9 @@ def test_detect_and_copy_new_file(local_tar): with make_file_replicator( local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: - while timer.is_alive(): - while replicate_files_on_change(src_dir, copy_file, timeout=0.2): - pass + replicate_files_on_change( + src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + ) # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -145,6 +144,8 @@ def test_detect_and_copy_new_file(local_tar): # Double check that contents is correct too. assert_file_contains(os.path.join(dest_parent_dir, "test/b.txt"), "goodbye") + timer.join() + def test_detect_and_copy_modified_file(local_tar): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: @@ -157,7 +158,6 @@ def test_detect_and_copy_modified_file(local_tar): timer = threading.Timer( 0.1, make_test_file, args=(src_dir, "a.txt", "hello again") ) - timer.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -168,9 +168,9 @@ def test_detect_and_copy_modified_file(local_tar): with make_file_replicator( local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: - while timer.is_alive(): - while replicate_files_on_change(src_dir, copy_file, timeout=0.2): - pass + replicate_files_on_change( + src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + ) # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -191,7 +191,6 @@ def test_detect_and_copy_new_file_in_new_directories(local_tar): timer = threading.Timer( 0.1, make_test_file, args=(src_dir, "a/b/c/d/e/a.txt", "hello again") ) - timer.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -204,9 +203,9 @@ def test_detect_and_copy_new_file_in_new_directories(local_tar): with make_file_replicator( local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: - while timer.is_alive(): - while replicate_files_on_change(src_dir, copy_file, timeout=0.2): - pass + replicate_files_on_change( + src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + ) # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) From a0e359232fa89467ff8a8e1d04d7910635ea4440 Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Mon, 28 Jan 2019 22:39:13 +0200 Subject: [PATCH 8/9] fix display reason for exit on timeout/keyboard interrupt --- file_replicator/lib.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/file_replicator/lib.py b/file_replicator/lib.py index f15666d..50b9f1b 100644 --- a/file_replicator/lib.py +++ b/file_replicator/lib.py @@ -149,14 +149,14 @@ def dispatch(self, event): super().dispatch(event) -class NoChangeTimeoutError(Exception): +class NoChangeTimeout(Exception): pass def raise_if_timeout(last_change, timeout): elapsed = time.time() - last_change if elapsed > timeout: - raise NoChangeTimeoutError(f"No changes detected for {elapsed} seconds.") + raise NoChangeTimeout(f"No changes detected for {elapsed} seconds.") def replicate_files_on_change( @@ -191,8 +191,8 @@ def replicate_files_on_change( if timeout: raise_if_timeout(event_handler.last_event_timestamp, timeout) time.sleep(0.5) - except (KeyboardInterrupt, NoChangeTimeoutError) as e: + except (KeyboardInterrupt, NoChangeTimeout) as e: observer.stop() if debugging: - print("Exitting on {e}") + print(f"Exitting on {type(e).__name__}: {e}") observer.join() From 748fd5da3ce87fa206739efe16571bc3576d2033 Mon Sep 17 00:00:00 2001 From: Petre Mierlutiu Date: Tue, 5 Feb 2019 22:29:24 +0200 Subject: [PATCH 9/9] updates to tests --- file_replicator/lib.py | 38 ++++++++++++++++++----- tests/test_lib.py | 69 +++++++++++++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/file_replicator/lib.py b/file_replicator/lib.py index 50b9f1b..6f8d862 100644 --- a/file_replicator/lib.py +++ b/file_replicator/lib.py @@ -153,24 +153,33 @@ class NoChangeTimeout(Exception): pass +class ConditionalTermination(Exception): + pass + + def raise_if_timeout(last_change, timeout): elapsed = time.time() - last_change if elapsed > timeout: raise NoChangeTimeout(f"No changes detected for {elapsed} seconds.") +TAIL_TIMEOUT = 2 + + def replicate_files_on_change( src_dir, copy_file, timeout=None, use_gitignore=True, debugging=False, - notify_observer_up=None, + observer_up_event=None, + terminate_event=None, ): """Wait for changes to files in src_dir and copy with copy_file(). If provided, the timeout indicates when to return after that many seconds of no change. """ + print("debug: replicate on change start") src_dir = os.path.abspath(src_dir) if use_gitignore: spec = get_pathspec(src_dir, use_gitignore) @@ -184,15 +193,30 @@ def replicate_files_on_change( if debugging: print("Starting observer") observer.start() - if notify_observer_up is not None: - notify_observer_up() + if observer_up_event is not None: + while not observer.is_alive(): + pass + # wait for event listeners to settle + time.sleep(0.5) + observer_up_event.set() + print("notified observer up") try: while True: + time.sleep(0.5) if timeout: raise_if_timeout(event_handler.last_event_timestamp, timeout) - time.sleep(0.5) - except (KeyboardInterrupt, NoChangeTimeout) as e: - observer.stop() + if terminate_event and terminate_event.is_set(): + raise ConditionalTermination("Termination condition was set.") + except (KeyboardInterrupt, NoChangeTimeout, ConditionalTermination) as e: if debugging: print(f"Exitting on {type(e).__name__}: {e}") - observer.join() + finally: + now = time.time() + # still, dispatch the existing change events before we part ways + while time.time() - now < TAIL_TIMEOUT and not observer.event_queue.empty(): + print("flushing events") + observer.dispatch_events(observer.event_queue, observer.timeout) + left = now + TAIL_TIMEOUT - time.time() + observer.stop() + observer.join(timeout=max(0, left)) + print(f"debug: finished replicate on change with {left} left") diff --git a/tests/test_lib.py b/tests/test_lib.py index 42c6588..e8ffbee 100644 --- a/tests/test_lib.py +++ b/tests/test_lib.py @@ -1,8 +1,10 @@ +from collections import namedtuple import contextlib import os import os.path import shutil import tempfile +import time import threading import pytest @@ -29,12 +31,20 @@ def temp_directory(): shutil.rmtree(directory) -def make_test_file(src_dir, relative_path, text): - """Create a test file of text.""" +def make_test_file(src_dir, relative_path, text, events=None): + """Create a test file of text, optionally blocking on event and notifying when done.""" + if events and events.wait_on: + # wait, but timeout -- in case something goes wrong + events.wait_on.wait(5) filename = os.path.join(src_dir, relative_path) os.makedirs(os.path.dirname(filename), exist_ok=True) with open(filename, "w") as f: f.write(text) + if events and events.created: + # allow file change to be picked up by a filesystem observer + time.sleep(0.1) + events.created.set() + print("notified created") def assert_file_contains(filename, text): @@ -110,7 +120,15 @@ def test_replicate_all_files(local_tar): assert_file_contains(os.path.join(src_dir, "b/c.txt"), "goodbye") -def test_detect_and_copy_new_file(local_tar): +EventPair = namedtuple("EventPair", ["wait_on", "created"]) + + +@pytest.fixture +def delay_events(): + return EventPair(threading.Event(), threading.Event()) + + +def test_detect_and_copy_new_file(local_tar, delay_events): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -118,7 +136,10 @@ def test_detect_and_copy_new_file(local_tar): make_test_file(src_dir, "a.txt", "hello") # Make another file in a short while (after the watcher has started). - timer = threading.Timer(0.1, make_test_file, args=(src_dir, "b.txt", "goodbye")) + delayed_t = threading.Thread( + target=make_test_file, args=(src_dir, "b.txt", "goodbye", delay_events) + ) + delayed_t.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -128,12 +149,19 @@ def test_detect_and_copy_new_file(local_tar): # Watch for changes and copy files, and stop after short while of inactvitiy. # The second file (see above) should be created during this internval. + print("before repl") with make_file_replicator( local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: replicate_files_on_change( - src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + src_dir, + copy_file, + observer_up_event=delay_events.wait_on, + terminate_event=delay_events.created, + debugging=True, ) + delayed_t.join() + print("after repl") # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -144,10 +172,8 @@ def test_detect_and_copy_new_file(local_tar): # Double check that contents is correct too. assert_file_contains(os.path.join(dest_parent_dir, "test/b.txt"), "goodbye") - timer.join() - -def test_detect_and_copy_modified_file(local_tar): +def test_detect_and_copy_modified_file(local_tar, delay_events): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -155,9 +181,10 @@ def test_detect_and_copy_modified_file(local_tar): make_test_file(src_dir, "a.txt", "hello") # Change that file in a short while (after the watcher has started). - timer = threading.Timer( - 0.1, make_test_file, args=(src_dir, "a.txt", "hello again") + delayed_t = threading.Thread( + target=make_test_file, args=(src_dir, "a.txt", "hello again", delay_events) ) + delayed_t.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -169,8 +196,13 @@ def test_detect_and_copy_modified_file(local_tar): local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: replicate_files_on_change( - src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + src_dir, + copy_file, + observer_up_event=delay_events.wait_on, + terminate_event=delay_events.created, + debugging=True, ) + delayed_t.join() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -180,7 +212,7 @@ def test_detect_and_copy_modified_file(local_tar): assert_file_contains(os.path.join(dest_parent_dir, "test/a.txt"), "hello again") -def test_detect_and_copy_new_file_in_new_directories(local_tar): +def test_detect_and_copy_new_file_in_new_directories(local_tar, delay_events): with temp_directory() as src_parent_dir, temp_directory() as dest_parent_dir: src_dir = os.path.join(src_parent_dir, "test") @@ -188,9 +220,11 @@ def test_detect_and_copy_new_file_in_new_directories(local_tar): make_test_file(src_dir, "a.txt", "hello") # Create a new file in nested new directories in a short while (after the watcher has started). - timer = threading.Timer( - 0.1, make_test_file, args=(src_dir, "a/b/c/d/e/a.txt", "hello again") + delayed_t = threading.Thread( + target=make_test_file, + args=(src_dir, "a/b/c/d/e/a.txt", "hello again", delay_events), ) + delayed_t.start() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt")) @@ -204,8 +238,13 @@ def test_detect_and_copy_new_file_in_new_directories(local_tar): local_tar, local_tar, src_dir, dest_parent_dir, ("bash",) ) as copy_file: replicate_files_on_change( - src_dir, copy_file, timeout=0.2, notify_observer_up=timer.start + src_dir, + copy_file, + observer_up_event=delay_events.wait_on, + terminate_event=delay_events.created, + debugging=True, ) + delayed_t.join() # Confirm we have the files we expect. assert os.path.exists(os.path.join(src_dir, "a.txt"))