Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/catalog/src/mdl_catalog/git_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ class RealGitRunner:

def run(self, args: list[str], cwd: Path) -> tuple[int, str]:
proc = subprocess.run( # noqa: S603,S607 - trusted git argv, no shell
["git", *args], cwd=str(cwd), capture_output=True, text=True
["git", *args],
cwd=str(cwd),
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
return proc.returncode, (proc.stdout or "") + (proc.stderr or "")

Expand Down
34 changes: 25 additions & 9 deletions packages/cli/src/mdl_cli/collab.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def changed_paths(base: str) -> list[str]:
proc = subprocess.run(
["git", "diff", "--name-only", f"{base}...HEAD"],
capture_output=True,
text=True,
text=True, encoding="utf-8", errors="replace",
timeout=30,
)
if proc.returncode != 0:
Expand Down Expand Up @@ -74,10 +74,18 @@ def expired_debt(model_dir: Path) -> list[dict]:

# --- §6.1 merge-driver wiring ---------------------------------------------------

# `eol=lf` pins tool-owned files to LF on checkout. Without it, Git-for-Windows'
# default core.autocrlf=true rewrites them to CRLF, which (a) changes committed vs
# generated bytes so `mdl generate` idempotency flaps, and (b) makes the on-disk byte
# size differ from the LF size, tripping the size-based staleness fingerprint so the
# canvas/LSP reports "model changed on disk" on every edit. `merge=mdl` keeps the
# semantic merge driver.
_GITATTRIBUTES_LINES = [
"{model}/**/*.yaml merge=mdl",
"**/.mdl/decisions.yaml merge=mdl",
"**/.mdl/state/**/*.json merge=mdl-state",
"{model}/**/*.yaml merge=mdl eol=lf",
"{model}/**/*.yml eol=lf",
"**/.mdl/decisions.yaml merge=mdl eol=lf",
"**/.mdl/lock.yaml eol=lf",
"**/.mdl/state/**/*.json merge=mdl-state eol=lf",
]


Expand All @@ -93,7 +101,7 @@ def ensure_git_hooks(repo_root: Path, model_root: str = "model") -> list[str]:
additions.append(line)
if additions:
body = existing.rstrip("\n") + ("\n" if existing else "") + "\n".join(additions)
ga.write_text(body + "\n")
ga.write_text(body + "\n", encoding="utf-8", newline="\n")
actions.append(f".gitattributes += {len(additions)} rule(s)")

def _git(*args: str) -> bool:
Expand Down Expand Up @@ -226,10 +234,12 @@ def scaffold_workspace_skeleton(
(tw / "models" / "staging").mkdir(parents=True, exist_ok=True)
(tw / "macros").mkdir(parents=True, exist_ok=True)
if not (tw / "dbt_project.yml").exists():
(tw / "dbt_project.yml").write_text(_DBT_PROJECT_STUB)
(tw / "dbt_project.yml").write_text(_DBT_PROJECT_STUB, encoding="utf-8", newline="\n")
written.append("transform/warehouse/ (dbt project)")
if not (tw / "profiles.yml").exists():
(tw / "profiles.yml").write_text(_PROFILES_STUB.format(target=dbt_target))
(tw / "profiles.yml").write_text(
_PROFILES_STUB.format(target=dbt_target), encoding="utf-8", newline="\n"
)
written.append("transform/warehouse/profiles.yml")
else:
written.append("(reusing the existing dbt project — no DuckDB project scaffolded)")
Expand All @@ -239,12 +249,18 @@ def scaffold_workspace_skeleton(
gh = root / ".github"
gh.mkdir(exist_ok=True)
if not (gh / "CODEOWNERS").exists():
(gh / "CODEOWNERS").write_text(_CODEOWNERS.format(model=model_root))
(gh / "CODEOWNERS").write_text(
_CODEOWNERS.format(model=model_root), encoding="utf-8", newline="\n"
)
written.append(".github/CODEOWNERS")

ws = root / f"{project_name}.code-workspace"
if not ws.exists():
ws.write_text(_WORKSPACE.format(model=model_root, dbt=dbt_project_dir))
ws.write_text(
_WORKSPACE.format(model=model_root, dbt=dbt_project_dir),
encoding="utf-8",
newline="\n",
)
written.append(ws.name)

written += ensure_git_hooks(root, model_root)
Expand Down
9 changes: 7 additions & 2 deletions packages/cli/src/mdl_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2726,7 +2726,7 @@ def export_json_schema(
for name, cls in kinds.items():
schema = cls.model_json_schema()
schema["$schema"] = "http://json-schema.org/draft-07/schema#"
(out / f"{name}.schema.json").write_text(_json.dumps(schema, indent=2))
(out / f"{name}.schema.json").write_text(_json.dumps(schema, indent=2), encoding="utf-8")
typer.secho(f"wrote {len(kinds)} schemas to {out}", fg=typer.colors.GREEN)


Expand Down Expand Up @@ -3758,7 +3758,12 @@ def _git_metadata(model_dir: Path) -> tuple[str | None, str | None]:
def _git(*args: str) -> str | None:
try:
out = subprocess.run( # noqa: S603,S607 - trusted git argv
["git", *args], cwd=str(model_dir), capture_output=True, text=True
["git", *args],
cwd=str(model_dir),
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
return out.stdout.strip() if out.returncode == 0 and out.stdout.strip() else None
except Exception: # noqa: BLE001
Expand Down
39 changes: 28 additions & 11 deletions packages/core/src/mdl_core/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@

from __future__ import annotations

import hashlib
from dataclasses import dataclass, field
from pathlib import Path

from mdl_core.diagnostics import Severity
from mdl_core.fsnames import fs_slug
from mdl_core.ids import is_ulid, new_ulid
from mdl_core.repo import PROJECT_FILE, ModelRepo, find_project_root
from mdl_core.validate import validate
Expand All @@ -29,15 +31,26 @@


def dir_fingerprint(model_dir: Path) -> str:
"""Cheap change detector over model YAML files (count/mtime/size), string-
encoded so it can round-trip through JSON."""
n, mtime, size = 0, 0.0, 0
for p in Path(model_dir).rglob("*.yaml"):
st = p.stat()
n += 1
mtime = max(mtime, st.st_mtime)
size += st.st_size
return f"{n}:{mtime}:{size}"
"""A change detector over model YAML files, NEWLINE-INSENSITIVE so it is identical on
a repo checked out with LF (POSIX) or CRLF (Git-for-Windows' autocrlf default).

The previous version summed raw `st.st_size`; CRLF adds a byte per line, so the
on-disk size differed from the LF size the client computed and the staleness check
fired on EVERY edit on Windows — the canvas/LSP was effectively read-only. We now hash
each file's content read in text mode (universal-newline normalises CRLF->LF), sorted
by relative path for determinism, so the fingerprint depends on content, not on how
git happened to translate line endings."""
h = hashlib.sha256()
for p in sorted(Path(model_dir).rglob("*.yaml"), key=lambda q: q.as_posix()):
try:
text = p.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError):
continue
h.update(p.relative_to(model_dir).as_posix().encode("utf-8"))
h.update(b"\0")
h.update(text.encode("utf-8"))
h.update(b"\0")
return h.hexdigest()


class StaleModelError(Exception):
Expand Down Expand Up @@ -94,8 +107,12 @@ def apply_command(
# --- helpers ----------------------------------------------------------------


def _slug(name: str) -> str:
return name.strip().lower().replace(" ", "_")
# The command engine slugs a user-supplied name into BOTH the object's identifier and its
# YAML filename, so the slug must be filesystem-safe on every OS. The old version only
# lowercased + replaced spaces, so a name with `/ : * ?`, a trailing dot, or a reserved
# device word (CON, NUL…) injected a path separator or crashed open() on Windows. Reuse
# the one hardened slug from mdl_core.fsnames.
_slug = fs_slug


def _id_from(p: dict, key: str = "id") -> str:
Expand Down
72 changes: 72 additions & 0 deletions packages/core/src/mdl_core/fsnames.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"""Filesystem-safe filename helpers — the one place object names become file names.

A modelled object's name (entity, domain, attribute, physical table…) is turned into a
YAML/SQL filename across the reverse writer, the interactive command engine, and the dbt
emitter. Windows is far stricter than POSIX about what a filename may be, so a name that
is fine on a Mac can crash `open()` or silently collide on a Windows box:

- illegal characters: ``< > : " / \\ | ? *`` and control chars (OSError 22);
- a name ending in a dot or space;
- reserved device names (CON, PRN, AUX, NUL, COM1-9, LPT1-9) — uncreatable *whatever*
the extension;
- case-insensitive collisions: ``Foo`` and ``FOO`` are the SAME file on Windows/macOS.

`fs_slug` produces a safe, lowercased slug (lowercasing also folds case collisions into
one string, which a caller can then dedup); `unique_namer` hands out collision-free names
within one write. Keeping this in one module means every writer stays portable and can't
drift apart.
"""

from __future__ import annotations

import re

# Illegal in a Windows filename, plus C0 control chars. Collapse every run to one "_".
_UNSAFE_FS = re.compile(r'[<>:"/\\|?*\x00-\x1f]+')

# Reserved DOS device names — uncreatable on Windows regardless of extension.
_RESERVED = {
"con", "prn", "aux", "nul",
*(f"com{i}" for i in range(1, 10)),
*(f"lpt{i}" for i in range(1, 10)),
}


def fs_slug(name: str) -> str:
"""A filesystem-safe, portable slug: lowercased (which also folds case-only
collisions), Windows-illegal characters and spaces collapsed to underscores, trailing
dots/spaces stripped, and reserved device names escaped. Never returns empty."""
s = _UNSAFE_FS.sub("_", name).lower().replace(" ", "_")
# Windows forbids a name ending in a dot or space; bare dots are confusing anyway.
s = s.strip("._")
if not s:
return "unnamed"
# A reserved device stem (case-insensitive) can't be a filename even as "con.yaml".
if s in _RESERVED:
s = f"_{s}"
return s


def unique_namer():
"""Return (unique, reset) — `unique(rel)` returns `rel`, or `rel` with a `-2`, `-3`…
suffix on the stem when that path was already handed out in this write. Slugging is
lossy (two names can map to one filename), so without this a later object silently
overwrites an earlier one. Keys are compared case-insensitively, because the target
filesystem may be too. Returns POSIX-style paths."""
used: set[str] = set()

def unique(rel: str) -> str:
rel = rel.replace("\\", "/")
key = rel.lower()
if key not in used:
used.add(key)
return rel
stem, dot, ext = rel.rpartition(".")
n = 2
while f"{stem}-{n}{dot}{ext}".lower() in used:
n += 1
cand = f"{stem}-{n}{dot}{ext}"
used.add(cand.lower())
return cand

return unique
8 changes: 6 additions & 2 deletions packages/core/src/mdl_core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,13 @@ def save(self, root: Path) -> None:
encoding="utf-8",
)

# prune shards for artifacts that no longer exist + the legacy file
# prune shards for artifacts that no longer exist + the legacy file. Compare with
# as_posix(): `wanted` holds forward-slash _shard_rel() keys, but relative_to()
# stringifies with the OS separator (backslash on Windows) — without normalising,
# NO shard matched on Windows and the whole generation-state store was wiped on
# every save (destroying the 3-way-merge base). Same class as writer._prune_stale.
for shard in state_dir.glob("*/*.json"):
if str(shard.relative_to(state_dir)) not in wanted:
if shard.relative_to(state_dir).as_posix() not in wanted:
shard.unlink()
legacy = state_dir / _LEGACY_FILE
if legacy.exists():
Expand Down
50 changes: 50 additions & 0 deletions packages/core/tests/test_fingerprint_newlines.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""The staleness fingerprint must be newline-insensitive.

On Git-for-Windows (core.autocrlf=true) model YAMLs check out with CRLF. The old
size-based fingerprint differed between an LF and a CRLF checkout, so the client's
staleness check fired on every edit and the canvas/LSP was effectively read-only on
Windows. dir_fingerprint now hashes newline-normalised content.
"""

from __future__ import annotations

from pathlib import Path

from mdl_core.commands import dir_fingerprint


def _write(root: Path, rel: str, text: str, *, crlf: bool) -> None:
p = root / rel
p.parent.mkdir(parents=True, exist_ok=True)
data = text.replace("\n", "\r\n") if crlf else text
# write bytes so the newline style is preserved exactly (no translation)
p.write_bytes(data.encode("utf-8"))


_MODEL = "id: 01J\nkind: logical_entity\nname: counterparty\nattributes:\n - name: id\n"


def test_fingerprint_identical_for_lf_and_crlf(tmp_path):
lf = tmp_path / "lf"
crlf = tmp_path / "crlf"
_write(lf, "logical/entities/counterparty.yaml", _MODEL, crlf=False)
_write(crlf, "logical/entities/counterparty.yaml", _MODEL, crlf=True)

# same content, different line endings -> the fingerprint must MATCH
assert dir_fingerprint(lf) == dir_fingerprint(crlf)


def test_fingerprint_changes_on_real_content_edit(tmp_path):
d = tmp_path / "m"
_write(d, "logical/entities/a.yaml", _MODEL, crlf=False)
fp1 = dir_fingerprint(d)
_write(d, "logical/entities/a.yaml", _MODEL + "definition: a party\n", crlf=False)
assert dir_fingerprint(d) != fp1, "a real content change must change the fingerprint"


def test_fingerprint_changes_when_a_file_is_added(tmp_path):
d = tmp_path / "m"
_write(d, "logical/entities/a.yaml", _MODEL, crlf=False)
fp1 = dir_fingerprint(d)
_write(d, "logical/entities/b.yaml", _MODEL, crlf=False)
assert dir_fingerprint(d) != fp1
49 changes: 49 additions & 0 deletions packages/core/tests/test_fsnames.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
"""The shared filesystem-safe name helper — the one place object names become filenames."""

from __future__ import annotations

from mdl_core.fsnames import fs_slug, unique_namer


def test_strips_windows_illegal_chars():
assert fs_slug("<root>") == "root"
assert fs_slug("Order:Line") == "order_line"
assert fs_slug("Foo/Bar") == "foo_bar"
assert fs_slug('a"b|c?d*e') == "a_b_c_d_e"
assert fs_slug("trailing.") == "trailing"
assert fs_slug(" ") == "unnamed"
assert fs_slug("Counterparty") == "counterparty"


def test_escapes_reserved_device_names():
# CON / NUL / COM1 / LPT9 are uncreatable on Windows even as con.yaml
assert fs_slug("CON") == "_con"
assert fs_slug("nul") == "_nul"
assert fs_slug("Com1") == "_com1"
assert fs_slug("LPT9") == "_lpt9"
# a normal name that merely CONTAINS a reserved word is fine
assert fs_slug("control") == "control"


def test_case_fold_collision_via_slug():
# Foo and FOO slug to the same string, so a caller can dedup them
assert fs_slug("Foo") == fs_slug("FOO") == "foo"


def test_unique_namer_disambiguates_collisions():
u = unique_namer()
assert u("logical/entities/foo.yaml") == "logical/entities/foo.yaml"
assert u("logical/entities/foo.yaml") == "logical/entities/foo-2.yaml"
assert u("logical/entities/foo.yaml") == "logical/entities/foo-3.yaml"


def test_unique_namer_is_case_insensitive():
# a case-insensitive filesystem treats Foo.yaml and foo.yaml as the same file
u = unique_namer()
assert u("Foo.yaml") == "Foo.yaml"
assert u("foo.yaml") == "foo-2.yaml"


def test_unique_namer_returns_posix():
u = unique_namer()
assert u("logical\\entities\\foo.yaml") == "logical/entities/foo.yaml"
Loading
Loading