From b0e963c6212e70fa51892cb178afb3b8634af94b Mon Sep 17 00:00:00 2001 From: Victor Azizi Date: Mon, 8 Jun 2026 12:58:23 +0000 Subject: [PATCH] Fix conflict on changing dependencies (#96) When a target file no longer depends on one of its former source files, its content changes while the remaining sources are all older than the target. The mtime-based "newer than all of its sources" heuristic in Write.conflict then wrongly reported a conflict. That heuristic is only reached after the filedb hash check has already confirmed the file on disk is exactly what Entangled last wrote (i.e. it was not edited outside of Entangled), so it could only ever produce false positives. Drop it and rely on the filedb comparison, which is the reliable way to detect external edits. Adds a regression test for the dependency-graph change. Co-Authored-By: Claude Opus 4.8 --- entangled/io/transaction.py | 11 ++++++---- test/io/test_transaction.py | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/entangled/io/transaction.py b/entangled/io/transaction.py index 179d1fe..4a9a40d 100644 --- a/entangled/io/transaction.py +++ b/entangled/io/transaction.py @@ -93,12 +93,15 @@ class Write(WriterBase): def conflict(self, fs: AbstractFileCache, db: FileDB) -> Conflict | None: if self.target not in fs: return None + # If the file on disk matches the filedb record, it is exactly what + # Entangled wrote last time and was not edited outside of Entangled, so + # it is safe to overwrite. Comparing against the filedb is more reliable + # than comparing modification times against the current set of sources: + # the latter gives false positives when the dependency graph changes, + # e.g. when a target no longer depends on one of its former sources + # (see issue #96). if fs[self.target].stat != db[self.target]: return Conflict(self.target, "changed outside the control of Entangled") - if self.sources: - if all(fs[s].stat < fs[self.target].stat for s in self.sources): - return Conflict(self.target, "newer than all of its sources: " + ", ".join( - f"`{s}`" for s in set(self.sources))) return None @override diff --git a/test/io/test_transaction.py b/test/io/test_transaction.py index 1874cd1..afab71c 100644 --- a/test/io/test_transaction.py +++ b/test/io/test_transaction.py @@ -1,5 +1,6 @@ from contextlib import chdir from pathlib import Path +from time import sleep from entangled.io.transaction import Transaction, Create, Write, Delete from entangled.io.filedb import filedb @@ -46,3 +47,42 @@ def test_transaction(tmp_path: Path): assert isinstance(t.actions[1], Delete) t.run() assert not Path("b").exists() + + +def test_changing_dependencies(tmp_path: Path): + """Regression test for issue #96: when a target no longer depends on one of + its former sources, Entangled should still be able to update it. The target + is then necessarily newer than its remaining sources, but as long as it was + not edited outside of Entangled (i.e. it matches the filedb), overwriting it + is safe and should not be reported as a conflict.""" + with chdir(tmp_path): + fs = FileCache() + + # the source files are older than anything generated from them + with open("input1", "w") as f: + _ = f.write("source one") + with open("input2", "w") as f: + _ = f.write("source two") + sleep(0.01) + + # first run: `output` is generated from both inputs + with filedb(fs=fs) as db: + t = Transaction(db) + t.write(Path("output"), "from one and two", + [Path("input1"), Path("input2")]) + assert isinstance(t.actions[0], Create) + t.run() + assert Path("output").exists() + + # `output` is now newer than its sources; `input2` no longer writes to + # it, so the content changes while only `input1` remains as a source + fs.reset() + with filedb(fs=fs) as db: + t = Transaction(db) + t.write(Path("output"), "from one only", [Path("input1")]) + assert isinstance(t.actions[0], Write) + # `output` was not touched outside of Entangled, so this is fine + assert t.all_ok() + t.run() + + assert "from one only" in Path("output").read_text()