diff --git a/README.md b/README.md index 3c58f44..deca7c5 100644 --- a/README.md +++ b/README.md @@ -333,7 +333,10 @@ deleted it again, `DELETE …/tokens/{id}` returning 204 under basic auth): list *is* the grant. A client-side filter would be a second opinion about someone else's authorisation. One repo failing does not abandon the rest, and a truncated listing is reported loudly — `--all` must never quietly - mean "the first 2000". + mean "the first 2000". `alice/notes` and `bob/notes` both want + `/notes`; the second is cloned to `/bob-notes` and the clash is + logged, because reporting it as "already present" would leave the user + believing they had pulled both. * `get [--into DIR] [--mode M]` — the download half of `add`. Defaults to `snapshot` mode unless the repo carries a `.granthi-sync.json` marker saying otherwise, so a plain synced folder @@ -360,6 +363,12 @@ deleted it again, `DELETE …/tokens/{id}` returning 204 under basic auth): refused unless `--force`. The seeded ignore file covers `.env`, `*.key`, `*.pem`, `id_rsa` and friends, and it governs snapshots too — the scratch index honours `.gitignore` exactly as a normal commit does. + The size guard measures what git *would* sync, ignore rules included + (including the machine's global excludes), because its own advice is "add + a .gitignore for what should not sync" and advice that changes nothing is + worse than none. It asks git through a **throwaway git dir outside the + folder**, so a refused `add` leaves no `.git` behind in a directory the + user never agreed to turn into a repo. * `watch [--interval 30] [--once]` — per folder, by mode. `mirror`: autocommit (`sync: `) → fetch → ff-pull if remote strictly ahead → push if local strictly ahead. `snapshot`: fetch → push a snapshot of the @@ -370,18 +379,31 @@ deleted it again, `DELETE …/tokens/{id}` returning 204 under basic auth): force, never merge** — the same policy as the mesh — **but the backup still happens**, because divergence is when work is most at risk. Retention pruning runs at most hourly. SIGTERM-clean. -* `snapshots [--limit 20]` — restore points, newest first. Read from - the **remote**, not a local cache: the feature exists for the case where - this machine is gone. +* `snapshots [--limit 20]` — restore points, newest first, **across + every device**, with the device that took each one. Read from the + **remote**, not a local cache: the feature exists for the case where this + machine is gone. + + The three scopes differ deliberately. Writing is device-scoped, so two + machines never overwrite each other. Pruning is device-scoped, so machine A + never applies its clock to machine B's refs. **Reading is not scoped** — a + replacement laptop has a new id, and scoping the read to it would print + "no restore points yet" while the backups sit on the forge. That defect + was live in the first draft and is now pinned by a test that restores a + dead machine's work from a fresh clone. * `restore --at [--into DIR]` — materialise one restore - point into a new directory; refuses a non-empty destination. + point into a new directory; refuses a non-empty destination. Accepts what + `snapshots` printed in either mode, including a mirror-mode `%cI` + timestamp. Two commits inside the same second share that timestamp, so an + ambiguous `--at` is **refused with the candidate ids** rather than + resolved by guessing. * `status` — table of linked folders, mode, last sync, divergence flags. * Run as a daemon on macOS with `client/launchd/ai.granthi.sync.plist` (edit the script path, then `launchctl bootstrap gui/$UID `). ## Tests -* `python3 -m unittest discover -s tests` — 143 tests. The v1.2 additions +* `python3 -m unittest discover -s tests` — 153 tests. The v1.2 additions cover: a snapshot capturing uncommitted work while HEAD, the index and the working tree stay byte-identical; snapshots landing outside `refs/heads`; an unchanged tree not being re-pushed; a diverged folder still being backed diff --git a/client/granthi_sync_client.py b/client/granthi_sync_client.py index 953ee57..aed0d4f 100644 --- a/client/granthi_sync_client.py +++ b/client/granthi_sync_client.py @@ -354,25 +354,41 @@ def _remember_snapshot_tree(folder, dev, tree): pass # a lost marker costs one redundant push, never correctness -def list_snapshots(folder, dev, remote="granthi"): - """Backup refs for this device on the remote, newest first. +def list_snapshots(folder, dev=None, remote="granthi"): + """Backup refs on the remote, newest first. Read from the REMOTE, not a local cache: the point of the feature is surviving the loss of this machine, so what the server holds is the answer that matters. + + `dev=None` means EVERY device, and that is what the read paths pass. + The three scopes differ on purpose: + + * writing is device-scoped, so two machines never overwrite each + other's snapshots; + * pruning is device-scoped, so machine A never applies its own clock + and retention to machine B's refs; + * reading is NOT scoped, because the case this feature exists for is + "the laptop died". A new machine has a new id, and scoping the read + to it would show an empty list while the backups sit on the forge. """ - rc, out = git(folder, "ls-remote", remote, f"{BACKUP_NS}/{dev}/*", - check=False) + pattern = f"{BACKUP_NS}/{dev}/*" if dev else f"{BACKUP_NS}/*" + rc, out = git(folder, "ls-remote", remote, pattern, check=False) if rc != 0: return [] + prefix = f"{BACKUP_NS}/{dev}/" if dev else f"{BACKUP_NS}/" snaps = [] for line in out.splitlines(): sha, _, ref = line.partition("\t") ref = ref.strip() - if not ref.startswith(f"{BACKUP_NS}/{dev}/"): + if not ref.startswith(prefix): continue - snaps.append({"sha": sha.strip(), "ref": ref, - "ts": ref.rsplit("/", 1)[-1]}) + rest = ref[len(f"{BACKUP_NS}/"):] + device, _, ts = rest.partition("/") + if not ts: + continue + snaps.append({"sha": sha.strip(), "ref": ref, "ts": ts, + "device": device}) return sorted(snaps, key=lambda s: s["ts"], reverse=True) @@ -831,21 +847,45 @@ def _get_all(cfg, args): """ repos, truncated = list_repos(cfg) base = os.path.abspath(args.into or ".") - linked = set(cfg.get("folders", {})) + folders = cfg.get("folders", {}) + # full_name already living at a path, so "already here" can be told apart + # from "a different repo wants that same path". + owner_of = {os.path.abspath(p): m.get("full_name") + for p, m in folders.items()} cloned = skipped = failed = 0 for repo in sorted(repos, key=lambda r: r.get("full_name") or ""): full_name = repo.get("full_name") if not full_name: continue - dest = os.path.join(base, full_name.rsplit("/", 1)[-1]) - if dest in linked or (os.path.exists(dest) and os.listdir(dest)): + owner, _, name = full_name.rpartition("/") + dest = os.path.join(base, name) + if owner_of.get(dest) == full_name: log(f"skip {full_name}: already present at {dest}") skipped += 1 continue + if dest in owner_of or (os.path.exists(dest) and os.listdir(dest)): + # alice/notes and bob/notes both want /notes. Reporting the + # second as "already here" would be a silent collision -- the user + # would believe they had pulled both. + taken_by = owner_of.get(dest) + qualified = os.path.join(base, f"{owner}-{name}" if owner else name) + if owner_of.get(qualified) == full_name or ( + os.path.exists(qualified) and os.listdir(qualified)): + log(f"skip {full_name}: already present at {qualified}") + skipped += 1 + continue + log(f"name clash on {dest}" + + (f" (held by {taken_by})" if taken_by else "") + + f": cloning {full_name} to {qualified} instead") + dest = qualified try: clone_one(cfg, full_name, dest, args.mode) + owner_of[os.path.abspath(dest)] = full_name cloned += 1 - except SystemExit as e: # one bad repo must not abandon the rest + except (SystemExit, RuntimeError) as e: + # One bad repo must not abandon the rest. clone_one raises + # SystemExit for a refused destination and RuntimeError from any + # failing git call -- both are one repo's problem, not the run's. log(f"FAILED {full_name}: {e}") failed += 1 log(f"get --all: {cloned} cloned, {skipped} already here, {failed} failed") @@ -857,12 +897,65 @@ def _get_all(cfg, args): def measure_folder(folder, max_files=ADD_MAX_FILES): - """(files, bytes) below .git, stopping once max_files is exceeded. + """(files, bytes) that would actually sync, stopping past max_files. + + Ignore rules are honoured, because the guard's own advice is "add a + .gitignore for what should not sync" -- and advice that changes nothing + is worse than no advice. git is asked with a THROWAWAY git dir outside + the folder, so a refused `add` leaves no .git behind in a directory the + user never agreed to turn into a repo. Bounded on purpose: the caller only needs to know whether the folder is surprisingly big, and walking a 2 TB drive to answer that would be the same mistake as syncing it. """ + listing = _git_would_sync(folder) + if listing is None: + return _walk_folder(folder, max_files) + files = total = 0 + for rel in listing: + path = os.path.join(folder, rel) + try: + if os.path.islink(path): + continue + total += os.path.getsize(path) + except OSError: + continue + files += 1 + if files > max_files: + break + return files, total + + +def _git_would_sync(folder): + """Paths git would take from `folder`, honouring .gitignore. None if git + could not answer, so the caller falls back to a plain walk.""" + scratch = tempfile.mkdtemp(prefix="granthi-measure-") + try: + git_dir = os.path.join(scratch, "git") + init = subprocess.run(["git", "init", "-q", "--bare", git_dir], + capture_output=True, text=True) + if init.returncode != 0: + return None + proc = subprocess.run( + ["git", f"--git-dir={git_dir}", f"--work-tree={folder}", + "status", "--porcelain", "--untracked-files=all", + "--ignored=no", "-z"], + capture_output=True, text=True) + if proc.returncode != 0: + return None + out = [] + for entry in proc.stdout.split("\0"): + if len(entry) > 3: + out.append(entry[3:]) + return out + except OSError: + return None + finally: + subprocess.run(["rm", "-rf", scratch], capture_output=True) + + +def _walk_folder(folder, max_files): files = total = 0 for root, dirs, names in os.walk(folder): dirs[:] = [d for d in dirs if d != ".git"] @@ -989,16 +1082,15 @@ def _sigterm(signum, frame): log(f"signal {signum} received; finishing current pass then exiting") -_LAST_PRUNE = 0.0 - - def watch_pass(now=None): - global _LAST_PRUNE cfg = load_config() dev = device_id(cfg) # Pruning talks to the remote for every folder, so it runs on a slow - # clock of its own rather than on every 30s pass. - due = (now or time.time()) - _LAST_PRUNE >= PRUNE_EVERY_SECONDS + # clock of its own rather than on every 30s pass. The clock is PERSISTED: + # `watch --once` under launchd or cron is a fresh process every time, and + # an in-memory timer would make "at most hourly" mean "every run". + now = now or time.time() + due = now - float(cfg.get("last_prune") or 0) >= PRUNE_EVERY_SECONDS for folder, meta in sorted(cfg.get("folders", {}).items()): if not os.path.isdir(folder): log(f"SKIP {folder}: missing") @@ -1024,7 +1116,7 @@ def watch_pass(now=None): # Retention failing must never stop the backup itself. log(f"prune failed in {folder} (backups unaffected): {e}") if due: - _LAST_PRUNE = now or time.time() + cfg["last_prune"] = now save_config(cfg) @@ -1058,7 +1150,7 @@ def cmd_snapshots(args): """Restore points, newest first. Both modes answer the same question.""" cfg = require_linked(load_config()) folder, meta = _folder_meta(cfg, args.folder) - dev = device_id(cfg) + this_dev = cfg.get("device_id") if meta.get("mode", "mirror") == "mirror": rc, out = git(folder, "log", "--format=%H\t%cI\t%s", "-n", str(args.limit), check=False) @@ -1070,13 +1162,16 @@ def cmd_snapshots(args): sha, ts, subject = (line.split("\t") + ["", ""])[:3] print(f" {ts} {sha[:12]} {subject}") return 0 - snaps = list_snapshots(folder, dev) + # Every device's backups, not just this one's -- see list_snapshots. + snaps = list_snapshots(folder) if not snaps: print("no restore points yet (nothing uncommitted has been backed up)") return 0 - print(f"restore points for {folder} (mode snapshot, device {dev[:8]}):") + print(f"restore points for {folder} (mode snapshot):") for snap in snaps[:args.limit]: - print(f" {snap['ts']} {snap['sha'][:12]}") + here = " (this device)" if snap["device"] == this_dev else "" + print(f" {snap['ts']} {snap['sha'][:12]} " + f"device {snap['device'][:8]}{here}") if len(snaps) > args.limit: print(f" ... {len(snaps) - args.limit} older (use --limit)") return 0 @@ -1091,15 +1186,36 @@ def cmd_restore(args): """ cfg = require_linked(load_config()) folder, meta = _folder_meta(cfg, args.folder) - dev = device_id(cfg) target = args.at sha = None if meta.get("mode", "mirror") == "snapshot": - for snap in list_snapshots(folder, dev): + # Across ALL devices: restoring is exactly the moment the device that + # took the backup may no longer exist. + for snap in list_snapshots(folder): if snap["ts"] == target or snap["sha"].startswith(target): sha = snap["sha"] git(folder, "fetch", "granthi", f"{snap['ref']}") break + else: + # Mirror mode lists commits by their %cI timestamp, and that first + # column is what a person copies. It is not a commit-ish, so match it + # against the log before falling through to rev-parse -- otherwise + # `restore --at ` fails and sends the + # user back to `snapshots`, which prints the same thing again. + rc, out = git(folder, "log", "--format=%H\t%cI", check=False) + if rc == 0: + hits = [line.partition("\t")[0] for line in out.splitlines() + if line.partition("\t")[2] == target] + if len(hits) > 1: + # Two commits in the same second share a %cI. Silently taking + # the newest would restore something the user did not pick. + listed = "\n".join(f" {h[:12]}" for h in hits) + raise SystemExit( + f"{target} matches {len(hits)} restore points in " + f"{folder}. Re-run with one of these ids instead:\n" + f"{listed}") + if hits: + sha = hits[0] if sha is None: rc, resolved = git(folder, "rev-parse", "--verify", "--quiet", f"{target}^{{commit}}", check=False) @@ -1108,8 +1224,10 @@ def cmd_restore(args): f"no restore point {target!r} for {folder} " f"(see: granthi-sync snapshots {folder})") sha = resolved.strip() + # A %cI timestamp carries ':' and '+', which make an awkward folder name. + suffix = re.sub(r"[^A-Za-z0-9]+", "-", target).strip("-") or sha[:12] dest = os.path.abspath( - args.into or f"{folder.rstrip(os.sep)}-restore-{target}") + args.into or f"{folder.rstrip(os.sep)}-restore-{suffix}") if os.path.exists(dest) and os.listdir(dest): raise SystemExit(f"refusing to restore over a non-empty path: {dest}") os.makedirs(dest, exist_ok=True) diff --git a/tests/test_client.py b/tests/test_client.py index d360328..9352ddd 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -10,6 +10,7 @@ import shutil import subprocess import sys import tempfile +import time import unittest from unittest import mock @@ -578,8 +579,45 @@ class TestSnapshots(GitScenarioBase): ref = client.push_snapshot(self.local, self.DEV) snaps = client.list_snapshots(self.local, self.DEV) self.assertEqual([s["ref"] for s in snaps], [ref]) + # writing is device-scoped: the ref carries THIS device's id, so two + # machines cannot overwrite each other + self.assertEqual(snaps[0]["device"], self.DEV) self.assertEqual(client.list_snapshots(self.local, "someone-else"), []) + def test_a_new_machine_can_see_the_dead_machine_s_backups(self): + """The case the whole feature exists for. Reads must NOT be scoped to + this device's id -- a replacement laptop has a new id, and scoping + would show an empty list while the backups sit on the forge.""" + self.write(self.local, "a.txt", "committed") + run_git(self.local, "add", "-A") + run_git(self.local, "commit", "-m", "c") + run_git(self.local, "push", "-u", "granthi", "main") + self.write(self.local, "a.txt", "PRECIOUS UNCOMMITTED WORK") + ref = client.push_snapshot(self.local, "laptop-that-died") + + # a fresh clone standing in for the replacement machine + newbox = os.path.join(self.tmp, "newbox") + subprocess.run(["git", "clone", "-q", "--origin", "granthi", + self.bare, newbox], check=True, capture_output=True, + env=GIT_ENV) + client.save_config({"gitea_base": self.tmp, "login": "alice", + "token": "t", "device_id": "brand-new-laptop", + "folders": {newbox: {"name": "cloud", + "full_name": "alice/cloud", + "branch": "main", + "mode": "snapshot"}}}) + + seen = client.list_snapshots(newbox) + self.assertEqual([s["ref"] for s in seen], [ref]) + self.assertEqual(seen[0]["device"], "laptop-that-died") + + # and it can actually restore it + dest = os.path.join(self.tmp, "recovered") + client.cmd_restore(argparse.Namespace(folder=newbox, at=seen[0]["ts"], + into=dest)) + with open(os.path.join(dest, "a.txt")) as f: + self.assertEqual(f.read(), "PRECIOUS UNCOMMITTED WORK") + class TestRetention(unittest.TestCase): """Retention is what keeps 30-second backups from being a disk leak -- @@ -900,5 +938,169 @@ class TestCredentialHelperIsolation(GitScenarioBase): self.assertNotIn("stale-token", out.stdout) +class TestMirrorRestore(GitScenarioBase): + """Mirror is the default for a plain folder, so its restore path is the + one most people will use -- and the timestamp `snapshots` prints has to + be a timestamp `restore` accepts, or the user just loops.""" + + def setUp(self): + super().setUp() + client.save_config({"gitea_base": self.tmp, "login": "alice", + "token": "t", "device_id": "d1", + "folders": {self.local: { + "name": "cloud", "full_name": "alice/cloud", + "branch": "main", "mode": "mirror"}}}) + + def test_restore_accepts_the_timestamp_snapshots_printed(self): + self.write(self.local, "a.txt", "the version I want back") + client.sync_folder(self.local, mode="mirror") + time.sleep(1.1) # %cI has one-second resolution + self.write(self.local, "a.txt", "later junk") + client.sync_folder(self.local, mode="mirror") + + with mock.patch("sys.stdout", new_callable=io.StringIO) as out: + client.cmd_snapshots(argparse.Namespace(folder=self.local, + limit=20)) + listed = [l.split() for l in out.getvalue().splitlines() + if l.startswith(" ")] + wanted_ts = listed[-1][0] # first column of the oldest entry + + dest = os.path.join(self.tmp, "mirror-restore") + client.cmd_restore(argparse.Namespace(folder=self.local, + at=wanted_ts, into=dest)) + with open(os.path.join(dest, "a.txt")) as f: + self.assertEqual(f.read(), "the version I want back") + + def test_two_commits_in_the_same_second_are_refused_not_guessed(self): + """%cI has one-second resolution. Picking one silently would restore + something the user did not choose.""" + self.write(self.local, "a.txt", "first") + client.sync_folder(self.local, mode="mirror") + self.write(self.local, "a.txt", "second") + client.sync_folder(self.local, mode="mirror") + stamps = run_git(self.local, "log", "--format=%cI").splitlines() + if len(set(stamps)) != 1: + self.skipTest("commits did not land in the same second") + with self.assertRaises(SystemExit) as caught: + client.cmd_restore(argparse.Namespace(folder=self.local, + at=stamps[0], into=None)) + self.assertIn("matches 2 restore points", str(caught.exception)) + + def test_restore_still_accepts_a_sha(self): + self.write(self.local, "a.txt", "one") + client.sync_folder(self.local, mode="mirror") + sha = run_git(self.local, "rev-parse", "HEAD") + dest = os.path.join(self.tmp, "by-sha") + client.cmd_restore(argparse.Namespace(folder=self.local, at=sha, + into=dest)) + self.assertTrue(os.path.exists(os.path.join(dest, "a.txt"))) + + def test_default_destination_is_a_usable_path(self): + self.write(self.local, "a.txt", "one") + client.sync_folder(self.local, mode="mirror") + with mock.patch("sys.stdout", new_callable=io.StringIO) as out: + client.cmd_snapshots(argparse.Namespace(folder=self.local, + limit=5)) + ts = [l.split() for l in out.getvalue().splitlines() + if l.startswith(" ")][0][0] + client.cmd_restore(argparse.Namespace(folder=self.local, at=ts, + into=None)) + made = [d for d in os.listdir(self.tmp) if d.startswith("local-restore-")] + self.assertEqual(len(made), 1, made) + self.assertNotIn(":", made[0]) + + +class TestMeasureHonoursGitignore(GitScenarioBase): + """The guard tells people to add a .gitignore. That advice has to work.""" + + def test_ignored_files_are_not_counted(self): + os.makedirs(os.path.join(self.local, "bulkdata")) + for i in range(30): + self.write(self.local, f"bulkdata/x{i}.bin", "y" * 100) + self.write(self.local, "real.txt", "mine") + before, _ = client.measure_folder(self.local) + self.write(self.local, ".gitignore", "bulkdata/\n") + after, _ = client.measure_folder(self.local) + self.assertGreater(before, 30) + self.assertEqual(after, 2) # real.txt + .gitignore + + def test_measure_leaves_no_git_dir_behind(self): + plain = os.path.join(self.tmp, "untouched") + os.makedirs(plain) + self.write(plain, "a.txt", "x") + client.measure_folder(plain) + self.assertEqual(os.listdir(plain), ["a.txt"]) + + +class TestGetAllRobustness(GitScenarioBase): + def setUp(self): + super().setUp() + self.forge = os.path.join(self.tmp, "forge") + for full in ("alice/notes", "bob/notes"): + path = os.path.join(self.forge, full + ".git") + os.makedirs(os.path.dirname(path), exist_ok=True) + subprocess.run(["git", "init", "-q", "--bare", "-b", "main", path], + check=True, capture_output=True, env=GIT_ENV) + seed = os.path.join(self.tmp, "seed-" + full.replace("/", "-")) + subprocess.run(["git", "clone", "-q", path, seed], check=True, + capture_output=True, env=GIT_ENV) + run_git(seed, "config", "user.name", "s") + run_git(seed, "config", "user.email", "s@s") + self.write(seed, "who.txt", full) + run_git(seed, "add", "-A") + run_git(seed, "commit", "-m", "seed") + run_git(seed, "push", "-q", "origin", "main") + client.save_config({"gitea_base": self.forge, "login": "alice", + "token": "t", "folders": {}}) + self.repos = [{"name": "notes", "full_name": "alice/notes"}, + {"name": "notes", "full_name": "bob/notes"}] + + def test_same_named_repos_from_two_owners_both_land(self): + into = os.path.join(self.tmp, "ws") + os.makedirs(into) + with mock.patch.object(client, "list_repos", + lambda c: (self.repos, False)), \ + mock.patch("sys.stdout", new_callable=io.StringIO) as out: + client.cmd_get(get_ns(all=True, into=into)) + self.assertIn("name clash", out.getvalue()) + with open(os.path.join(into, "notes", "who.txt")) as f: + self.assertEqual(f.read(), "alice/notes") + with open(os.path.join(into, "bob-notes", "who.txt")) as f: + self.assertEqual(f.read(), "bob/notes") + self.assertEqual(len(client.load_config()["folders"]), 2) + + def test_a_git_failure_on_one_repo_does_not_abandon_the_rest(self): + into = os.path.join(self.tmp, "ws2") + os.makedirs(into) + real_clone = client.clone_one + + def flaky(cfg, full_name, dest, mode=None): + if full_name == "alice/notes": + raise RuntimeError("git clone failed: pretend network blip") + return real_clone(cfg, full_name, dest, mode) + + with mock.patch.object(client, "list_repos", + lambda c: (self.repos, False)), \ + mock.patch.object(client, "clone_one", flaky), \ + mock.patch("sys.stdout", new_callable=io.StringIO) as out: + rc = client.cmd_get(get_ns(all=True, into=into)) + self.assertEqual(rc, 1) # reported, not hidden + self.assertIn("FAILED alice/notes", out.getvalue()) + self.assertTrue(os.path.exists(os.path.join(into, "notes", "who.txt"))) + + +class TestPruneClockIsPersisted(GitScenarioBase): + def test_watch_once_does_not_prune_every_run(self): + client.save_config({"gitea_base": self.tmp, "login": "alice", + "token": "t", "device_id": "d1", "folders": {}}) + with mock.patch.object(client, "prune_snapshots") as pruner: + client.watch_pass(now=1_000_000.0) + self.assertEqual(client.load_config()["last_prune"], 1_000_000.0) + client.watch_pass(now=1_000_060.0) # a minute later: not due + client.watch_pass(now=1_003_700.0) # an hour later: due again + self.assertEqual(client.load_config()["last_prune"], 1_003_700.0) + self.assertEqual(pruner.call_count, 0) # no snapshot folders linked + + if __name__ == "__main__": unittest.main()