diff --git a/docs/reference/configuration/directories.md b/docs/reference/configuration/directories.md index b6491212..31d48a6a 100644 --- a/docs/reference/configuration/directories.md +++ b/docs/reference/configuration/directories.md @@ -63,7 +63,8 @@ directories: The **automatic** 24 h-TTL refresh that happens during a normal `dl`/`search` run **refuses to refresh and exits** (naming the offending clone) if that clone has uncommitted changes to tracked files or unpushed local commits. Untracked files (new - service folders, `__pycache__`) never block it. The **manual** + service folders, `__pycache__`), Python bytecode (`.pyc`, `.pyo`, also when the repo + commits it) and `.DS_Store` never block it. The **manual** `unshackle util refresh-services` command instead **hard-resets** the clone to upstream, discarding any local edits. diff --git a/tests/core/test_service_repo.py b/tests/core/test_service_repo.py index 606ada9c..fa1f5f27 100644 --- a/tests/core/test_service_repo.py +++ b/tests/core/test_service_repo.py @@ -1,4 +1,4 @@ -"""Unit tests for remote git service-repo resolution. No network, no real git.""" +"""Unit tests for remote git service-repo resolution. No network; only the bytecode tests run a real git.""" from __future__ import annotations @@ -245,6 +245,66 @@ def test_changed_services_up_to_date(monkeypatch): assert service_repo.changed_services(Path("/x"), "same", "same") == [] +@pytest.fixture +def bytecode_clone(tmp_path): + """A clone of a repo that commits a ``.pyc``, with that ``.pyc`` rewritten as an import does.""" + git = service_repo.binaries.Git + if not git: + pytest.skip("git not found") + upstream, dest = tmp_path / "upstream", tmp_path / "clone" + (upstream / "EXAMPLE" / "__pycache__").mkdir(parents=True) + (upstream / "EXAMPLE" / "__init__.py").write_text("X = 1\n") + (upstream / "EXAMPLE" / "__pycache__" / "__init__.cpython-312.pyc").write_bytes(b"old") + + def run(*args: str, cwd: Path = upstream) -> None: + subprocess.run( + [git, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", *args], + cwd=cwd, + check=True, + capture_output=True, + ) + + run("init", "-q", "-b", "main") + run("add", "-A") + run("commit", "-qm", "init") + run("clone", "-q", str(upstream), str(dest), cwd=tmp_path) + (dest / "EXAMPLE" / "__pycache__" / "__init__.cpython-312.pyc").write_bytes(b"rewritten") + return upstream, dest, run + + +def test_rewritten_bytecode_does_not_block_pull(bytecode_clone, tmp_path): + upstream, dest, run = bytecode_clone + assert service_repo.is_dirty(dest) is False + assert service_repo.local_dirty_services(dest) == [] + (upstream / "EXAMPLE" / "__init__.py").write_text("X = 2\n") + (upstream / "EXAMPLE" / "__pycache__" / "__init__.cpython-312.pyc").write_bytes(b"new") + run("commit", "-qam", "update") + service_repo.pull(dest, tmp_path / "stamp") + assert (dest / "EXAMPLE" / "__init__.py").read_text() == "X = 2\n" + + +def test_source_edit_still_dirty_beside_bytecode(bytecode_clone): + _, dest, _ = bytecode_clone + (dest / "EXAMPLE" / "__init__.py").write_text("X = 3\n") + assert service_repo.is_dirty(dest) is True + assert service_repo.local_dirty_services(dest) == ["EXAMPLE"] + + +def test_untracked_bytecode_does_not_block_pull(bytecode_clone, tmp_path): + upstream, dest, run = bytecode_clone + (dest / "EXAMPLE" / "__pycache__" / "__init__.cpython-314.pyc").write_bytes(b"local") + (dest / "OTHER").mkdir() + (dest / "OTHER" / "__init__.py").write_text("") + (dest / "OTHER" / "__init__.cpython-314-x86_64-linux-gnu.so").write_bytes(b"compiled") + (upstream / "EXAMPLE" / "__init__.py").write_text("X = 2\n") + (upstream / "EXAMPLE" / "__pycache__" / "__init__.cpython-314.pyc").write_bytes(b"upstream") + run("add", "-A") + run("commit", "-qm", "update") + service_repo.pull(dest, tmp_path / "stamp") + assert (dest / "EXAMPLE" / "__init__.py").read_text() == "X = 2\n" + assert (dest / "OTHER" / "__init__.cpython-314-x86_64-linux-gnu.so").read_bytes() == b"compiled" + + @pytest.mark.parametrize("order", [["a", "b"], ["b", "a"]]) def test_collision_first_source_wins(tmp_path, order): """Discovery dedupes by tag honoring list order: the FIRST source to define a tag wins, diff --git a/unshackle/core/service_repo.py b/unshackle/core/service_repo.py index 4e445f46..a9a3f6e6 100644 --- a/unshackle/core/service_repo.py +++ b/unshackle/core/service_repo.py @@ -32,57 +32,24 @@ def stamp_for(dest: Path) -> Path: SHORTHAND_RE = re.compile(r"^[\w.-]+/[\w.-]+$") -# Importing services rewrites tracked bytecode when upstream mistakenly commits -# ``__pycache__`` (e.g. playtv). That must not block refresh / look like source edits. -_JUNK_NAME_SUFFIXES = (".pyc", ".pyo") -_JUNK_BASENAMES = frozenset({".DS_Store"}) - - -def is_junk_path(path: str) -> bool: - """True for import/OS noise that is never a meaningful service-source edit.""" - normalized = path.replace("\\", "/").strip() - if not normalized: - return False - parts = normalized.split("/") - if "__pycache__" in parts: - return True - name = parts[-1] - if name in _JUNK_BASENAMES: - return True - return name.endswith(_JUNK_NAME_SUFFIXES) - - -def porcelain_paths(porcelain: str) -> list[str]: - """Paths from ``git status --porcelain`` lines (handles rename ``old -> new``).""" - paths: list[str] = [] - for raw in porcelain.splitlines(): - if len(raw) < 4: - continue - # XYpath or XYold -> new - entry = raw[3:] - if " -> " in entry: - entry = entry.split(" -> ", 1)[1] - # quoted paths are rare here; strip optional quotes - if len(entry) >= 2 and entry[0] == entry[-1] == '"': - entry = entry[1:-1] - paths.append(entry) - return paths +JUNK_GLOBS = ("**/__pycache__/**", "**/*.pyc", "**/*.pyo", "**/.DS_Store") +JUNK = tuple(f":(glob){g}" for g in JUNK_GLOBS) +NOT_JUNK = (".", *(f":(exclude,glob){g}" for g in JUNK_GLOBS)) def discard_junk_workdir(dest: Path) -> None: - """Restore tracked junk paths so ``git pull`` is not blocked by rewritten ``.pyc`` files.""" - if not binaries.Git: - return + """Restore tracked bytecode the import rewrote and delete untracked ``__pycache__`` files, so + neither can block ``git pull --ff-only``. It never touches compiled ``.so``/``.pyd`` modules.""" git = str(binaries.Git) - status = subprocess.run( - [git, "-C", str(dest), "status", "--porcelain", "--untracked-files=no"], capture_output=True + subprocess.run([git, "-C", str(dest), "clean", "-fdqx", "--", JUNK[0]], capture_output=True) # nosec B603 + changed = subprocess.run([git, "-C", str(dest), "diff", "--name-only", "-z", "--", *JUNK], capture_output=True) # nosec B603 + if not changed.stdout: + return + subprocess.run( + [git, "--literal-pathspecs", "-C", str(dest), "checkout", "--pathspec-from-file=-", "--pathspec-file-nul"], + input=changed.stdout, + capture_output=True, ) # nosec B603 - if status.returncode != 0 or not status.stdout.strip(): - return - junk = [p for p in porcelain_paths(status.stdout.decode(errors="ignore")) if is_junk_path(p)] - if not junk: - return - subprocess.run([git, "-C", str(dest), "checkout", "--", *junk], capture_output=True) # nosec B603 def is_repo_spec(value: object) -> bool: @@ -173,8 +140,6 @@ def pull(dest: Path, stamp: Path) -> None: if not binaries.Git: write_stamp(stamp) return - # Drop rewritten bytecode before the dirty check / pull so a tracked ``.pyc`` cannot - # false-trigger DirtyServiceRepo or block ``git pull --ff-only``. discard_junk_workdir(dest) # never clobber local edits: refuse to refresh a dirty clone and let the caller exit cleanly if is_dirty(dest): @@ -215,16 +180,12 @@ def head(dest: Path) -> Optional[str]: def local_dirty_services(dest: Path) -> list[str]: """Top-level service dirs with uncommitted edits to tracked files (lost on a force reset).""" - r = subprocess.run([str(binaries.Git), "-C", str(dest), "diff", "--name-only", "HEAD"], capture_output=True) # nosec B603 + r = subprocess.run( + [str(binaries.Git), "-C", str(dest), "diff", "--name-only", "HEAD", "--", *NOT_JUNK], capture_output=True + ) # nosec B603 if r.returncode != 0: return [] - return sorted( - { - line.split("/")[0] - for line in r.stdout.decode(errors="ignore").splitlines() - if line and not is_junk_path(line) - } - ) + return sorted({line.split("/")[0] for line in r.stdout.decode(errors="ignore").splitlines() if line}) def changed_services(dest: Path, old: Optional[str], new: Optional[str]) -> list[str]: @@ -266,20 +227,15 @@ def force_sync(dest: Path, stamp: Path) -> None: def is_dirty(dest: Path) -> bool: """True if the clone has uncommitted edits to tracked files or local commits not on its upstream. - Ignores untracked files (``git pull`` does not remove them) and tracked import/OS noise - (``__pycache__/``, ``*.pyc`` / ``*.pyo``, ``.DS_Store``). Upstream sometimes commits a - ``.pyc``; importing under another interpreter rewrites it and must not look like a - service-source edit. + This function ignores untracked files, tracked bytecode and ``.DS_Store``: importing a service + rewrites a committed ``.pyc`` whose source modification time changed on checkout. """ git = str(binaries.Git) - # modifications to tracked files only (--untracked-files=no skips untracked __pycache__ etc.) status = subprocess.run( - [git, "-C", str(dest), "status", "--porcelain", "--untracked-files=no"], capture_output=True + [git, "-C", str(dest), "status", "--porcelain", "--untracked-files=no", "--", *NOT_JUNK], capture_output=True ) # nosec B603 - if status.returncode == 0: - meaningful = [p for p in porcelain_paths(status.stdout.decode(errors="ignore")) if not is_junk_path(p)] - if meaningful: - return True + if status.returncode == 0 and status.stdout.strip(): + return True # local commits ahead of upstream (committed but not pushed); no upstream → treat as clean ahead = subprocess.run([git, "-C", str(dest), "rev-list", "--count", "@{u}..HEAD"], capture_output=True) # nosec B603 if ahead.returncode == 0: diff --git a/unshackle/core/services.py b/unshackle/core/services.py index 68fcaa84..6e7f2397 100644 --- a/unshackle/core/services.py +++ b/unshackle/core/services.py @@ -28,10 +28,6 @@ log = logging.getLogger("services") DIRTY_REPOS: list[str] = [] SHADOWED: list[str] = [] -# Service clones live inside the tree; writing bytecode there rewrites tracked ``.pyc`` -# files (when upstream committed them) and floods untracked ``__pycache__`` dirs. -sys.dont_write_bytecode = True - def discover_services() -> tuple[list[Path], list[str]]: """Resolve the configured service dirs and glob every ``TAG/__init__.py``.