[TRTLLM-14628][fix] Do not trust just-written mtimes in sync_tree copy-back

scripts/build_wheel.py

@@ -21,6 +21,7 @@
 import sys
 import sysconfig
 import tempfile
+import time
 import warnings
 from argparse import ArgumentParser, ArgumentTypeError
 from contextlib import contextmanager
@@ -614,7 +615,38 @@ def _tar_pipe_copy(src: Path, dst: Path) -> bool:
     return producer.returncode == 0 and consumer.returncode == 0
 
 
-def sync_tree(src, dst, exclude: Sequence[str] = ()):
+# How recently a source file must have been written for its mtime to be
+# untrustworthy as a change marker. Inode timestamps come from a coarse clock
+# (one timer tick on Linux) and some filesystems store whole seconds, so a file
+# rewritten shortly after being copied can still report the mtime the copy
+# recorded. A size+mtime comparison would then call it unchanged and leave a
+# stale copy behind. Two seconds covers a whole-second-granularity destination
+# (which can make an mtime look up to a second older than it is) on top of the
+# tick granularity of the source.
+_MTIME_RACE_WINDOW = 2.0
+
+
+def _demote_racy_mtime(dst_file: Path, src_stat: Optional[os.stat_result],
+                       now: float) -> None:
+    """Break the mtime match for a copy whose source was just written.
+
+    A copy normally records the source's mtime so the next sync can skip it.
+    That is only sound once the source mtime has aged out of the window above;
+    before that the source can change again without its mtime moving. Backdating
+    the copy makes the next sync's comparison mismatch, so the file is re-copied
+    instead of silently kept stale. It costs one extra copy of files written
+    right before a sync, and converges: the re-copy records the real mtime.
+    """
+    if src_stat is None or src_stat.st_mtime <= now - _MTIME_RACE_WINDOW:
+        return
+    try:
+        os.utime(dst_file,
+                 (src_stat.st_atime, src_stat.st_mtime - _MTIME_RACE_WINDOW))
+    except OSError:
+        pass
+
+
+def sync_tree(src: Path, dst: Path, exclude: Sequence[str] = ()) -> None:
     """Mirror the src directory into dst, touching only what changed.
 
     Replaces the rmtree+copytree pattern for artifact copy-back: files are
@@ -623,29 +655,51 @@ def sync_tree(src, dst, exclude: Sequence[str] = ()):
     destination (which may be a slow network filesystem). A missing dst is
     populated via a streamed tar pipeline instead of per-file copies.
     Symlinks are dereferenced like copytree(symlinks=False); mtimes are
-    preserved so the next sync can compare against them. exclude lists
-    fnmatch patterns for entry names to skip.
+    preserved so the next sync can compare against them, except for sources
+    written within _MTIME_RACE_WINDOW of the copy, whose mtimes cannot yet
+    prove the content settled. exclude lists fnmatch patterns for entry names
+    to skip.
     """
     import fnmatch
 
     src = Path(src).resolve()
     dst = Path(dst)
+    now = time.time()
 
-    def excluded(name):
+    def excluded(name: str) -> bool:
         return any(fnmatch.fnmatch(name, pat) for pat in exclude)
 
+    def demote_racy_mtimes() -> None:
+        # A cold populate (tar or copytree) copies source mtimes verbatim, so
+        # apply the same guard the incremental path applies per file. Walk the
+        # source rather than the freshly written destination: the source is
+        # local and warm, and only the few racy entries need a write.
+        for root, dirs, files in os.walk(src, followlinks=True):
+            dirs[:] = [d for d in dirs if not excluded(d)]
+            rel = Path(root).relative_to(src)
+            for name in files:
+                if excluded(name):
+                    continue
+                try:
+                    src_stat = (Path(root) / name).stat()
+                except OSError:
+                    continue
+                _demote_racy_mtime(dst / rel / name, src_stat, now)
+
     if dst.is_symlink():
         dst.unlink()
     elif dst.exists() and src == dst.resolve():
         return
 
     if not dst.exists():
         if not exclude and _tar_pipe_copy(src, dst):
+            demote_racy_mtimes()
             return
         copytree(src,
                  dst,
                  symlinks=False,
                  ignore=shutil.ignore_patterns(*exclude) if exclude else None)
+        demote_racy_mtimes()
         return
 
     for root, dirs, files in os.walk(src, followlinks=True):
@@ -667,18 +721,24 @@ def excluded(name):
         for name in files:
             src_file = Path(root) / name
             dst_file = dst_root / name
+            src_stat = None
             try:
                 src_stat = src_file.stat()
                 dst_stat = dst_file.stat()
