fix: don't remove obsolete file if it matches excludes pattern (#714)

* fix: don't remove obsolete file if it matches excludes pattern

* rename arg from file_path to glob_pattern

synthtool/metadata.py

@@ -12,15 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import fnmatch
 import locale
 import os
 import pathlib
 import shutil
 import subprocess
 import sys
 import tempfile
-import time
 import threading
+import time
 from typing import Dict, Iterable, List
 
 import google.protobuf.json_format
@@ -40,11 +41,16 @@ def get_environment_bool(var_name: str) -> bool:
 
 _track_obsolete_files = get_environment_bool("SYNTHTOOL_TRACK_OBSOLETE_FILES")
 
+# The list of file patterns excluded during a copy() or move() operation.
+_excluded_patterns: List[str] = []
+
 
 def reset() -> None:
     """Clear all metadata so far."""
     global _metadata
     _metadata = metadata_pb2.Metadata()
+    global _excluded_patterns
+    _excluded_patterns = []
 
 
 def get():
@@ -56,6 +62,13 @@ def add_git_source(**kwargs) -> None:
     _metadata.sources.add(git=metadata_pb2.GitSource(**kwargs))
 
 
+def add_pattern_excluded_during_copy(glob_pattern: str) -> None:
+    """Adds a file excluded during copy.
+
+    Used to avoid deleting an obsolete file that is excluded."""
+    _excluded_patterns.append(glob_pattern)
+
+
 def add_generator_source(**kwargs) -> None:
     """Adds a generator source to the current metadata."""
     _metadata.sources.add(generator=metadata_pb2.GeneratorSource(**kwargs))
@@ -107,11 +120,22 @@ def _remove_obsolete_files(old_metadata):
     """
     old_files = set(old_metadata.generated_files)
     new_files = set(_metadata.generated_files)
+    excluded_patterns = set([pattern for pattern in _excluded_patterns])
     obsolete_files = old_files - new_files
     for file_path in git_ignore(obsolete_files):
         try:
-            logger.info(f"Removing obsolete file {file_path}...")
-            os.unlink(file_path)
+            matched_pattern = False
+            for pattern in excluded_patterns:
+                if fnmatch.fnmatch(file_path, pattern):
+                    matched_pattern = True
+                    break
+            if matched_pattern:
+                logger.info(
+                    f"Leaving obsolete file {file_path} because it matched excluded pattern {pattern} during copy."
+                )
+            else:
+                logger.info(f"Removing obsolete file {file_path}...")
+                os.unlink(file_path)
         except FileNotFoundError:
             pass  # Already deleted.  That's OK.
 

synthtool/transforms.py

@@ -21,6 +21,7 @@
 
 from synthtool import _tracked_paths
 from synthtool.log import logger
+from synthtool import metadata
 
 PathOrStr = Union[str, Path]
 ListOfPathsOrStrs = Iterable[Union[str, Path]]
@@ -178,6 +179,9 @@ def move(
     """
     copied = False
 
+    for excluded_pattern in excludes or []:
+        metadata.add_pattern_excluded_during_copy(str(excluded_pattern))
+
     for source in _expand_paths(sources):
         if destination is None:
             canonical_destination = _tracked_paths.relativize(source)

tests/test_metadata.py

@@ -21,7 +21,7 @@
 
 import pytest
 
-from synthtool import metadata
+from synthtool import _tracked_paths, metadata, transforms
 from synthtool.tmp import tmpdir
 
 
@@ -178,6 +178,25 @@ def test_old_file_removed(source_tree, preserve_track_obsolete_file_flag):
     assert os.path.exists("code/c")
 
 
+def test_excluded_file_not_removed(source_tree, preserve_track_obsolete_file_flag):
+    metadata.set_track_obsolete_files(True)
+    _tracked_paths.add(source_tree.tmpdir / "build")
+
+    with metadata.MetadataTrackerAndWriter(source_tree.tmpdir / "synth.metadata"):
+        source_tree.write("code/b")
+        source_tree.write("code/c")
+
+    metadata.reset()
+    # Create a second source tree and copy it into the first.
+    with metadata.MetadataTrackerAndWriter(source_tree.tmpdir / "synth.metadata"):
+        # exclude code/c from being copied should mean it doesn't get deleted.
+        transforms.move(source_tree.tmpdir / "build", excludes=["code/c"])
+
+    # Confirm remove_obsolete_files deletes b but not c.
+    assert not os.path.exists("code/b")
+    assert os.path.exists("code/c")
+
+
 def test_nothing_happens_when_disabled(source_tree, preserve_track_obsolete_file_flag):
     metadata.set_track_obsolete_files(True)