Skip to content

Commit 3322c00

Browse files
authored
Merge pull request #42 from input-output-hk/fix-installer-recipes
fix: correct broken doctor recipes and harden installer-recipes gate
2 parents 60077dc + 57ade6c commit 3322c00

8 files changed

Lines changed: 80 additions & 23 deletions

File tree

.github/scripts/verify_installers.py

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import shutil
2929
import subprocess
3030
import sys
31+
import tempfile
3132
import tomllib
3233
from pathlib import Path
3334

@@ -100,32 +101,76 @@ def _is_dir(d: Path) -> bool:
100101
return False
101102

102103

103-
def lookup_path() -> str:
104+
def lookup_path(extra: list[Path] | None = None) -> str:
104105
"""PATH augmented with the tool-managed bin dirs (for the presence check)."""
105106
parts = os.environ.get("PATH", "").split(os.pathsep)
106107
parts += [str(d) for d in EXTRA_BIN_DIRS if _is_dir(d)]
108+
parts += [str(d) for d in (extra or []) if _is_dir(d)]
107109
return os.pathsep.join(parts)
108110

109111

110-
def verify_one(dep_id: str, arg: str, binaries: list[str], command: str) -> bool:
111-
"""Run one install command; report whether its binaries are reachable."""
112+
def build_command(installer: str, template: str, arg: str) -> tuple[str, list[Path]]:
113+
"""Render a recipe's install command and any extra bin dirs to look in.
114+
115+
Mirrors `Installer::command()`; `.rstrip()` matches its bare `aikup install`
116+
(empty arg ⇒ latest, no trailing space).
117+
118+
For `nix`, each recipe is redirected into its own throwaway profile. A real
119+
user installs ONE tool, but this gate installs every nix recipe into a
120+
single environment, where two packages that ship the same file collide
121+
(e.g. rustup and cargo both provide `cargo.bash`). Per-recipe profiles
122+
remove that false conflict; the nixpkgs attr installed is exactly what
123+
`doctor` prints — only the profile location differs, in the same spirit as
124+
the EXTRA_BIN_DIRS PATH augmentation above.
125+
"""
126+
command = template.format(arg=arg).rstrip()
127+
extra: list[Path] = []
128+
if installer == "nix":
129+
profile = Path(tempfile.mkdtemp(prefix="nixprof-")) / "profile"
130+
command = command.replace(
131+
"nix profile install ",
132+
f"nix profile install --profile {profile} ",
133+
1,
134+
)
135+
extra.append(profile / "bin")
136+
return command, extra
137+
138+
139+
def verify_one(
140+
dep_id: str, binaries: list[str], command: str, extra_bin_dirs: list[Path]
141+
) -> bool:
142+
"""Run one install command; report whether its binaries are reachable.
143+
144+
Success is defined by the binary being reachable afterwards, NOT by the
145+
install command's exit code. Some recipes exit non-zero for reasons that
146+
don't reflect a broken recipe — e.g. brew's rustup post-install step fails
147+
on a runner that already ships a Rust toolchain, yet `rustup` is installed
148+
and on PATH. We surface a non-zero exit as a warning but only fail when a
149+
declared binary is actually missing.
150+
"""
112151
print(f"::group::{dep_id} via {command}", flush=True)
113-
ok = True
114-
try:
115-
subprocess.run(command, shell=True, check=True)
116-
except subprocess.CalledProcessError as e:
117-
print(f" ✗ install command exited {e.returncode}", flush=True)
118-
ok = False
152+
install_rc = subprocess.run(command, shell=True).returncode
119153

120-
path = lookup_path()
154+
path = lookup_path(extra_bin_dirs)
155+
missing = False
121156
for binary in binaries:
122157
if shutil.which(binary, path=path):
123158
print(f" ✓ {binary} on PATH", flush=True)
124159
else:
125160
print(f" ✗ {binary} NOT found after install", flush=True)
126-
ok = False
161+
missing = True
162+
163+
if install_rc != 0:
164+
if missing:
165+
print(f" ✗ install command exited {install_rc}", flush=True)
166+
else:
167+
print(
168+
f" ! install command exited {install_rc}, but binaries are "
169+
f"present — treating as OK",
170+
flush=True,
171+
)
127172
print("::endgroup::", flush=True)
128-
return ok
173+
return not missing
129174

