Selaa lähdekoodia

Implement file:// URL support for LFS

Refactor LFS client architecture to support multiple URL schemes:

- Create base LFSClient class with common interface
- Implement HTTPLFSClient for http/https URLs (existing functionality)
- Implement FileLFSClient for file:// URLs that directly accesses local
  LFS stores
Jelmer Vernooij 2 kuukautta sitten
vanhempi
commit
8cfb4c9ab9
3 muutettua tiedostoa jossa 263 lisäystä ja 15 poistoa
  1. 3 2
      NEWS
  2. 146 10
      dulwich/lfs.py
  3. 114 3
      tests/test_lfs.py

+ 3 - 2
NEWS

@@ -12,8 +12,9 @@
    vendor. Regression from 0.24.2.  (Jelmer Vernooij, #1945)
 
  * Fix LFS URL validation to prevent DNS resolution errors when ``lfs.url`` is
-   configured with an invalid value. Add support for ``file://`` URLs to match
-   git-lfs behavior. (Jelmer Vernooij, #1951)
+   configured with an invalid value. Implement full support for ``file://`` URLs
+   to access local LFS repositories, matching git-lfs behavior.
+   (Jelmer Vernooij, #1951)
 
 0.24.6	2025-10-19
 

+ 146 - 10
dulwich/lfs.py

@@ -383,22 +383,56 @@ def _is_valid_lfs_url(url: str) -> bool:
 
 
 class LFSClient:
-    """LFS client for network operations."""
+    """Base class for LFS client operations."""
 
     def __init__(self, url: str, config: Optional["Config"] = None) -> None:
         """Initialize LFS client.
 
         Args:
-            url: LFS server URL
+            url: LFS server URL (http://, https://, or file://)
             config: Optional git config for authentication/proxy settings
         """
         self._base_url = url.rstrip("/") + "/"  # Ensure trailing slash for urljoin
         self.config = config
-        self._pool_manager: Optional[urllib3.PoolManager] = None
+
+    @property
+    def url(self) -> str:
+        """Get the LFS server URL without trailing slash."""
+        return self._base_url.rstrip("/")
+
+    def download(self, oid: str, size: int, ref: Optional[str] = None) -> bytes:
+        """Download an LFS object.
+
+        Args:
+            oid: Object ID (SHA256)
+            size: Expected size
+            ref: Optional ref name
+
+        Returns:
+            Object content
+        """
+        raise NotImplementedError
+
+    def upload(
+        self, oid: str, size: int, content: bytes, ref: Optional[str] = None
+    ) -> None:
+        """Upload an LFS object.
+
+        Args:
+            oid: Object ID (SHA256)
+            size: Object size
+            content: Object content
+            ref: Optional ref name
+        """
+        raise NotImplementedError
 
     @classmethod
     def from_config(cls, config: "Config") -> Optional["LFSClient"]:
-        """Create LFS client from git config."""
+        """Create LFS client from git config.
+
+        Returns the appropriate subclass (HTTPLFSClient or FileLFSClient)
+        based on the URL scheme.
+        """
         # Try to get LFS URL from config first
         try:
             url = config.get((b"lfs",), b"url").decode()
@@ -411,7 +445,16 @@ class LFSClient:
                     f"Invalid lfs.url in config: {url!r}. "
                     "URL must be an absolute URL with scheme http://, https://, or file://."
                 )
-            return cls(url, config)
+
+            # Return appropriate client based on scheme
+            parsed = urlparse(url)
+            if parsed.scheme in ("http", "https"):
+                return HTTPLFSClient(url, config)
+            elif parsed.scheme == "file":
+                return FileLFSClient(url, config)
+            else:
+                # This shouldn't happen if _is_valid_lfs_url works correctly
+                raise ValueError(f"Unsupported LFS URL scheme: {parsed.scheme}")
 
         # Fall back to deriving from remote URL (same as git-lfs)
         try:
@@ -439,14 +482,24 @@ class LFSClient:
             if not _is_valid_lfs_url(lfs_url):
                 return None
 
-            return LFSClient(lfs_url, config)
+            # Derived URLs are always http/https
+            return HTTPLFSClient(lfs_url, config)
 
         return None
 
-    @property
-    def url(self) -> str:
-        """Get the LFS server URL without trailing slash."""
-        return self._base_url.rstrip("/")
+
+class HTTPLFSClient(LFSClient):
+    """LFS client for HTTP/HTTPS operations."""
+
+    def __init__(self, url: str, config: Optional["Config"] = None) -> None:
+        """Initialize HTTP LFS client.
+
+        Args:
+            url: LFS server URL (http:// or https://)
+            config: Optional git config for authentication/proxy settings
+        """
+        super().__init__(url, config)
+        self._pool_manager: Optional[urllib3.PoolManager] = None
 
     def _get_pool_manager(self) -> "urllib3.PoolManager":
         """Get urllib3 pool manager with git config applied."""
@@ -655,5 +708,88 @@ class LFSClient:
                     raise LFSError(f"Verification failed with status {response.status}")
 
 
+class FileLFSClient(LFSClient):
+    """LFS client for file:// URLs that accesses local filesystem."""
+
+    def __init__(self, url: str, config: Optional["Config"] = None) -> None:
+        """Initialize File LFS client.
+
+        Args:
+            url: LFS server URL (file://)
+            config: Optional git config (unused for file:// URLs)
+        """
+        super().__init__(url, config)
+
+        # Convert file:// URL to filesystem path
+        from urllib.request import url2pathname
+
+        parsed = urlparse(url)
+        if parsed.scheme != "file":
+            raise ValueError(f"FileLFSClient requires file:// URL, got {url!r}")
+
+        # url2pathname handles the conversion properly across platforms
+        path = url2pathname(parsed.path)
+        self._local_store = LFSStore(path)
+
+    def download(self, oid: str, size: int, ref: Optional[str] = None) -> bytes:
+        """Download an LFS object from local filesystem.
+
+        Args:
+            oid: Object ID (SHA256)
+            size: Expected size
+            ref: Optional ref name (ignored for file:// URLs)
+
+        Returns:
+            Object content
+
+        Raises:
+            LFSError: If object not found or size mismatch
+        """
+        try:
+            with self._local_store.open_object(oid) as f:
+                content = f.read()
+        except KeyError as exc:
+            raise LFSError(f"Object not found: {oid}") from exc
+
+        # Verify size
+        if len(content) != size:
+            raise LFSError(f"Size mismatch: expected {size}, got {len(content)}")
+
+        # Verify SHA256
+        actual_oid = hashlib.sha256(content).hexdigest()
+        if actual_oid != oid:
+            raise LFSError(f"OID mismatch: expected {oid}, got {actual_oid}")
+
+        return content
+
+    def upload(
+        self, oid: str, size: int, content: bytes, ref: Optional[str] = None
+    ) -> None:
+        """Upload an LFS object to local filesystem.
+
+        Args:
+            oid: Object ID (SHA256)
+            size: Object size
+            content: Object content
+            ref: Optional ref name (ignored for file:// URLs)
+
+        Raises:
+            LFSError: If size or OID mismatch
+        """
+        # Verify size
+        if len(content) != size:
+            raise LFSError(f"Size mismatch: expected {size}, got {len(content)}")
+
+        # Verify SHA256
+        actual_oid = hashlib.sha256(content).hexdigest()
+        if actual_oid != oid:
+            raise LFSError(f"OID mismatch: expected {oid}, got {actual_oid}")
+
+        # Store the object
+        stored_oid = self._local_store.write_object([content])
+        if stored_oid != oid:
+            raise LFSError(f"Storage OID mismatch: expected {oid}, got {stored_oid}")
+
+
 class LFSError(Exception):
     """LFS-specific error."""

+ 114 - 3
tests/test_lfs.py

@@ -25,6 +25,7 @@ import json
 import os
 import shutil
 import tempfile
+from pathlib import Path
 
 from dulwich import porcelain
 from dulwich.lfs import LFSFilterDriver, LFSPointer, LFSStore
@@ -991,7 +992,7 @@ class LFSClientTests(TestCase):
         super().setUp()
         import threading
 
-        from dulwich.lfs import LFSClient
+        from dulwich.lfs import HTTPLFSClient
         from dulwich.lfs_server import run_lfs_server
 
         # Create temporary directory for LFS storage
@@ -1011,8 +1012,8 @@ class LFSClientTests(TestCase):
 
         self.addCleanup(cleanup_server)
 
-        # Create LFS client pointing to our test server
-        self.client = LFSClient(self.server_url)
+        # Create HTTP LFS client pointing to our test server
+        self.client = HTTPLFSClient(self.server_url)
 
     def test_client_url_normalization(self) -> None:
         """Test that client URL is normalized correctly."""
@@ -1196,3 +1197,113 @@ class LFSClientTests(TestCase):
         self.assertIsNotNone(client2)
         assert client2 is not None  # for mypy
         self.assertEqual(client2.url, "https://example.com/user/repo.git/info/lfs")
+
+
+class FileLFSClientTests(TestCase):
+    """Tests for FileLFSClient with file:// URLs."""
+
+    def setUp(self) -> None:
+        super().setUp()
+        # Create temporary directory for LFS storage
+        self.test_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.test_dir)
+
+        # Create LFS store and populate with test data
+        from dulwich.lfs import FileLFSClient, LFSStore
+
+        self.lfs_store = LFSStore.create(self.test_dir)
+        self.test_content = b"Test file content for FileLFSClient"
+        self.test_oid = self.lfs_store.write_object([self.test_content])
+
+        # Create FileLFSClient pointing to the test directory
+        # Use Path.as_uri() to create proper file:// URLs on all platforms
+        file_url = Path(self.test_dir).as_uri()
+        self.client = FileLFSClient(file_url)
+
+    def test_download_existing_object(self) -> None:
+        """Test downloading an existing object from file:// URL."""
+        content = self.client.download(self.test_oid, len(self.test_content))
+        self.assertEqual(content, self.test_content)
+
+    def test_download_missing_object(self) -> None:
+        """Test downloading a non-existent object raises LFSError."""
+        from dulwich.lfs import LFSError
+
+        fake_oid = "0" * 64
+        with self.assertRaises(LFSError) as cm:
+            self.client.download(fake_oid, 100)
+        self.assertIn("Object not found", str(cm.exception))
+
+    def test_download_size_mismatch(self) -> None:
+        """Test download with wrong size raises LFSError."""
+        from dulwich.lfs import LFSError
+
+        with self.assertRaises(LFSError) as cm:
+            self.client.download(self.test_oid, 999)  # Wrong size
+        self.assertIn("Size mismatch", str(cm.exception))
+
+    def test_upload_new_object(self) -> None:
+        """Test uploading a new object to file:// URL."""
+        import hashlib
+
+        new_content = b"New content to upload"
+        new_oid = hashlib.sha256(new_content).hexdigest()
+
+        # Upload
+        self.client.upload(new_oid, len(new_content), new_content)
+
+        # Verify it was stored
+        with self.lfs_store.open_object(new_oid) as f:
+            stored_content = f.read()
+        self.assertEqual(stored_content, new_content)
+
+    def test_upload_size_mismatch(self) -> None:
+        """Test upload with mismatched size raises LFSError."""
+        from dulwich.lfs import LFSError
+
+        content = b"test"
+        oid = "0" * 64
+
+        with self.assertRaises(LFSError) as cm:
+            self.client.upload(oid, 999, content)  # Wrong size
+        self.assertIn("Size mismatch", str(cm.exception))
+
+    def test_upload_oid_mismatch(self) -> None:
+        """Test upload with mismatched OID raises LFSError."""
+        from dulwich.lfs import LFSError
+
+        content = b"test"
+        wrong_oid = "0" * 64  # Won't match actual SHA256
+
+        with self.assertRaises(LFSError) as cm:
+            self.client.upload(wrong_oid, len(content), content)
+        self.assertIn("OID mismatch", str(cm.exception))
+
+    def test_from_config_creates_file_client(self) -> None:
+        """Test that from_config creates FileLFSClient for file:// URLs."""
+        from dulwich.config import ConfigFile
+        from dulwich.lfs import FileLFSClient, LFSClient
+
+        config = ConfigFile()
+        file_url = Path(self.test_dir).as_uri()
+        config.set((b"lfs",), b"url", file_url.encode())
+
+        client = LFSClient.from_config(config)
+        self.assertIsInstance(client, FileLFSClient)
+        assert client is not None  # for mypy
+        self.assertEqual(client.url, file_url)
+
+    def test_round_trip(self) -> None:
+        """Test uploading and then downloading an object."""
+        import hashlib
+
+        content = b"Round trip test content"
+        oid = hashlib.sha256(content).hexdigest()
+
+        # Upload
+        self.client.upload(oid, len(content), content)
+
+        # Download
+        downloaded = self.client.download(oid, len(content))
+
+        self.assertEqual(downloaded, content)