Skip to content

Commit d466802

Browse files
fix(dsl): port corepack PM version pin to Python DSL; fail-fast on legacy pipeline(step) form (#134)
1 parent 4afad53 commit d466802

7 files changed

Lines changed: 302 additions & 17 deletions

File tree

crates/hm-dsl-engine/harmont-py/harmont/__init__.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,28 @@ def my_pipeline() -> Step: ...
8282
8383
Returns:
8484
A v0 IR ``dict`` in factory form, or a decorator in decorator form.
85+
86+
Raises:
87+
TypeError: When called with the legacy variadic ``Step`` form
88+
(``pipeline(step)`` / ``pipeline(a, b)``). The factory now takes
89+
a single list of leaves.
8590
"""
8691
if args and isinstance(args[0], (list, tuple)):
8792
return _pipeline_factory(args[0], **kwargs)
93+
# Legacy form: leaves passed as positional Step args (pre-CLI-9
94+
# `pipeline(step)` / `pipeline(a, b)`). Without this guard the call would
95+
# fall through to the decorator and fail far downstream with a cryptic
96+
# AttributeError. Fail fast with the migration hint instead.
97+
if args and all(isinstance(a, Step) for a in args):
98+
msg = (
99+
"hm.pipeline() takes a single list of leaves, not variadic Step "
100+
"arguments\n"
101+
f" observed: {len(args)} positional Step "
102+
f"argument{'s' if len(args) != 1 else ''}\n"
103+
" → wrap the leaves in a list, e.g. "
104+
"hm.pipeline([step]) or hm.pipeline([a, b])"
105+
)
106+
raise TypeError(msg)
88107
return _decorator.pipeline(*args, **kwargs)
89108

90109

crates/hm-dsl-engine/harmont-py/harmont/_detect.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
class DetectedToolchain:
1717
runtime: Runtime | None = None
1818
pm: Pm | None = None
19+
pm_version: str | None = None
1920

2021

2122
def detect_from_package_json(package_json: dict) -> DetectedToolchain:
2223
runtime: Runtime | None = None
2324
pm: Pm | None = None
25+
pm_version: str | None = None
2426

2527
engines = package_json.get("engines")
2628
if isinstance(engines, dict):
@@ -35,23 +37,25 @@ def detect_from_package_json(package_json: dict) -> DetectedToolchain:
3537
if pm is None:
3638
pm_field = package_json.get("packageManager")
3739
if isinstance(pm_field, str):
38-
name = pm_field.split("@")[0]
40+
parts = pm_field.split("@")
41+
name = parts[0]
42+
ver = parts[1] if len(parts) > 1 else ""
3943
if name == "pnpm":
4044
pm = "pnpm"
4145
elif name == "bun":
4246
pm = "bun"
4347
elif name == "npm":
4448
pm = "npm"
4549
elif name == "yarn":
46-
parts = pm_field.split("@")
47-
ver = parts[1] if len(parts) > 1 else ""
4850
try:
4951
major = int(ver.split(".")[0])
5052
except (ValueError, IndexError):
5153
major = 1
5254
pm = "yarn-berry" if major >= 2 else "yarn-classic"
55+
if pm is not None and ver:
56+
pm_version = ver
5357

54-
return DetectedToolchain(runtime=runtime, pm=pm)
58+
return DetectedToolchain(runtime=runtime, pm=pm, pm_version=pm_version)
5559

5660

5761
def detect_from_lockfiles(files: list[str]) -> DetectedToolchain:
@@ -86,4 +90,5 @@ def detect(path: str) -> DetectedToolchain:
8690
return DetectedToolchain(
8791
runtime=from_pkg.runtime if from_pkg.runtime is not None else from_lock.runtime,
8892
pm=from_pkg.pm if from_pkg.pm is not None else from_lock.pm,
93+
pm_version=from_pkg.pm_version,
8994
)

crates/hm-dsl-engine/harmont-py/harmont/_js.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,32 @@
6969
}
7070

7171

72-
def _pm_bootstrap(pm: PackageManager, runtime: Runtime) -> str | None:
72+
def _pm_bootstrap(pm: PackageManager, runtime: Runtime, version: str | None = None) -> str | None:
7373
"""Command to bring ``pm`` onto the runtime image, or ``None`` when the PM
7474
already ships with the runtime (npm with node, bun/deno with their own
75-
runtime)."""
75+
runtime).
76+
77+
When ``version`` is set (from the project's ``packageManager`` field),
78+
corepack-managed PMs pin that exact version with ``corepack install -g``."""
7679
if pm == "npm":
7780
return None # bundled with node
7881
if pm == "bun":
7982
return None if runtime == "bun" else bun_install_cmd()
8083
if pm == "deno":
8184
return None # bundled with deno
8285
if pm == "pnpm":
83-
return "corepack enable pnpm"
86+
return (
87+
f"corepack enable pnpm && corepack install -g pnpm@{version}"
88+
if version is not None
89+
else "corepack enable pnpm"
90+
)
8491
# corepack resolves the exact yarn from the `packageManager` field; its
8592
# bundled default is classic 1.x, which suits yarn-classic.
86-
return "corepack enable"
93+
return (
94+
f"corepack enable yarn && corepack install -g yarn@{version}"
95+
if version is not None
96+
else "corepack enable"
97+
)
8798

8899

89100
def _validate_version(runtime: Runtime, version: str) -> None:
@@ -171,6 +182,7 @@ def _make_js(
171182

172183
# --- Node / Bun runtime ---
173184
detected_pm = detected.pm if detected else None
185+
pm_version = detected.pm_version if detected else None
174186
resolved_pm: PackageManager = (
175187
pm
176188
if pm is not None
@@ -205,14 +217,22 @@ def _make_js(
205217
)
206218

207219
# Layer the package manager onto the runtime image when it isn't bundled.
208-
bootstrap = _pm_bootstrap(resolved_pm, runtime)
220+
# When a pinned version is detected, the bootstrap step's snapshot must be
221+
# rebuilt whenever package.json (hence the pin) changes; otherwise it is a
222+
# deterministic, forever-cacheable install.
223+
bootstrap = _pm_bootstrap(resolved_pm, runtime, pm_version)
224+
bootstrap_cache = (
225+
CacheOnChange(paths=(f"{path}/package.json",))
226+
if pm_version is not None
227+
else CacheForever(env_keys=())
228+
)
209229
pm_ready = (
210230
runtime_installed
211231
if bootstrap is None
212232
else runtime_installed.sh(
213233
bootstrap,
214234
label=f":{lang_tag}: {resolved_pm}",
215-
cache=CacheForever(env_keys=()),
235+
cache=bootstrap_cache,
216236
)
217237
)
218238

crates/hm-dsl-engine/harmont-py/tests/test_detect.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,31 @@ def test_engines_deno(self) -> None:
3030

3131
def test_package_manager_pnpm(self) -> None:
3232
pkg = {"packageManager": "pnpm@8.15.4"}
33-
assert detect_from_package_json(pkg) == DetectedToolchain(pm="pnpm")
33+
assert detect_from_package_json(pkg) == DetectedToolchain(pm="pnpm", pm_version="8.15.4")
3434

3535
def test_package_manager_bun(self) -> None:
3636
pkg = {"packageManager": "bun@1.1.0"}
37-
assert detect_from_package_json(pkg) == DetectedToolchain(pm="bun")
37+
assert detect_from_package_json(pkg) == DetectedToolchain(pm="bun", pm_version="1.1.0")
3838

3939
def test_package_manager_npm(self) -> None:
4040
pkg = {"packageManager": "npm@10.2.4"}
41-
assert detect_from_package_json(pkg) == DetectedToolchain(pm="npm")
41+
assert detect_from_package_json(pkg) == DetectedToolchain(pm="npm", pm_version="10.2.4")
4242

4343
def test_package_manager_yarn_classic(self) -> None:
4444
pkg = {"packageManager": "yarn@1.22.22"}
45-
assert detect_from_package_json(pkg) == DetectedToolchain(pm="yarn-classic")
45+
assert detect_from_package_json(pkg) == DetectedToolchain(
46+
pm="yarn-classic", pm_version="1.22.22"
47+
)
4648

4749
def test_package_manager_yarn_berry(self) -> None:
4850
pkg = {"packageManager": "yarn@4.0.0"}
49-
assert detect_from_package_json(pkg) == DetectedToolchain(pm="yarn-berry")
51+
assert detect_from_package_json(pkg) == DetectedToolchain(
52+
pm="yarn-berry", pm_version="4.0.0"
53+
)
54+
55+
def test_package_manager_without_version_omits_pm_version(self) -> None:
56+
pkg = {"packageManager": "pnpm"}
57+
assert detect_from_package_json(pkg) == DetectedToolchain(pm="pnpm", pm_version=None)
5058

5159
def test_ignores_unknown_package_manager(self) -> None:
5260
pkg = {"packageManager": "unknown@1.0"}
@@ -60,7 +68,7 @@ def test_engines_bun_overrides_package_manager(self) -> None:
6068
def test_engines_node_plus_package_manager_pnpm(self) -> None:
6169
pkg = {"engines": {"node": ">=18"}, "packageManager": "pnpm@8"}
6270
result = detect_from_package_json(pkg)
63-
assert result == DetectedToolchain(runtime="node", pm="pnpm")
71+
assert result == DetectedToolchain(runtime="node", pm="pnpm", pm_version="8")
6472

6573

6674
class TestDetectFromLockfiles:
@@ -130,4 +138,12 @@ def test_yarn_berry_from_package_manager(self, tmp_path) -> None:
130138
pkg = json.dumps({"packageManager": "yarn@4.5.0"})
131139
(tmp_path / "package.json").write_text(pkg)
132140
(tmp_path / "yarn.lock").touch()
133-
assert detect(str(tmp_path)) == DetectedToolchain(pm="yarn-berry")
141+
assert detect(str(tmp_path)) == DetectedToolchain(pm="yarn-berry", pm_version="4.5.0")
142+
143+
def test_pm_version_propagates_through_detect(self, tmp_path) -> None:
144+
pkg = json.dumps({"packageManager": "pnpm@10.33.0"})
145+
(tmp_path / "package.json").write_text(pkg)
146+
(tmp_path / "pnpm-lock.yaml").touch()
147+
result = detect(str(tmp_path))
148+
assert result.pm == "pnpm"
149+
assert result.pm_version == "10.33.0"

crates/hm-dsl-engine/harmont-py/tests/test_js.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,54 @@ def test_skips_detection_when_pm_set(self, tmp_path) -> None:
344344
assert "pnpm install --frozen-lockfile" in p.install().cmd
345345

346346

347+
# ---------------------------------------------------------------------------
348+
# Corepack version pin (parity with the TypeScript DSL)
349+
# ---------------------------------------------------------------------------
350+
351+
352+
class TestCorepackPin:
353+
def test_pnpm_command_includes_version(self, tmp_path) -> None:
354+
(tmp_path / "package.json").write_text(json.dumps({"packageManager": "pnpm@10.33.0"}))
355+
(tmp_path / "pnpm-lock.yaml").touch()
356+
deps = js.project(path=str(tmp_path)).install()
357+
corepack = deps.parent
358+
assert corepack.cmd == "corepack enable pnpm && corepack install -g pnpm@10.33.0"
359+
360+
def test_yarn_berry_command_includes_version(self, tmp_path) -> None:
361+
(tmp_path / "package.json").write_text(json.dumps({"packageManager": "yarn@4.5.0"}))
362+
(tmp_path / "yarn.lock").touch()
363+
deps = js.project(path=str(tmp_path)).install()
364+
corepack = deps.parent
365+
assert corepack.cmd == "corepack enable yarn && corepack install -g yarn@4.5.0"
366+
367+
def test_command_has_no_version_when_field_absent(self, tmp_path) -> None:
368+
(tmp_path / "package.json").write_text("{}")
369+
(tmp_path / "pnpm-lock.yaml").touch()
370+
deps = js.project(path=str(tmp_path)).install()
371+
corepack = deps.parent
372+
assert corepack.cmd == "corepack enable pnpm"
373+
374+
def test_explicit_pm_omits_version(self) -> None:
375+
# An explicit pm option skips detection entirely, so no pin is applied.
376+
deps = js.project(pm="pnpm").install()
377+
corepack = deps.parent
378+
assert corepack.cmd == "corepack enable pnpm"
379+
380+
def test_cache_watches_package_json_when_pinned(self, tmp_path) -> None:
381+
(tmp_path / "package.json").write_text(json.dumps({"packageManager": "pnpm@10.33.0"}))
382+
(tmp_path / "pnpm-lock.yaml").touch()
383+
deps = js.project(path=str(tmp_path)).install()
384+
corepack = deps.parent
385+
assert corepack.cache.paths == (f"{tmp_path}/package.json",)
386+
387+
def test_cache_is_forever_when_no_field(self, tmp_path) -> None:
388+
(tmp_path / "package.json").write_text("{}")
389+
(tmp_path / "pnpm-lock.yaml").touch()
390+
deps = js.project(path=str(tmp_path)).install()
391+
corepack = deps.parent
392+
assert corepack.cache == hm.CacheForever(env_keys=())
393+
394+
347395
# ---------------------------------------------------------------------------
348396
# ts alias
349397
# ---------------------------------------------------------------------------

crates/hm-dsl-engine/harmont-py/tests/test_pipeline.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,23 @@ def test_pipeline_factory_rejects_no_leaves():
2525
_factory([])
2626

2727

28+
def test_pipeline_rejects_legacy_single_step_form():
29+
# Pre-CLI-9 `pipeline(step)` must fail fast with the migration hint, not
30+
# silently route to the @hm.pipeline decorator and blow up downstream.
31+
step = scratch().sh("echo", label="echo")
32+
with pytest.raises(TypeError, match="single list of leaves") as exc:
33+
pipeline(step)
34+
assert "hm.pipeline([step])" in str(exc.value)
35+
36+
37+
def test_pipeline_rejects_legacy_variadic_step_form():
38+
a = scratch().sh("a", label="a")
39+
b = scratch().sh("b", label="b")
40+
with pytest.raises(TypeError, match="single list of leaves") as exc:
41+
pipeline(a, b)
42+
assert "hm.pipeline([a, b])" in str(exc.value)
43+
44+
2845
def test_pipeline_default_image_lowers_to_dict():
2946
p = pipeline(
3047
[scratch().sh("echo", label="a", image="ubuntu:24.04")],

0 commit comments

Comments
 (0)