Merge pull request #954 from koordinates/fix-create-parts-if-missing

Bugfix: don't check out all datasets when creating the WC

CHANGELOG.md

@@ -7,6 +7,7 @@ _When adding new entries to the changelog, please include issue/PR numbers where
 ## 0.15.1 (UNRELEASED)
 
 - Prevented committing local changes to linked datasets. [#953](https://github.com/koordinates/kart/pull/953)
+- Fixes a bug where even datasets marked as `--no-checkout` are checked out when the working copy is first created. [#954](https://github.com/koordinates/kart/pull/954)
 
 ## 0.15.0
 

kart/checkout.py

@@ -249,7 +249,9 @@ def checkout(
     elif parts_to_create:
         # Possibly we needn't auto-create any working copy here at all, but lots of tests currently depend on it.
         repo.working_copy.create_parts_if_missing(
-            parts_to_create, reset_to=repo.head_commit
+            parts_to_create,
+            reset_to=repo.head_commit,
+            non_checkout_datasets=non_checkout_datasets,
         )
 
 

kart/working_copy.py

@@ -169,7 +169,9 @@ def workdir_diff_cache(self):
         w = self.workdir
         return w.workdir_diff_cache() if w else None
 
-    def create_parts_if_missing(self, parts_to_create, reset_to=DONT_RESET):
+    def create_parts_if_missing(
+        self, parts_to_create, reset_to=DONT_RESET, non_checkout_datasets=None
+    ):
         """
         Creates the given parts if they are missing and can be created. Returns any created parts themselves.
         parts_to_create is a collection of PartType enum values.
@@ -189,7 +191,7 @@ def create_parts_if_missing(self, parts_to_create, reset_to=DONT_RESET):
 
         if reset_to != self.DONT_RESET:
             for p in created_parts:
-                p.reset(reset_to)
+                p.reset(reset_to, non_checkout_datasets=non_checkout_datasets)
 
         return created_parts
 
@@ -311,7 +313,9 @@ def reset(
             # is newly created since it won't otherwise contain any data yet. The extra parameters (repo_key_filter
             # and track_changes_as_dirty) don't have any effect for a WC part that is newly created.
             created_parts = self.create_parts_if_missing(
-                create_parts_if_missing, reset_to=commit_or_tree
+                create_parts_if_missing,
+                reset_to=commit_or_tree,
+                non_checkout_datasets=non_checkout_datasets,
             )
 
         for p in self.parts():

tests/linked_storage/test_workingcopy.py

@@ -39,3 +39,59 @@ def test_read_only_linked_datasets(
             r.stderr.splitlines()[-1]
             == "Error: Aborting commit due to changes to linked datasets."
         )
+
+
+def test_linked_non_checkout_datasets(
+    data_archive,
+    tmp_path,
+    chdir,
+    cli_runner,
+    s3_test_data_point_cloud,
+    s3_test_data_raster,
+    check_lfs_hashes,
+    check_tile_is_reflinked,
+):
+    repo_path = tmp_path / "linked-dataset-repo"
+    r = cli_runner.invoke(["init", repo_path])
+    assert r.exit_code == 0
+
+    with chdir(repo_path):
+        r = cli_runner.invoke(
+            [
+                "import",
+                "--link",
+                "--no-checkout",
+                s3_test_data_point_cloud,
+                "--dataset-path=auckland",
+            ]
+        )
+        assert r.exit_code == 0, r.stderr
+
+        r = cli_runner.invoke(
+            [
+                "import",
+                "--link",
+                "--no-checkout",
+                s3_test_data_raster,
+                "--dataset-path=erorisk_si",
+            ]
+        )
+        assert r.exit_code == 0, r.stderr
+
+        repo = KartRepo(repo_path)
+        check_lfs_hashes(repo, expected_file_count=0)
+
+        # Make sure we can checkout a single dataset without fetching the other.
+        r = cli_runner.invoke(["checkout", "--dataset=auckland"])
+        assert r.exit_code == 0, r.stderr
+
+        check_lfs_hashes(repo, expected_file_count=16)
+        assert (repo_path / "auckland").is_dir()
+        assert not (repo_path / "erorisk_si").exists()
+
+        for x in range(4):
+            for y in range(4):
+                assert (repo_path / "auckland" / f"auckland_{x}_{y}.laz").is_file()
+                check_tile_is_reflinked(
+                    repo_path / "auckland" / f"auckland_{x}_{y}.laz", repo
+                )