+                # Trust the match only once the source mtime has aged past the
+                # race window; a just-written source can be rewritten again
+                # without the mtime moving, which would strand a stale copy.
                 if (src_stat.st_size == dst_stat.st_size
-                        and abs(src_stat.st_mtime - dst_stat.st_mtime) < 1e-3):
+                        and abs(src_stat.st_mtime - dst_stat.st_mtime) < 1e-3
+                        and src_stat.st_mtime <= now - _MTIME_RACE_WINDOW):
                     continue
             except OSError:
                 pass
             if dst_file.is_dir() and not dst_file.is_symlink():
                 rmtree(dst_file)
             # copy2: mtime must survive for the next sync's comparison.
             shutil.copy2(src_file, dst_file)
+            _demote_racy_mtime(dst_file, src_stat, now)
 
 
 def stage_python_package(project_dir: Path, staging_dir: Path) -> None:

tests/unittest/scripts/test_build_wheel_copy_back.py

@@ -30,7 +30,9 @@
 import os
 import shutil
 import stat
+import time
 from pathlib import Path
+from typing import Callable
 
 import pytest
 
@@ -41,11 +43,34 @@
 
 
 @pytest.fixture(scope="module")
-def sync_tree():
+def build_wheel_module():
     spec = importlib.util.spec_from_file_location("build_wheel", SCRIPT_PATH)
     module = importlib.util.module_from_spec(spec)
     spec.loader.exec_module(module)
-    return module.sync_tree
+    return module
+
+
+@pytest.fixture
+def sync_tree(build_wheel_module):
+    return build_wheel_module.sync_tree
+
+
+@pytest.fixture(params=["tar", "copytree"])
+def cold_backend(request, build_wheel_module, monkeypatch):
+    """Force sync_tree's cold populate down each backend in turn.
+
+    A missing destination is filled by _tar_pipe_copy when tar is on PATH and
+    by copytree otherwise. Both preserve mtimes, so the mtime-collision tests
+    that consume this fixture must hold on either; without pinning the backend
+    a run only covers whichever one the environment happens to select. The tar
+    case skips when tar is absent so it can never silently alias to copytree.
+    """
+    if request.param == "tar":
+        if shutil.which("tar") is None:
+            pytest.skip("tar unavailable; cannot exercise the tar cold path")
+    else:
+        monkeypatch.setattr(build_wheel_module, "_tar_pipe_copy", lambda src, dst: False)
+    return request.param
 
 
 def old_copy(src, dst, exclude=()):
@@ -183,6 +208,59 @@ def test_warm_noop_is_stable(sync_tree, tmp_path):
     assert_trees_equal(new_dir, old_dir)
 
 
+def test_rewrite_without_mtime_change_is_not_skipped(
+    sync_tree: Callable[..., None], cold_backend: str, tmp_path: Path
+) -> None:
+    """A same-size rewrite that leaves the source mtime untouched must copy.
+
+    Inode timestamps come from a coarse clock, so a file rewritten shortly
+    after being copied can still report the mtime the copy recorded, and a
+    size+mtime comparison would call it unchanged. utime pins the mtime to the
+    copied value, making that collision deterministic instead of a race.
+    cold_backend runs this once per cold-populate path (tar and copytree).
+    """
+    src = tmp_path / "src"
+    src.mkdir()
+    mod = src / "mod.py"
+    mod.write_bytes(b"def f():\n    return 42\n")
+    new_dir = tmp_path / "new"
+    sync_tree(src, new_dir)
+
+    copied = mod.stat()
+    mod.write_bytes(b"def f():\n    return 99\n")  # same size
+    os.utime(mod, (copied.st_atime, copied.st_mtime))
+    sync_tree(src, new_dir)
+
+    assert (new_dir / "mod.py").read_bytes() == b"def f():\n    return 99\n"
+
+
+def test_settled_file_is_not_recopied(
+    sync_tree: Callable[..., None], cold_backend: str, tmp_path: Path
+) -> None:
+    """A file whose mtime has aged out of the race window is left alone.
+
+    Pins the incremental behaviour itself: the destination is diverged behind
+    sync_tree's back, so surviving the re-sync proves the file was skipped
+    rather than rewritten. cold_backend runs this once per cold-populate path
+    (tar and copytree).
+    """
+    src = tmp_path / "src"
+    src.mkdir()
+    aged = src / "aged.bin"
+    aged.write_bytes(b"aged\n")
+    old = time.time() - 3600
+    os.utime(aged, (old, old))
+    new_dir = tmp_path / "new"
+    sync_tree(src, new_dir)
+
+    dst_file = new_dir / "aged.bin"
+    dst_file.write_bytes(b"kept\n")  # same size as the source
+    os.utime(dst_file, (old, old))
+    sync_tree(src, new_dir)
+
+    assert dst_file.read_bytes() == b"kept\n"
+
+
 def test_same_src_dst_is_noop(sync_tree, tmp_path):
     """sync_tree onto itself must not wipe the tree.