Skip to content

Commit e01688c

Browse files
committed
fixup pathing
1 parent 18c56ea commit e01688c

3 files changed

Lines changed: 78 additions & 16 deletions

File tree

dimos/core/native_module.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,19 @@ def _maybe_build(self) -> None:
259259

260260
if exe.exists() and not needs_rebuild:
261261
return
262+
262263
if self.config.build_command is None:
263264
raise FileNotFoundError(
264265
f"Executable not found: {exe}. "
265266
"Set build_command in config to auto-build, or build it manually."
266267
)
268+
269+
# Don't unlink the exe before rebuilding — the build command is
270+
# responsible for replacing it. For nix builds the exe lives inside
271+
# a read-only store; `nix build -o` atomically swaps the output
272+
# symlink without touching store contents.
267273
logger.info(
268-
"Executable not found, running build",
274+
"Rebuilding" if needs_rebuild else "Executable not found, building",
269275
executable=str(exe),
270276
build_command=self.config.build_command,
271277
)
@@ -296,7 +302,7 @@ def _maybe_build(self) -> None:
296302
# Update the change cache so next check is clean
297303
if self.config.rebuild_on_change:
298304
cache_name = f"native_{type(self).__name__}_build"
299-
did_change(cache_name, self.config.rebuild_on_change)
305+
did_change(cache_name, self.config.rebuild_on_change, cwd=self.config.cwd)
300306

301307
def _collect_topics(self) -> dict[str, str]:
302308
"""Extract LCM topic strings from blueprint-assigned stream transports."""

dimos/utils/change_detect.py

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,23 @@ def _get_cache_dir() -> Path:
4646

4747

4848
def _resolve_paths(paths: Sequence[str | Path], cwd: str | Path | None = None) -> list[Path]:
49-
"""Expand globs/directories into a sorted list of individual file paths."""
49+
"""Expand globs/directories into a sorted list of individual file paths.
50+
51+
When *cwd* is provided, relative paths and glob patterns are resolved
52+
against it. When *cwd* is ``None``, every entry must be absolute (or an
53+
absolute glob); a relative path will raise :class:`ValueError` so that
54+
callers don't silently resolve against an unpredictable process CWD.
55+
"""
5056
files: set[Path] = set()
5157
for entry in paths:
5258
entry_str = str(entry)
53-
# Resolve relative paths against cwd when provided
54-
if cwd is not None and not Path(entry_str).is_absolute():
59+
is_relative = not Path(entry_str).is_absolute()
60+
if is_relative:
61+
if cwd is None:
62+
raise ValueError(
63+
f"Relative path {entry_str!r} passed to change detection without a cwd. "
64+
"Either provide an absolute path or pass cwd= so relatives can be resolved."
65+
)
5566
entry_str = str(Path(cwd) / entry_str)
5667
# Try glob expansion first (handles both glob patterns and plain paths)
5768
expanded = glob_mod.glob(entry_str, recursive=True)
@@ -91,24 +102,57 @@ def did_change(
91102
paths: Sequence[str | Path],
92103
cwd: str | Path | None = None,
93104
) -> bool:
94-
"""Check if any files/dirs matching the given paths have changed since last check.
105+
"""Check if any files/dirs matching *paths* have changed since last check.
106+
107+
Examples::
108+
109+
# Absolute paths — no cwd needed
110+
did_change("my_build", ["/src/main.cpp", "/src/utils/*.hpp"])
111+
112+
# Relative paths — must pass cwd
113+
did_change("my_build", ["src/main.cpp", "common/*.hpp"], cwd="/home/user/project")
114+
115+
# Track a whole directory (walked recursively)
116+
did_change("assets", ["/data/models/"], cwd="/project")
117+
118+
# Second call with no file changes → False
119+
did_change("my_build", ["/src/main.cpp"]) # True (first call, no cache)
120+
did_change("my_build", ["/src/main.cpp"]) # False (nothing changed)
121+
122+
# After editing a file → True again
123+
Path("/src/main.cpp").write_text("// changed")
124+
did_change("my_build", ["/src/main.cpp"]) # True
125+
126+
# Relative path without cwd → ValueError
127+
did_change("bad", ["src/main.cpp"]) # raises ValueError
95128
96129
Args:
97-
cache_name: Unique identifier for this cache (e.g. ``"mymodule_build_cache"``).
98-
Different cache names track independently.
99-
paths: List of file paths, directory paths, or glob patterns.
130+
cache_name: Unique identifier for this cache. Different names track independently.
131+
paths: File paths, directory paths, or glob patterns.
100132
Directories are walked recursively.
101-
Globs are expanded with :func:`glob.glob`.
102-
cwd: Optional working directory for resolving relative paths.
133+
Relative paths require *cwd*; without it a ``ValueError`` is raised.
134+
cwd: Working directory for resolving relative paths.
103135
104136
Returns:
105-
``True`` if any file has changed (or if no previous cache exists).
106-
``False`` if all files are identical to the cached state.
137+
``True`` if any file has changed (or first call with no prior cache).
138+
``False`` if all files match the cached state, or if no files were found.
107139
"""
108140
if not paths:
109141
return False
110142

111143
files = _resolve_paths(paths, cwd=cwd)
144+
145+
# If none of the monitored paths resolve to actual files (e.g. source
146+
# files don't exist on this branch or checkout), don't claim anything
147+
# changed — deleting a working binary because we can't find the sources
148+
# to compare against is destructive.
149+
if not files:
150+
logger.warning(
151+
"No source files found for change detection, skipping rebuild check",
152+
cache_name=cache_name,
153+
)
154+
return False
155+
112156
current_hash = _hash_files(files)
113157

114158
cache_dir = _get_cache_dir()

dimos/utils/test_change_detect.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,19 @@ def test_empty_paths_returns_false() -> None:
108108

109109

110110
def test_nonexistent_path_warns(caplog: pytest.LogCaptureFixture) -> None:
111-
"""A non-existent path logs a warning and doesn't crash."""
111+
"""A non-existent absolute path logs a warning and doesn't crash."""
112112
result = did_change("missing_test", ["/nonexistent/path/to/file.c"])
113-
# First call with no resolvable files still returns True (no cache)
114-
assert isinstance(result, bool)
113+
# No resolvable files → returns False (skip rebuild)
114+
assert result is False
115+
116+
117+
def test_relative_path_without_cwd_raises() -> None:
118+
"""Relative paths without cwd= should raise ValueError."""
119+
with pytest.raises(ValueError, match="Relative path.*without a cwd"):
120+
did_change("rel_test", ["some/relative/path.c"])
121+
122+
123+
def test_relative_path_with_cwd(src_dir: Path) -> None:
124+
"""Relative paths should resolve against the provided cwd."""
125+
assert did_change("cwd_test", ["src/a.c"], cwd=src_dir.parent) is True
126+
assert did_change("cwd_test", ["src/a.c"], cwd=src_dir.parent) is False

0 commit comments

Comments
 (0)