130175

131176
def main() -> int:
@@ -155,8 +200,8 @@ def main() -> int:
155200

156201
failures: list[str] = []
157202
for dep_id, arg, binaries in methods:
158-
command = template.format(arg=arg)
159-
if not verify_one(dep_id, arg, binaries, command):
203+
command, extra_bin_dirs = build_command(args.installer, template, arg)
204+
if not verify_one(dep_id, binaries, command, extra_bin_dirs):
160205
failures.append(dep_id)
161206

162207
print()

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
[![CI](https://github.com/input-output-hk/cardano-init/actions/workflows/ci.yml/badge.svg)](https://github.com/input-output-hk/cardano-init/actions/workflows/ci.yml)
44
[![Code Quality](https://github.com/input-output-hk/cardano-init/actions/workflows/github-code-scanning/codeql/badge.svg)](https://github.com/input-output-hk/cardano-init/actions/workflows/github-code-scanning/codeql)
55
[![Scheduled Smoke](https://github.com/input-output-hk/cardano-init/actions/workflows/scheduled-smoke.yml/badge.svg)](https://github.com/input-output-hk/cardano-init/actions/workflows/scheduled-smoke.yml)
6+
[![Installer Recipes](https://github.com/input-output-hk/cardano-init/actions/workflows/installer-recipes.yml/badge.svg)](https://github.com/input-output-hk/cardano-init/actions/workflows/installer-recipes.yml)
67

78
**Go from zero to a running Cardano protocol in one command.**
89

docs/ARCHITECTURE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ doctor/
301301
- **Two-tier inputs.** The selection yields **required** deps = `{just}` (universal task runner) ∪ the `system_deps` of all selected tools (unioned, deduped); and **recommended** deps (soft notes, never blocking). The two-tier mechanism stands, but there is **currently no recommended dep**: the former `process-compose`/≥2-infra case existed only to smooth a multi-service top-level `just dev`, which no longer exists (the top level no longer aggregates `dev`; long-running services start per-component — TECH_SPEC §7.2/§9.1). `just` is a base/derived dep owned by no tool.
302302
- **Installers vs deps: the key model.** An **installer** is just another dependency. Code owns a *closed* `Installer` vocabulary (`Brew`, `Apt`, `Dnf`, `Pacman`, `Winget`, `Nix`, `Go`, `Cargo`, `Npm`, `Aikup`, `CardanoUp`, `Curl`, `PowerShell`); each declares its detect-binaries, a command template (`brew install {arg}`, `npm install -g {arg}`, `curl -sSfL {arg} | sh`, …), and a **`bootstrap` list of dep ids**. An **empty `bootstrap` list ⇒ terminal** (we detect it, never install it: system package managers, `nix`, the OS shells); a **non-empty list ⇒ bootstrappable** by installing any one of those deps in order (`npm``["node"]`, `aikup``["aikup"]`, `cargo``["rustup","rust"]`). This is what makes the catalog a graph rather than a flat list.
303303
- **Recipes live in data.** Per-dep recipes are an embedded TOML file (`registry/deps.toml`), keyed by dep id: `binaries` (presence check), `docs` (universal fallback), and an ordered `install` list of `{ installer = arg }` methods. Installer names are validated against the code enum at load (unknown installer → load error, like an unknown `Role`). See §8.1 for why code/data split this way.
304-
- **Resolver (`resolve`, pure, recursive).** A dep is present if any of its `binaries` is on `PATH`. For a missing dep, the walk is **two-pass over the ordered `install` methods**: Pass 1 returns the first method whose installer is **detected** (a one-step command); only if none is directly available does Pass 2 walk the methods again and, for the first **bootstrappable** installer, recurse to satisfy one of its `bootstrap` deps and prepend those steps. The result is an ordered, possibly multi-step **plan** (e.g. `aiken` missing with no `nix`/`aikup` → install `aikup` via `npm`, then `aikup install latest`). Two passes — rather than bootstrapping each method before trying later ones — are exactly why the `nix` path needs no `aikup` when `nix` is present (a single method is still chosen per dep). Cycle detection guards the walk; `docs` is the fallback when nothing resolves (advice never empty, FR-20). Version constraints are out of scope for v1 (presence only); doctor output is **host-dependent by design** (not part of the byte-identical generation contract). Full algorithm in TECH_SPEC §9.4.
304+
- **Resolver (`resolve`, pure, recursive).** A dep is present if any of its `binaries` is on `PATH`. For a missing dep, the walk is **two-pass over the ordered `install` methods**: Pass 1 returns the first method whose installer is **detected** (a one-step command); only if none is directly available does Pass 2 walk the methods again and, for the first **bootstrappable** installer, recurse to satisfy one of its `bootstrap` deps and prepend those steps. The result is an ordered, possibly multi-step **plan** (e.g. `aiken` missing with no `nix`/`aikup` → install `aikup` via `npm`, then `aikup install`). Two passes — rather than bootstrapping each method before trying later ones — are exactly why the `nix` path needs no `aikup` when `nix` is present (a single method is still chosen per dep). Cycle detection guards the walk; `docs` is the fallback when nothing resolves (advice never empty, FR-20). Version constraints are out of scope for v1 (presence only); doctor output is **host-dependent by design** (not part of the byte-identical generation contract). Full algorithm in TECH_SPEC §9.4.
305305
- **Infrastructure deps** install via `cardano-up` (the `CardanoUp` installer); `cardano-up` is itself a dep in `registry/deps.toml` (bootstrappable via its own installer methods). Auto-installing it arrives with the DX.05 install command; bootstrapping `cardano-up` when absent may follow post-RC (ROADMAP).
306306
- **Project scan (no metadata file).** The standalone doctor derives its target set by scanning the cwd: each contract role directory present is matched against the `detect` signatures of the tools that fill that role; an identified tool contributes its `system_deps`, and an unmatched directory is reported as *unrecognized*. A **`protocol/`** directory (the fullstack fused component, §3.2) is scanned by a dedicated branch against the tools that declare a `[fullstack]` template; the identified tool is a real registry tool, so its `system_deps` feed the required set through the normal `registry.get` path (unlike the synthetic infra driver). Signatures are tool-author **data** in `registry/tools/<tool>.toml` (`detect = [...]`), either a bare path (existence) or `{ file, contains }` (content) — the content form keeps generic filenames like `package.json` from mislabeling foreign projects without claiming to validate viability. Full algorithm + schema in TECH_SPEC §9.6.
307307
- **Boundary:** `mod.rs`/`installers.rs`/`catalog.rs` are pure and unit-tested with synthetic `Environment`s; only `probe.rs` touches the system (PATH/OS probes + the project scan). `doctor` depends on `registry`/`contract`, never on `cli`.
@@ -330,7 +330,7 @@ install = [ { npm = "@aiken-lang/aikup" }, { curl = "https://install.aiken-lang
330330
[aiken]
331331
binaries = ["aiken"]
332332
docs = "https://aiken-lang.org/installation-instructions"
333-
install = [ { aikup = "latest" }, { nix = "aiken" } ]
333+
install = [ { aikup = "" }, { nix = "aiken" } ]
334334
```
335335

336336
**Referential integrity (tests):** every `system_deps` id (plus the base dep `just`) has a `registry/deps.toml` entry; every installer named in the data exists in the `Installer` enum; every dep id in an installer's `bootstrap` list exists. The full field-by-field schema and the resolver algorithm are in TECH_SPEC §9.

docs/TECH_SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ The two passes are what make a directly-usable installer win over bootstrapping
544544
{ "id": "node", "required": true, "present": true },
545545
{ "id": "aiken", "required": true, "present": false,
546546
"plan": [ { "installer": "npm", "command": "npm install -g @aiken-lang/aikup" },
547-
{ "installer": "aikup", "command": "aikup install latest" } ],
547+
{ "installer": "aikup", "command": "aikup install" } ],
548548
"docs": "https://aiken-lang.org/installation-instructions" }
549549
]
550550
}}

registry/deps.toml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ install = [{ brew = "node" }, { apt = "nodejs" }, { winget = "OpenJS.NodeJS" },
2727
[aiken]
2828
binaries = ["aiken"]
2929
docs = "https://aiken-lang.org/installation-instructions"
30-
install = [{ aikup = "latest" }, { nix = "aiken" }]
30+
install = [{ aikup = "" }, { nix = "aiken" }]
3131

3232
[sbt]
3333
binaries = ["sbt"]
3434
docs = "https://www.scala-sbt.org/download.html"
35-
install = [{ brew = "sbt" }, { apt = "sbt" }, { nix = "sbt" }]
35+
# No stock `apt` package: sbt ships only via its own scala-sbt apt repo, which a
36+
# single-package recipe can't add — so apt is deliberately omitted (use brew/nix).
37+
install = [{ brew = "sbt" }, { nix = "sbt" }]
3638

3739
[jvm]
3840
binaries = ["java"]
@@ -76,4 +78,6 @@ install = [{ brew = "go" }, { apt = "golang" }, { winget = "GoLang.Go" }, { nix
7678
[process-compose]
7779
binaries = ["process-compose"]
7880
docs = "https://f1bonacc1.github.io/process-compose/"
79-
install = [{ brew = "process-compose" }, { go = "github.com/f1bonacc1/process-compose@latest" }, { nix = "process-compose" }]
81+
# process-compose is not in homebrew-core; it lives in the maintainer's tap.
82+
# `brew install` auto-taps a tap-qualified formula.
83+
install = [{ brew = "f1bonacc1/tap/process-compose" }, { go = "github.com/f1bonacc1/process-compose@latest" }, { nix = "process-compose" }]

src/doctor/catalog.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ mod tests {
152152
assert_eq!(aiken.binaries, vec!["aiken".to_string()]);
153153
// First method is aikup, second is nix (order = preference).
154154
assert_eq!(aiken.install[0].installer, Installer::Aikup);
155-
assert_eq!(aiken.install[0].arg, "latest");
155+
// Empty arg ⇒ `aikup install` (latest); "latest" is not a valid tag.
156+
assert_eq!(aiken.install[0].arg, "");
156157
assert_eq!(aiken.install[1].installer, Installer::Nix);
157158
}
158159

src/doctor/installers.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,9 @@ impl Installer {
142142
Installer::Go => format!("go install {arg}"),
143143
Installer::Cargo => format!("cargo install {arg}"),
144144
Installer::Npm => format!("npm install -g {arg}"),
145+
// An empty arg means "latest" — aikup installs the newest release
146+
// when given no version, so avoid a stray trailing space.
147+
Installer::Aikup if arg.is_empty() => "aikup install".to_string(),
145148
Installer::Aikup => format!("aikup install {arg}"),
146149
Installer::CardanoUp => format!("cardano-up install {arg}"),
147150
Installer::Curl => format!("curl -sSfL {arg} | sh"),
@@ -209,5 +212,8 @@ mod tests {
209212
Installer::Curl.command("https://sh.rustup.rs"),
210213
"curl -sSfL https://sh.rustup.rs | sh"
211214
);
215+
// Empty aikup arg renders bare `aikup install` (latest), no trailing space.
216+
assert_eq!(Installer::Aikup.command(""), "aikup install");
217+
assert_eq!(Installer::Aikup.command("v1.1.0"), "aikup install v1.1.0");
212218
}
213219
}

src/doctor/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ mod tests {
239239
#[test]
240240
fn multi_step_bootstrap_aiken_via_npm() {
241241
// aiken missing, no nix/aikup, but npm present → bootstrap aikup via npm,
242-
// then `aikup install latest`.
242+
// then `aikup install` (bare ⇒ latest).
243243
let report = resolve_all(
244244
&["aiken".to_string()],
245245
&catalog(),
@@ -251,7 +251,7 @@ mod tests {
251251
assert_eq!(aiken.plan[0].installer, Installer::Npm);
252252
assert_eq!(aiken.plan[0].command, "npm install -g @aiken-lang/aikup");
253253
assert_eq!(aiken.plan[1].installer, Installer::Aikup);
254-
assert_eq!(aiken.plan[1].command, "aikup install latest");
254+
assert_eq!(aiken.plan[1].command, "aikup install");
255255
}
256256

257257
#[test]

0 commit comments

Comments
 (0)