Merge pull request #930 from koordinates/fix-lfs-clean

Fix `kart lfs+ gc` to work with BYOD datasets
koordinates · Oct 30, 2023 · f6873f4 · f6873f4
2 parents 169261a + 5d3f508
commit f6873f4
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git a/kart/lfs_commands/__init__.py b/kart/lfs_commands/__init__.py
@@ -424,22 +424,21 @@ def gc(ctx, dry_run):
     """
     repo = ctx.obj.repo
 
-    remote_name = repo.head_remote_name_or_default
-    if not remote_name:
-        raise InvalidOperation(
-            "LFS files cannot be garbage collected unless there is a remote to refetch them from."
-        )
-
     unpushed_lfs_oids = set()
     for commit_id, path_match_result, pointer_blob in rev_list_tile_pointer_files(
         repo, ["--branches"], ["--remotes"]
     ):
-        unpushed_lfs_oids.add(get_hash_from_pointer_file(pointer_blob))
+        pointer_dict = pointer_file_bytes_to_dict(pointer_blob)
+        if pointer_dict.get("url"):
+            continue
+        unpushed_lfs_oids.add(get_hash_from_pointer_file(pointer_dict))
 
     spatial_filter = repo.spatial_filter
     checked_out_lfs_oids = set()
+    non_checkout_datasets = repo.non_checkout_datasets
     for dataset in repo.datasets("HEAD", filter_dataset_type=ALL_TILE_DATASET_TYPES):
-        checked_out_lfs_oids.update(dataset.tile_lfs_hashes(spatial_filter))
+        if dataset.path not in non_checkout_datasets:
+            checked_out_lfs_oids.update(dataset.tile_lfs_hashes(spatial_filter))
 
     to_delete = set()
     total_size_to_delete = 0
@@ -464,7 +463,7 @@ def gc(ctx, dry_run):
     if to_delete_once_pushed:
         size_desc = human_readable_bytes(total_size_to_delete_once_pushed)
         click.echo(
-            f"Can't delete {len(to_delete_once_pushed)} LFS blobs ({size_desc}) from the cache since they have not been pushed to the remote"
+            f"Can't delete {len(to_delete_once_pushed)} LFS blobs ({size_desc}) from the cache since they have not been pushed to a remote"
         )
 
     size_desc = human_readable_bytes(total_size_to_delete)

diff --git a/tests/byod/test_imports.py b/tests/byod/test_imports.py
@@ -127,6 +127,21 @@ def test_byod_point_cloud_import(
             "  Found nothing to fetch",
         ]
 
+        r = cli_runner.invoke(["lfs+", "gc", "--dry-run"])
+        assert r.exit_code == 0, r.stderr
+        assert r.stdout.splitlines() == [
+            "Running gc with --dry-run: found 0 LFS blobs (0B) to delete from the cache"
+        ]
+
+        r = cli_runner.invoke(["checkout", "--not-dataset=auckland"])
+        assert r.exit_code == 0, r.stderr
+
+        r = cli_runner.invoke(["lfs+", "gc"])
+        assert r.stdout.splitlines() == [
+            "Deleting 16 LFS blobs (373KiB) from the cache..."
+        ]
+        check_lfs_hashes(repo, expected_file_count=0)
+
 
 @pytest.mark.slow
 def test_byod_raster_import(

diff --git a/tests/point_cloud/test_workingcopy.py b/tests/point_cloud/test_workingcopy.py
@@ -1142,7 +1142,7 @@ def test_lfs_gc(cli_runner, data_archive, monkeypatch):
         r = cli_runner.invoke(["lfs+", "gc", "--dry-run"])
         assert r.exit_code == 0, r.stderr
         assert r.stdout.splitlines() == [
-            "Can't delete 4 LFS blobs (82KiB) from the cache since they have not been pushed to the remote",
+            "Can't delete 4 LFS blobs (82KiB) from the cache since they have not been pushed to a remote",
             "Running gc with --dry-run: found 0 LFS blobs (0B) to delete from the cache",
         ]