Skip to content

Commit

Permalink
Merge pull request #957 from koordinates/suppress-minor-changes-fix
Browse files Browse the repository at this point in the history
Fixes a bug where minor XML changes are not suppressed properly
  • Loading branch information
olsen232 committed Dec 11, 2023
2 parents ebd653a + 69c9a04 commit 5f7a6dd
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ _When adding new entries to the changelog, please include issue/PR numbers where

- 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)
- Fixes a bug where minor changes to auxiliary metadata (.aux.xml) files that Kart is supposed to ignore were preventing branch changes. [#957](https://github.com/koordinates/kart/pull/957)

## 0.15.0

Expand Down
6 changes: 6 additions & 0 deletions kart/tile/tile_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,12 @@ def diff_to_working_copy(
new_half_delta = tilename, new_tile_summary
else:
new_half_delta = tilename, {"sourceName": workdir_path.name}
expected_pam_path = workdir_path.with_name(
workdir_path.name + PAM_SUFFIX
)
pams = find_similar_files_case_insensitive(expected_pam_path)
if len(pams) == 1:
new_half_delta[1]["pamSourceName"] = pams[0].name

if old_half_delta is None and new_half_delta is None:
# This can happen by eg editing a .tif.aux.xml file that's not attached to anything.
Expand Down
19 changes: 16 additions & 3 deletions kart/workdir.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,26 @@ def is_dirty(self):
self.get_tree_id(), filter_dataset_type=self.SUPPORTED_DATASET_TYPE
)
workdir_diff_cache = self.workdir_diff_cache()
changed_pam_datasets = set()
# First pass - see if any tiles have changed. If yes, return dirty, if no, return clean.
# Exception - if only PAM files have changed, we need to do a second pass.
for dataset in datasets:
ds_tiles_path_pattern = dataset.get_tile_path_pattern(
parent_path=dataset.path
)
for tile_path in workdir_diff_cache.dirty_paths_for_dataset(dataset):
if ds_tiles_path_pattern.fullmatch(tile_path):
return True
if not tile_path.endswith(PAM_SUFFIX):
return True
else:
changed_pam_datasets.add(dataset)

# Second pass - run the actual diff code on datasets with changed PAM files.
# Changes to PAM files may or may not be "minor" diffs which are hidden from the user.
for dataset in changed_pam_datasets:
if dataset.diff_to_working_copy(workdir_diff_cache):
return True

return False

def _is_head(self, commit_or_tree):
Expand Down Expand Up @@ -600,7 +613,7 @@ def _all_names_in_tile_delta(tile_delta):
for tile_summary in tile_delta.old_value, tile_delta.new_value:
if tile_summary is None:
continue
for key in ("name", "sourceName", "pamName", "sourcePamName"):
for key in ("name", "sourceName", "pamName", "pamSourceName"):
if key in tile_summary:
yield tile_summary[key]

Expand Down Expand Up @@ -812,7 +825,7 @@ def _reset_workdir_index_after_commit(
for tile_delta in tile_diff.values():
if tile_delta.type in ("update", "delete"):
old_val = tile_delta.old_value
for key in ("name", "sourceName", "pamName", "sourcePamName"):
for key in ("name", "sourceName", "pamName", "pamSourceName"):
if key in old_val:
workdir_index.remove_all([f"{dataset.path}/{old_val[key]}"])

Expand Down
10 changes: 10 additions & 0 deletions tests/raster/test_pam_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ def test_add_stats_to_new_pam(
r = cli_runner.invoke(["diff", "--exit-code"])
assert r.exit_code == 0

r = cli_runner.invoke(["reset", "--discard-changes"])
assert r.exit_code == 0

assert not pam_path.exists()

# Real changes are not suppressed:
pam_path.write_text("<whatever />")

Expand All @@ -169,6 +174,11 @@ def test_add_stats_to_new_pam(
r = cli_runner.invoke(["diff", "--exit-code"])
assert r.exit_code == 1

r = cli_runner.invoke(["reset", "--discard-changes"])
assert r.exit_code == 0

assert not pam_path.exists()


def test_add_stats_to_existing_pam(
cli_runner,
Expand Down

0 comments on commit 5f7a6dd

Please sign in to comment.