diff --git a/src/git_pulsar/cli.py b/src/git_pulsar/cli.py index a0d5d76..e9a2ef1 100644 --- a/src/git_pulsar/cli.py +++ b/src/git_pulsar/cli.py @@ -14,9 +14,7 @@ from . import daemon, ops, service, system from .config import CONFIG_FILE, Config from .constants import ( - APP_LABEL, DEFAULT_IGNORES, - HOMEBREW_LABEL, LOG_FILE, PID_FILE, REGISTRY_FILE, @@ -38,27 +36,6 @@ def _get_ref(repo: GitRepo) -> str: return ops.get_backup_ref(repo.current_branch()) -def _is_service_enabled() -> bool: - """Checks if the system service is currently loaded and active. - - Supports both launchd (macOS) and systemd (Linux). - - Returns: - bool: True if the service is active/loaded, False otherwise. - """ - if sys.platform == "darwin": - res = subprocess.run(["launchctl", "list"], capture_output=True, text=True) - return HOMEBREW_LABEL in res.stdout - elif sys.platform.startswith("linux"): - res = subprocess.run( - ["systemctl", "--user", "is-active", f"{APP_LABEL}.timer"], - capture_output=True, - text=True, - ) - return res.stdout.strip() == "active" - return False - - def _analyze_logs(hours: int = 24) -> list[str]: """ Scans the daemon log for error messages that occurred within a recent time window. @@ -194,7 +171,7 @@ def show_status() -> None: pid_running = False # Check if the system service is scheduled/enabled. - service_enabled = _is_service_enabled() + service_enabled = service.is_service_enabled() if pid_running: status_text = "Active (Running)" @@ -218,11 +195,9 @@ def show_status() -> None: # Check registration status. is_registered = False - if REGISTRY_FILE.exists(): - with open(REGISTRY_FILE, "r") as f: - registered = {line.strip() for line in f if line.strip()} - if str(cwd) in registered: - is_registered = True + # Use the system helper (returns list[Path]) + if cwd in system.get_registered_repos(): + is_registered = True if not is_registered: console.print( @@ -277,8 +252,7 @@ def show_status() -> None: # Display global repository count if not currently in a repository. elif REGISTRY_FILE.exists(): - with open(REGISTRY_FILE) as f: - count = len([line for line in f if line.strip()]) + count = len(system.get_registered_repos()) console.print(f"[dim]Watching {count} repositories.[/dim]") @@ -314,11 +288,9 @@ def list_repos() -> None: table.add_column("Status") table.add_column("Last Backup", justify="right", style="dim") - with open(REGISTRY_FILE, "r") as f: - lines = [line.strip() for line in f if line.strip()] + repos = system.get_registered_repos() - for path_str in lines: - path = Path(path_str) + for path in repos: display_path = str(path).replace(str(Path.home()), "~") status_text = "Unknown" @@ -362,19 +334,17 @@ def unregister_repo() -> None: console.print("Registry is empty.", style="yellow") return - with open(REGISTRY_FILE, "r") as f: - lines = [line.strip() for line in f if line.strip()] - - if cwd not in lines: + current_paths = [str(p) for p in system.get_registered_repos()] + if cwd not in current_paths: console.print( f"Current path not registered: [cyan]{cwd}[/cyan]", style="yellow" ) return with open(REGISTRY_FILE, "w") as f: - for line in lines: - if line != cwd: - f.write(f"{line}\n") + for path in current_paths: + if path != cwd: + f.write(f"{path}\n") console.print(f"✔ Unregistered: [cyan]{cwd}[/cyan]", style="green") @@ -386,17 +356,15 @@ def run_doctor() -> None: # Verify and clean the registry. with console.status("[bold blue]Checking Registry...", spinner="dots"): - if not REGISTRY_FILE.exists(): + repos = system.get_registered_repos() + if not repos and not REGISTRY_FILE.exists(): console.print(" [green]✔ Registry empty/clean.[/green]") else: - with open(REGISTRY_FILE, "r") as f: - lines = [line.strip() for line in f if line.strip()] - valid_lines = [] fixed = False - for line in lines: - if Path(line).exists(): - valid_lines.append(line) + for p in repos: + if p.exists(): + valid_lines.append(str(p)) else: fixed = True @@ -411,7 +379,7 @@ def run_doctor() -> None: # Check daemon status. with console.status("[bold blue]Checking Daemon...", spinner="dots"): - if _is_service_enabled(): + if service.is_service_enabled(): console.print(" [green]✔ Daemon is active.[/green]") else: console.print( diff --git a/src/git_pulsar/config.py b/src/git_pulsar/config.py index 91c42bf..028adcd 100644 --- a/src/git_pulsar/config.py +++ b/src/git_pulsar/config.py @@ -100,6 +100,9 @@ class Config: files: FilesConfig = field(default_factory=FilesConfig) daemon: DaemonConfig = field(default_factory=DaemonConfig) + # Cache for the base global configuration + _global_cache: "Config | None" = None + @classmethod def load(cls, repo_path: Path | None = None) -> "Config": """Loads and merges configuration from defaults, global, and local sources. @@ -110,11 +113,15 @@ def load(cls, repo_path: Path | None = None) -> "Config": Returns: Config: The fully merged configuration object. """ - instance = cls() - - # 1. Load Global Config - if CONFIG_FILE.exists(): - instance._merge_from_file(CONFIG_FILE) + # 1. Load or Retrieve Global Config + if cls._global_cache is None: + instance = cls() + if CONFIG_FILE.exists(): + instance._merge_from_file(CONFIG_FILE) + cls._global_cache = instance + + # Start with a copy of the cached global config + instance = replace(cls._global_cache) # 2. Load Local Config (if applicable) if repo_path: diff --git a/src/git_pulsar/daemon.py b/src/git_pulsar/daemon.py index 0a05833..534c0ba 100644 --- a/src/git_pulsar/daemon.py +++ b/src/git_pulsar/daemon.py @@ -181,8 +181,7 @@ def is_repo_busy(repo_path: Path, interactive: bool = False) -> bool: except OSError: pass # File vanished (race resolved). - # B. Wait-and-see (Micro-retry) to handle transient operations. - time.sleep(1.0) + # B. Fail fast if lock still exists if lock_file.exists(): return True @@ -205,7 +204,8 @@ def has_large_files(repo_path: Path, config: Config) -> bool: try: cmd = ["git", "ls-files", "--others", "--modified", "--exclude-standard"] candidates = subprocess.check_output(cmd, cwd=repo_path, text=True).splitlines() - except subprocess.CalledProcessError: + except subprocess.CalledProcessError as e: + logger.warning(f"Large file scan failed for {repo_path.name}: {e}") return False for name in candidates: @@ -221,7 +221,8 @@ def has_large_files(repo_path: Path, config: Config) -> bool: ) SYSTEM.notify("Backup Aborted", f"File >{limit_mb}MB detected: {name}") return True - except OSError: + except OSError as e: + logger.warning(f"Failed to check size of file {name}: {e}") continue return False @@ -361,7 +362,8 @@ def _get_ref_timestamp(repo: GitRepo, ref: str) -> int: try: ts = repo._run(["log", "-1", "--format=%ct", ref]) return int(ts.strip()) - except Exception: + except Exception as e: + logger.debug(f"Could not get timestamp for {ref}: {e}") return 0 @@ -392,15 +394,10 @@ def run_backup(original_path_str: str, interactive: bool = False) -> None: if not current_branch: return - slug = system.get_identity_slug() - namespace = config.core.backup_branch - # Define Refs - local_backup_ref = f"refs/heads/{namespace}/{slug}/{current_branch}" - remote_backup_ref = ( - f"refs/remotes/{config.core.remote_name}/" - f"{namespace}/{slug}/{current_branch}" - ) + local_backup_ref = ops.get_backup_ref(current_branch) + ref_suffix = local_backup_ref.replace("refs/heads/", "") + remote_backup_ref = f"refs/remotes/{config.core.remote_name}/{ref_suffix}" # --- COMMIT PHASE --- last_commit_ts = _get_ref_timestamp(repo, local_backup_ref) @@ -409,6 +406,10 @@ def run_backup(original_path_str: str, interactive: bool = False) -> None: if time_since_commit >= config.daemon.commit_interval: with temporary_index(repo_path) as env: # Stage current working directory into temp index. + # Use wrapper method if available, or repo._run(["add", "."], env=env) + repo.add_all() + # Note: GitRepo.add_all() in wrapper doesn't accept env. + # Keeping manual run with env. repo._run(["add", "."], env=env) # Write Tree. @@ -430,9 +431,16 @@ def run_backup(original_path_str: str, interactive: bool = False) -> None: if should_commit: timestamp = datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S") + + # Use wrapper method commit_oid = repo.commit_tree( - tree_oid, parents, f"Shadow backup {timestamp}", env=env + tree=tree_oid, + parents=parents, + message=f"Shadow backup {timestamp}", + env=env, ) + + # Use wrapper method repo.update_ref(local_backup_ref, commit_oid, parent_backup) if interactive: @@ -492,7 +500,9 @@ def main(interactive: bool = False) -> None: """ setup_logging(interactive) - if not REGISTRY_FILE.exists(): + repos = [str(p) for p in system.get_registered_repos()] + + if not repos: if interactive: console.print( "[yellow]Registry empty. Run 'git-pulsar' in " @@ -500,9 +510,6 @@ def main(interactive: bool = False) -> None: ) return - with open(REGISTRY_FILE, "r") as f: - repos = [line.strip() for line in f if line.strip()] - # Set a timeout handler for stalled network mounts. def timeout_handler(_signum: int, _frame: FrameType | None) -> None: raise TimeoutError("Repo access timed out") diff --git a/src/git_pulsar/git_wrapper.py b/src/git_pulsar/git_wrapper.py index cf85b97..21f4e6b 100644 --- a/src/git_pulsar/git_wrapper.py +++ b/src/git_pulsar/git_wrapper.py @@ -1,7 +1,12 @@ +import logging import subprocess from pathlib import Path from typing import Optional +from .constants import APP_NAME + +logger = logging.getLogger(APP_NAME) + class GitRepo: """A wrapper around the Git command-line interface for a specific repository. @@ -164,7 +169,8 @@ def list_refs(self, pattern: str) -> list[str]: try: output = self._run(["for-each-ref", "--format=%(refname)", pattern]) return output.splitlines() if output else [] - except Exception: + except Exception as e: + logger.warning(f"Git error listing refs for {pattern}: {e}") return [] def get_last_commit_time(self, branch: str) -> str: @@ -193,7 +199,8 @@ def rev_parse(self, rev: str) -> Optional[str]: """ try: return self._run(["rev-parse", rev]) - except Exception: + except Exception as e: + logger.debug(f"rev-parse failed for '{rev}': {e}") return None def write_tree(self, env: Optional[dict] = None) -> str: @@ -226,7 +233,11 @@ def commit_tree( cmd = ["commit-tree", tree, "-m", message] for p in parents: cmd.extend(["-p", p]) - return self._run(cmd, env=env) + try: + return self._run(cmd, env=env) + except Exception as e: + logger.warning(f"Failed to commit tree {tree}: {e}") + raise def update_ref(self, ref: str, new_oid: str, old_oid: Optional[str] = None) -> None: """Safely updates a reference to a new object ID. @@ -241,7 +252,11 @@ def update_ref(self, ref: str, new_oid: str, old_oid: Optional[str] = None) -> N cmd = ["update-ref", "-m", "Pulsar backup", ref, new_oid] if old_oid: cmd.append(old_oid) - self._run(cmd) + try: + self._run(cmd) + except Exception as e: + logger.warning(f"Failed to update ref {ref}: {e}") + raise def get_untracked_files(self) -> list[str]: """Lists files that are not tracked by git and are not ignored. diff --git a/src/git_pulsar/ops.py b/src/git_pulsar/ops.py index e092c45..f56a381 100644 --- a/src/git_pulsar/ops.py +++ b/src/git_pulsar/ops.py @@ -1,3 +1,4 @@ +import logging import os import shutil import subprocess @@ -10,10 +11,11 @@ from rich.panel import Panel from . import system -from .constants import BACKUP_NAMESPACE +from .constants import APP_NAME, BACKUP_NAMESPACE from .git_wrapper import GitRepo console = Console() +logger = logging.getLogger(APP_NAME) def get_backup_ref(branch: str) -> str: @@ -167,6 +169,7 @@ def restore_file(path_str: str, force: bool = False) -> None: repo.checkout(backup_ref, file=path_str) console.print("[bold green]SUCCESS:[/bold green] Restore complete.") except Exception as e: + logger.error(f"Failed to restore {path_str}: {e}") console.print(f"[bold red]ERROR:[/bold red] Failed to restore: {e}") sys.exit(1) @@ -187,15 +190,17 @@ def sync_session() -> None: spinner="dots", ): try: + # Only fetch backups related to the current branch repo._run( [ "fetch", "origin", - f"refs/heads/{BACKUP_NAMESPACE}/*:refs/heads/{BACKUP_NAMESPACE}/*", + f"refs/heads/{BACKUP_NAMESPACE}/*/{current_branch}:refs/heads/{BACKUP_NAMESPACE}/*/{current_branch}", ], capture=True, ) - except Exception: + except Exception as e: + logger.warning(f"Fetch error: {e}") console.print( "[yellow][bold]WARNING:[/bold] Fetch warning: network might be down " "(checking local cache).[/yellow]" @@ -265,6 +270,7 @@ def sync_session() -> None: "[bold green]SUCCESS:[/bold green] Session synced. You may resume work." ) except Exception as e: + logger.warning(f"Sync failed: {e}") console.print(f"[bold red]ERROR:[/bold red] Sync failed: {e}") sys.exit(1) @@ -352,6 +358,7 @@ def finalize_work() -> None: console.print(f" Your backup history remains in refs/{BACKUP_NAMESPACE}/...") except Exception as e: + logger.error(f"Finalize failed: {e}") console.print(f"\n[bold red]ERROR:[/bold red] Error during finalize: {e}") sys.exit(1) @@ -441,5 +448,6 @@ def add_ignore(pattern: str) -> None: if confirm == "y": repo._run(["rm", "--cached", pattern], capture=False) console.print(" Removed from index (file preserved on disk).") - except Exception: + except Exception as e: + logger.warning(f"Failed to remove tracked files: {e}") pass diff --git a/src/git_pulsar/service.py b/src/git_pulsar/service.py index 67c95ad..939025c 100644 --- a/src/git_pulsar/service.py +++ b/src/git_pulsar/service.py @@ -5,11 +5,30 @@ from rich.console import Console -from .constants import APP_LABEL, LOG_FILE +from .constants import APP_LABEL, HOMEBREW_LABEL, LOG_FILE console = Console() +def is_service_enabled() -> bool: + """Checks if the system service is currently loaded and active. + + Returns: + bool: True if the service is active/loaded, False otherwise. + """ + if sys.platform == "darwin": + res = subprocess.run(["launchctl", "list"], capture_output=True, text=True) + return HOMEBREW_LABEL in res.stdout + elif sys.platform.startswith("linux"): + res = subprocess.run( + ["systemctl", "--user", "is-active", f"{APP_LABEL}.timer"], + capture_output=True, + text=True, + ) + return res.stdout.strip() == "active" + return False + + def get_executable() -> str: """Locates the installed daemon executable in the system path. diff --git a/src/git_pulsar/system.py b/src/git_pulsar/system.py index 27fce47..1d2e03e 100644 --- a/src/git_pulsar/system.py +++ b/src/git_pulsar/system.py @@ -8,13 +8,27 @@ from rich.console import Console -from .constants import APP_NAME, BACKUP_NAMESPACE, MACHINE_ID_FILE, MACHINE_NAME_FILE +from .constants import ( + APP_NAME, + BACKUP_NAMESPACE, + MACHINE_ID_FILE, + MACHINE_NAME_FILE, + REGISTRY_FILE, +) from .git_wrapper import GitRepo console = Console() logger = logging.getLogger(APP_NAME) +def get_registered_repos() -> list[Path]: + """Reads the registry file and returns a list of registered repository paths.""" + if not REGISTRY_FILE.exists(): + return [] + with open(REGISTRY_FILE, "r") as f: + return [Path(line.strip()) for line in f if line.strip()] + + class SystemStrategy: """Base class defining the interface for system-level interactions.""" @@ -43,7 +57,8 @@ def is_under_load(self) -> bool: load_1m, _, _ = os.getloadavg() cpu_count = os.cpu_count() or 1 return load_1m > (cpu_count * 2.5) - except OSError: + except OSError as e: + logger.warning(f"Failed to determine system load: {e}") return False def notify(self, title: str, message: str) -> None: @@ -69,7 +84,8 @@ def get_battery(self) -> tuple[int, bool]: match = re.search(r"(\d+)%", out) percent = int(match.group(1)) if match else 100 return percent, is_plugged - except Exception: + except Exception as e: + logger.warning(f"MacOS battery check failed: {e}") return 100, True def notify(self, title: str, message: str) -> None: @@ -79,8 +95,8 @@ def notify(self, title: str, message: str) -> None: script = f'display notification "{clean_msg}" with title "{title}"' try: subprocess.run(["osascript", "-e", script], stderr=subprocess.DEVNULL) - except Exception: - pass + except Exception as e: + logger.warning(f"Notification failed: {e}") class LinuxStrategy(SystemStrategy): @@ -99,7 +115,8 @@ def get_battery(self) -> tuple[int, bool]: with open(bat_path / "status", "r") as f: is_plugged = f.read().strip() != "Discharging" return percent, is_plugged - except Exception: + except Exception as e: + logger.warning(f"Linux battery check failed: {e}") pass return 100, True @@ -108,7 +125,7 @@ def notify(self, title: str, message: str) -> None: try: subprocess.run(["notify-send", title, message], stderr=subprocess.DEVNULL) except FileNotFoundError: - pass + logger.warning("notify-send not available") def get_system() -> SystemStrategy: @@ -188,8 +205,8 @@ def get_machine_id() -> str: uuid = data[0].get("IOPlatformUUID") if isinstance(uuid, str) and uuid.strip(): return uuid.strip() - except Exception: - pass + except Exception as e: + logger.warning(f"Failed to extract IOPlatformUUID: {e}") # Secondary: stable-ish local name try: @@ -201,8 +218,8 @@ def get_machine_id() -> str: ) if res.returncode == 0 and res.stdout.strip(): return res.stdout.strip() - except Exception: - pass + except Exception as e: + logger.warning(f"Failed to extract LocalHostName: {e}") # Generic fallback (not a true machine ID) name = socket.gethostname() @@ -270,7 +287,8 @@ def _fetch_remote_identities(repo: GitRepo) -> set[str]: if "--" in slug: name, _ = slug.split("--", 1) used_names.add(name) - except ValueError: + except ValueError as e: + logger.warning(f"Failed to parse slug from ref '{ref}': {e}") continue return used_names diff --git a/tests/test_cli.py b/tests/test_cli.py index 73b827f..e6f350f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,7 +25,7 @@ def test_show_status_displays_timestamps( # Mock Registry to include the current path registry_path = tmp_path / "registry" registry_path.write_text(str(tmp_path)) - mocker.patch("git_pulsar.cli.REGISTRY_FILE", registry_path) + mocker.patch("git_pulsar.system.REGISTRY_FILE", registry_path) # Mock Config loading mocker.patch("git_pulsar.config.Config.load", return_value=Config()) @@ -96,6 +96,8 @@ def test_setup_repo_triggers_identity_config(tmp_path: Path, mocker: MagicMock) # Use a fake registry so we don't pollute the real user's registry mock_registry = tmp_path / "registry" + mocker.patch("git_pulsar.constants.REGISTRY_FILE", mock_registry) + # Mock system.configure_identity mock_config_id = mocker.patch("git_pulsar.system.configure_identity") diff --git a/tests/test_config.py b/tests/test_config.py index a6e4b9a..2afecfb 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,11 +1,22 @@ """Tests for the configuration management subsystem.""" from pathlib import Path +from typing import Any from unittest.mock import MagicMock +import pytest + from git_pulsar.config import Config +@pytest.fixture(autouse=True) +def clear_config_cache() -> Any: + """Ensures every test starts with a clean config cache.""" + Config._global_cache = None + yield + Config._global_cache = None + + def test_config_defaults() -> None: """Verifies that the configuration initializes with sensible defaults.""" conf = Config() diff --git a/tests/test_git_wrapper.py b/tests/test_git_wrapper.py new file mode 100644 index 0000000..2b33937 --- /dev/null +++ b/tests/test_git_wrapper.py @@ -0,0 +1,25 @@ +from pathlib import Path +from unittest.mock import MagicMock + +from git_pulsar.git_wrapper import GitRepo + + +def test_list_refs_logs_error_on_failure( + mocker: MagicMock, caplog: MagicMock, tmp_path: Path +) -> None: + """Verifies that git failures are logged instead of passing silently.""" + # Mock subprocess to raise an exception + mocker.patch("subprocess.run", side_effect=Exception("Git is broken")) + + # Create a fake .git directory so GitRepo accepts the path + (tmp_path / ".git").mkdir() + repo = GitRepo(tmp_path) + + # Run the method + results = repo.list_refs("refs/heads/*") + + # Assert it handled the error gracefully + assert results == [] + + # Assert it logged the warning + assert "Git error listing refs" in caplog.text diff --git a/tests/test_ops.py b/tests/test_ops.py index 910e08a..d8f7d90 100644 --- a/tests/test_ops.py +++ b/tests/test_ops.py @@ -155,12 +155,12 @@ def mock_run(cmd: list[str], *args: Any, **kwargs: Any) -> str: ops.sync_session() - # Verify fetch of all namespaces. + # Verify fetch of specific branch only repo._run.assert_any_call( [ "fetch", "origin", - f"refs/heads/{BACKUP_NAMESPACE}/*:refs/heads/{BACKUP_NAMESPACE}/*", + f"refs/heads/{BACKUP_NAMESPACE}/*/main:refs/heads/{BACKUP_NAMESPACE}/*/main", ], capture=True, ) diff --git a/tests/test_system.py b/tests/test_system.py index 257b637..eae6c8b 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -183,3 +183,16 @@ def test_configure_identity_skips_existing(tmp_path: Path, mocker: MagicMock) -> # Should exit early without asking for input mock_console.input.assert_not_called() + + +def test_get_registered_repos_parses_cleanly(tmp_path: Path, mocker: MagicMock) -> None: + """Verifies that the registry helper strips whitespace and empty lines.""" + reg_file = tmp_path / "registry" + reg_file.write_text("\n /path/one \n\n/path/two\n") + + mocker.patch("git_pulsar.system.REGISTRY_FILE", reg_file) + + repos = system.get_registered_repos() + assert len(repos) == 2 + assert Path("/path/one") in repos + assert Path("/path/two") in repos