rolled back change that made tar its own filter type -- big misunderstanding

Author: james-rl

src/runloop_api_client/lib/_ignore.py

@@ -1,9 +1,8 @@
 from __future__ import annotations
 
 import os
-import tarfile
 from abc import ABC, abstractmethod
-from typing import Callable, Iterable, Optional, Sequence
+from typing import Iterable, Optional, Sequence
 from pathlib import Path, PurePosixPath
 from dataclasses import dataclass
 
@@ -12,7 +11,6 @@
     "IgnoreMatcher",
     "DockerIgnoreMatcher",
     "FilePatternMatcher",
-    "TarFilterMatcher",
     "read_ignorefile",
     "compile_ignore",
     "path_match",
@@ -290,74 +288,6 @@ def iter_included_files(
             yield file_path
 
 
-TarFilter = Callable[[tarfile.TarInfo], Optional[tarfile.TarInfo]]
-
-
-def _compute_included_dirs_from_files(included_files: set[str]) -> set[str]:
-    """Return all directory ancestors (plus ``'.'``) for a set of file paths."""
-
-    included_dirs: set[str] = {"."}
-    for rel in included_files:
-        parent = PurePosixPath(rel).parent
-        while True:
-            as_posix = parent.as_posix() or "."
-            included_dirs.add(as_posix)
-            if as_posix == ".":
-                break
-            parent = parent.parent
-    return included_dirs
-
-
-class TarFilterMatcher:
-    """Adapt an :class:`IgnoreMatcher` to a :class:`TarFilter`-compatible callable.
-
-    This helper precomputes the set of included files under ``root`` using the
-    provided :class:`IgnoreMatcher` and converts that into a simple tar filter:
-
-    - Only files returned by ``matcher.iter_paths(root)`` are included.
-    - Directory entries are included only when they are ancestors of at least
-      one included file (plus the root ``'.'`` entry).
-
-    Member names passed to ``__call__`` are expected to be relative to
-    ``root`` and to use POSIX ``'/'`` separators, matching the behaviour of
-    ``build_directory_tar`` in :mod:`runloop_api_client.lib.context_loader`.
-    """
-
-    def __init__(self, root: Path, matcher: IgnoreMatcher) -> None:
-        self._root = root.resolve()
-
-        # Compute the set of included files as relative POSIX paths.
-        # Note: the majority of the work being performed here is simply to deal with the path to the root.
-        included_files: set[str] = set()
-        for path in matcher.iter_paths(self._root):
-            rel = path.resolve().relative_to(self._root)
-            rel_posix = PurePosixPath(rel).as_posix()
-            included_files.add(rel_posix)
-
-        included_dirs = _compute_included_dirs_from_files(included_files)
-
-        self._included_files = included_files
-        self._included_dirs = included_dirs
-
-    def __call__(self, ti: tarfile.TarInfo) -> Optional[tarfile.TarInfo]:
-        name = ti.name
-
-        # The root of the archive is always kept.
-        if name == ".":
-            return ti
-
-        if ti.isdir():
-            if name in self._included_dirs:
-                return ti
-            return None
-
-        # Non-directory entries (files, symlinks, etc.) are kept only if their
-        # relative path is in the included file set.
-        if name in self._included_files:
-            return ti
-        return None
-
-
 class IgnoreMatcher(ABC):
     """Abstract interface for ignore matchers like .dockerignore and .gitignore.
 

src/runloop_api_client/sdk/async_.py

@@ -3,7 +3,7 @@
 from __future__ import annotations
 
 import asyncio
-from typing import Dict, Mapping, Optional, Sequence
+from typing import Dict, Mapping, Optional
 from pathlib import Path
 from datetime import timedelta
 from typing_extensions import Unpack
@@ -24,11 +24,10 @@
 from .._types import Timeout, NotGiven, not_given
 from .._client import DEFAULT_MAX_RETRIES, AsyncRunloop
 from ._helpers import detect_content_type
-from ..lib._ignore import IgnoreMatcher, TarFilterMatcher, FilePatternMatcher
 from .async_devbox import AsyncDevbox
 from .async_snapshot import AsyncSnapshot
 from .async_blueprint import AsyncBlueprint
-from ..lib.context_loader import build_directory_tar
+from ..lib.context_loader import TarFilter, build_directory_tar
 from .async_storage_object import AsyncStorageObject
 from ..types.object_create_params import ContentType
 
@@ -376,7 +375,7 @@ async def upload_from_dir(
         name: Optional[str] = None,
         metadata: Optional[Dict[str, str]] = None,
         ttl: Optional[timedelta] = None,
-        ignore: IgnoreMatcher | Sequence[str] | str | None = None,
+        ignore: TarFilter | None = None,
         **options: Unpack[LongRequestOptions],
     ) -> AsyncStorageObject:
         """Create and upload an object from a local directory.
@@ -391,17 +390,10 @@ async def upload_from_dir(
         :type metadata: Optional[Dict[str, str]]
         :param ttl: Optional Time-To-Live, after which the object is automatically deleted
         :type ttl: Optional[timedelta]
-        :param ignore: Optional ignore configuration controlling which files from
-            ``dir_path`` are included in the uploaded tarball. This may be:
-
-            - An :class:`~runloop_api_client.lib._ignore.IgnoreMatcher`
-              implementation such as :class:`~runloop_api_client.lib._ignore.DockerIgnoreMatcher`
-              or :class:`~runloop_api_client.lib._ignore.FilePatternMatcher`.
-            - A single pattern string.
-            - A sequence of pattern strings.
-
-            Patterns follow Docker-style semantics (``!`` negation, ``**`` support).
-        :type ignore: Optional[IgnoreMatcher | Sequence[str] | str]
+        :param ignore: Optional tar filter function compatible with
+            :meth:`tarfile.TarFile.add`. If provided, it will be called for each
+            member to allow modification or exclusion (by returning ``None``).
+        :type ignore: Optional[TarFilter]
         :param options: See :typeddict:`~runloop_api_client.sdk._types.LongRequestOptions`
             for available options
         :return: Wrapper for the uploaded object
@@ -416,17 +408,7 @@ async def upload_from_dir(
         ttl_ms = int(ttl.total_seconds()) * 1000 if ttl else None
 
         def synchronous_io() -> bytes:
-            matcher: IgnoreMatcher | None
-            if ignore is None:
-                matcher = None
-            elif isinstance(ignore, IgnoreMatcher):
-                matcher = ignore
-            else:
-                matcher = FilePatternMatcher(ignore)  # type: ignore[arg-type]
-
-            if matcher is None:
-                return build_directory_tar(path)
-            return build_directory_tar(path, tar_filter=TarFilterMatcher(path, matcher))
+            return build_directory_tar(path, tar_filter=ignore)
 
         tar_bytes = await asyncio.to_thread(synchronous_io)
 

src/runloop_api_client/sdk/sync.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from typing import Dict, Mapping, Optional, Sequence
+from typing import Dict, Mapping, Optional
 from pathlib import Path
 from datetime import timedelta
 from typing_extensions import Unpack
@@ -26,9 +26,8 @@
 from ._helpers import detect_content_type
 from .snapshot import Snapshot
 from .blueprint import Blueprint
-from ..lib._ignore import IgnoreMatcher, TarFilterMatcher, FilePatternMatcher
 from .storage_object import StorageObject
-from ..lib.context_loader import build_directory_tar
+from ..lib.context_loader import TarFilter, build_directory_tar
 from ..types.object_create_params import ContentType
 
 
@@ -375,7 +374,7 @@ def upload_from_dir(
         name: Optional[str] = None,
         metadata: Optional[Dict[str, str]] = None,
         ttl: Optional[timedelta] = None,
-        ignore: IgnoreMatcher | Sequence[str] | str | None = None,
+        ignore: TarFilter | None = None,
         **options: Unpack[LongRequestOptions],
     ) -> StorageObject:
         """Create and upload an object from a local directory.
@@ -390,17 +389,10 @@ def upload_from_dir(
         :type metadata: Optional[Dict[str, str]]
         :param ttl: Optional Time-To-Live, after which the object is automatically deleted
         :type ttl: Optional[timedelta]
-        :param ignore: Optional ignore configuration controlling which files from
-            ``dir_path`` are included in the uploaded tarball. This may be:
-
-            - An :class:`~runloop_api_client.lib._ignore.IgnoreMatcher`
-              implementation such as :class:`~runloop_api_client.lib._ignore.DockerIgnoreMatcher`
-              or :class:`~runloop_api_client.lib._ignore.FilePatternMatcher`.
-            - A single pattern string.
-            - A sequence of pattern strings.
-
-            Patterns follow Docker-style semantics (``!`` negation, ``**`` support).
-        :type ignore: Optional[IgnoreMatcher | Sequence[str] | str]
+        :param ignore: Optional tar filter function compatible with
+            :meth:`tarfile.TarFile.add`. If provided, it will be called for each
+            member to allow modification or exclusion (by returning ``None``).
+        :type ignore: Optional[TarFilter]
         :param options: See :typeddict:`~runloop_api_client.sdk._types.LongRequestOptions`
             for available options
         :return: Wrapper for the uploaded object
@@ -414,19 +406,7 @@ def upload_from_dir(
         name = name or f"{path.name}.tar.gz"
         ttl_ms = int(ttl.total_seconds()) * 1000 if ttl else None
 
-        # Pick the right matcher
-        matcher: IgnoreMatcher | None
-        if ignore is None:
-            matcher = None
-        elif isinstance(ignore, IgnoreMatcher):
-            matcher = ignore
-        else:
-            matcher = FilePatternMatcher(ignore)  # type: ignore[arg-type]
-
-        if matcher is None:
-            tar_bytes = build_directory_tar(path)
-        else:
-            tar_bytes = build_directory_tar(path, tar_filter=TarFilterMatcher(path, matcher))
+        tar_bytes = build_directory_tar(path, tar_filter=ignore)
 
         obj = self.create(name=name, content_type="tgz", metadata=metadata, ttl_ms=ttl_ms, **options)
         obj.upload_content(tar_bytes)

tests/sdk/test_async_clients.py

@@ -425,8 +425,13 @@ async def test_upload_from_dir_with_inline_ignore_patterns(
 
         client = AsyncStorageObjectOps(mock_async_client)
 
-        # Inline patterns: drop logs and anything under build/
-        obj = await client.upload_from_dir(test_dir, ignore=["*.log", "build/"])
+        # Tar filter: drop logs and anything under build/
+        def ignore_logs_and_build(ti: tarfile.TarInfo) -> tarfile.TarInfo | None:
+            if ti.name.endswith(".log") or ti.name.startswith("build/"):
+                return None
+            return ti
+
+        obj = await client.upload_from_dir(test_dir, ignore=ignore_logs_and_build)
 
         assert isinstance(obj, AsyncStorageObject)
         uploaded_content = http_client.put.call_args[1]["content"]

tests/sdk/test_clients.py

@@ -25,7 +25,6 @@
     BlueprintOps,
     StorageObjectOps,
 )
-from runloop_api_client.lib._ignore import FilePatternMatcher
 from runloop_api_client.lib.polling import PollingConfig
 
 
@@ -486,7 +485,7 @@ def test_upload_from_dir_with_string_path(
     def test_upload_from_dir_respects_filter(
         self, mock_client: Mock, object_view: MockObjectView, tmp_path: Path
     ) -> None:
-        """upload_from_dir should respect ignore patterns when provided."""
+        """upload_from_dir should respect a tar filter when provided."""
         mock_client.objects.create.return_value = object_view
 
         test_dir = tmp_path / "ctx"
@@ -504,10 +503,13 @@ def test_upload_from_dir_respects_filter(
 
         client = StorageObjectOps(mock_client)
 
-        # Ignore patterns: drop logs and anything under build/
-        matcher = FilePatternMatcher(["*.log", "build/"])
+        # Tar filter: drop logs and anything under build/
+        def ignore_logs_and_build(ti: tarfile.TarInfo) -> tarfile.TarInfo | None:
+            if ti.name.endswith(".log") or ti.name.startswith("build/"):
+                return None
+            return ti
 
-        obj = client.upload_from_dir(test_dir, ignore=matcher)
+        obj = client.upload_from_dir(test_dir, ignore=ignore_logs_and_build)
 
         assert isinstance(obj, StorageObject)
         uploaded_content = http_client.put.call_args[1]["content"]

tests/test_utils/test_context_loader.py

@@ -4,18 +4,14 @@
 
 from runloop_api_client.lib._ignore import (
     IgnorePattern,
-    TarFilterMatcher,
     FilePatternMatcher,
     is_ignored,
     path_match,
     compile_ignore,
     read_ignorefile,
     iter_included_files,
 )
-from runloop_api_client.lib.context_loader import (
-    build_directory_tar,
-    build_docker_context_tar,
-)
+from runloop_api_client.lib.context_loader import build_docker_context_tar
 
 
 def test_segment_match_basic_globs():
@@ -184,24 +180,3 @@ def test_build_docker_context_tar_supports_file_pattern_matcher(tmp_path: Path)
 
     assert "keep.bin" in names
     assert "ignore.txt" not in names
-
-
-def test_tar_filter_matcher_respects_patterns(tmp_path: Path) -> None:
-    """TarFilterMatcher should apply FilePatternMatcher patterns at tar level."""
-
-    root = tmp_path
-    (root / "keep.txt").write_text("keep", encoding="utf-8")
-    (root / "ignore.log").write_text("ignore", encoding="utf-8")
-    build_dir = root / "build"
-    build_dir.mkdir()
-    (build_dir / "ignored.txt").write_text("ignored", encoding="utf-8")
-
-    matcher = FilePatternMatcher(["*.log", "build/"])
-    tar_bytes = build_directory_tar(root, tar_filter=TarFilterMatcher(root, matcher))
-
-    with tarfile.open(fileobj=io.BytesIO(tar_bytes), mode="r:gz") as tf:
-        names = {m.name for m in tf.getmembers()}
-
-    assert "keep.txt" in names
-    assert "ignore.log" not in names
-    assert not any(name.startswith("build/") for name in names)