diff --git a/src/basic_memory/cli/commands/cloud/rclone_commands.py b/src/basic_memory/cli/commands/cloud/rclone_commands.py index 0894241e..72343c9b 100644 --- a/src/basic_memory/cli/commands/cloud/rclone_commands.py +++ b/src/basic_memory/cli/commands/cloud/rclone_commands.py @@ -9,11 +9,14 @@ This module provides simplified, project-scoped rclone operations: Replaces tenant-wide sync with project-scoped workflows. """ +import re import subprocess from dataclasses import dataclass +from functools import lru_cache from pathlib import Path from typing import Optional +from loguru import logger from rich.console import Console from basic_memory.cli.commands.cloud.rclone_installer import is_rclone_installed @@ -21,6 +24,9 @@ from basic_memory.utils import normalize_project_path console = Console() +# Minimum rclone version for --create-empty-src-dirs support +MIN_RCLONE_VERSION_EMPTY_DIRS = (1, 64, 0) + class RcloneError(Exception): """Exception raised for rclone command errors.""" @@ -43,6 +49,42 @@ def check_rclone_installed() -> None: ) +@lru_cache(maxsize=1) +def get_rclone_version() -> tuple[int, int, int] | None: + """Get rclone version as (major, minor, patch) tuple. + + Returns: + Version tuple like (1, 64, 2), or None if version cannot be determined. + + Note: + Result is cached since rclone version won't change during runtime. + """ + try: + result = subprocess.run(["rclone", "version"], capture_output=True, text=True, timeout=10) + # Parse "rclone v1.64.2" or "rclone v1.60.1-DEV" + match = re.search(r"v(\d+)\.(\d+)\.(\d+)", result.stdout) + if match: + version = (int(match.group(1)), int(match.group(2)), int(match.group(3))) + logger.debug(f"Detected rclone version: {version}") + return version + except Exception as e: + logger.warning(f"Could not determine rclone version: {e}") + return None + + +def supports_create_empty_src_dirs() -> bool: + """Check if installed rclone supports --create-empty-src-dirs flag. + + Returns: + True if rclone version >= 1.64.0, False otherwise. + """ + version = get_rclone_version() + if version is None: + # If we can't determine version, assume older and skip the flag + return False + return version >= MIN_RCLONE_VERSION_EMPTY_DIRS + + @dataclass class SyncProject: """Project configured for cloud sync. @@ -218,7 +260,6 @@ def project_bisync( "bisync", str(local_path), remote_path, - "--create-empty-src-dirs", "--resilient", "--conflict-resolve=newer", "--max-delete=25", @@ -229,6 +270,10 @@ def project_bisync( str(state_path), ] + # Add --create-empty-src-dirs if rclone version supports it (v1.64+) + if supports_create_empty_src_dirs(): + cmd.append("--create-empty-src-dirs") + if verbose: cmd.append("--verbose") else: diff --git a/tests/test_rclone_commands.py b/tests/test_rclone_commands.py index e68dc109..caefb6d3 100644 --- a/tests/test_rclone_commands.py +++ b/tests/test_rclone_commands.py @@ -6,16 +6,19 @@ from unittest.mock import MagicMock, patch import pytest from basic_memory.cli.commands.cloud.rclone_commands import ( + MIN_RCLONE_VERSION_EMPTY_DIRS, RcloneError, SyncProject, bisync_initialized, check_rclone_installed, get_project_bisync_state, get_project_remote, + get_rclone_version, project_bisync, project_check, project_ls, project_sync, + supports_create_empty_src_dirs, ) @@ -194,10 +197,12 @@ def test_project_sync_no_local_path(mock_is_installed): @patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") @patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") @patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") -def test_project_bisync_success(mock_bisync_init, mock_run, mock_is_installed): +@patch("basic_memory.cli.commands.cloud.rclone_commands.supports_create_empty_src_dirs") +def test_project_bisync_success(mock_supports_flag, mock_bisync_init, mock_run, mock_is_installed): """Test successful project bisync.""" mock_is_installed.return_value = True mock_bisync_init.return_value = True # Already initialized + mock_supports_flag.return_value = True # Mock version check mock_run.return_value = MagicMock(returncode=0) project = SyncProject( @@ -221,9 +226,8 @@ def test_project_bisync_success(mock_bisync_init, mock_run, mock_is_installed): @patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") -@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") @patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") -def test_project_bisync_requires_resync_first_time(mock_bisync_init, mock_run, mock_is_installed): +def test_project_bisync_requires_resync_first_time(mock_bisync_init, mock_is_installed): """Test that first bisync requires --resync flag.""" mock_is_installed.return_value = True mock_bisync_init.return_value = False # Not initialized @@ -238,16 +242,19 @@ def test_project_bisync_requires_resync_first_time(mock_bisync_init, mock_run, m project_bisync(project, "my-bucket") assert "requires --resync" in str(exc_info.value) - mock_run.assert_not_called() @patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") @patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") @patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") -def test_project_bisync_with_resync_flag(mock_bisync_init, mock_run, mock_is_installed): +@patch("basic_memory.cli.commands.cloud.rclone_commands.supports_create_empty_src_dirs") +def test_project_bisync_with_resync_flag( + mock_supports_flag, mock_bisync_init, mock_run, mock_is_installed +): """Test bisync with --resync flag for first time.""" mock_is_installed.return_value = True mock_bisync_init.return_value = False # Not initialized + mock_supports_flag.return_value = True # Mock version check mock_run.return_value = MagicMock(returncode=0) project = SyncProject( @@ -266,10 +273,14 @@ def test_project_bisync_with_resync_flag(mock_bisync_init, mock_run, mock_is_ins @patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") @patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") @patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") -def test_project_bisync_dry_run_skips_init_check(mock_bisync_init, mock_run, mock_is_installed): +@patch("basic_memory.cli.commands.cloud.rclone_commands.supports_create_empty_src_dirs") +def test_project_bisync_dry_run_skips_init_check( + mock_supports_flag, mock_bisync_init, mock_run, mock_is_installed +): """Test that dry-run skips initialization check.""" mock_is_installed.return_value = True mock_bisync_init.return_value = False # Not initialized + mock_supports_flag.return_value = True # Mock version check mock_run.return_value = MagicMock(returncode=0) project = SyncProject( @@ -479,3 +490,158 @@ def test_project_ls_checks_rclone_installed(mock_is_installed): assert "rclone is not installed" in str(exc_info.value) mock_is_installed.assert_called_once() + + +# Tests for rclone version detection + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +def test_get_rclone_version_parses_standard_version(mock_run): + """Test parsing standard rclone version output.""" + # Clear the lru_cache before test + get_rclone_version.cache_clear() + + mock_run.return_value = MagicMock( + stdout="rclone v1.64.2\n- os/version: darwin 23.0.0\n- os/arch: arm64\n" + ) + + version = get_rclone_version() + + assert version == (1, 64, 2) + mock_run.assert_called_once() + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +def test_get_rclone_version_parses_dev_version(mock_run): + """Test parsing rclone dev version output like v1.60.1-DEV.""" + get_rclone_version.cache_clear() + + mock_run.return_value = MagicMock(stdout="rclone v1.60.1-DEV\n- os/version: linux 5.15.0\n") + + version = get_rclone_version() + + assert version == (1, 60, 1) + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +def test_get_rclone_version_handles_invalid_output(mock_run): + """Test handling of invalid rclone version output.""" + get_rclone_version.cache_clear() + + mock_run.return_value = MagicMock(stdout="not a valid version string") + + version = get_rclone_version() + + assert version is None + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +def test_get_rclone_version_handles_exception(mock_run): + """Test handling of subprocess exception.""" + get_rclone_version.cache_clear() + + mock_run.side_effect = Exception("Command failed") + + version = get_rclone_version() + + assert version is None + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +def test_get_rclone_version_handles_timeout(mock_run): + """Test handling of subprocess timeout.""" + get_rclone_version.cache_clear() + from subprocess import TimeoutExpired + + mock_run.side_effect = TimeoutExpired(cmd="rclone version", timeout=10) + + version = get_rclone_version() + + assert version is None + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.get_rclone_version") +def test_supports_create_empty_src_dirs_true_for_new_version(mock_get_version): + """Test supports_create_empty_src_dirs returns True for v1.64+.""" + mock_get_version.return_value = (1, 64, 2) + + assert supports_create_empty_src_dirs() is True + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.get_rclone_version") +def test_supports_create_empty_src_dirs_true_for_exact_min_version(mock_get_version): + """Test supports_create_empty_src_dirs returns True for exactly v1.64.0.""" + mock_get_version.return_value = (1, 64, 0) + + assert supports_create_empty_src_dirs() is True + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.get_rclone_version") +def test_supports_create_empty_src_dirs_false_for_old_version(mock_get_version): + """Test supports_create_empty_src_dirs returns False for v1.60.""" + mock_get_version.return_value = (1, 60, 1) + + assert supports_create_empty_src_dirs() is False + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.get_rclone_version") +def test_supports_create_empty_src_dirs_false_for_unknown_version(mock_get_version): + """Test supports_create_empty_src_dirs returns False when version unknown.""" + mock_get_version.return_value = None + + assert supports_create_empty_src_dirs() is False + + +def test_min_rclone_version_constant(): + """Test MIN_RCLONE_VERSION_EMPTY_DIRS constant is set correctly.""" + assert MIN_RCLONE_VERSION_EMPTY_DIRS == (1, 64, 0) + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") +@patch("basic_memory.cli.commands.cloud.rclone_commands.supports_create_empty_src_dirs") +def test_project_bisync_includes_empty_dirs_flag_when_supported( + mock_supports_flag, mock_bisync_init, mock_run, mock_is_installed +): + """Test project_bisync includes --create-empty-src-dirs when supported.""" + mock_is_installed.return_value = True + mock_bisync_init.return_value = True + mock_supports_flag.return_value = True + mock_run.return_value = MagicMock(returncode=0) + + project = SyncProject( + name="research", + path="app/data/research", + local_sync_path="/tmp/research", + ) + + project_bisync(project, "my-bucket") + + cmd = mock_run.call_args[0][0] + assert "--create-empty-src-dirs" in cmd + + +@patch("basic_memory.cli.commands.cloud.rclone_commands.is_rclone_installed") +@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run") +@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized") +@patch("basic_memory.cli.commands.cloud.rclone_commands.supports_create_empty_src_dirs") +def test_project_bisync_excludes_empty_dirs_flag_when_not_supported( + mock_supports_flag, mock_bisync_init, mock_run, mock_is_installed +): + """Test project_bisync excludes --create-empty-src-dirs for older rclone.""" + mock_is_installed.return_value = True + mock_bisync_init.return_value = True + mock_supports_flag.return_value = False # Old rclone version + mock_run.return_value = MagicMock(returncode=0) + + project = SyncProject( + name="research", + path="app/data/research", + local_sync_path="/tmp/research", + ) + + project_bisync(project, "my-bucket") + + cmd = mock_run.call_args[0][0] + assert "--create-empty-src-dirs" not in cmd