From 8375d989c2970c9c91f1aa3f93783ee4ac41571b Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Tue, 6 Oct 2026 02:52:49 -0400 Subject: [PATCH 1/5] setup cases: a failed load before step 7 must clear setup too (fails on 1.0.2) The shared setup case now also covers the ruling on #468: - a second, different setup before any open is attempted is a UsageError; - an open the loader refuses before step 7 (the stub's fingerprint-other variant: an incompatible artifact, reason "fingerprint") must clear the setup record while no image has completed step 7, so the next, different setup is accepted. An open step may now name its own stub variant; each runner copies every variant a case opens once, and checks a loader refusal by its class and reason. The variant, its reason and its class come from _stubshared.plan and sdk.json's loader.refusals, not from a list here. This commit carries no fix: on 1.0.2 code the case fails at step 3, where the setup after the refused load is still a UsageError. refs #468 Co-Authored-By: Claude Opus 5.5 (1M context) --- go/chtypes/setup_cases_test.go | 37 +++++++-- python/tests/abi1/test_public_setup_cases.py | 51 +++++++++--- rust/tests/api_v1_setup_cases.rs | 61 ++++++++++---- scripts/abi-v1/emit/setup_cases.py | 87 +++++++++++++++----- tests/fixtures/abi-v1/setup-cases.json | 22 +++++ ts/test/abi1/setup-cases.test.ts | 47 ++++++++--- 6 files changed, 241 insertions(+), 64 deletions(-) diff --git a/go/chtypes/setup_cases_test.go b/go/chtypes/setup_cases_test.go index 91132813..d9015506 100644 --- a/go/chtypes/setup_cases_test.go +++ b/go/chtypes/setup_cases_test.go @@ -22,11 +22,13 @@ type setupCaseExpect struct { Status string `json:"status"` ChCode *int32 `json:"ch_code"` ChName *string `json:"ch_name"` + Reason string `json:"reason"` } type setupCaseStep struct { Op string `json:"op"` Timezone string `json:"timezone"` + Variant string `json:"variant"` Expect setupCaseExpect `json:"expect"` } @@ -71,6 +73,21 @@ func setupStepOutcome(want setupCaseExpect, err error) string { if err == nil { return fmt.Sprintf("want a %s error, got success", want.Class) } + switch want.Class { + case "artifact_incompatible", "artifact_corrupt": + sentinel := ErrArtifactIncompatible + if want.Class == "artifact_corrupt" { + sentinel = ErrArtifactCorrupt + } + var ae *ArtifactError + if !errors.Is(err, sentinel) || !errors.As(err, &ae) { + return fmt.Sprintf("want a %s error, got %T: %v", want.Class, err, err) + } + if want.Reason != "" && ae.Reason != want.Reason { + return fmt.Sprintf("reason %q, want %q", ae.Reason, want.Reason) + } + return "" + } var matched bool switch want.Class { case "schema": @@ -107,15 +124,25 @@ func setupStepOutcome(want setupCaseExpect, err error) string { return "" } -// TestSetupCases runs every shared setup case: one fresh image per case, no -// setup recorded at its start, and each step through the public API. +// TestSetupCases runs every shared setup case: one fresh image per variant the +// case opens, no setup recorded at its start, and each step through the public +// API. func TestSetupCases(t *testing.T) { stubDir(t) // skips loudly without the stubs for _, c := range loadSetupCases(t) { t.Run(c.ID, func(t *testing.T) { resetSetup(t) t.Setenv("CHTYPES_ALLOW_UNVERIFIED_LIBRARY", "1") - image := stubFile(t, c.Variant) + images := map[string]string{} + image := func(variant string) string { + if variant == "" { + variant = c.Variant + } + if images[variant] == "" { + images[variant] = stubFile(t, variant) + } + return images[variant] + } if len(c.Steps) == 0 { t.Fatal("a case with no steps") } @@ -125,12 +152,12 @@ func TestSetupCases(t *testing.T) { case "setup": err = Setup(SetupOptions{Timezone: s.Timezone}) case "open": - _, err = OpenUnverified(image, true) + _, err = OpenUnverified(image(s.Variant), true) default: t.Fatalf("step %d: an op this runner does not know: %q", i, s.Op) } if diff := setupStepOutcome(s.Expect, err); diff != "" { - t.Fatalf("step %d (%s %q): %s", i, s.Op, s.Timezone, diff) + t.Fatalf("step %d (%s %q %s): %s", i, s.Op, s.Timezone, s.Variant, diff) } } }) diff --git a/python/tests/abi1/test_public_setup_cases.py b/python/tests/abi1/test_public_setup_cases.py index f3606e65..11872cab 100644 --- a/python/tests/abi1/test_public_setup_cases.py +++ b/python/tests/abi1/test_public_setup_cases.py @@ -1,9 +1,10 @@ """The process setup's shared public-API cases (tests/fixtures/abi-v1/setup-cases.json, generated by scripts/abi-v1/emit/setup_cases.py and read by all four bindings), run -through the public API over a fresh copy of a stub library. +through the public API over fresh copies of the stub libraries. -Each case gets a fresh image (`stub_copy`) and starts with no setup recorded -(`clean_process`). Without `$CHTYPES_ABI1_STUBS` every case skips loudly by name. +Each case gets a fresh image per variant it opens and starts with no setup +recorded (`clean_process`). Without `$CHTYPES_ABI1_STUBS` every case skips loudly +by name. """ from __future__ import annotations @@ -15,6 +16,8 @@ import pytest from chtypes import ( + ArtifactCorruptError, + ArtifactIncompatibleError, InternalError, SchemaError, Status, @@ -34,6 +37,8 @@ "unsupported": UnsupportedError, "usage": UsageError, "internal": InternalError, + "artifact_incompatible": ArtifactIncompatibleError, + "artifact_corrupt": ArtifactCorruptError, } @@ -56,6 +61,8 @@ def _outcome(want: dict, error: BaseException | None) -> str: return f"ch_code {error.ch_code}, want {want['ch_code']}" if "ch_name" in want and error.ch_name != want["ch_name"]: return f"ch_name {error.ch_name!r}, want {want['ch_name']!r}" + if "reason" in want and error.reason != want["reason"]: + return f"reason {error.reason!r}, want {want['reason']!r}" return "" @@ -63,31 +70,49 @@ def _outcome(want: dict, error: BaseException | None) -> str: def test_setup_case( setup_case: dict, stubs_dir, stubs_manifest, tmp_path, clean_process, monkeypatch ) -> None: - image = tmp_path / f"{setup_case['variant']}-image.so" - shutil.copyfile(stub_path(stubs_dir, stubs_manifest["variants"][setup_case["variant"]]), image) - image.chmod(0o755) + images: dict[str, str] = {} + + def image(variant: str | None) -> str: + """The case's one fresh copy of `variant` (the case's own by default).""" + variant = variant or setup_case["variant"] + if variant not in images: + target = tmp_path / f"{variant}-image.so" + shutil.copyfile(stub_path(stubs_dir, stubs_manifest["variants"][variant]), target) + target.chmod(0o755) + images[variant] = str(target) + return images[variant] + monkeypatch.setenv("CHTYPES_ALLOW_UNVERIFIED_LIBRARY", "1") steps = setup_case["steps"] assert steps, "a case with no steps" - warned = False + warned: set[str] = set() for i, step in enumerate(steps): error: BaseException | None = None try: if step["op"] == "setup": setup(timezone=step["timezone"]) elif step["op"] == "open": - # An unverified open warns once per path: on this copy's first open. - expect_warning = not warned - warned = True + path = image(step.get("variant")) + # An unverified open warns once per path: on each copy's first open. + expect_warning = path not in warned + warned.add(path) with ( pytest.warns(UserWarning, match="UNVERIFIED") if expect_warning else contextlib.nullcontext() ): - open_unverified(str(image), allow=True) + open_unverified(path, allow=True) else: pytest.fail(f"step {i}: an op this runner does not know: {step['op']!r}") - except (SchemaError, UnsupportedError, UsageError, InternalError) as exc: + except ( + SchemaError, + UnsupportedError, + UsageError, + InternalError, + ArtifactIncompatibleError, + ArtifactCorruptError, + ) as exc: error = exc diff = _outcome(step["expect"], error) - assert not diff, f"step {i} ({step['op']} {step.get('timezone', '')!r}): {diff}" + where = f"{step['op']} {step.get('timezone', '')!r} {step.get('variant', '')}" + assert not diff, f"step {i} ({where}): {diff}" diff --git a/rust/tests/api_v1_setup_cases.rs b/rust/tests/api_v1_setup_cases.rs index 2ead3b32..176b52c4 100644 --- a/rust/tests/api_v1_setup_cases.rs +++ b/rust/tests/api_v1_setup_cases.rs @@ -10,6 +10,7 @@ //! //! `CHTYPES_ABI1_STUBS` unset: this suite skips LOUDLY by name and passes. +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::Command; @@ -72,6 +73,20 @@ fn outcome(want: &Value, got: &Result<(), Error>) -> Result<(), String> { Ok(()) => return Err(format!("want a {class} error, got success")), Err(e) => e, }; + // A loader refusal: its class, and the reason word it carries. + if class == "artifact_incompatible" || class == "artifact_corrupt" { + let refusal = match (class, err) { + ("artifact_incompatible", Error::ArtifactIncompatible(r)) + | ("artifact_corrupt", Error::ArtifactCorrupt(r)) => r, + (_, other) => return Err(format!("want a {class} error, got {other:?}")), + }; + if let Some(reason) = want.get("reason").and_then(Value::as_str) { + if refusal.reason != reason { + return Err(format!("reason {:?}, want {reason:?}", refusal.reason)); + } + } + return Ok(()); + } let call = match (class, err) { ("schema", Error::Schema(c)) | ("unsupported", Error::Unsupported(c)) @@ -107,27 +122,37 @@ fn outcome(want: &Value, got: &Result<(), Error>) -> Result<(), String> { Ok(()) } -/// The child: one case, on a fresh copy of its stub, from a process with no -/// setup recorded. +/// The child: one case, on a fresh copy of each stub it opens, from a process +/// with no setup recorded. fn run_case(stubs: &Path, case: &Value) { let id = case["id"].as_str().expect("case id"); - let variant = case["variant"].as_str().expect("case variant"); + let case_variant = case["variant"].as_str().expect("case variant"); let manifest: Value = serde_json::from_slice(&std::fs::read(stubs.join("stubs.json")).expect("read stubs.json")) .expect("parse stubs.json"); - let source = stubs.join( - Path::new( - manifest["variants"][variant]["path"] - .as_str() - .expect("variant path"), - ) - .file_name() - .expect("file name"), - ); let dir = std::env::temp_dir().join(format!("chtypes_setup_case_{}", std::process::id())); std::fs::create_dir_all(&dir).expect("create the case directory"); - let image = dir.join(format!("{variant}-image.so")); - std::fs::copy(&source, &image).expect("copy the stub"); + // The case's one fresh copy of each variant, made on its first open. + let mut images: BTreeMap = BTreeMap::new(); + let mut image = |variant: &str| -> PathBuf { + images + .entry(variant.to_string()) + .or_insert_with(|| { + let source = stubs.join( + Path::new( + manifest["variants"][variant]["path"] + .as_str() + .expect("variant path"), + ) + .file_name() + .expect("file name"), + ); + let copy = dir.join(format!("{variant}-image.so")); + std::fs::copy(&source, ©).expect("copy the stub"); + copy + }) + .clone() + }; // SAFETY: the child runs this one test on one thread (--test-threads=1), // so nothing reads or writes the environment concurrently. unsafe { std::env::set_var(ENV_UNVERIFIED, "1") }; @@ -137,16 +162,20 @@ fn run_case(stubs: &Path, case: &Value) { for (i, step) in steps.iter().enumerate() { let op = step["op"].as_str().expect("step op"); let zone = step.get("timezone").and_then(Value::as_str).unwrap_or(""); + let variant = step + .get("variant") + .and_then(Value::as_str) + .unwrap_or(case_variant); let got: Result<(), Error> = match op { "setup" => chtypes::setup(SetupOptions { timezone: Some(zone.to_string()), defaults: Vec::new(), }), - "open" => Library::open_unverified(&image, true).map(|_| ()), + "open" => Library::open_unverified(image(variant), true).map(|_| ()), other => panic!("case {id} step {i}: an op this runner does not know: {other:?}"), }; if let Err(diff) = outcome(&step["expect"], &got) { - panic!("case {id} step {i} ({op} {zone:?}): {diff}"); + panic!("case {id} step {i} ({op} {zone:?} {variant}): {diff}"); } } let _ = std::fs::remove_dir_all(&dir); diff --git a/scripts/abi-v1/emit/setup_cases.py b/scripts/abi-v1/emit/setup_cases.py index 98b6eee8..afc52d48 100644 --- a/scripts/abi-v1/emit/setup_cases.py +++ b/scripts/abi-v1/emit/setup_cases.py @@ -1,7 +1,7 @@ """tests/fixtures/abi-v1/setup-cases.json: the process setup's public-API cases (docs/reference/bindings-v1.md section 6), defined once and run by every -binding's public-API suite through its PUBLIC API, against a fresh copy of a -stub library that scripts/abi-v1/build-stubs.sh builds. +binding's public-API suite through its PUBLIC API, against fresh copies of the +stub libraries that scripts/abi-v1/build-stubs.sh builds. WHY A SEPARATE FILE. tests/fixtures/abi-v1/cases.json is call-level: each of its cases is one C call through a binding's generated layer, and its runners @@ -10,10 +10,11 @@ memo, so it lives here. Only the public-API suites read it, and it never reaches the call-level runners or their reports (scripts/abi-v1/parity.py). -HOW A RUNNER RUNS A CASE. A case names a stub `variant`. The runner: +HOW A RUNNER RUNS A CASE. A case names a stub `variant`, and an open step may +name another one. The runner: - 1. copies that variant's library to a fresh path, once per case, so the - case gets a fresh image (the stub keeps its image zone per image, as the + 1. copies each variant the case uses to a fresh path, once per case, so the + case gets fresh images (the stub keeps its image zone per image, as the library does: _stubshared.IMAGE_ZONE); 2. starts with no setup recorded and no image ever set up (a fresh process, or the binding's own test-only reset); @@ -22,24 +23,34 @@ outcome differs from its `expect`: * {"op": "setup", "timezone": ""}: the binding's `setup` with that image zone and no defaults; - * {"op": "open"}: the binding's unverified open of the case's ONE copy - (Go `OpenUnverified(path, true)`, Python `open_unverified(path, - allow=True)`, TypeScript `openUnverified(path, { allow: true })`, Rust - `Library::open_unverified(path, true)`), the same file for every open - step of the case. + * {"op": "open"} or {"op": "open", "variant": ""}: the binding's + unverified open (Go `OpenUnverified(path, true)`, Python + `open_unverified(path, allow=True)`, TypeScript `openUnverified(path, + { allow: true })`, Rust `Library::open_unverified(path, true)`) of the + case's ONE copy of that variant (the case's own `variant` when the + step names none), the same file for every open step that names it. `expect` is {"ok": true}, or the error the step must raise: `class` is a key of -spec/abi-v1/sdk.json's errors.classes (each binding's own class for it), and an +spec/abi-v1/sdk.json's errors.classes (each binding's own class for it). An error the library answered also names the `status`, `ch_code` and `ch_name` it -must carry, verbatim. +must carry, verbatim; a loader refusal names its `reason`, sdk.json's word. THE CASE. Setup latches only once an image completes load step 7 (chs_initialize, -then chs_set_defaults when there are defaults). A step 7 failure before any image -has completed it clears the setup record, so a corrected setup can be recorded -and the next open runs step 7 with it; a failed step 7 is never remembered, so -the same open runs step 7 again. Once an image has completed step 7, a different -setup is a UsageError. The bad zone, its refusal and the stub rule behind both -come from _stubshared.IMAGE_ZONE, which the stub is generated from too. +then chs_set_defaults when there are defaults). Before that: + + * a second, different setup before any open is attempted is a UsageError, + never a silent last-wins; + * ANY failed open clears the setup record, whatever failed: here a load the + loader refuses at step 4 (_stubshared.plan's "fingerprint-other" variant, + an incompatible artifact), and a zone the library refuses at step 7. The + next setup is then accepted, whatever it is, and the next open runs with + it; + * a failed open is never remembered, so the same open runs again. + +Once an image has completed step 7, a different setup is a UsageError. The bad +zone, its refusal and the stub rule behind both come from _stubshared.IMAGE_ZONE, +which the stub is generated from too; the refused load and its reason come from +_stubshared.plan, which build-stubs.sh builds from. """ from __future__ import annotations @@ -65,6 +76,33 @@ def _class_of(model, status: str) -> str: return cls +# The stub variant a loader refuses before step 7, through the unverified open: +# an incompatible artifact, the failure the issue was reported on. +REFUSED_VARIANT = "fingerprint-other" + + +def _refused_load(model) -> tuple[str, dict]: + """(the variant, the expectation for opening it): its reason from + _stubshared.plan, its class and step from sdk.json's loader.refusals. The + step must come before step 7, and the unverified open must reach it (it + skips steps 1 and 5).""" + variant = next((v for v in _stubshared.plan(model) if v.name == REFUSED_VARIANT), None) + if variant is None: + raise ValueError(f"_stubshared.plan has no {REFUSED_VARIANT!r} variant") + base = variant.reason.split(":", 1)[0] + rows = [r for r in model.sdk["loader"]["refusals"] if r["reason"] == base] + if not rows: + raise ValueError(f"spec/abi-v1/sdk.json's loader.refusals has no reason {base!r}") + steps = {r["step"] for r in rows} + if steps & {1, 5} or max(steps) >= 7: + raise ValueError(f"{REFUSED_VARIANT}: reason {base!r} is refused at step(s) {sorted(steps)}, " + "which the unverified open skips or which is not before step 7") + classes = {r["error"] for r in rows} + if len(classes) != 1 or not classes <= set(model.sdk["errors"]["classes"]): + raise ValueError(f"{REFUSED_VARIANT}: reason {base!r} maps to error classes {sorted(classes)}") + return variant.name, {"class": classes.pop(), "reason": variant.reason} + + def _latch_case(model) -> dict: rule = _stubshared.IMAGE_ZONE fn = model.function(rule["fn"]) @@ -79,12 +117,21 @@ def _latch_case(model) -> dict: # A different setup is misuse, the class of CHS_INVALID_ARGUMENT, which is # also what the library answers a different spelling with. misuse = {"class": _class_of(model, rule["conflict_status"])} + refused_variant, refused_load = _refused_load(model) return { "id": "setup.latches_only_on_success", "variant": "ok", "steps": [ - # A zone the library refuses: setup records it, and the open that - # reaches step 7 raises the library's own refusal. + # Before any open is attempted, a second, different setup is + # misuse: the first stands, never a silent last-wins. + {"op": "setup", "timezone": OTHER_ZONE, "expect": OK}, + {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, + # A load the loader refuses before step 7. No image has completed + # step 7, so the failed open clears the record. + {"op": "open", "variant": refused_variant, "expect": refused_load}, + # So a different setup is accepted now: a zone the library + # refuses. The open that reaches step 7 raises the library's own + # refusal, and clears the record again. {"op": "setup", "timezone": rule["bad_zone"], "expect": OK}, {"op": "open", "expect": refused}, # No image has completed step 7, so the record was cleared: the diff --git a/tests/fixtures/abi-v1/setup-cases.json b/tests/fixtures/abi-v1/setup-cases.json index acaa1b99..165ca997 100644 --- a/tests/fixtures/abi-v1/setup-cases.json +++ b/tests/fixtures/abi-v1/setup-cases.json @@ -4,6 +4,28 @@ { "id": "setup.latches_only_on_success", "steps": [ + { + "expect": { + "ok": true + }, + "op": "setup", + "timezone": "Asia/Tokyo" + }, + { + "expect": { + "class": "usage" + }, + "op": "setup", + "timezone": "Europe/Berlin" + }, + { + "expect": { + "class": "artifact_incompatible", + "reason": "fingerprint" + }, + "op": "open", + "variant": "fingerprint-other" + }, { "expect": { "ok": true diff --git a/ts/test/abi1/setup-cases.test.ts b/ts/test/abi1/setup-cases.test.ts index b99a1758..351236e2 100644 --- a/ts/test/abi1/setup-cases.test.ts +++ b/ts/test/abi1/setup-cases.test.ts @@ -2,10 +2,10 @@ * The process setup's shared public-API cases * (`tests/fixtures/abi-v1/setup-cases.json`, generated by * `scripts/abi-v1/emit/setup_cases.py` and read by all four bindings), run - * through the public API over a fresh copy of a stub library. + * through the public API over fresh copies of the stub libraries. * - * Each case gets a fresh image (its own copy of the stub) and starts with no - * setup recorded. Without `$CHTYPES_ABI1_STUBS` every case SKIPS LOUDLY by + * Each case gets a fresh image per variant it opens (its own copy of that + * stub) and starts with no setup recorded. Without `$CHTYPES_ABI1_STUBS` every case SKIPS LOUDLY by * name; it never passes silently. */ @@ -13,7 +13,17 @@ import { chmodSync, copyFileSync, mkdtempSync, readFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { describe, expect, it, vi } from 'vitest'; -import { InternalError, openUnverified, SchemaError, Status, setup, UnsupportedError, UsageError } from '../../src/index.js'; +import { + ArtifactCorruptError, + ArtifactIncompatibleError, + InternalError, + openUnverified, + SchemaError, + Status, + setup, + UnsupportedError, + UsageError, +} from '../../src/index.js'; import { resetSetupForTests } from '../../src/setup.js'; const STUBS_DIR = process.env.CHTYPES_ABI1_STUBS; @@ -27,11 +37,13 @@ interface Expect { readonly status?: string; readonly ch_code?: number; readonly ch_name?: string; + readonly reason?: string; } interface Step { readonly op: string; readonly timezone?: string; + readonly variant?: string; readonly expect: Expect; } @@ -48,6 +60,8 @@ const CLASSES: Readonly Error> unsupported: UnsupportedError, usage: UsageError, internal: InternalError, + artifact_incompatible: ArtifactIncompatibleError, + artifact_corrupt: ArtifactCorruptError, }; /** `CHS_INVALID_ARGUMENT` -> `Status.InvalidArgument`: the generated vocabulary's own spelling of a status name. */ @@ -73,6 +87,8 @@ function outcome(want: Expect, error: unknown): string { } if (want.ch_code !== undefined && e.chCode !== want.ch_code) return `ch_code ${e.chCode}, want ${want.ch_code}`; if (want.ch_name !== undefined && e.chName !== want.ch_name) return `ch_name ${e.chName}, want ${want.ch_name}`; + const reason = (error as { reason?: unknown }).reason; + if (want.reason !== undefined && reason !== want.reason) return `reason ${String(reason)}, want ${want.reason}`; return ''; } @@ -86,9 +102,19 @@ describe.skipIf(!stubsAvailable)('the shared setup cases, through the public API it.each(setupCases.map((c) => [c.id, c] as const))('%s', (_id, c) => { expect(c.steps.length).toBeGreaterThan(0); const dir = mkdtempSync(path.join(os.tmpdir(), 'setup-case-')); - const image = path.join(dir, `${c.variant}-image.so`); - copyFileSync(path.join(STUBS_DIR as string, `${c.variant}.so`), image); - chmodSync(image, 0o755); + const images = new Map(); + /** The case's one fresh copy of `variant` (the case's own by default). */ + const image = (variant: string | undefined): string => { + const v = variant ?? c.variant; + let copy = images.get(v); + if (copy === undefined) { + copy = path.join(dir, `${v}-image.so`); + copyFileSync(path.join(STUBS_DIR as string, `${v}.so`), copy); + chmodSync(copy, 0o755); + images.set(v, copy); + } + return copy; + }; resetSetupForTests(); const saved = process.env.CHTYPES_ALLOW_UNVERIFIED_LIBRARY; const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); @@ -98,14 +124,15 @@ describe.skipIf(!stubsAvailable)('the shared setup cases, through the public API let error: unknown; try { if (step.op === 'setup') setup({ timezone: step.timezone ?? '' }); - else if (step.op === 'open') openUnverified(image, { allow: true }); + else if (step.op === 'open') openUnverified(image(step.variant), { allow: true }); else throw new Error(`step ${i}: an op this runner does not know: ${step.op}`); } catch (err) { - if (!(err instanceof SchemaError || err instanceof UnsupportedError || err instanceof UsageError || err instanceof InternalError)) throw err; + const known = Object.values(CLASSES).some((cls) => err instanceof cls); + if (!known) throw err; error = err; } const diff = outcome(step.expect, error); - expect(diff, `step ${i} (${step.op} ${JSON.stringify(step.timezone ?? '')})`).toBe(''); + expect(diff, `step ${i} (${step.op} ${JSON.stringify(step.timezone ?? '')} ${step.variant ?? ''})`).toBe(''); }); } finally { warn.mockRestore(); From 6c11397f247ebf3bb971dd42fff0033050962f7d Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Tue, 6 Oct 2026 03:03:33 -0400 Subject: [PATCH 2/5] setup: any failed open before the first step 7 clears setup (#468) 1.0.2 settled the setup around load step 7 only, so a setup stayed recorded after an open that failed earlier (the fetch, the signature, an incompatible artifact, a missing symbol), and a different setup was refused until the process restarted. Option 2 as ruled: while no image has completed step 7, ANY failed open clears the record; once one has, today's rule holds; and a second, different setup before any open is attempted is still a UsageError. Each binding now settles around the whole public open, not step 7: - an image that completes step 7 latches the setup, under the image lock that already holds its commit and load; - every public open (Go Registry.For/OpenUnverified/OpenLinked, Python Registry.for_version/open_unverified, TS registry.for/openUnverified, Rust Registry::for_version/Library::open_unverified) reads a generation when it begins and, on any failure, clears the record under that same lock, unless the setup has latched or a setup was recorded after the open began. The step-7-only callbacks from 1.0.2 are gone (Python's loader settle=, TS ImageSetup.settle). TS moves the spelling check inside the settled open, so a refused spelling clears the record there as it does in the other three. docs/reference/bindings-v1.md section 6 rules 1, 2 and 4 and the settings guide say so, including that an open with no setup after a failure records the empty setup. Each CHANGELOG has an [Unreleased] Fixed entry; no version bump. refs #468 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/guides/settings.md | 2 +- docs/reference/bindings-v1.md | 6 +-- go/CHANGELOG.md | 4 ++ go/chtypes/library.go | 27 +++++------ go/chtypes/linked.go | 8 ++- go/chtypes/registry.go | 11 ++++- go/chtypes/settings_test.go | 55 +++++++++++++++++++++ go/chtypes/setup.go | 63 ++++++++++++++++-------- python/CHANGELOG.md | 4 ++ python/src/chtypes/_abi1/_loader.py | 29 +++-------- python/src/chtypes/_setup.py | 67 ++++++++++++++++++-------- python/src/chtypes/library.py | 47 ++++++++++-------- python/src/chtypes/registry.py | 12 ++++- python/tests/abi1/test_public_open.py | 56 +++++++++++++++++---- rust/CHANGELOG.md | 4 ++ rust/src/library.rs | 42 +++++++++++----- rust/src/registry.rs | 33 ++++++++++--- rust/src/setup.rs | 57 +++++++++++++++------- rust/tests/api_v1_setup_failed_open.rs | 57 ++++++++++++++++++++++ ts/CHANGELOG.md | 4 ++ ts/src/abi1/loader.ts | 14 +----- ts/src/library.ts | 15 ++++-- ts/src/registry.ts | 10 ++-- ts/src/setup.ts | 38 ++++++++++----- ts/test/settings-setup.test.ts | 37 +++++++++++++- 25 files changed, 517 insertions(+), 185 deletions(-) create mode 100644 rust/tests/api_v1_setup_failed_open.rs diff --git a/docs/guides/settings.md b/docs/guides/settings.md index 3f065fc7..b3e1b69d 100644 --- a/docs/guides/settings.md +++ b/docs/guides/settings.md @@ -19,7 +19,7 @@ per-call settings > handle compile profile > setup defaults > ClickHouse's | setup defaults | `setup(defaults=…)` | the whole process, applied to every library when it opens | | ClickHouse defaults | the vendored build | everything | -**The setup defaults latch.** `setup` is called once, before the first library opens, and the defaults it records are applied to each library as it loads and never again. A second `setup` with the same zone and defaults is a no-op; a different one is a `UsageError` naming both, and the first stands. If `setup` was never called, the first open commits the empty setup: no defaults, and the library's default zone, `UTC`. The latch closes only when a library completes its setup. If the first open fails there, because the library refuses the zone or a default, the record is cleared, so a corrected `setup` is accepted and the next open uses it ([`reference/bindings-v1.md` §6](../reference/bindings-v1.md#6-setup-and-from-a-request-to-a-loaded-library)). So call `setup` first. **There is no runtime setter in any binding.** 0.x's `set_default_settings` (and Go's `SetDefaultSettings`, which existed only on the linked build) is deleted ([`reference/bindings-v1.md` §7](../reference/bindings-v1.md#7-what-v0-api-is-deleted-and-why)); per-call settings and the compile profile carry everything that changes after setup. All four bindings reach all four channels now, Go included. +**The setup defaults latch.** `setup` is called once, before the first library opens, and the defaults it records are applied to each library as it loads and never again. A second `setup` with the same zone and defaults is a no-op; a different one is a `UsageError` naming both, and the first stands. If `setup` was never called, the first open commits the empty setup: no defaults, and the library's default zone, `UTC`. The latch closes only when a library completes its setup. Until then, any open that fails clears the record, whatever failed: the fetch, the signature, an artifact that will not load, or the library refusing the zone or a default. A corrected `setup` is then accepted and the next open uses it. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry an open ([`reference/bindings-v1.md` §6](../reference/bindings-v1.md#6-setup-and-from-a-request-to-a-loaded-library)). So call `setup` first. **There is no runtime setter in any binding.** 0.x's `set_default_settings` (and Go's `SetDefaultSettings`, which existed only on the linked build) is deleted ([`reference/bindings-v1.md` §7](../reference/bindings-v1.md#7-what-v0-api-is-deleted-and-why)); per-call settings and the compile profile carry everything that changes after setup. All four bindings reach all four channels now, Go included.
Go diff --git a/docs/reference/bindings-v1.md b/docs/reference/bindings-v1.md index 711fbd9e..11eafae7 100644 --- a/docs/reference/bindings-v1.md +++ b/docs/reference/bindings-v1.md @@ -427,10 +427,10 @@ A JSON object from handle kind (its type name, `chs_schema` and the rest) to the The image zone and the default settings are process-wide in ClickHouse: vendored MergeTree code reads the server zone directly (`measured` by the artifact producer on 26.7), and defaults are read by every call. So both are chosen once, before traffic, through `setup`, and every binding gives `setup` the same rule: -1. **`setup` records; it does not load.** Called before any library is open, it records the zone spelling and the defaults. Called again with the same zone spelling, byte for byte, and the same defaults, it is a no-op. Called with a different zone or different defaults while a setup is recorded, it is a `UsageError` naming both, and the recorded setup stands. This is the library's own process-once rule for `chs_initialize`, applied by the binding while no image has been set up to ask. A record lasts until it latches (rule 2), or until a step 7 failure clears it (rule 4). -2. **The first successful step 7 latches it.** If `setup` was never called, the first open records the empty setup: the empty zone, which the library reads as `UTC`, and no defaults. The setup latches when the first image completes load step 7 successfully: `chs_initialize`, then `chs_set_defaults` when there are defaults. From then on `setup` succeeds only with exactly the setup in effect, and is otherwise a `UsageError` naming both. Call `setup` first. +1. **`setup` records; it does not load.** Called before any library is open, it records the zone spelling and the defaults. Called again with the same zone spelling, byte for byte, and the same defaults, it is a no-op. Called with a different zone or different defaults while a setup is recorded, it is a `UsageError` naming both, and the recorded setup stands. So two components that disagree before any open is attempted get a refusal, never a silent last-wins. This is the library's own process-once rule for `chs_initialize`, applied by the binding while no image has been set up to ask. A record lasts until it latches (rule 2), or until a failed open clears it (rule 4). +2. **The first successful step 7 latches it.** If no setup is recorded when an open starts (`setup` was never called, or a failed open cleared the record), the open records the empty setup: the empty zone, which the library reads as `UTC`, and no defaults. The setup latches when the first image completes load step 7 successfully: `chs_initialize`, then `chs_set_defaults` when there are defaults. From then on `setup` succeeds only with exactly the setup in effect, and is otherwise a `UsageError` naming both. Call `setup` first. 3. **Every image is set up at load step 7, once.** The loader calls `chs_initialize` with the recorded zone, as counted bytes (empty for the empty zone), and then, when there are defaults, `chs_set_defaults` with them, under the setup guard. Nothing calls either again for that image once step 7 has completed. -4. **What the library answers is what the caller gets, and a refused setup can be corrected.** A zone `DateLUT` will not load is a `SchemaError` from `chs_initialize`, and a default the library refuses is the error `chs_set_defaults` answers. Either is raised by the open that hit step 7, and that open returns no library. A failed step 7 is never remembered: the next open of the same image runs step 7 again. If no image has completed step 7 yet, the binding also clears its setup record, under the setup guard, so the caller may call `setup` again with any setup, and the next open runs step 7 with it. Once the setup has latched, a failure clears nothing. The library's own process-once rule stays the backstop: an image whose `chs_initialize` succeeded keeps its zone even if `chs_set_defaults` then failed, and a different spelling than the image already has (otherwise possible only across TypeScript isolates, or with a second binding loaded into one process) is `CHS_INVALID_ARGUMENT` from `chs_initialize`, a `UsageError` naming both, the same class rule 1 raises. +4. **What the library answers is what the caller gets, and a failed open can be corrected.** A zone `DateLUT` will not load is a `SchemaError` from `chs_initialize`, and a default the library refuses is the error `chs_set_defaults` answers. Either is raised by the open that hit step 7, and that open returns no library. **Until an image has completed step 7, any open that fails clears the setup record, whatever failed:** the version spelling, the fetch, the signature, loader steps 1 to 6 (an incompatible or corrupt artifact, a missing symbol), or step 7 itself. The binding clears the record under the setup guard, unless a setup was recorded after that open began. The caller may then call `setup` again with any setup, and the next open runs with it. An open with no `setup` after the failure records the empty setup (rule 2), so a caller that retries an open should call `setup` again first. A failed open is never remembered: the next open of the same request or image runs every step again. Once the setup has latched, a failure clears nothing. The library's own process-once rule stays the backstop: an image whose `chs_initialize` succeeded keeps its zone even if `chs_set_defaults` then failed, and a different spelling than the image already has (otherwise possible only across TypeScript isolates, or with a second binding loaded into one process) is `CHS_INVALID_ARGUMENT` from `chs_initialize`, a `UsageError` naming both, the same class rule 1 raises. 5. **Defaults never change during traffic.** There is no public setter, in any binding. If the regenerated description drops `chs_set_defaults` (the decision accepts "no setter at all"), the `defaults` field of `setup` is deleted with it, and per-call `settings` and the compile profile carry everything. The per-call zone needs no setup: it is the `session_timezone` key of a call's settings (§2, the call options). diff --git a/go/CHANGELOG.md b/go/CHANGELOG.md index 5a5f3bbf..6cec7a37 100644 --- a/go/CHANGELOG.md +++ b/go/CHANGELOG.md @@ -6,6 +6,10 @@ The four bindings in this repository are released together and give one answer, ## [Unreleased] +### Fixed + +- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `Setup(SetupOptions{Timezone: "Asia/Tokyo"})`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `Setup` was then a `*UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry.For`, `OpenUnverified` and `OpenLinked`), so the next `Setup` is accepted, whatever it is. An open with no `Setup` after the failure uses the empty setup, so call `Setup` again before you retry. Two things are unchanged. A second, different `Setup` before any open is attempted is still a `*UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `*UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. + ## [1.0.2] — 2026-10-06 ### Changed diff --git a/go/chtypes/library.go b/go/chtypes/library.go index 6f6fd41d..2c1b67ea 100644 --- a/go/chtypes/library.go +++ b/go/chtypes/library.go @@ -5,7 +5,6 @@ package chtypes // exactly one ABI call and takes no lock. import ( - "errors" "path/filepath" "sync" @@ -47,11 +46,11 @@ func imageKey(kind, path string) string { // openImage loads (or returns the already loaded) image for key. Every image is // set up at load step 7, once, under the process setup, which the first open -// records. images.mu is held from the commit to the settle, so every open -// loads under the record it settles, and Setup cannot change that record in -// between (it refuses a different setup while one is recorded). A step 7 -// failure is the loader's *abi1.CallError, and only that clears the record; -// a failed load is never cached, so the next open runs step 7 again. +// records. images.mu is held from the commit to the latch, so an image latches +// the record it loaded under, and Setup cannot change that record in between +// (it refuses a different setup while one is recorded). A failed load is never +// cached, so the next open runs every step again; the open that called this +// settles a failure with failedOpen, whatever failed. func openImage(key string, load func(zone, defaults []byte) (*abi1.Table, error), path string, resolved *Resolved) (*Library, error) { images.mu.Lock() defer images.mu.Unlock() @@ -63,16 +62,10 @@ func openImage(key string, load func(zone, defaults []byte) (*abi1.Table, error) return l, nil } tbl, err := load(zone, defaults) - var step7 *abi1.CallError - switch { - case err == nil: - settleSetup(true) - case errors.As(err, &step7): - settleSetup(false) - } if err != nil { return nil, loadError(err) } + latchSetup() info, err := decodeBuildInfo([]byte(tbl.BuildInfo())) if err != nil { return nil, &ArtifactError{ @@ -96,7 +89,13 @@ func openImage(key string, load func(zone, defaults []byte) (*abi1.Table, error) // CHTYPES_ALLOW_UNVERIFIED_LIBRARY=1 is set, and warns once per path. It runs // step 7 under the process setup like any open, is not reachable through a // Registry, and its Library has no Resolved record. -func OpenUnverified(path string, allow bool) (*Library, error) { +func OpenUnverified(path string, allow bool) (l *Library, err error) { + gen := setupGeneration() + defer func() { + if err != nil { + failedOpen(gen) + } + }() return openImage(imageKey("unverified", path), func(zone, defaults []byte) (*abi1.Table, error) { return abi1.OpenUnverified(path, allow, zone, defaults) }, path, nil) diff --git a/go/chtypes/linked.go b/go/chtypes/linked.go index 5d03e83b..7c24fba8 100644 --- a/go/chtypes/linked.go +++ b/go/chtypes/linked.go @@ -9,7 +9,13 @@ import "github.com/wave-rf/chtypes/go/internal/abi1" // loader steps 3, 4, 6 and 7 (step 6 is satisfied by the linker) and skips 1, // 2 and 5; one code path serves both modes, and only the table filler // differs. It needs a core build tree via CGO_LDFLAGS, and -tags chtypes_linked. -func OpenLinked() (*Library, error) { +func OpenLinked() (l *Library, err error) { + gen := setupGeneration() + defer func() { + if err != nil { + failedOpen(gen) + } + }() return openImage("linked", func(zone, defaults []byte) (*abi1.Table, error) { return abi1.LoadLinked(abi1.OpenLinked(), zone, defaults) }, "", nil) diff --git a/go/chtypes/registry.go b/go/chtypes/registry.go index a18877a5..0159afd6 100644 --- a/go/chtypes/registry.go +++ b/go/chtypes/registry.go @@ -125,12 +125,21 @@ func (r *Registry) ForContext(ctx context.Context, request string) (*Library, er return r.open(ctx, request, r.autofetch) } -func (r *Registry) open(ctx context.Context, request string, mayFetch bool) (*Library, error) { +func (r *Registry) open(ctx context.Context, request string, mayFetch bool) (_ *Library, err error) { r.mu.Lock() defer r.mu.Unlock() if l := r.memo[request]; l != nil { return l, nil } + // A failed open clears the setup record while no image has completed load + // step 7, whatever failed: the spelling, the fetch, the signature or any + // load step (bindings-v1.md section 6, rule 4). + gen := setupGeneration() + defer func() { + if err != nil { + failedOpen(gen) + } + }() opts := r.fetch.internal() req := ocifetch.Request{Spelling: request} res, err := ocifetch.ResolveInstalled(req, "", opts) diff --git a/go/chtypes/settings_test.go b/go/chtypes/settings_test.go index 7fdf4516..473c0830 100644 --- a/go/chtypes/settings_test.go +++ b/go/chtypes/settings_test.go @@ -147,3 +147,58 @@ func TestFirstOpenCommitsTheEmptySetup(t *testing.T) { t.Error("a setup after the first open commit must be refused: it is not the one in effect") } } + +// TestAFailedFetchBeforeStep7ClearsTheSetup: a registry open that fails before +// any load (nothing installed, autofetch off) clears the setup record while no +// image has completed step 7, so a different setup is accepted afterwards. +func TestAFailedFetchBeforeStep7ClearsTheSetup(t *testing.T) { + resetSetup(t) + if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { + t.Fatal(err) + } + reg, err := NewRegistry(WithFetchOptions(FetchOptions{CacheDir: t.TempDir(), SystemDirs: []string{t.TempDir()}, Offline: true}), WithAutoFetch(false)) + if err != nil { + t.Fatal(err) + } + if _, err := reg.For("26.8"); !errors.Is(err, ErrArtifactMissing) { + t.Fatalf("For on an empty cache = %v, want ErrArtifactMissing", err) + } + if err := Setup(SetupOptions{Timezone: "UTC"}); err != nil { + t.Errorf("a different setup after a failed open = %v, want it accepted", err) + } + // A refused version spelling is a failed open too. + var ue *UsageError + if _, err := reg.For("v26.8"); !errors.As(err, &ue) { + t.Fatalf("For(v26.8) = %v, want a *UsageError", err) + } + if err := Setup(SetupOptions{Timezone: "Europe/Berlin"}); err != nil { + t.Errorf("a different setup after a refused spelling = %v, want it accepted", err) + } + // Before any open is attempted, a second, different setup is still misuse. + if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); !errors.As(err, &ue) { + t.Errorf("a second, different setup before any open = %v, want a *UsageError", err) + } +} + +// TestAFailedOpenClearsOnlyItsOwnRecordAndNeverALatchedOne: a failed open never +// clears a setup recorded after it began, and once an image has completed step +// 7 a failure clears nothing. +func TestAFailedOpenClearsOnlyItsOwnRecordAndNeverALatchedOne(t *testing.T) { + resetSetup(t) + began := setupGeneration() + if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { + t.Fatal(err) + } + failedOpen(began) // began before Asia/Tokyo was recorded + if err := Setup(SetupOptions{Timezone: "UTC"}); err == nil { + t.Error("a failed open cleared a setup recorded after it began") + } + latchSetup() + failedOpen(setupGeneration()) + if err := Setup(SetupOptions{Timezone: "UTC"}); err == nil { + t.Error("a failed open cleared a latched setup") + } + if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { + t.Errorf("the latched setup again = %v, want a no-op", err) + } +} diff --git a/go/chtypes/setup.go b/go/chtypes/setup.go index f16bd536..645497fa 100644 --- a/go/chtypes/setup.go +++ b/go/chtypes/setup.go @@ -3,8 +3,8 @@ package chtypes // setup.go — the process setup (bindings-v1.md section 6). The image zone and // the default settings are process-wide in ClickHouse, so both are chosen once, // before traffic: Setup records them, and every image gets them at load step 7. -// The record latches once an image completes step 7; a step 7 failure before -// that clears it, so a corrected setup can be recorded. +// The record latches once an image completes step 7; before that, any failed +// open clears it, so a corrected setup can be recorded. import ( "reflect" @@ -22,13 +22,19 @@ type SetupOptions struct { } // setup is the setup guard: the record, and whether any image has completed -// load step 7 under it. Every open holds images.mu around its commit, its load -// and its settle, so the record a load ran under is the one it settles. +// load step 7 under it. An open commits the record, loads and latches under +// images.mu, and a failed open clears the record under images.mu too, so a +// clear never lands between another open's commit and its latch. var setup struct { mu sync.Mutex recorded bool latched bool // an image completed load step 7 under the record - opts SetupOptions + // gen counts the records Setup made and the records failed opens cleared. + // An open reads it when it begins, and clears the record on failure only + // if it is unchanged: a failed open never clears a setup recorded after it + // began. + gen uint64 + opts SetupOptions } func sameSetup(a, b SetupOptions) bool { @@ -64,14 +70,17 @@ func quoteForMessage(s string) string { // first setup stands. The first open records the empty setup if Setup was // never called. The setup latches once an image completes load step 7 // (chs_initialize, then chs_set_defaults when there are defaults); from then -// on Setup succeeds only with exactly the setup in effect. If step 7 fails -// before any image has completed it, the record is cleared, so a corrected -// Setup is accepted and the next open runs step 7 with it. Call it first. +// on Setup succeeds only with exactly the setup in effect. Until then, any +// open that fails, whatever failed (the fetch, the signature, an incompatible +// artifact, a missing symbol or step 7), clears the record: the next Setup is +// accepted, and an open with no Setup after the failure records the empty +// setup. Call it first. func Setup(opts SetupOptions) error { setup.mu.Lock() defer setup.mu.Unlock() if !setup.recorded { setup.recorded = true + setup.gen++ setup.opts = SetupOptions{Timezone: opts.Timezone, Defaults: copyMap(opts.Defaults)} return nil } @@ -94,21 +103,37 @@ func commitSetup() (zone []byte, defaults []byte, err error) { return []byte(setup.opts.Timezone), defaults, nil } -// settleSetup records how load step 7 ended, under the setup guard. A success -// latches the setup in effect. A failure before any image has completed step 7 -// clears the record, so Setup accepts a corrected setup; once the setup has -// latched, a failure changes nothing (the library's own process-once rule -// answers a different zone on an image that already has one). -func settleSetup(completed bool) { +// setupGeneration is read when an open begins, for failedOpen. +func setupGeneration() uint64 { setup.mu.Lock() defer setup.mu.Unlock() - if completed { - setup.latched = true + return setup.gen +} + +// latchSetup is called, under images.mu, when an image completes load step 7: +// from then on the setup in effect stands. +func latchSetup() { + setup.mu.Lock() + defer setup.mu.Unlock() + setup.latched = true +} + +// failedOpen settles an open that failed, whatever failed. While no image has +// completed step 7 it clears the record, so Setup accepts a corrected setup, +// unless a setup was recorded after the open began (gen moved). Once the setup +// has latched it changes nothing: the library's own process-once rule answers +// a different zone on an image that already has one. It takes images.mu, so it +// never lands between another open's commit and its latch. +func failedOpen(gen uint64) { + images.mu.Lock() + defer images.mu.Unlock() + setup.mu.Lock() + defer setup.mu.Unlock() + if setup.latched || setup.gen != gen { return } - if !setup.latched { - setup.recorded, setup.opts = false, SetupOptions{} - } + setup.recorded, setup.opts = false, SetupOptions{} + setup.gen++ } func copyMap(m map[string]string) map[string]string { diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md index 85a24287..2027fe41 100644 --- a/python/CHANGELOG.md +++ b/python/CHANGELOG.md @@ -6,6 +6,10 @@ The four bindings in this repository are released together and give one answer, ## [Unreleased] +### Fixed + +- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup(timezone="Asia/Tokyo")`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then a `UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry.for_version`, `Registry(preload=…)` and `open_unverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. + ## [1.0.2] — 2026-10-06 There is no 1.0.1 of this binding: 1.0.1 was a Go-only fix to that module's metadata. From 1.0.2 the four bindings release together again. diff --git a/python/src/chtypes/_abi1/_loader.py b/python/src/chtypes/_abi1/_loader.py index e20b640c..eb9354e0 100644 --- a/python/src/chtypes/_abi1/_loader.py +++ b/python/src/chtypes/_abi1/_loader.py @@ -30,7 +30,7 @@ import platform import re import warnings -from collections.abc import Callable, Mapping +from collections.abc import Mapping from dataclasses import dataclass from . import _decls, _errmap, _errors @@ -165,7 +165,6 @@ def _open( skip_step5: bool, timezone: bytes = b"", defaults: bytes = b"", - settle: Callable[[bool], None] | None = None, ) -> LoadResult: if not skip_step1: _check_glibc(path, predicate) @@ -205,20 +204,12 @@ def _open( # 6): the image zone (empty = UTC), then the default settings when there # are any. After every handshake check and before any other call. A # failure is the library's own refusal, the call error its status maps to - # (never a loader refusal reason). `settle`, when given, is told how step 7 - # ended: the public layer's setup guard latches on success and clears its - # record on a failure before any image has completed step 7. Nothing here - # remembers a failure, so the next open runs step 7 again. - try: - api.initialize(timezone) - if defaults: - api.set_defaults(defaults) - except Exception: - if settle is not None: - settle(False) - raise - if settle is not None: - settle(True) + # (never a loader refusal reason). Nothing here remembers a failure, so + # the next open runs every step again; the public layer settles the setup + # with the open's outcome. + api.initialize(timezone) + if defaults: + api.set_defaults(defaults) return LoadResult(api=api, build_info=info, path=path, raw=raw) @@ -239,14 +230,13 @@ def open( *, timezone: bytes = b"", defaults: bytes = b"", - settle: Callable[[bool], None] | None = None, ) -> LoadResult: """Load and verify a v1 artifact at `path` against its (already verified elsewhere) signed `predicate`, then set the image zone (`timezone`, empty = UTC) and the default settings (`defaults`, a JSON object of string values, empty for none): step 7. Raises an artifact error naming the exact refusal on any loader failure, or the call error step 7's own status maps - to. `settle`, when given, is told whether step 7 completed.""" + to.""" return _open( path, predicate, @@ -254,7 +244,6 @@ def open( skip_step5=False, timezone=timezone, defaults=defaults, - settle=settle, ) @@ -277,7 +266,6 @@ def open_unverified( allow: bool = False, timezone: bytes = b"", defaults: bytes = b"", - settle: Callable[[bool], None] | None = None, ) -> LoadResult: """For core's local builds and the linked mode ONLY (plan section 3.1, Q-b): skips step 1 when no predicate is given, and always skips step 5. @@ -298,5 +286,4 @@ def open_unverified( skip_step5=True, timezone=timezone, defaults=defaults, - settle=settle, ) diff --git a/python/src/chtypes/_setup.py b/python/src/chtypes/_setup.py index ab4a0eae..5cdd3749 100644 --- a/python/src/chtypes/_setup.py +++ b/python/src/chtypes/_setup.py @@ -4,11 +4,11 @@ `setup` records; it does not load. The first open records the empty setup if `setup` was never called. The record latches once an image completes loader step 7 (`chs_initialize`, then `chs_set_defaults` when there are defaults), and -from then on `setup` succeeds only with exactly the setup in effect. A step 7 -failure before any image has completed it clears the record, so a corrected -setup can be recorded and the next open runs step 7 with it. Every image is set -up once, at loader step 7, by `library.open_image` and `open_unverified`, under -this module's lock: the setup guard. +from then on `setup` succeeds only with exactly the setup in effect. Until then, +any open that fails, whatever failed, clears the record (`open_failed`), so a +corrected setup can be recorded. Every image is set up once, at loader step 7, +by `library.open_image` and `open_unverified`, under this module's lock: the +setup guard, which every load, latch and clear holds. """ from __future__ import annotations @@ -42,13 +42,19 @@ def defaults_json(self) -> bytes: _state: SetupState | None = None # Whether any image has completed loader step 7 under the record. _latched = False +# Counts the records `setup` made and the records failed opens cleared. An open +# reads it when it begins, and clears the record on failure only if it is +# unchanged: a failed open never clears a setup recorded after it began. +_generation = 0 -def _record(state: SetupState) -> SetupState: +def _record(state: SetupState, *, by_setup: bool = False) -> SetupState: """Record `state`, or confirm it equals the one in effect. Holds LOCK.""" - global _state + global _state, _generation if _state is None: _state = state + if by_setup: + _generation += 1 return state if _state != state: raise misuse( @@ -64,18 +70,34 @@ def effective() -> SetupState: return _record(_state if _state is not None else SetupState("", {})) -def settle(completed: bool) -> None: - """How loader step 7 ended, reported by the loader under LOCK. A success - latches the setup in effect. A failure before any image has completed step - 7 clears the record, so `setup` accepts a corrected setup; once the setup - has latched, a failure changes nothing (the library's own process-once - rule answers a different zone on an image that already has one).""" - global _state, _latched +def generation() -> int: + """Read when an open begins, for `open_failed`.""" + with LOCK: + return _generation + + +def latch() -> None: + """An image completed loader step 7: from then on the setup in effect + stands. Called by an open with LOCK held, right after the load.""" + global _latched + with LOCK: + _latched = True + + +def open_failed(began: int) -> None: + """Settle an open that failed, whatever failed. While no image has completed + step 7 it clears the record, so `setup` accepts a corrected setup, unless a + setup was recorded after the open began (`began` is `generation()` then). + Once the setup has latched it changes nothing: the library's own + process-once rule answers a different zone on an image that already has one. + LOCK serializes it with every load, so it never lands between another + open's commit and its latch.""" + global _state, _generation with LOCK: - if completed: - _latched = True - elif not _latched: - _state = None + if _latched or _generation != began: + return + _state = None + _generation += 1 def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> None: @@ -91,8 +113,11 @@ def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> N defaults, this is a no-op. Called with a different zone or different defaults it is a `UsageError` naming both, and the first setup stands. If it is never called, the first open records the empty setup. The setup latches - once an image completes loader step 7; if step 7 fails before that, the - record is cleared and a corrected setup is accepted. Call it first. + once an image completes loader step 7. Until then, any open that fails, + whatever failed (the fetch, the signature, an incompatible artifact, a + missing symbol or step 7), clears the record: the next `setup` is accepted, + and an open with no `setup` after the failure records the empty setup. Call + it first. """ if timezone is not None and not isinstance(timezone, str): raise TypeError(f"timezone must be str, not {type(timezone).__name__}") @@ -101,7 +126,7 @@ def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> N string_map_json(defaults, "defaults") # the type check, once mapping = dict(defaults) with LOCK: - _record(SetupState(timezone or "", mapping)) + _record(SetupState(timezone or "", mapping), by_setup=True) def _reset_for_tests() -> None: diff --git a/python/src/chtypes/library.py b/python/src/chtypes/library.py index d92b8747..70fe5d0a 100644 --- a/python/src/chtypes/library.py +++ b/python/src/chtypes/library.py @@ -57,8 +57,9 @@ def open_image(path: str, predicate: object, resolved: object | None) -> Library """Open (or find) the image at `path`, verified against `predicate`. A new image runs loader steps 1 to 7 once, under the setup in effect, and - tells the setup guard how step 7 ended (`_setup.settle`). A failed load is - never cached, so the next open of the same image runs step 7 again. An + latches the setup when step 7 completes (`_setup.latch`). A failed load is + never cached, so the next open of the same image runs every step again; the + caller settles a failure with `_setup.open_failed`, whatever failed. An image already open is still checked against this signed statement (steps 1, 4 and 5): a mismatch refuses this request and leaves the image open for the requests it did match. @@ -75,8 +76,8 @@ def open_image(path: str, predicate: object, resolved: object | None) -> Library predicate, # type: ignore[arg-type] timezone=state.zone_bytes(), defaults=state.defaults_json(), - settle=_setup.settle, ) + _setup.latch() library = Library(load, resolved) _IMAGES[key] = library return library @@ -88,24 +89,30 @@ def open_unverified(path: str | os.PathLike[str], *, allow: bool = False) -> Lib `allow` is passed AND `CHTYPES_ALLOW_UNVERIFIED_LIBRARY=1` is set, warns once per path, and skips loader steps 1 and 5. It runs step 7 under the process setup like any open. It is not reachable through a registry, and its - `Library` has no `resolved`.""" + `Library` has no `resolved`. Like any open, a failure clears the setup + record while no image has completed step 7 (`_setup.open_failed`).""" spelled = os.fspath(path) - _loader.check_unverified_allowed(spelled, allow) - with _setup.LOCK: - state = _setup.effective() - existing = _IMAGES.get(_image_key(spelled)) - if existing is not None: - return existing - load = _loader.open_unverified( - spelled, - allow=allow, - timezone=state.zone_bytes(), - defaults=state.defaults_json(), - settle=_setup.settle, - ) - library = Library(load, None) - _IMAGES[_image_key(spelled)] = library - return library + began = _setup.generation() + try: + _loader.check_unverified_allowed(spelled, allow) + with _setup.LOCK: + state = _setup.effective() + existing = _IMAGES.get(_image_key(spelled)) + if existing is not None: + return existing + load = _loader.open_unverified( + spelled, + allow=allow, + timezone=state.zone_bytes(), + defaults=state.defaults_json(), + ) + _setup.latch() + library = Library(load, None) + _IMAGES[_image_key(spelled)] = library + return library + except Exception: + _setup.open_failed(began) + raise @contextlib.contextmanager diff --git a/python/src/chtypes/registry.py b/python/src/chtypes/registry.py index 275d6a4f..4cc95d41 100644 --- a/python/src/chtypes/registry.py +++ b/python/src/chtypes/registry.py @@ -25,7 +25,7 @@ from dataclasses import dataclass from pathlib import Path -from . import errors +from . import _setup, errors from ._ocifetch import Options, Request, Resolved, ensure, list_installed, resolve_installed from ._ocifetch import _constants as _fetch_constants from ._ocifetch._dsse import TrustedKey @@ -170,7 +170,15 @@ def _open(self, request: str, *, allow_fetch: bool) -> Library: hit = self._memo.get(request) if hit is not None: return hit - library = self._resolve_and_open(request, allow_fetch) + # A failed open clears the setup record while no image has completed + # load step 7, whatever failed: the spelling, the fetch, the + # signature or any load step (bindings-v1.md section 6, rule 4). + began = _setup.generation() + try: + library = self._resolve_and_open(request, allow_fetch) + except Exception: + _setup.open_failed(began) + raise self._memo[request] = library return library diff --git a/python/tests/abi1/test_public_open.py b/python/tests/abi1/test_public_open.py index b41f52d6..e99b32f9 100644 --- a/python/tests/abi1/test_public_open.py +++ b/python/tests/abi1/test_public_open.py @@ -10,6 +10,9 @@ from chtypes import ( ArtifactCorruptError, ArtifactError, + ArtifactMissingError, + FetchOptions, + Registry, SchemaError, UsageError, _setup, @@ -158,23 +161,56 @@ def test_open_unverified_needs_both_opt_ins_and_has_no_resolved(stub_copy, monke def test_a_defaults_failure_at_step_7_clears_the_record_too( stub_copy, monkeypatch, clean_process ) -> None: - # The shared setup case (test_public_setup_cases.py) covers a zone the - # library refuses; the stub cannot refuse a default, so this forces the - # chs_set_defaults half of step 7 to fail. The rule is the same: no image - # has completed step 7, so the record is cleared and a corrected setup is - # accepted, and the open that follows runs step 7 again under it. - path, predicate = stub_copy() + # The shared setup case (test_public_setup_cases.py) covers a refused load + # and a zone the library refuses; the stub cannot refuse a default, so this + # forces the chs_set_defaults half of step 7 to fail. The rule is the same: + # no image has completed step 7, so the failed open clears the record, a + # corrected setup is accepted, and the open that follows runs step 7 again. + path, _ = stub_copy() + monkeypatch.setenv("CHTYPES_ALLOW_UNVERIFIED_LIBRARY", "1") def refuse(self, d): raise SchemaError(1, 115, "UNKNOWN_SETTING", b"Unknown setting no_such_setting") monkeypatch.setattr(_decls.Api, "set_defaults", refuse) setup(timezone="Asia/Tokyo", defaults={"no_such_setting": "1"}) - with pytest.raises(SchemaError) as info: - library.open_image(path, predicate, None) + with pytest.raises(SchemaError) as info, pytest.warns(UserWarning, match="UNVERIFIED"): + open_unverified(path, allow=True) assert info.value.ch_code == 115 setup(timezone="Asia/Tokyo") # the corrected setup: no longer refused - first = library.open_image(path, predicate, None) + first = open_unverified(path, allow=True) with pytest.raises(UsageError): setup(timezone="Asia/Tokyo", defaults={"no_such_setting": "1"}) # now latched - assert library.open_image(path, predicate, None) is first + assert open_unverified(path, allow=True) is first + + +def test_a_failed_fetch_before_step_7_clears_the_record(tmp_path, clean_process) -> None: + # A registry open that fails before any load (nothing installed, autofetch + # off) is a failed open like any other: no image has completed step 7, so + # the record is cleared and a different setup is accepted afterwards. + setup(timezone="Asia/Tokyo") + registry = Registry( + fetch=FetchOptions(cache_dir=tmp_path / "cache", system_dirs=[], offline=True), + autofetch=False, + ) + with pytest.raises(ArtifactMissingError): + registry.for_version("26.8") + setup(timezone="UTC") # accepted: the failed open cleared the record + with pytest.raises(UsageError): + registry.for_version("v26.8") # a refused spelling is a failed open too + setup(timezone="Europe/Berlin") # accepted + with pytest.raises(UsageError): + setup(timezone="Asia/Tokyo") # before any open is attempted: the first stands + + +def test_a_failed_open_never_clears_a_later_or_a_latched_setup(clean_process) -> None: + began = _setup.generation() + setup(timezone="Asia/Tokyo") + _setup.open_failed(began) # it began before Asia/Tokyo was recorded + with pytest.raises(UsageError): + setup(timezone="UTC") + _setup.latch() + _setup.open_failed(_setup.generation()) + with pytest.raises(UsageError): + setup(timezone="UTC") + setup(timezone="Asia/Tokyo") # the latched setup again: a no-op diff --git a/rust/CHANGELOG.md b/rust/CHANGELOG.md index 77d9436b..d2e57d7c 100644 --- a/rust/CHANGELOG.md +++ b/rust/CHANGELOG.md @@ -6,6 +6,10 @@ The four bindings in this repository are released together and give one answer, ## [Unreleased] +### Fixed + +- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup(SetupOptions { timezone: Some("Asia/Tokyo".into()), ..Default::default() })`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then an `Error::Usage` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry::for_version`, `RegistryOptions::preload` and `Library::open_unverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still an `Error::Usage`, never a silent last-wins. Once a library has completed step 7, a different setup is still an `Error::Usage`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. + ## [1.0.2] — 2026-10-06 There is no 1.0.1 of this binding: 1.0.1 was a Go-only fix to that module's metadata. From 1.0.2 the four bindings release together again. diff --git a/rust/src/library.rs b/rust/src/library.rs index 700e2dc1..e716f65c 100644 --- a/rust/src/library.rs +++ b/rust/src/library.rs @@ -69,6 +69,15 @@ fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } +/// Settle an open that failed, whatever failed (`crate::setup`'s rule): under +/// the image list's lock, so the clear never lands between another open's +/// commit and its latch. `began` is `setup::generation()` read when the open +/// began. +pub(crate) fn settle_failed_open(began: u64) { + let _images = lock(&IMAGES); + setup::clear_after_failed_open(began); +} + /// Open the image at `path`, or return the one this process already opened. /// `predicate` is the fetch layer's verified statement, verbatim; `None` is an /// unverified open. @@ -99,27 +108,24 @@ pub(crate) fn open_image( return Ok(Arc::clone(&image.library)); } - // The image list's lock is held from the commit to the settle, so the - // record this load runs under is the one it settles, and `setup` cannot - // change it in between (it refuses a different setup while one is - // recorded). Only a step 7 failure (`LoadError::Call`) clears the record, - // and a failed load is never pushed, so the next open runs step 7 again. + // The image list's lock is held from the commit to the latch, so an image + // latches the record it loaded under, and `setup` cannot change that record + // in between (it refuses a different setup while one is recorded). A failed + // load is never pushed, so the next open runs every step again; the open + // that called this settles a failure with `settle_failed_open`, whatever + // failed. let setup = setup::commit(); let loaded = loader::load(LoadInput { library_path: &canonical, predicate, timezone: &setup.timezone, defaults: setup.defaults.as_deref(), - }); - match &loaded { - Ok(_) => setup::settle(true), - Err(LoadError::Call(_)) => setup::settle(false), - Err(LoadError::Refused(_)) => {} - } - let loaded = loaded.map_err(|e| match e { + }) + .map_err(|e| match e { LoadError::Refused(r) => Error::from_refusal(r), LoadError::Call(c) => Error::from_call(c), })?; + setup::latch(); let build_info = decode::build_info(&loaded.build_info, &loaded.build_info_raw) .map_err(|detail| refusal("build_info_malformed", detail))?; let library = Arc::new(Library { @@ -148,7 +154,17 @@ impl Library { /// It is not reachable through a [`crate::Registry`], and the library it /// returns has no [`Library::resolved`]. pub fn open_unverified(path: impl AsRef, allow: bool) -> Result> { - let path = path.as_ref(); + // Like any open: a failure clears the setup record while no image has + // completed load step 7, whatever failed. + let began = setup::generation(); + let opened = Self::open_unverified_once(path.as_ref(), allow); + if opened.is_err() { + settle_failed_open(began); + } + opened + } + + fn open_unverified_once(path: &Path, allow: bool) -> Result> { let env_on = std::env::var(UNVERIFIED_ENV).is_ok_and(|v| v == "1"); if !(allow && env_on) { return Err(Error::usage(format!( diff --git a/rust/src/registry.rs b/rust/src/registry.rs index 8e9ae108..de4b489a 100644 --- a/rust/src/registry.rs +++ b/rust/src/registry.rs @@ -26,9 +26,10 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex}; use crate::error::{Error, Result}; -use crate::library::{Library, open_image}; +use crate::library::{Library, open_image, settle_failed_open}; use crate::ocifetch::constants::ENV_AUTOFETCH_NAME; use crate::ocifetch::ensure::{self, Options, Resolved}; +use crate::setup; /// What the fetch layer is configured with, under its own names /// (`docs/guides/fetch-v1.md`). Every field defaults to the fetch layer's own @@ -192,6 +193,27 @@ impl Registry { if let Some(o) = opened.iter().find(|o| o.request == request) { return Ok(Arc::clone(&o.library)); } + // A failed open clears the setup record while no image has completed + // load step 7, whatever failed: the spelling, the fetch, the signature + // or any load step (bindings-v1.md section 6, rule 4). + let began = setup::generation(); + let library = match self.resolve_and_open(request, may_fetch) { + Ok(library) => library, + Err(e) => { + settle_failed_open(began); + return Err(e); + } + }; + opened.push(Opened { + request: request.to_string(), + library: Arc::clone(&library), + }); + Ok(library) + } + + /// Resolve a request, fetching when allowed, and open its image: the part + /// of [`Registry::open`] whose failure settles the setup. + fn resolve_and_open(&self, request: &str, may_fetch: bool) -> Result> { let platform = self.fetch.platform.clone().unwrap_or_else(host_platform); let resolved = match ensure::resolve_installed(request, &platform, self.fetch.to_options())? { @@ -212,16 +234,11 @@ impl Registry { }; // The adapter: the fetch layer's record to the loader's input. The // predicate goes across exactly as returned, never re-encoded. - let library = open_image( + open_image( &resolved.library_path.clone(), Some(&resolved.predicate.clone()), Some(resolved), - )?; - opened.push(Opened { - request: request.to_string(), - library: Arc::clone(&library), - }); - Ok(library) + ) } } diff --git a/rust/src/setup.rs b/rust/src/setup.rs index 46c446d8..a6fd2630 100644 --- a/rust/src/setup.rs +++ b/rust/src/setup.rs @@ -14,10 +14,12 @@ //! called, the first open records the empty setup: the empty zone, which //! the library reads as `UTC`, and no defaults. Once an image completes //! step 7 (`chs_initialize`, then `chs_set_defaults` when there are -//! defaults), [`setup`] succeeds only with exactly the setup in effect. If -//! step 7 fails before any image has completed it, the record is cleared, -//! so a corrected [`setup`] is accepted and the next open runs step 7 with -//! it. Call [`setup`] first. +//! defaults), [`setup`] succeeds only with exactly the setup in effect. +//! Until then, any open that fails, whatever failed (the fetch, the +//! signature, an incompatible artifact, a missing symbol or step 7), +//! clears the record: the next [`setup`] is accepted, and an open with no +//! [`setup`] after the failure records the empty setup. Call [`setup`] +//! first. //! 3. There is no public setter for defaults during traffic. use std::sync::Mutex; @@ -47,17 +49,24 @@ pub(crate) struct Recorded { } /// The setup guard: the record, and whether any image has completed load step -/// 7 under it. Every open holds the image list's lock from its commit to its -/// settle (`crate::library::open_image`), so the record a load ran under is the -/// one it settles. +/// 7 under it. An open commits, loads and latches under the image list's lock +/// (`crate::library::open_image`), and a failed open clears under it too +/// (`crate::library::settle_failed_open`), so a clear never lands between +/// another open's commit and its latch. static STATE: Mutex = Mutex::new(State { recorded: None, latched: false, + generation: 0, }); struct State { recorded: Option, latched: bool, + /// Counts the records [`setup`] made and the records failed opens cleared. + /// An open reads it when it begins, and clears the record on failure only + /// if it is unchanged: a failed open never clears a setup recorded after it + /// began. + generation: u64, } fn recorded_of(options: &SetupOptions) -> Result { @@ -103,6 +112,7 @@ pub fn setup(options: SetupOptions) -> Result<()> { match &state.recorded { None => { state.recorded = Some(wanted); + state.generation += 1; Ok(()) } Some(current) if *current == wanted => Ok(()), @@ -127,18 +137,31 @@ pub(crate) fn commit() -> Recorded { .clone() } -/// How load step 7 ended, under the setup guard. A success latches the setup -/// in effect. A failure before any image has completed step 7 clears the -/// record, so [`setup`] accepts a corrected setup; once the setup has latched, -/// a failure changes nothing (the library's own process-once rule answers a -/// different zone on an image that already has one). -pub(crate) fn settle(completed: bool) { +/// Read when an open begins, for [`clear_after_failed_open`]. +pub(crate) fn generation() -> u64 { + STATE.lock().unwrap_or_else(|e| e.into_inner()).generation +} + +/// An image completed load step 7: from then on the setup in effect stands. +/// Called under the image list's lock, right after the load. +pub(crate) fn latch() { + STATE.lock().unwrap_or_else(|e| e.into_inner()).latched = true; +} + +/// Settle an open that failed, whatever failed. While no image has completed +/// step 7 it clears the record, so [`setup`] accepts a corrected setup, unless +/// a setup was recorded after the open began (`began` is [`generation`] then). +/// Once the setup has latched it changes nothing: the library's own +/// process-once rule answers a different zone on an image that already has one. +/// Called only through `crate::library::settle_failed_open`, which holds the +/// image list's lock. +pub(crate) fn clear_after_failed_open(began: u64) { let mut state = STATE.lock().unwrap_or_else(|e| e.into_inner()); - if completed { - state.latched = true; - } else if !state.latched { - state.recorded = None; + if state.latched || state.generation != began { + return; } + state.recorded = None; + state.generation += 1; } #[cfg(test)] diff --git a/rust/tests/api_v1_setup_failed_open.rs b/rust/tests/api_v1_setup_failed_open.rs new file mode 100644 index 00000000..c7775dc7 --- /dev/null +++ b/rust/tests/api_v1_setup_failed_open.rs @@ -0,0 +1,57 @@ +//! A failed open before any image has completed load step 7 clears the setup +//! record, whatever failed (`docs/reference/bindings-v1.md` §6, rule 4), here +//! for the failures that need no library at all: a request nothing installed +//! answers (autofetch off), and a refused version spelling. It runs in its own +//! process, as one test, because `setup` is process-wide and nothing public +//! resets it. It needs no stubs and no network. + +use chtypes::{Error, FetchOptions, Registry, RegistryOptions, SetupOptions}; + +fn zone(name: &str) -> SetupOptions { + SetupOptions { + timezone: Some(name.to_string()), + defaults: Vec::new(), + } +} + +#[test] +fn a_failed_open_before_step_7_clears_the_setup() { + let cache = + std::env::temp_dir().join(format!("setup_failed_open_cache_{}", std::process::id())); + std::fs::create_dir_all(&cache).expect("create an empty cache"); + let registry = Registry::new(RegistryOptions { + fetch: FetchOptions { + cache_dir: Some(cache.to_string_lossy().into_owned()), + system_dirs: Some(Vec::new()), + offline: true, + ..Default::default() + }, + autofetch: Some(false), + ..Default::default() + }) + .expect("constructing a registry opens nothing"); + + // Nothing installed answers the request: a failed open, before any load. + chtypes::setup(zone("Asia/Tokyo")).expect("the first setup records"); + let err = registry.for_version("26.8").unwrap_err(); + assert!( + matches!(err, Error::ArtifactMissing(_)), + "an empty cache with autofetch off: want ArtifactMissing, got {err:?}" + ); + chtypes::setup(zone("UTC")) + .expect("the failed open cleared the record, so a different setup is accepted"); + + // A refused version spelling is a failed open too. + let err = registry.for_version("v26.8").unwrap_err(); + assert!( + matches!(err, Error::Usage(_)), + "a refused spelling: want Usage, got {err:?}" + ); + chtypes::setup(zone("Europe/Berlin")).expect("the refused spelling cleared the record too"); + + // Before any open is attempted, a second, different setup is still misuse. + let err = chtypes::setup(zone("Asia/Tokyo")).unwrap_err(); + assert!(matches!(err, Error::Usage(_)), "{err:?}"); + + let _ = std::fs::remove_dir_all(&cache); +} diff --git a/ts/CHANGELOG.md b/ts/CHANGELOG.md index 48e075d5..d92f5aee 100644 --- a/ts/CHANGELOG.md +++ b/ts/CHANGELOG.md @@ -6,6 +6,10 @@ The four bindings in this repository are released together and give one answer, ## [Unreleased] +### Fixed + +- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup({ timezone: 'Asia/Tokyo' })`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then a `UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`registry.for`, `Registry.open({ preload })` and `openUnverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. + ## [1.0.2] — 2026-10-06 There is no 1.0.1 of this binding: 1.0.1 was a Go-only fix to that module's metadata. From 1.0.2 the four bindings release together again. diff --git a/ts/src/abi1/loader.ts b/ts/src/abi1/loader.ts index fb40dcdc..f470421f 100644 --- a/ts/src/abi1/loader.ts +++ b/ts/src/abi1/loader.ts @@ -67,8 +67,6 @@ export interface ImageSetup { readonly timezone?: string; /** Seeded once with `chs_set_defaults` when there are any. */ readonly defaults?: Readonly>; - /** Told how step 7 ended, when given: the public layer's setup guard latches on success and clears its record on a failure before any image has completed step 7 (`../setup.ts`). */ - readonly settle?: (completed: boolean) => void; } /** The loader's own input, decoupled from the fetch layer's `Resolved` type: the adapter in `../registry.ts` joins them and passes the predicate verbatim. */ @@ -260,19 +258,9 @@ export function openAbi1(input: LoadInput): LoadedImage { /** * Step 7: `chs_initialize(zone)`, then `chs_set_defaults` when there are defaults. A failure is the call's own error, mapped by the status table; the zone asked for is named in the message either way. - * `setup.settle`, when given, is told how step 7 ended. The caller caches the image only after this returns, so a failure is never remembered and the next open runs step 7 again. + * The caller caches the image only after this returns, so a failure is never remembered and the next open runs every step again; the public layer settles the setup with the open's outcome (`../setup.ts`). */ function initializeImage(image: LoadedImage, setup: ImageSetup): void { - try { - runStep7(image, setup); - } catch (err) { - setup.settle?.(false); - throw err; - } - setup.settle?.(true); -} - -function runStep7(image: LoadedImage, setup: ImageSetup): void { const timezone = setup.timezone ?? ''; try { image.calls.initialize(Buffer.from(timezone, 'utf8')); diff --git a/ts/src/library.ts b/ts/src/library.ts index 12601bdb..06c91cd5 100644 --- a/ts/src/library.ts +++ b/ts/src/library.ts @@ -16,7 +16,7 @@ import { } from './documents.js'; import type { Resolved } from './ocifetch/index.js'; import { Schema, type CompileOptions } from './schema.js'; -import { commitSetup, settleSetup } from './setup.js'; +import { commitSetup, latchSetup, settleFailedOpen, setupGeneration } from './setup.js'; import { type BytesIn, bytesIn, encodeSettings } from './settings.js'; export class Library { @@ -125,6 +125,15 @@ export function libraryOf(image: LoadedImage, resolved: Resolved | undefined): L * no `resolved`, unless a registry had already opened the same image. */ export function openUnverified(path: string, options: { readonly allow: boolean }): Library { - const setup = commitSetup(); - return libraryOf(loadUnverified(path, { allow: options.allow, timezone: setup.timezone, defaults: setup.defaults, settle: settleSetup }), undefined); + const began = setupGeneration(); + try { + const setup = commitSetup(); + const image = loadUnverified(path, { allow: options.allow, timezone: setup.timezone, defaults: setup.defaults }); + latchSetup(); + return libraryOf(image, undefined); + } catch (err) { + // Like any open: a failure clears the setup record while no image has completed step 7. + settleFailedOpen(began); + throw err; + } } diff --git a/ts/src/registry.ts b/ts/src/registry.ts index da510721..581d7db4 100644 --- a/ts/src/registry.ts +++ b/ts/src/registry.ts @@ -37,7 +37,7 @@ import { } from './ocifetch/index.js'; import { ENV_AUTOFETCH_NAME, SPELLING_REGEX } from './ocifetch/constants.gen.js'; import { withEnvironment } from './env.js'; -import { commitSetup, settleSetup } from './setup.js'; +import { commitSetup, latchSetup, settleFailedOpen, setupGeneration } from './setup.js'; /** The fetch layer's options (bases, cache directory, system directories, trusted keys, token, allow-unsigned, offline, frozen, lock path, ...) without its test-only hooks. */ export type FetchOptions = Omit; @@ -80,7 +80,6 @@ export class Registry { static async open(options: RegistryOptions = {}): Promise { const registry = new Registry(options); for (const request of options.preload ?? []) { - checkSpelling(request); await registry.#memoized(request, false); } return registry; @@ -88,7 +87,6 @@ export class Registry { /** Open a version: the installed build the request names, fetched first when autofetch is on and none is installed. */ async for(request: string): Promise { - checkSpelling(request); return this.#memoized(request, this.#autofetch); } @@ -105,6 +103,8 @@ export class Registry { #memoized(request: string, mayFetch: boolean): Promise { const memo = this.#memo.get(request); if (memo !== undefined) return memo; + // A failed open clears the setup record while no image has completed load step 7, whatever failed: the spelling, the fetch, the signature or any load step (bindings-v1.md §6, rule 4). + const began = setupGeneration(); const opening = this.#openRequest(request, mayFetch).then( (library) => { if (!this.#opened.includes(library)) this.#opened.push(library); @@ -112,6 +112,7 @@ export class Registry { }, (err: unknown) => { this.#memo.delete(request); // a failure is never remembered + settleFailedOpen(began); throw err; }, ); @@ -120,6 +121,7 @@ export class Registry { } async #openRequest(request: string, mayFetch: boolean): Promise { + checkSpelling(request); // a refused spelling is a failed open too, settled by #memoized const platform: PlatformKey | undefined = this.#fetch.platform ?? hostPlatformKey(os.platform(), os.arch()); if (platform === undefined) { throw new ArtifactMissingError(`chtypes: no v1 artifact is published for this host (${os.platform()}-${os.arch()})`); @@ -141,8 +143,8 @@ export class Registry { platform: resolved.platform, timezone: setup.timezone, defaults: setup.defaults, - settle: settleSetup, }); + latchSetup(); return libraryOf(image, resolved); } } diff --git a/ts/src/setup.ts b/ts/src/setup.ts index c0170cef..7e61c75d 100644 --- a/ts/src/setup.ts +++ b/ts/src/setup.ts @@ -11,12 +11,14 @@ * the first open records the empty setup: the empty zone, which the * library reads as UTC, and no defaults. Once an image completes loader * step 7 (`chs_initialize`, then `chs_set_defaults` when there are - * defaults), `setup` succeeds only with exactly the setup in effect. If - * step 7 fails before any image has completed it, the record is cleared, - * so a corrected `setup` is accepted and the next open runs step 7 with it. + * defaults), `setup` succeeds only with exactly the setup in effect + * (`latchSetup`). Until then, any open that fails, whatever failed (the + * fetch, the signature, an incompatible artifact, a missing symbol or + * step 7), clears the record (`settleFailedOpen`): the next `setup` is + * accepted, and an open with no `setup` after the failure records the + * empty setup. * 3. Every image is set up at loader step 7, once, from this record - * (`./abi1/loader.ts`), which reports how step 7 ended to `settleSetup`; - * nothing sets either again for that image. + * (`./abi1/loader.ts`); nothing sets either again for that image. * * This is state of the isolate: every worker thread calls it identically, and * a worker whose zone differs is refused by the library at its first open. @@ -40,6 +42,8 @@ export interface ProcessSetup { let recorded: ProcessSetup | undefined; /** Whether any image has completed loader step 7 under the record. */ let latched = false; +/** Counts the records `setup` made and the records failed opens cleared. An open reads it when it begins, and clears the record on failure only if it is unchanged: a failed open never clears a setup recorded after it began. */ +let generation = 0; function describe(s: ProcessSetup): string { const keys = Object.keys(s.defaults).sort(); @@ -63,6 +67,7 @@ export function setup(options: SetupOptions = {}): void { const next: ProcessSetup = { timezone, defaults: { ...defaults } }; if (recorded === undefined) { recorded = next; + generation += 1; return; } if (!same(recorded, next)) { @@ -76,16 +81,23 @@ export function commitSetup(): ProcessSetup { return recorded; } +/** Read when an open begins, for `settleFailedOpen`. */ +export function setupGeneration(): number { + return generation; +} + +/** An image completed loader step 7: from then on the setup in effect stands. An open calls it synchronously after the load that committed the record, so it latches the record that load ran under. */ +export function latchSetup(): void { + latched = true; +} + /** - * How loader step 7 ended, reported by the loader. A success latches the setup in effect. A failure before any image has completed step 7 clears the record, so `setup` accepts a corrected setup; once the setup has latched, a failure changes nothing (the library's own process-once rule answers a different zone on an image that already has one). - * An isolate runs this synchronously with the commit and the load that preceded it, so the record it settles is the one that load ran under. + * Settle an open that failed, whatever failed. While no image has completed step 7 it clears the record, so `setup` accepts a corrected setup, unless a setup was recorded after the open began (`began` is `setupGeneration()` then). Once the setup has latched it changes nothing: the library's own process-once rule answers a different zone on an image that already has one. */ -export function settleSetup(completed: boolean): void { - if (completed) { - latched = true; - return; - } - if (!latched) recorded = undefined; +export function settleFailedOpen(began: number): void { + if (latched || generation !== began) return; + recorded = undefined; + generation += 1; } /** Test only: forget the recorded setup and the latch. Not part of the public API. */ diff --git a/ts/test/settings-setup.test.ts b/ts/test/settings-setup.test.ts index 3a89166a..3621de2e 100644 --- a/ts/test/settings-setup.test.ts +++ b/ts/test/settings-setup.test.ts @@ -3,10 +3,15 @@ * `setup` (`docs/reference/bindings-v1.md` §2, §3 and §6). Pure: no library. */ +import { mkdtempSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { beforeEach, describe, expect, it } from 'vitest'; import { UsageError } from '../src/abi1/errors.js'; +import { ArtifactMissingError } from '../src/ocifetch/errors.js'; +import { Registry } from '../src/registry.js'; import { bytesIn, encodeColumns, encodeParams, encodeSettings } from '../src/settings.js'; -import { commitSetup, resetSetupForTests, setup } from '../src/setup.js'; +import { commitSetup, latchSetup, resetSetupForTests, settleFailedOpen, setup, setupGeneration } from '../src/setup.js'; describe('settings values are strings, and only strings', () => { it('serializes a map of strings with the stock encoder, verbatim', () => { @@ -93,3 +98,33 @@ describe('setup: the process-once rule', () => { expect(() => setup({ defaults: { a: 1 as unknown as string } })).toThrow(TypeError); }); }); + +describe('setup: a failed open before the first step 7 clears the record', () => { + beforeEach(() => resetSetupForTests()); + + it('clears it after a registry open that fails before any load (nothing installed, autofetch off)', async () => { + setup({ timezone: 'Asia/Tokyo' }); + const cacheDir = mkdtempSync(path.join(os.tmpdir(), 'setup-latch-empty-cache-')); + const registry = await Registry.open({ fetch: { cacheDir, systemDirs: [], offline: true }, autofetch: false }); + await expect(registry.for('26.8')).rejects.toBeInstanceOf(ArtifactMissingError); + expect(() => setup({ timezone: 'UTC' })).not.toThrow(); + }); + + it('clears it after a refused version spelling too: any failed open', async () => { + setup({ timezone: 'Asia/Tokyo' }); + const registry = await Registry.open({ autofetch: false }); + await expect(registry.for('v26.8')).rejects.toBeInstanceOf(UsageError); + expect(() => setup({ timezone: 'UTC' })).not.toThrow(); + }); + + it('never clears a setup recorded after the open began, nor a latched one', () => { + const began = setupGeneration(); + setup({ timezone: 'Asia/Tokyo' }); + settleFailedOpen(began); // it began before Asia/Tokyo was recorded + expect(() => setup({ timezone: 'UTC' })).toThrow(UsageError); + latchSetup(); + settleFailedOpen(setupGeneration()); + expect(() => setup({ timezone: 'UTC' })).toThrow(UsageError); + expect(() => setup({ timezone: 'Asia/Tokyo' })).not.toThrow(); + }); +}); From b8c3e6b5781d6a7c578bd66716168fefe10e8be2 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Tue, 6 Oct 2026 03:23:44 -0400 Subject: [PATCH 3/5] setup cases: a failed open unlocks, never clears (fails on the clear head) The ruling on #468 changed before merge from "clear" to "unlock": after an open that attempted a load and failed, the setup record is KEPT but becomes replaceable, so a plain retry runs under the recorded zone, never the empty setup; and the binding's own input checks (a refused spelling, an unverified open without the caller's opt-in) unlock nothing. The shared cases now pin that: - setup.latches_only_on_success: a refused spelling and a withheld opt-in each leave a different setup refused; a refused load makes the record replaceable; a replaced record is locked again; a plain retry after a step 7 refusal is refused again under the recorded zone; the corrected setup opens, and the image reports that zone. - setup.a_retry_after_a_failed_open_keeps_the_recorded_setup: a refused load, then a plain retry with no new setup, which must load under the recorded zone. The stub gains a zone probe (_stubshared.ZONE_PROBE): chs_type_validate of exactly "!Z:" returns the zone the image holds, so each runner reads it back through its public validate_type. Open steps may now carry "request" (a registry over an empty cache) and "allow" (the caller's opt-in). This commit carries no binding change: on the clear head both cases fail, at the setup after the refused spelling and at the retry's image zone. refs #468 Co-Authored-By: Claude Opus 5.5 (1M context) --- go/chtypes/setup_cases_test.go | 57 +++++++--- python/tests/abi1/test_public_setup_cases.py | 34 ++++-- rust/tests/api_v1_setup_cases.rs | 72 ++++++++++--- scripts/abi-v1/emit/_stubshared.py | 14 +++ scripts/abi-v1/emit/setup_cases.py | 103 ++++++++++++++----- scripts/abi-v1/emit/stub.py | 81 ++++++++++++--- tests/fixtures/abi-v1/setup-cases.json | 79 ++++++++++++-- ts/test/abi1/setup-cases.test.ts | 31 ++++-- 8 files changed, 388 insertions(+), 83 deletions(-) diff --git a/go/chtypes/setup_cases_test.go b/go/chtypes/setup_cases_test.go index d9015506..28ba5f27 100644 --- a/go/chtypes/setup_cases_test.go +++ b/go/chtypes/setup_cases_test.go @@ -23,22 +23,32 @@ type setupCaseExpect struct { ChCode *int32 `json:"ch_code"` ChName *string `json:"ch_name"` Reason string `json:"reason"` + // ImageZone, on a successful open, is the zone the image was set up + // with, read back with the document's image_zone_probe. + ImageZone *string `json:"image_zone"` } type setupCaseStep struct { Op string `json:"op"` Timezone string `json:"timezone"` Variant string `json:"variant"` + Request string `json:"request"` + Allow *bool `json:"allow"` Expect setupCaseExpect `json:"expect"` } +type setupCasesDoc struct { + ImageZoneProbe string `json:"image_zone_probe"` + Cases []setupCase `json:"cases"` +} + type setupCase struct { ID string `json:"id"` Variant string `json:"variant"` Steps []setupCaseStep `json:"steps"` } -func loadSetupCases(t *testing.T) []setupCase { +func loadSetupCases(t *testing.T) setupCasesDoc { t.Helper() _, file, _, ok := runtime.Caller(0) if !ok { @@ -49,16 +59,17 @@ func loadSetupCases(t *testing.T) []setupCase { if err != nil { t.Fatal(err) } - var doc struct { - Cases []setupCase `json:"cases"` - } + var doc setupCasesDoc if err := json.Unmarshal(raw, &doc); err != nil { t.Fatalf("%s: %v", path, err) } if len(doc.Cases) == 0 { t.Fatalf("%s: zero cases; a suite that ran nothing must not pass", path) } - return doc.Cases + if doc.ImageZoneProbe == "" { + t.Fatalf("%s: no image_zone_probe", path) + } + return doc } // setupStepOutcome compares one step's outcome with its expectation, and @@ -129,7 +140,8 @@ func setupStepOutcome(want setupCaseExpect, err error) string { // API. func TestSetupCases(t *testing.T) { stubDir(t) // skips loudly without the stubs - for _, c := range loadSetupCases(t) { + doc := loadSetupCases(t) + for _, c := range doc.Cases { t.Run(c.ID, func(t *testing.T) { resetSetup(t) t.Setenv("CHTYPES_ALLOW_UNVERIFIED_LIBRARY", "1") @@ -148,16 +160,37 @@ func TestSetupCases(t *testing.T) { } for i, s := range c.Steps { var err error - switch s.Op { - case "setup": + var lib *Library + switch { + case s.Op == "setup": err = Setup(SetupOptions{Timezone: s.Timezone}) - case "open": - _, err = OpenUnverified(image(s.Variant), true) + case s.Op == "open" && s.Request != "": + // A registry over an empty cache, offline, autofetch off. + var reg *Registry + reg, err = NewRegistry(WithFetchOptions(FetchOptions{ + CacheDir: t.TempDir(), SystemDirs: []string{t.TempDir()}, Offline: true, + }), WithAutoFetch(false)) + if err != nil { + t.Fatalf("step %d: constructing a registry: %v", i, err) + } + lib, err = reg.For(s.Request) + case s.Op == "open": + lib, err = OpenUnverified(image(s.Variant), s.Allow == nil || *s.Allow) default: t.Fatalf("step %d: an op this runner does not know: %q", i, s.Op) } - if diff := setupStepOutcome(s.Expect, err); diff != "" { - t.Fatalf("step %d (%s %q %s): %s", i, s.Op, s.Timezone, s.Variant, diff) + diff := setupStepOutcome(s.Expect, err) + if diff == "" && s.Expect.ImageZone != nil { + zone, perr := lib.ValidateType(doc.ImageZoneProbe) + switch { + case perr != nil: + diff = fmt.Sprintf("reading the image zone back: %v", perr) + case zone != *s.Expect.ImageZone: + diff = fmt.Sprintf("the image was set up with zone %q, want %q", zone, *s.Expect.ImageZone) + } + } + if diff != "" { + t.Fatalf("step %d (%s %q %s%s): %s", i, s.Op, s.Timezone, s.Variant, s.Request, diff) } } }) diff --git a/python/tests/abi1/test_public_setup_cases.py b/python/tests/abi1/test_public_setup_cases.py index 11872cab..748d0dad 100644 --- a/python/tests/abi1/test_public_setup_cases.py +++ b/python/tests/abi1/test_public_setup_cases.py @@ -18,7 +18,9 @@ from chtypes import ( ArtifactCorruptError, ArtifactIncompatibleError, + FetchOptions, InternalError, + Registry, SchemaError, Status, UnsupportedError, @@ -30,7 +32,9 @@ from .conftest import REPO_ROOT, stub_path SETUP_CASES_PATH = REPO_ROOT / "tests" / "fixtures" / "abi-v1" / "setup-cases.json" -_SETUP_CASES: list[dict] = json.loads(SETUP_CASES_PATH.read_text(encoding="utf-8"))["cases"] +_SETUP_DOC: dict = json.loads(SETUP_CASES_PATH.read_text(encoding="utf-8")) +_SETUP_CASES: list[dict] = _SETUP_DOC["cases"] +IMAGE_ZONE_PROBE: bytes = _SETUP_DOC["image_zone_probe"].encode() CLASSES = { "schema": SchemaError, @@ -88,20 +92,30 @@ def image(variant: str | None) -> str: warned: set[str] = set() for i, step in enumerate(steps): error: BaseException | None = None + opened = None try: if step["op"] == "setup": setup(timezone=step["timezone"]) + elif step["op"] == "open" and "request" in step: + # A registry over an empty cache, offline, autofetch off. + cache = tmp_path / f"cache-{i}" + empty = FetchOptions(cache_dir=cache, system_dirs=[], offline=True) + registry = Registry(fetch=empty, autofetch=False) + opened = registry.for_version(step["request"]) elif step["op"] == "open": path = image(step.get("variant")) - # An unverified open warns once per path: on each copy's first open. - expect_warning = path not in warned - warned.add(path) + allow = step.get("allow", True) + # An unverified open with both opt-ins warns once per path: on + # each copy's first such open. Without them it warns nothing. + expect_warning = allow and path not in warned + if allow: + warned.add(path) with ( pytest.warns(UserWarning, match="UNVERIFIED") if expect_warning else contextlib.nullcontext() ): - open_unverified(path, allow=True) + opened = open_unverified(path, allow=allow) else: pytest.fail(f"step {i}: an op this runner does not know: {step['op']!r}") except ( @@ -114,5 +128,13 @@ def image(variant: str | None) -> str: ) as exc: error = exc diff = _outcome(step["expect"], error) - where = f"{step['op']} {step.get('timezone', '')!r} {step.get('variant', '')}" + if not diff and "image_zone" in step["expect"]: + zone = opened.validate_type(IMAGE_ZONE_PROBE) + want_zone = step["expect"]["image_zone"] + if zone != want_zone.encode(): + diff = f"the image was set up with zone {zone!r}, want {want_zone!r}" + where = ( + f"{step['op']} {step.get('timezone', '')!r} " + f"{step.get('variant', '')}{step.get('request', '')}" + ) assert not diff, f"step {i} ({where}): {diff}" diff --git a/rust/tests/api_v1_setup_cases.rs b/rust/tests/api_v1_setup_cases.rs index 176b52c4..bb4d2437 100644 --- a/rust/tests/api_v1_setup_cases.rs +++ b/rust/tests/api_v1_setup_cases.rs @@ -13,8 +13,9 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::Arc; -use chtypes::{Error, Library, SetupOptions, status}; +use chtypes::{Error, FetchOptions, Library, Registry, RegistryOptions, SetupOptions, status}; use serde_json::Value; const ENV_STUBS: &str = "CHTYPES_ABI1_STUBS"; @@ -29,10 +30,15 @@ fn repo_root() -> PathBuf { .to_path_buf() } -fn load_cases() -> Vec { +/// The document's image zone probe, and its cases. +fn load_cases() -> (String, Vec) { let path = repo_root().join("tests/fixtures/abi-v1/setup-cases.json"); let doc: Value = serde_json::from_slice(&std::fs::read(&path).expect("read setup-cases.json")) .expect("parse setup-cases.json"); + let probe = doc["image_zone_probe"] + .as_str() + .expect("setup-cases.json has an image_zone_probe") + .to_string(); let cases = doc["cases"] .as_array() .expect("setup-cases.json has a cases array") @@ -42,7 +48,7 @@ fn load_cases() -> Vec { "{}: zero cases; a suite that ran nothing must not pass", path.display() ); - cases + (probe, cases) } /// A status name (`CHS_REJECTED`) as the generated constant's value. @@ -124,7 +130,7 @@ fn outcome(want: &Value, got: &Result<(), Error>) -> Result<(), String> { /// The child: one case, on a fresh copy of each stub it opens, from a process /// with no setup recorded. -fn run_case(stubs: &Path, case: &Value) { +fn run_case(stubs: &Path, probe: &str, case: &Value) { let id = case["id"].as_str().expect("case id"); let case_variant = case["variant"].as_str().expect("case variant"); let manifest: Value = @@ -166,16 +172,58 @@ fn run_case(stubs: &Path, case: &Value) { .get("variant") .and_then(Value::as_str) .unwrap_or(case_variant); - let got: Result<(), Error> = match op { - "setup" => chtypes::setup(SetupOptions { + let request = step.get("request").and_then(Value::as_str); + let allow = step.get("allow").and_then(Value::as_bool).unwrap_or(true); + let mut opened: Option> = None; + let got: Result<(), Error> = match (op, request) { + ("setup", _) => chtypes::setup(SetupOptions { timezone: Some(zone.to_string()), defaults: Vec::new(), }), - "open" => Library::open_unverified(image(variant), true).map(|_| ()), - other => panic!("case {id} step {i}: an op this runner does not know: {other:?}"), + ("open", Some(request)) => { + // A registry over an empty cache, offline, autofetch off. + let cache = dir.join(format!("cache-{i}")); + std::fs::create_dir_all(&cache).expect("create an empty cache"); + let registry = Registry::new(RegistryOptions { + fetch: FetchOptions { + cache_dir: Some(cache.to_string_lossy().into_owned()), + system_dirs: Some(Vec::new()), + offline: true, + ..Default::default() + }, + autofetch: Some(false), + ..Default::default() + }) + .expect("constructing a registry opens nothing"); + registry.for_version(request).map(|l| opened = Some(l)) + } + ("open", None) => { + Library::open_unverified(image(variant), allow).map(|l| opened = Some(l)) + } + (other, _) => { + panic!("case {id} step {i}: an op this runner does not know: {other:?}") + } }; - if let Err(diff) = outcome(&step["expect"], &got) { - panic!("case {id} step {i} ({op} {zone:?} {variant}): {diff}"); + let mut checked = outcome(&step["expect"], &got); + if let (Ok(()), Some(want)) = (&checked, step["expect"].get("image_zone")) { + let want = want.as_str().expect("image_zone is a string"); + let library = opened + .as_ref() + .expect("a successful open returns its library"); + checked = match library.validate_type(probe) { + Err(e) => Err(format!("reading the image zone back: {e:?}")), + Ok(zone) if zone.as_bytes() != want.as_bytes() => Err(format!( + "the image was set up with zone {:?}, want {want:?}", + String::from_utf8_lossy(zone.as_bytes()) + )), + Ok(_) => Ok(()), + }; + } + if let Err(diff) = checked { + panic!( + "case {id} step {i} ({op} {zone:?} {variant} {}): {diff}", + request.unwrap_or("") + ); } } let _ = std::fs::remove_dir_all(&dir); @@ -190,7 +238,7 @@ fn setup_cases() { ); return; }; - let cases = load_cases(); + let (probe, cases) = load_cases(); if let Some(id) = std::env::var_os(ENV_CASE) { let id = id.to_string_lossy().into_owned(); @@ -198,7 +246,7 @@ fn setup_cases() { .iter() .find(|c| c["id"].as_str() == Some(id.as_str())) .unwrap_or_else(|| panic!("no setup case {id:?}")); - run_case(&stubs, case); + run_case(&stubs, &probe, case); return; } diff --git a/scripts/abi-v1/emit/_stubshared.py b/scripts/abi-v1/emit/_stubshared.py index 8df26ab6..e0497cd6 100644 --- a/scripts/abi-v1/emit/_stubshared.py +++ b/scripts/abi-v1/emit/_stubshared.py @@ -190,6 +190,20 @@ def doc_render(template: str, param: bytes) -> dict: "conflict_message": "the image zone is already set, to a different spelling", } +# The image zone probe: the one way a test reads back the zone an image was set +# up with, through a binding's PUBLIC API. A call to `fn` whose `param` is +# exactly `probe` returns, as its output, the zone the image holds +# (IMAGE_ZONE's committed spelling, empty when none was committed yet), +# instead of its echo. The setup cases (emit/setup_cases.py) use it to prove an +# open ran step 7 under the zone the caller recorded, not under the empty +# setup. A real library has no such call: this is the stub's alone. +ZONE_PROBE = { + "fn": "chs_type_validate", + "param": "type_expr", + "out": "out", + "probe": "!Z:", +} + # The symbol a loader checks before any other (step 3): it gets its own named # variant ("no-abi-version", reason "not_v1") rather than folding into the # generic missing- sweep, because a real loader distinguishes "this is diff --git a/scripts/abi-v1/emit/setup_cases.py b/scripts/abi-v1/emit/setup_cases.py index afc52d48..3e5f37b0 100644 --- a/scripts/abi-v1/emit/setup_cases.py +++ b/scripts/abi-v1/emit/setup_cases.py @@ -24,33 +24,50 @@ * {"op": "setup", "timezone": ""}: the binding's `setup` with that image zone and no defaults; * {"op": "open"} or {"op": "open", "variant": ""}: the binding's - unverified open (Go `OpenUnverified(path, true)`, Python - `open_unverified(path, allow=True)`, TypeScript `openUnverified(path, - { allow: true })`, Rust `Library::open_unverified(path, true)`) of the - case's ONE copy of that variant (the case's own `variant` when the - step names none), the same file for every open step that names it. + unverified open with both opt-ins (Go `OpenUnverified(path, true)`, + Python `open_unverified(path, allow=True)`, TypeScript + `openUnverified(path, { allow: true })`, Rust + `Library::open_unverified(path, true)`) of the case's ONE copy of that + variant (the case's own `variant` when the step names none), the same + file for every open step that names it. With "allow": false the + caller's own opt-in is withheld (the environment's stays set); + * {"op": "open", "request": ""}: a registry open of that + request, over an empty cache, offline, with autofetch off (Go + `NewRegistry(...).For`, Python `Registry(...).for_version`, + TypeScript `(await Registry.open(...)).for`, Rust + `Registry::new(...).for_version`). `expect` is {"ok": true}, or the error the step must raise: `class` is a key of spec/abi-v1/sdk.json's errors.classes (each binding's own class for it). An error the library answered also names the `status`, `ch_code` and `ch_name` it -must carry, verbatim; a loader refusal names its `reason`, sdk.json's word. +must carry, verbatim; a loader refusal names its `reason`, sdk.json's word. A +successful open may also name `image_zone`: the zone the opened image was set +up with at step 7, read back through the library with the document's +`image_zone_probe` (_stubshared.ZONE_PROBE: `validate_type` of exactly that +text, in each binding's own spelling). -THE CASE. Setup latches only once an image completes load step 7 (chs_initialize, -then chs_set_defaults when there are defaults). Before that: +THE RULE THE CASES PIN. Setup latches only once an image completes load step 7 +(chs_initialize, then chs_set_defaults when there are defaults). Before that: * a second, different setup before any open is attempted is a UsageError, never a silent last-wins; - * ANY failed open clears the setup record, whatever failed: here a load the - loader refuses at step 4 (_stubshared.plan's "fingerprint-other" variant, - an incompatible artifact), and a zone the library refuses at step 7. The - next setup is then accepted, whatever it is, and the next open runs with - it; + * the binding's own input checks unlock nothing: a refused version spelling, + or an unverified open without the caller's opt-in, fails before any load is + attempted, and a different setup is still a UsageError afterwards; + * an open that attempted a load and failed, whatever failed (here a load the + loader refuses at step 4, _stubshared.plan's "fingerprint-other" variant, + and a zone the library refuses at step 7), KEEPS the setup record but makes + it replaceable: the next open with no new setup runs under the recorded + setup, never the empty one, and a different setup replaces the record; + * a replaced record is locked again, so a second, different setup before the + next open is a UsageError; * a failed open is never remembered, so the same open runs again. Once an image has completed step 7, a different setup is a UsageError. The bad zone, its refusal and the stub rule behind both come from _stubshared.IMAGE_ZONE, -which the stub is generated from too; the refused load and its reason come from -_stubshared.plan, which build-stubs.sh builds from. +and the probe from _stubshared.ZONE_PROBE, which the stub is generated from too; +the refused load and its reason come from _stubshared.plan, which +build-stubs.sh builds from. """ from __future__ import annotations @@ -103,6 +120,10 @@ def _refused_load(model) -> tuple[str, dict]: return variant.name, {"class": classes.pop(), "reason": variant.reason} +# A refused version spelling: a "v" prefix, refused before any resolve. +REFUSED_SPELLING = "v26.8" + + def _latch_case(model) -> dict: rule = _stubshared.IMAGE_ZONE fn = model.function(rule["fn"]) @@ -126,23 +147,30 @@ def _latch_case(model) -> dict: # misuse: the first stands, never a silent last-wins. {"op": "setup", "timezone": OTHER_ZONE, "expect": OK}, {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, + # The binding's own input checks fail before any load is attempted + # and unlock nothing: a refused version spelling, and an unverified + # open without the caller's opt-in. + {"op": "open", "request": REFUSED_SPELLING, "expect": misuse}, + {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, + {"op": "open", "allow": False, "expect": misuse}, + {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, # A load the loader refuses before step 7. No image has completed - # step 7, so the failed open clears the record. + # step 7, so the failed open makes the record replaceable. {"op": "open", "variant": refused_variant, "expect": refused_load}, - # So a different setup is accepted now: a zone the library - # refuses. The open that reaches step 7 raises the library's own - # refusal, and clears the record again. + # So a different setup replaces it: a zone the library refuses. + # The replaced record is locked again until the next open. {"op": "setup", "timezone": rule["bad_zone"], "expect": OK}, + {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, + # The open that reaches step 7 raises the library's own refusal, + # and makes the record replaceable again; a plain retry runs step + # 7 again under the RECORDED zone, never the empty one, and is + # refused again. {"op": "open", "expect": refused}, - # No image has completed step 7, so the record was cleared: the - # same setup records again, and the open runs step 7 again rather - # than remembering the failure. - {"op": "setup", "timezone": rule["bad_zone"], "expect": OK}, {"op": "open", "expect": refused}, - # A corrected setup is no longer refused, and the same image now - # opens under it. + # A corrected setup replaces it, and the same image now opens + # under it. {"op": "setup", "timezone": GOOD_ZONE, "expect": OK}, - {"op": "open", "expect": OK}, + {"op": "open", "expect": {**OK, "image_zone": GOOD_ZONE}}, # Step 7 has completed: the setup is latched, and a different one # is misuse. {"op": "setup", "timezone": OTHER_ZONE, "expect": misuse}, @@ -150,14 +178,35 @@ def _latch_case(model) -> dict: } +def _retry_case(model) -> dict: + """A transient failure, then a plain retry with no new setup: the retry + loads under the zone the caller recorded, read back from the image.""" + refused_variant, refused_load = _refused_load(model) + misuse = {"class": _class_of(model, _stubshared.IMAGE_ZONE["conflict_status"])} + return { + "id": "setup.a_retry_after_a_failed_open_keeps_the_recorded_setup", + "variant": "ok", + "steps": [ + {"op": "setup", "timezone": OTHER_ZONE, "expect": OK}, + # The failed open: an artifact the loader refuses before step 7. + {"op": "open", "variant": refused_variant, "expect": refused_load}, + # The plain retry, with no new setup, opens another artifact under + # the RECORDED zone, never the empty setup. + {"op": "open", "expect": {**OK, "image_zone": OTHER_ZONE}}, + {"op": "setup", "timezone": GOOD_ZONE, "expect": misuse}, + ], + } + + def build_cases(model) -> list[dict]: - return [_latch_case(model)] + return [_latch_case(model), _retry_case(model)] def render(model) -> str: doc = { "_generated": banner(model), "schema": 1, + "image_zone_probe": _stubshared.ZONE_PROBE["probe"], "cases": build_cases(model), } return json.dumps(doc, indent=2, sort_keys=True) + "\n" diff --git a/scripts/abi-v1/emit/stub.py b/scripts/abi-v1/emit/stub.py index 72b0f395..c59bf1d9 100644 --- a/scripts/abi-v1/emit/stub.py +++ b/scripts/abi-v1/emit/stub.py @@ -63,6 +63,8 @@ CHS_REJECTED and commits nothing, the first other spelling is committed, and after that the same spelling is CHS_OK and a different one is CHS_INVALID_ARGUMENT, per image (each dlopen'd copy has its own state). + A probe (_stubshared.ZONE_PROBE) reads the committed zone back through + `chs_type_validate`, so a public-API test can see which zone step 7 used. * `chs_live_handles` reports this image's own live count per handle kind from five atomic counters (incremented on mint, decremented the instant a handle's refcount reaches zero), never by walking a registry. @@ -816,6 +818,29 @@ def _one_create(fn) -> list[str]: ] +def _image_zone_state() -> str: + """The image zone's state, file scope: chs_initialize's process-once rule + (_image_zone) writes it, and the zone probe (_zone_probe) reads it. Static + storage, so each dlopen'd copy of the stub (each image) has its own.""" + return """ +/* ------------------------------------------------------------- the image zone + (scripts/abi-v1/emit/_stubshared.py IMAGE_ZONE and ZONE_PROBE) */ + +static atomic_flag chs_stub_zone_lock = ATOMIC_FLAG_INIT; +static uint8_t *chs_stub_zone = NULL; +static size_t chs_stub_zone_len = 0; +static int chs_stub_zone_set = 0; + +static void chs_stub_zone_acquire(void) { + while (atomic_flag_test_and_set(&chs_stub_zone_lock)) { /* spin: the critical sections are a few instructions */ } +} + +static void chs_stub_zone_release(void) { + atomic_flag_clear(&chs_stub_zone_lock); +} +""" + + def _image_zone(fn) -> list[str]: """The stub's stand-in for the image zone's process-once rule (_stubshared.IMAGE_ZONE): after the input checks and any status @@ -832,10 +857,6 @@ def _image_zone(fn) -> list[str]: return [ " {", " /* the image zone's process-once rule: see scripts/abi-v1/emit/_stubshared.py IMAGE_ZONE */", - " static atomic_flag held_lock = ATOMIC_FLAG_INIT;", - " static uint8_t *held = NULL;", - " static size_t held_len = 0;", - " static int held_set = 0;", f" static const char bad[] = {_c_str(rule['bad_zone'])};", f" if ({p}_len == sizeof(bad) - 1 && memcmp({p}, bad, sizeof(bad) - 1) == 0) {{", f" static const char name[] = {_c_str(rule['ch_name'])};", @@ -844,16 +865,16 @@ def _image_zone(fn) -> list[str]: f" return {rule['status']};", " }", " int conflict = 0;", - " while (atomic_flag_test_and_set(&held_lock)) { /* spin: the critical section is a few instructions */ }", - " if (!held_set) {", - f" held = {p}_len ? (uint8_t *) malloc({p}_len) : NULL;", - f" if (held) memcpy(held, {p}, {p}_len);", - f" held_len = {p}_len;", - " held_set = 1;", - f" }} else if (held_len != {p}_len || ({p}_len > 0 && memcmp(held, {p}, {p}_len) != 0)) {{", + " chs_stub_zone_acquire();", + " if (!chs_stub_zone_set) {", + f" chs_stub_zone = {p}_len ? (uint8_t *) malloc({p}_len) : NULL;", + f" if (chs_stub_zone) memcpy(chs_stub_zone, {p}, {p}_len);", + f" chs_stub_zone_len = {p}_len;", + " chs_stub_zone_set = 1;", + f" }} else if (chs_stub_zone_len != {p}_len || ({p}_len > 0 && memcmp(chs_stub_zone, {p}, {p}_len) != 0)) {{", " conflict = 1;", " }", - " atomic_flag_clear(&held_lock);", + " chs_stub_zone_release();", " if (conflict) {", f" static const char msg[] = {_c_str(rule['conflict_message'])};", f" chs_stub_set_err(err, chs_stub_make_error({rule['conflict_status']}, 0, \"\", 0, msg, sizeof(msg) - 1));", @@ -863,6 +884,40 @@ def _image_zone(fn) -> list[str]: ] +def _zone_probe(fn) -> list[str]: + """The image zone probe (_stubshared.ZONE_PROBE): when the named parameter + is exactly the probe, the call returns the zone this image holds (empty + when none was committed) in its output, instead of the echo. Only the + function the probe names gets it.""" + rule = _stubshared.ZONE_PROBE + if fn.name != rule["fn"]: + return [] + p, out = rule["param"], rule["out"] + if not any(q.name == p and q.kind == "bytes_in" for q in fn.params): + raise ValueError(f"{fn.name}: _stubshared.ZONE_PROBE names {p!r}, which is not one of its bytes_in parameters") + if not any(q.name == out and q.kind == "out_handle" and q.type == BUF_HANDLE and not q.nullable for q in fn.params): + raise ValueError(f"{fn.name}: _stubshared.ZONE_PROBE names {out!r}, which is not its required chs_buf output") + return [ + " {", + " /* the image zone probe: see scripts/abi-v1/emit/_stubshared.py ZONE_PROBE */", + f" static const char probe[] = {_c_str(rule['probe'])};", + f" if ({p}_len == sizeof(probe) - 1 && memcmp({p}, probe, sizeof(probe) - 1) == 0) {{", + f" if ({out} == NULL) {{", + *_c_fixed_error("CHS_INVALID_ARGUMENT", f"{out}: required", indent=" "), + " }", + " chs_stub_zone_acquire();", + " size_t n = chs_stub_zone_len;", + " uint8_t *copy = (uint8_t *) malloc(n + 1);", + " if (n) memcpy(copy, chs_stub_zone, n);", + " chs_stub_zone_release();", + f" *{out} = chs_stub_make_buf(copy, n);", + " chs_stub_set_err(err, NULL);", + " return CHS_OK;", + " }", + " }", + ] + + def _c_bytes_lit(data: bytes) -> str: """A C string literal for arbitrary bytes, every byte octal-escaped.""" return '"' + "".join(f"\\{b:03o}" for b in data) + '"' @@ -1022,6 +1077,7 @@ def _gen_generic(model, fn) -> str: body += _status_injection(fn) body += _one_create(fn) body += _image_zone(fn) + body += _zone_probe(fn) body += _document_mode(model, fn) body += _fill_outputs(model, fn) err_param = next((p for p in fn.params if p.kind == "out_error"), None) @@ -1169,6 +1225,7 @@ def render_stub_c(model) -> str: f"{bi_begin}\n{_build_info_section(model)}\n{bi_end}", _ctor_section(), _unbound_section(), + _image_zone_state(), ] # chs_build_info and chs_abi_version need their own omit/override handling, # woven around the hand-written and special bodies below. diff --git a/tests/fixtures/abi-v1/setup-cases.json b/tests/fixtures/abi-v1/setup-cases.json index 165ca997..9edd4453 100644 --- a/tests/fixtures/abi-v1/setup-cases.json +++ b/tests/fixtures/abi-v1/setup-cases.json @@ -18,6 +18,34 @@ "op": "setup", "timezone": "Europe/Berlin" }, + { + "expect": { + "class": "usage" + }, + "op": "open", + "request": "v26.8" + }, + { + "expect": { + "class": "usage" + }, + "op": "setup", + "timezone": "Europe/Berlin" + }, + { + "allow": false, + "expect": { + "class": "usage" + }, + "op": "open" + }, + { + "expect": { + "class": "usage" + }, + "op": "setup", + "timezone": "Europe/Berlin" + }, { "expect": { "class": "artifact_incompatible", @@ -33,6 +61,13 @@ "op": "setup", "timezone": "Not/AZone" }, + { + "expect": { + "class": "usage" + }, + "op": "setup", + "timezone": "Europe/Berlin" + }, { "expect": { "ch_code": 36, @@ -42,13 +77,6 @@ }, "op": "open" }, - { - "expect": { - "ok": true - }, - "op": "setup", - "timezone": "Not/AZone" - }, { "expect": { "ch_code": 36, @@ -67,6 +95,7 @@ }, { "expect": { + "image_zone": "Europe/Berlin", "ok": true }, "op": "open" @@ -80,7 +109,43 @@ } ], "variant": "ok" + }, + { + "id": "setup.a_retry_after_a_failed_open_keeps_the_recorded_setup", + "steps": [ + { + "expect": { + "ok": true + }, + "op": "setup", + "timezone": "Asia/Tokyo" + }, + { + "expect": { + "class": "artifact_incompatible", + "reason": "fingerprint" + }, + "op": "open", + "variant": "fingerprint-other" + }, + { + "expect": { + "image_zone": "Asia/Tokyo", + "ok": true + }, + "op": "open" + }, + { + "expect": { + "class": "usage" + }, + "op": "setup", + "timezone": "Europe/Berlin" + } + ], + "variant": "ok" } ], + "image_zone_probe": "!Z:", "schema": 1 } diff --git a/ts/test/abi1/setup-cases.test.ts b/ts/test/abi1/setup-cases.test.ts index 351236e2..cf426463 100644 --- a/ts/test/abi1/setup-cases.test.ts +++ b/ts/test/abi1/setup-cases.test.ts @@ -17,7 +17,9 @@ import { ArtifactCorruptError, ArtifactIncompatibleError, InternalError, + type Library, openUnverified, + Registry, SchemaError, Status, setup, @@ -38,12 +40,16 @@ interface Expect { readonly ch_code?: number; readonly ch_name?: string; readonly reason?: string; + /** On a successful open: the zone the image was set up with, read back with the document's `image_zone_probe`. */ + readonly image_zone?: string; } interface Step { readonly op: string; readonly timezone?: string; readonly variant?: string; + readonly request?: string; + readonly allow?: boolean; readonly expect: Expect; } @@ -53,7 +59,8 @@ interface SetupCase { readonly steps: readonly Step[]; } -const setupCases: readonly SetupCase[] = (JSON.parse(readFileSync(SETUP_CASES_PATH, 'utf8')) as { cases: SetupCase[] }).cases; +const setupDoc = JSON.parse(readFileSync(SETUP_CASES_PATH, 'utf8')) as { image_zone_probe: string; cases: SetupCase[] }; +const setupCases: readonly SetupCase[] = setupDoc.cases; const CLASSES: Readonly Error>> = { schema: SchemaError, @@ -99,7 +106,7 @@ describe('the setup cases file', () => { }); describe.skipIf(!stubsAvailable)('the shared setup cases, through the public API', () => { - it.each(setupCases.map((c) => [c.id, c] as const))('%s', (_id, c) => { + it.each(setupCases.map((c) => [c.id, c] as const))('%s', async (_id, c) => { expect(c.steps.length).toBeGreaterThan(0); const dir = mkdtempSync(path.join(os.tmpdir(), 'setup-case-')); const images = new Map(); @@ -120,20 +127,30 @@ describe.skipIf(!stubsAvailable)('the shared setup cases, through the public API const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); process.env.CHTYPES_ALLOW_UNVERIFIED_LIBRARY = '1'; try { - c.steps.forEach((step, i) => { + for (const [i, step] of c.steps.entries()) { let error: unknown; + let opened: Library | undefined; try { if (step.op === 'setup') setup({ timezone: step.timezone ?? '' }); - else if (step.op === 'open') openUnverified(image(step.variant), { allow: true }); + else if (step.op === 'open' && step.request !== undefined) { + // A registry over an empty cache, offline, autofetch off. + const cacheDir = mkdtempSync(path.join(dir, 'cache-')); + const registry = await Registry.open({ fetch: { cacheDir, systemDirs: [], offline: true }, autofetch: false }); + opened = await registry.for(step.request); + } else if (step.op === 'open') opened = openUnverified(image(step.variant), { allow: step.allow ?? true }); else throw new Error(`step ${i}: an op this runner does not know: ${step.op}`); } catch (err) { const known = Object.values(CLASSES).some((cls) => err instanceof cls); if (!known) throw err; error = err; } - const diff = outcome(step.expect, error); - expect(diff, `step ${i} (${step.op} ${JSON.stringify(step.timezone ?? '')} ${step.variant ?? ''})`).toBe(''); - }); + let diff = outcome(step.expect, error); + if (diff === '' && step.expect.image_zone !== undefined) { + const zone = (opened as Library).validateType(setupDoc.image_zone_probe).toString('utf8'); + if (zone !== step.expect.image_zone) diff = `the image was set up with zone ${JSON.stringify(zone)}, want ${JSON.stringify(step.expect.image_zone)}`; + } + expect(diff, `step ${i} (${step.op} ${JSON.stringify(step.timezone ?? '')} ${step.variant ?? ''}${step.request ?? ''})`).toBe(''); + } } finally { warn.mockRestore(); resetSetupForTests(); From dbbf4b2efe53e5de655fe97d0ce8a90babd9f597 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Tue, 6 Oct 2026 03:29:51 -0400 Subject: [PATCH 4/5] setup cases: the Rust runner runs every case before its verdict The parent used to stop at the first failing case, so a later case's result was never seen. It now runs each case in its own process, reports every failure, then fails once. Still no binding change: on the clear head both cases must fail in Rust too. refs #468 Co-Authored-By: Claude Opus 5.5 (1M context) --- rust/tests/api_v1_setup_cases.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/rust/tests/api_v1_setup_cases.rs b/rust/tests/api_v1_setup_cases.rs index bb4d2437..f78656a4 100644 --- a/rust/tests/api_v1_setup_cases.rs +++ b/rust/tests/api_v1_setup_cases.rs @@ -250,7 +250,9 @@ fn setup_cases() { return; } + // Every case runs, and every failure is reported, before the verdict. let exe = std::env::current_exe().expect("this test binary's path"); + let mut failures = Vec::new(); for case in &cases { let id = case["id"].as_str().expect("case id"); let out = Command::new(&exe) @@ -260,15 +262,17 @@ fn setup_cases() { .expect("re-run this test binary for one case"); let stdout = String::from_utf8_lossy(&out.stdout); let stderr = String::from_utf8_lossy(&out.stderr); - assert!( - out.status.success(), - "setup case {id} failed:\n{stdout}\n{stderr}" - ); let ran = format!("setup case {id}: ran "); - assert!( - stdout.contains(&ran), - "setup case {id}: the child did not run it (no {ran:?} line):\n{stdout}\n{stderr}" - ); - println!("setup case {id}: passed in its own process"); + if !out.status.success() { + eprintln!("setup case {id}: FAILED in its own process"); + failures.push(format!("setup case {id} failed:\n{stdout}\n{stderr}")); + } else if !stdout.contains(&ran) { + failures.push(format!( + "setup case {id}: the child did not run it (no {ran:?} line):\n{stdout}\n{stderr}" + )); + } else { + println!("setup case {id}: passed in its own process"); + } } + assert!(failures.is_empty(), "{}", failures.join("\n")); } From 069a6eca3f45b9dff849f04aca56dc7386df77a4 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Tue, 6 Oct 2026 03:35:13 -0400 Subject: [PATCH 5/5] setup: a failed open unlocks the setup, it never clears it (#468) The lead refined the ruling before merge: clearing the record failed open, because a plain retry after a transient failure silently loaded under the empty setup (UTC) instead of the zone the caller chose. Now, while no image has completed load step 7: - an open that attempted a load and failed (resolve or fetch, signature, an incompatible artifact, a missing symbol, step 7) KEEPS the record but marks it replaceable: a retry with no new setup runs under the recorded zone and defaults, and a different setup replaces the record; - a replaced record is locked again, and so is any record an open commits for its load, so no setup replaces a record under a load; - the binding's own input checks fail before anything is attempted and unlock nothing: a refused version spelling (checked before the attempt begins) and an unverified open without both opt-ins (checked before the commit, through a gate the loader now exposes); - two different setups before any open is attempted are still a UsageError. After the first successful step 7 nothing changes. The generation guard stays: a failed open still never unlocks a setup that was recorded or replaced after it began (another caller's setup, replacing an unlocked record during a slow fetch, must stay locked). - Go: setup.go replaceable; failedOpen unlocks; Registry.open treats an ocifetch.SpellingError as misuse; OpenUnverified asks the new abi1.CheckUnverifiedAllowed first. - Python: _setup.py _replaceable; open_failed unlocks; Registry._open parses the request before the attempt; open_unverified checks opt-ins first. - TypeScript: setup.ts replaceable; settleFailedOpen unlocks; the spelling check is back before the settled opening; openUnverified asks the loader's exported checkUnverifiedAllowed first. - Rust: setup.rs replaceable and unlock_after_failed_open; Registry::open parses the spelling first; open_unverified checks opt-ins first. Section 6 rules 1, 2 and 4, the settings guide and the four [Unreleased] CHANGELOG entries say so. refs #468 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/guides/settings.md | 2 +- docs/reference/bindings-v1.md | 6 +- go/CHANGELOG.md | 2 +- go/chtypes/library.go | 7 +- go/chtypes/registry.go | 14 +++- go/chtypes/settings_test.go | 54 +++++++------ go/chtypes/setup.go | 85 ++++++++++++--------- go/internal/abi1/loader.go | 15 +++- python/CHANGELOG.md | 2 +- python/src/chtypes/_setup.py | 101 ++++++++++++++----------- python/src/chtypes/library.py | 10 ++- python/src/chtypes/registry.py | 21 ++--- python/tests/abi1/test_public_open.py | 35 +++++---- rust/CHANGELOG.md | 2 +- rust/src/library.rs | 35 +++++---- rust/src/registry.rs | 10 ++- rust/src/setup.rs | 91 ++++++++++++---------- rust/tests/api_v1_setup_failed_open.rs | 47 +++++++----- ts/CHANGELOG.md | 2 +- ts/src/abi1/index.ts | 2 +- ts/src/abi1/loader.ts | 11 ++- ts/src/library.ts | 6 +- ts/src/registry.ts | 5 +- ts/src/setup.ts | 44 ++++++----- ts/test/settings-setup.test.ts | 17 +++-- 25 files changed, 371 insertions(+), 255 deletions(-) diff --git a/docs/guides/settings.md b/docs/guides/settings.md index b3e1b69d..bb03113f 100644 --- a/docs/guides/settings.md +++ b/docs/guides/settings.md @@ -19,7 +19,7 @@ per-call settings > handle compile profile > setup defaults > ClickHouse's | setup defaults | `setup(defaults=…)` | the whole process, applied to every library when it opens | | ClickHouse defaults | the vendored build | everything | -**The setup defaults latch.** `setup` is called once, before the first library opens, and the defaults it records are applied to each library as it loads and never again. A second `setup` with the same zone and defaults is a no-op; a different one is a `UsageError` naming both, and the first stands. If `setup` was never called, the first open commits the empty setup: no defaults, and the library's default zone, `UTC`. The latch closes only when a library completes its setup. Until then, any open that fails clears the record, whatever failed: the fetch, the signature, an artifact that will not load, or the library refusing the zone or a default. A corrected `setup` is then accepted and the next open uses it. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry an open ([`reference/bindings-v1.md` §6](../reference/bindings-v1.md#6-setup-and-from-a-request-to-a-loaded-library)). So call `setup` first. **There is no runtime setter in any binding.** 0.x's `set_default_settings` (and Go's `SetDefaultSettings`, which existed only on the linked build) is deleted ([`reference/bindings-v1.md` §7](../reference/bindings-v1.md#7-what-v0-api-is-deleted-and-why)); per-call settings and the compile profile carry everything that changes after setup. All four bindings reach all four channels now, Go included. +**The setup defaults latch.** `setup` is called once, before the first library opens, and the defaults it records are applied to each library as it loads and never again. A second `setup` with the same zone and defaults is a no-op; a different one is a `UsageError` naming both, and the first stands. If `setup` was never called, the first open commits the empty setup: no defaults, and the library's default zone, `UTC`. The latch closes only when a library completes its setup. Until then, an open that attempts a load and fails keeps the setup but unlocks it, whatever failed: the fetch, the signature, an artifact that will not load, or the library refusing the zone or a default. A retry with no new `setup` uses the recorded zone and defaults, and a different `setup` is accepted and replaces them. A refused version spelling, or an unverified open without its opt-ins, fails before any load and unlocks nothing ([`reference/bindings-v1.md` §6](../reference/bindings-v1.md#6-setup-and-from-a-request-to-a-loaded-library)). So call `setup` first. **There is no runtime setter in any binding.** 0.x's `set_default_settings` (and Go's `SetDefaultSettings`, which existed only on the linked build) is deleted ([`reference/bindings-v1.md` §7](../reference/bindings-v1.md#7-what-v0-api-is-deleted-and-why)); per-call settings and the compile profile carry everything that changes after setup. All four bindings reach all four channels now, Go included.
Go diff --git a/docs/reference/bindings-v1.md b/docs/reference/bindings-v1.md index 11eafae7..e109dc91 100644 --- a/docs/reference/bindings-v1.md +++ b/docs/reference/bindings-v1.md @@ -427,10 +427,10 @@ A JSON object from handle kind (its type name, `chs_schema` and the rest) to the The image zone and the default settings are process-wide in ClickHouse: vendored MergeTree code reads the server zone directly (`measured` by the artifact producer on 26.7), and defaults are read by every call. So both are chosen once, before traffic, through `setup`, and every binding gives `setup` the same rule: -1. **`setup` records; it does not load.** Called before any library is open, it records the zone spelling and the defaults. Called again with the same zone spelling, byte for byte, and the same defaults, it is a no-op. Called with a different zone or different defaults while a setup is recorded, it is a `UsageError` naming both, and the recorded setup stands. So two components that disagree before any open is attempted get a refusal, never a silent last-wins. This is the library's own process-once rule for `chs_initialize`, applied by the binding while no image has been set up to ask. A record lasts until it latches (rule 2), or until a failed open clears it (rule 4). -2. **The first successful step 7 latches it.** If no setup is recorded when an open starts (`setup` was never called, or a failed open cleared the record), the open records the empty setup: the empty zone, which the library reads as `UTC`, and no defaults. The setup latches when the first image completes load step 7 successfully: `chs_initialize`, then `chs_set_defaults` when there are defaults. From then on `setup` succeeds only with exactly the setup in effect, and is otherwise a `UsageError` naming both. Call `setup` first. +1. **`setup` records; it does not load.** Called before any library is open, it records the zone spelling and the defaults. Called again with the same zone spelling, byte for byte, and the same defaults, it is a no-op. Called with a different zone or different defaults, it is a `UsageError` naming both, and the recorded setup stands, unless a failed open has unlocked the record (rule 4). So two components that disagree before any open is attempted get a refusal, never a silent last-wins. This is the library's own process-once rule for `chs_initialize`, applied by the binding while no image has been set up to ask. +2. **The first successful step 7 latches it.** If `setup` was never called, the first open records the empty setup: the empty zone, which the library reads as `UTC`, and no defaults. The setup latches when the first image completes load step 7 successfully: `chs_initialize`, then `chs_set_defaults` when there are defaults. From then on `setup` succeeds only with exactly the setup in effect, is otherwise a `UsageError` naming both, and nothing unlocks it again. Call `setup` first. 3. **Every image is set up at load step 7, once.** The loader calls `chs_initialize` with the recorded zone, as counted bytes (empty for the empty zone), and then, when there are defaults, `chs_set_defaults` with them, under the setup guard. Nothing calls either again for that image once step 7 has completed. -4. **What the library answers is what the caller gets, and a failed open can be corrected.** A zone `DateLUT` will not load is a `SchemaError` from `chs_initialize`, and a default the library refuses is the error `chs_set_defaults` answers. Either is raised by the open that hit step 7, and that open returns no library. **Until an image has completed step 7, any open that fails clears the setup record, whatever failed:** the version spelling, the fetch, the signature, loader steps 1 to 6 (an incompatible or corrupt artifact, a missing symbol), or step 7 itself. The binding clears the record under the setup guard, unless a setup was recorded after that open began. The caller may then call `setup` again with any setup, and the next open runs with it. An open with no `setup` after the failure records the empty setup (rule 2), so a caller that retries an open should call `setup` again first. A failed open is never remembered: the next open of the same request or image runs every step again. Once the setup has latched, a failure clears nothing. The library's own process-once rule stays the backstop: an image whose `chs_initialize` succeeded keeps its zone even if `chs_set_defaults` then failed, and a different spelling than the image already has (otherwise possible only across TypeScript isolates, or with a second binding loaded into one process) is `CHS_INVALID_ARGUMENT` from `chs_initialize`, a `UsageError` naming both, the same class rule 1 raises. +4. **What the library answers is what the caller gets, and a failed open can be corrected.** A zone `DateLUT` will not load is a `SchemaError` from `chs_initialize`, and a default the library refuses is the error `chs_set_defaults` answers. Either is raised by the open that hit step 7, and that open returns no library. **Until an image has completed step 7, an open that attempted a load and failed unlocks the setup record, whatever failed:** the resolve or the fetch, the signature, loader steps 1 to 6 (an incompatible or corrupt artifact, a missing symbol), or step 7 itself. The record is kept, so a retry with no new `setup` runs under the recorded zone and defaults, never under the empty setup. A different `setup` is accepted and replaces the record. A replaced record is locked again, and so is any record an open claims for its load. The binding unlocks the record under the setup guard, and never one that was recorded or replaced after the failed open began. The binding's own input checks fail before any load is attempted, and unlock nothing: a refused version spelling, or an unverified open without both opt-ins. A failed open is never remembered: the next open of the same request or image runs every step again. Once the setup has latched, a failure unlocks nothing. The library's own process-once rule stays the backstop: an image whose `chs_initialize` succeeded keeps its zone even if `chs_set_defaults` then failed, and a different spelling than the image already has (otherwise possible only across TypeScript isolates, or with a second binding loaded into one process) is `CHS_INVALID_ARGUMENT` from `chs_initialize`, a `UsageError` naming both, the same class rule 1 raises. 5. **Defaults never change during traffic.** There is no public setter, in any binding. If the regenerated description drops `chs_set_defaults` (the decision accepts "no setter at all"), the `defaults` field of `setup` is deleted with it, and per-call `settings` and the compile profile carry everything. The per-call zone needs no setup: it is the `session_timezone` key of a call's settings (§2, the call options). diff --git a/go/CHANGELOG.md b/go/CHANGELOG.md index 6cec7a37..6422c1c0 100644 --- a/go/CHANGELOG.md +++ b/go/CHANGELOG.md @@ -8,7 +8,7 @@ The four bindings in this repository are released together and give one answer, ### Fixed -- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `Setup(SetupOptions{Timezone: "Asia/Tokyo"})`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `Setup` was then a `*UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry.For`, `OpenUnverified` and `OpenLinked`), so the next `Setup` is accepted, whatever it is. An open with no `Setup` after the failure uses the empty setup, so call `Setup` again before you retry. Two things are unchanged. A second, different `Setup` before any open is attempted is still a `*UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `*UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. +- An open that attempted a load and failed now unlocks the setup, whatever failed, while no library has completed load step 7 (#468). After `Setup(SetupOptions{Timezone: "Asia/Tokyo"})`, an open could fail before step 7: at the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol. That left the setup locked, and a different `Setup` was a `*UsageError` until the process restarted. 1.0.2 handled only a failure in step 7 itself (#458). Now any failed attempt keeps the recorded setup but makes it replaceable. A plain retry with no new `Setup` runs under the recorded zone and defaults, never the empty setup. A different `Setup` is accepted and replaces the record, which is then locked again. This holds for `Registry.For`, `OpenUnverified` and `OpenLinked`. The caller's own misuse fails before any load is attempted and unlocks nothing: a refused version spelling, or `OpenUnverified(path, false)`. Two different `Setup` calls before any open is attempted are still a `*UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `*UsageError`. No name or signature changes. All four bindings run the same cases, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. ## [1.0.2] — 2026-10-06 diff --git a/go/chtypes/library.go b/go/chtypes/library.go index 2c1b67ea..29fa3ad5 100644 --- a/go/chtypes/library.go +++ b/go/chtypes/library.go @@ -50,7 +50,7 @@ func imageKey(kind, path string) string { // the record it loaded under, and Setup cannot change that record in between // (it refuses a different setup while one is recorded). A failed load is never // cached, so the next open runs every step again; the open that called this -// settles a failure with failedOpen, whatever failed. +// settles a failure with failedOpen, whatever failed in the attempt. func openImage(key string, load func(zone, defaults []byte) (*abi1.Table, error), path string, resolved *Resolved) (*Library, error) { images.mu.Lock() defer images.mu.Unlock() @@ -90,6 +90,11 @@ func openImage(key string, load func(zone, defaults []byte) (*abi1.Table, error) // step 7 under the process setup like any open, is not reachable through a // Registry, and its Library has no Resolved record. func OpenUnverified(path string, allow bool) (l *Library, err error) { + // The caller's two opt-ins are checked before anything is attempted, so a + // refusal here is misuse and unlocks nothing (bindings-v1.md section 6). + if err := abi1.CheckUnverifiedAllowed(path, allow); err != nil { + return nil, loadError(err) + } gen := setupGeneration() defer func() { if err != nil { diff --git a/go/chtypes/registry.go b/go/chtypes/registry.go index 0159afd6..8f19a06a 100644 --- a/go/chtypes/registry.go +++ b/go/chtypes/registry.go @@ -8,6 +8,7 @@ package chtypes import ( "context" + "errors" "fmt" "os" "runtime" @@ -131,12 +132,15 @@ func (r *Registry) open(ctx context.Context, request string, mayFetch bool) (_ * if l := r.memo[request]; l != nil { return l, nil } - // A failed open clears the setup record while no image has completed load - // step 7, whatever failed: the spelling, the fetch, the signature or any - // load step (bindings-v1.md section 6, rule 4). + // An open that attempted a load and failed unlocks the setup record while + // no image has completed load step 7, whatever failed: the resolve, the + // fetch, the signature or any load step. A refused version spelling is the + // caller's own misuse, refused before anything is attempted, and unlocks + // nothing (bindings-v1.md section 6, rule 4). gen := setupGeneration() + misuse := false defer func() { - if err != nil { + if err != nil && !misuse { failedOpen(gen) } }() @@ -144,6 +148,8 @@ func (r *Registry) open(ctx context.Context, request string, mayFetch bool) (_ * req := ocifetch.Request{Spelling: request} res, err := ocifetch.ResolveInstalled(req, "", opts) if err != nil { + var spelling *ocifetch.SpellingError + misuse = errors.As(err, &spelling) return nil, fetchError(err) } if res == nil { diff --git a/go/chtypes/settings_test.go b/go/chtypes/settings_test.go index 473c0830..295f262a 100644 --- a/go/chtypes/settings_test.go +++ b/go/chtypes/settings_test.go @@ -148,10 +148,12 @@ func TestFirstOpenCommitsTheEmptySetup(t *testing.T) { } } -// TestAFailedFetchBeforeStep7ClearsTheSetup: a registry open that fails before -// any load (nothing installed, autofetch off) clears the setup record while no -// image has completed step 7, so a different setup is accepted afterwards. -func TestAFailedFetchBeforeStep7ClearsTheSetup(t *testing.T) { +// TestAFailedFetchBeforeStep7UnlocksTheSetup: a registry open that attempted +// to resolve and failed (nothing installed, autofetch off) unlocks the setup +// record while no image has completed step 7, so a different setup replaces it. +// A refused version spelling is misuse, refused before anything is attempted, +// and unlocks nothing. +func TestAFailedFetchBeforeStep7UnlocksTheSetup(t *testing.T) { resetSetup(t) if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { t.Fatal(err) @@ -160,30 +162,38 @@ func TestAFailedFetchBeforeStep7ClearsTheSetup(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := reg.For("26.8"); !errors.Is(err, ErrArtifactMissing) { - t.Fatalf("For on an empty cache = %v, want ErrArtifactMissing", err) - } - if err := Setup(SetupOptions{Timezone: "UTC"}); err != nil { - t.Errorf("a different setup after a failed open = %v, want it accepted", err) - } - // A refused version spelling is a failed open too. var ue *UsageError + // A refused spelling first: it unlocks nothing. if _, err := reg.For("v26.8"); !errors.As(err, &ue) { t.Fatalf("For(v26.8) = %v, want a *UsageError", err) } - if err := Setup(SetupOptions{Timezone: "Europe/Berlin"}); err != nil { - t.Errorf("a different setup after a refused spelling = %v, want it accepted", err) + if err := Setup(SetupOptions{Timezone: "UTC"}); !errors.As(err, &ue) { + t.Errorf("a different setup after a refused spelling = %v, want a *UsageError", err) + } + if _, err := reg.For("26.8"); !errors.Is(err, ErrArtifactMissing) { + t.Fatalf("For on an empty cache = %v, want ErrArtifactMissing", err) + } + // The failed open kept the record (a retry runs under it, never the empty + // setup) and made it replaceable. + setup.mu.Lock() + kept, replaceable := setup.recorded && setup.opts.Timezone == "Asia/Tokyo", setup.replaceable + setup.mu.Unlock() + if !kept || !replaceable { + t.Errorf("after a failed open: record kept = %v, replaceable = %v; want both", kept, replaceable) + } + if err := Setup(SetupOptions{Timezone: "UTC"}); err != nil { + t.Errorf("a different setup after a failed open = %v, want it to replace the record", err) } - // Before any open is attempted, a second, different setup is still misuse. - if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); !errors.As(err, &ue) { - t.Errorf("a second, different setup before any open = %v, want a *UsageError", err) + // The replaced record is locked again. + if err := Setup(SetupOptions{Timezone: "Europe/Berlin"}); !errors.As(err, &ue) { + t.Errorf("a second, different setup after the replacement = %v, want a *UsageError", err) } } -// TestAFailedOpenClearsOnlyItsOwnRecordAndNeverALatchedOne: a failed open never -// clears a setup recorded after it began, and once an image has completed step -// 7 a failure clears nothing. -func TestAFailedOpenClearsOnlyItsOwnRecordAndNeverALatchedOne(t *testing.T) { +// TestAFailedOpenUnlocksOnlyItsOwnRecordAndNeverALatchedOne: a failed open +// never unlocks a setup recorded after it began, and once an image has +// completed step 7 a failure unlocks nothing. +func TestAFailedOpenUnlocksOnlyItsOwnRecordAndNeverALatchedOne(t *testing.T) { resetSetup(t) began := setupGeneration() if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { @@ -191,12 +201,12 @@ func TestAFailedOpenClearsOnlyItsOwnRecordAndNeverALatchedOne(t *testing.T) { } failedOpen(began) // began before Asia/Tokyo was recorded if err := Setup(SetupOptions{Timezone: "UTC"}); err == nil { - t.Error("a failed open cleared a setup recorded after it began") + t.Error("a failed open unlocked a setup recorded after it began") } latchSetup() failedOpen(setupGeneration()) if err := Setup(SetupOptions{Timezone: "UTC"}); err == nil { - t.Error("a failed open cleared a latched setup") + t.Error("a failed open unlocked a latched setup") } if err := Setup(SetupOptions{Timezone: "Asia/Tokyo"}); err != nil { t.Errorf("the latched setup again = %v, want a no-op", err) diff --git a/go/chtypes/setup.go b/go/chtypes/setup.go index 645497fa..f095ef24 100644 --- a/go/chtypes/setup.go +++ b/go/chtypes/setup.go @@ -3,8 +3,9 @@ package chtypes // setup.go — the process setup (bindings-v1.md section 6). The image zone and // the default settings are process-wide in ClickHouse, so both are chosen once, // before traffic: Setup records them, and every image gets them at load step 7. -// The record latches once an image completes step 7; before that, any failed -// open clears it, so a corrected setup can be recorded. +// The record latches once an image completes step 7; before that, an open +// that attempted a load and failed unlocks it: the record stays, and a +// different Setup may replace it. import ( "reflect" @@ -21,18 +22,22 @@ type SetupOptions struct { Defaults map[string]string } -// setup is the setup guard: the record, and whether any image has completed -// load step 7 under it. An open commits the record, loads and latches under -// images.mu, and a failed open clears the record under images.mu too, so a -// clear never lands between another open's commit and its latch. +// setup is the setup guard: the record, whether any image has completed load +// step 7 under it, and whether a failed open has made it replaceable. An open +// commits the record (which locks it again), loads and latches under images.mu, +// and a failed open unlocks the record under images.mu too, so an unlock never +// lands between another open's commit and its latch. var setup struct { mu sync.Mutex recorded bool latched bool // an image completed load step 7 under the record - // gen counts the records Setup made and the records failed opens cleared. - // An open reads it when it begins, and clears the record on failure only - // if it is unchanged: a failed open never clears a setup recorded after it - // began. + // replaceable: an open that attempted a load failed since the record was + // last set or committed, and nothing has latched, so a different Setup + // replaces the record instead of being refused. + replaceable bool + // gen counts the records Setup made or replaced. An open reads it when it + // begins, and unlocks the record on failure only if it is unchanged: a + // failed open never unlocks a setup recorded after it began. gen uint64 opts SetupOptions } @@ -70,32 +75,39 @@ func quoteForMessage(s string) string { // first setup stands. The first open records the empty setup if Setup was // never called. The setup latches once an image completes load step 7 // (chs_initialize, then chs_set_defaults when there are defaults); from then -// on Setup succeeds only with exactly the setup in effect. Until then, any -// open that fails, whatever failed (the fetch, the signature, an incompatible -// artifact, a missing symbol or step 7), clears the record: the next Setup is -// accepted, and an open with no Setup after the failure records the empty -// setup. Call it first. +// on Setup succeeds only with exactly the setup in effect. Until then, an open +// that attempted a load and failed, whatever failed (the fetch, the signature, +// an incompatible artifact, a missing symbol or step 7), keeps the record but +// makes it replaceable: a retry with no new Setup runs under the recorded +// setup, and a different Setup replaces it. A refused version spelling or an +// unverified open without the caller's opt-in fails before any load is +// attempted, and unlocks nothing. Call it first. func Setup(opts SetupOptions) error { setup.mu.Lock() defer setup.mu.Unlock() - if !setup.recorded { - setup.recorded = true - setup.gen++ - setup.opts = SetupOptions{Timezone: opts.Timezone, Defaults: copyMap(opts.Defaults)} + switch { + case !setup.recorded: + case sameSetup(setup.opts, opts): return nil + case setup.replaceable && !setup.latched: + default: + return usageError("setup is already %s; refusing a different setup, %s", describeSetup(setup.opts), describeSetup(opts)) } - if sameSetup(setup.opts, opts) { - return nil - } - return usageError("setup is already %s; refusing a different setup, %s", describeSetup(setup.opts), describeSetup(opts)) + // A first record, or a replacement after a failed open: either way the + // new record is locked until the next failed open. + setup.recorded, setup.replaceable = true, false + setup.gen++ + setup.opts = SetupOptions{Timezone: opts.Timezone, Defaults: copyMap(opts.Defaults)} + return nil } // commitSetup returns the setup every image is loaded under, recording the -// empty setup when Setup was never called. +// empty setup when Setup was never called. The load that follows claims the +// record, so it is locked again: no Setup replaces it under a load. func commitSetup() (zone []byte, defaults []byte, err error) { setup.mu.Lock() defer setup.mu.Unlock() - setup.recorded = true + setup.recorded, setup.replaceable = true, false defaults, err = stringMapJSON(setup.opts.Defaults) if err != nil { return nil, nil, err @@ -103,7 +115,7 @@ func commitSetup() (zone []byte, defaults []byte, err error) { return []byte(setup.opts.Timezone), defaults, nil } -// setupGeneration is read when an open begins, for failedOpen. +// setupGeneration is read when an open's attempt begins, for failedOpen. func setupGeneration() uint64 { setup.mu.Lock() defer setup.mu.Unlock() @@ -115,25 +127,26 @@ func setupGeneration() uint64 { func latchSetup() { setup.mu.Lock() defer setup.mu.Unlock() - setup.latched = true + setup.latched, setup.replaceable = true, false } -// failedOpen settles an open that failed, whatever failed. While no image has -// completed step 7 it clears the record, so Setup accepts a corrected setup, -// unless a setup was recorded after the open began (gen moved). Once the setup -// has latched it changes nothing: the library's own process-once rule answers -// a different zone on an image that already has one. It takes images.mu, so it -// never lands between another open's commit and its latch. +// failedOpen settles an open that attempted a load and failed, whatever +// failed. While no image has completed step 7 it unlocks the record: the record +// stays, so a retry runs under it, and a different Setup may replace it. It +// leaves alone a setup recorded or replaced after the open began (gen moved), +// and once the setup has latched it changes nothing: the library's own +// process-once rule answers a different zone on an image that already has one. +// It takes images.mu, so it never lands between another open's commit and its +// latch. func failedOpen(gen uint64) { images.mu.Lock() defer images.mu.Unlock() setup.mu.Lock() defer setup.mu.Unlock() - if setup.latched || setup.gen != gen { + if setup.latched || !setup.recorded || setup.gen != gen { return } - setup.recorded, setup.opts = false, SetupOptions{} - setup.gen++ + setup.replaceable = true } func copyMap(m map[string]string) map[string]string { diff --git a/go/internal/abi1/loader.go b/go/internal/abi1/loader.go index 4cbbb58d..f684101e 100644 --- a/go/internal/abi1/loader.go +++ b/go/internal/abi1/loader.go @@ -138,6 +138,17 @@ func (e *UnverifiedRefusedError) Error() string { return fmt.Sprintf("abi1: OpenUnverified(%s) refused: pass explicit=true AND set %s=1", e.Path, unverifiedEnv) } +// CheckUnverifiedAllowed is OpenUnverified's own gate, alone: nil only when +// explicit is true AND CHTYPES_ALLOW_UNVERIFIED_LIBRARY=1 is set, otherwise an +// *UnverifiedRefusedError. The public layer asks it before an open is +// attempted, so the caller's misuse is told apart from a failed load. +func CheckUnverifiedAllowed(path string, explicit bool) error { + if !explicit || os.Getenv(unverifiedEnv) != "1" { + return &UnverifiedRefusedError{Path: path} + } + return nil +} + // OpenUnverified loads a library with NO predicate verification: steps 1 and // 5 are skipped (plan §3.1). It is for core's own local builds and the // linked-mode smoke path, and refuses unless BOTH explicit=true AND @@ -146,8 +157,8 @@ func (e *UnverifiedRefusedError) Error() string { // this but a caller that asked for it by name. It runs step 7 like any open, // under the timezone and defaults given. func OpenUnverified(path string, explicit bool, timezone, defaults []byte) (*Table, error) { - if !explicit || os.Getenv(unverifiedEnv) != "1" { - return nil, &UnverifiedRefusedError{Path: path} + if err := CheckUnverifiedAllowed(path, explicit); err != nil { + return nil, err } warnUnverifiedOnce(path) return load(LoadInput{LibraryPath: path, Predicate: nil, Timezone: timezone, Defaults: defaults}) diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md index 2027fe41..60e0422b 100644 --- a/python/CHANGELOG.md +++ b/python/CHANGELOG.md @@ -8,7 +8,7 @@ The four bindings in this repository are released together and give one answer, ### Fixed -- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup(timezone="Asia/Tokyo")`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then a `UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry.for_version`, `Registry(preload=…)` and `open_unverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. +- An open that attempted a load and failed now unlocks the setup, whatever failed, while no library has completed load step 7 (#468). After `setup(timezone="Asia/Tokyo")`, an open could fail before step 7: at the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol. That left the setup locked, and a different `setup` was a `UsageError` until the process restarted. 1.0.2 handled only a failure in step 7 itself (#458). Now any failed attempt keeps the recorded setup but makes it replaceable. A plain retry with no new `setup` runs under the recorded zone and defaults, never the empty setup. A different `setup` is accepted and replaces the record, which is then locked again. This holds for `Registry.for_version`, `Registry(preload=…)` and `open_unverified`. The caller's own misuse fails before any load is attempted and unlocks nothing: a refused version spelling, or `open_unverified(path)` without `allow=True`. Two different `setup` calls before any open is attempted are still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same cases, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. ## [1.0.2] — 2026-10-06 diff --git a/python/src/chtypes/_setup.py b/python/src/chtypes/_setup.py index 5cdd3749..5bec280f 100644 --- a/python/src/chtypes/_setup.py +++ b/python/src/chtypes/_setup.py @@ -5,10 +5,11 @@ `setup` was never called. The record latches once an image completes loader step 7 (`chs_initialize`, then `chs_set_defaults` when there are defaults), and from then on `setup` succeeds only with exactly the setup in effect. Until then, -any open that fails, whatever failed, clears the record (`open_failed`), so a -corrected setup can be recorded. Every image is set up once, at loader step 7, -by `library.open_image` and `open_unverified`, under this module's lock: the -setup guard, which every load, latch and clear holds. +an open that attempted a load and failed unlocks the record (`open_failed`): it +stays, so a retry runs under it, and a different setup may replace it. Every +image is set up once, at loader step 7, by `library.open_image` and +`open_unverified`, under this module's lock: the setup guard, which every load, +latch and unlock holds. """ from __future__ import annotations @@ -42,36 +43,36 @@ def defaults_json(self) -> bytes: _state: SetupState | None = None # Whether any image has completed loader step 7 under the record. _latched = False -# Counts the records `setup` made and the records failed opens cleared. An open -# reads it when it begins, and clears the record on failure only if it is -# unchanged: a failed open never clears a setup recorded after it began. +# Whether an open that attempted a load failed since the record was last set or +# committed, with nothing latched: a different `setup` then replaces the record +# instead of being refused. +_replaceable = False +# Counts the records `setup` made or replaced. An open reads it when its attempt +# begins, and unlocks the record on failure only if it is unchanged: a failed +# open never unlocks a setup recorded after it began. _generation = 0 -def _record(state: SetupState, *, by_setup: bool = False) -> SetupState: - """Record `state`, or confirm it equals the one in effect. Holds LOCK.""" - global _state, _generation - if _state is None: - _state = state - if by_setup: - _generation += 1 - return state - if _state != state: - raise misuse( - f"setup is already in effect ({_state.describe()}) and cannot be changed to " - f"({state.describe()}); call setup() once, before the first open" - ) - return _state +def _misuse(current: SetupState, wanted: SetupState) -> Exception: + return misuse( + f"setup is already in effect ({current.describe()}) and cannot be changed to " + f"({wanted.describe()}); call setup() once, before the first open" + ) def effective() -> SetupState: """The setup in effect, recording the empty setup if none was recorded. - Called by an open, with LOCK held.""" - return _record(_state if _state is not None else SetupState("", {})) + Called by an open, with LOCK held. The load that follows claims the record, + so it is locked again: no `setup` replaces it under a load.""" + global _state, _replaceable + if _state is None: + _state = SetupState("", {}) + _replaceable = False + return _state def generation() -> int: - """Read when an open begins, for `open_failed`.""" + """Read when an open's attempt begins, for `open_failed`.""" with LOCK: return _generation @@ -79,25 +80,25 @@ def generation() -> int: def latch() -> None: """An image completed loader step 7: from then on the setup in effect stands. Called by an open with LOCK held, right after the load.""" - global _latched + global _latched, _replaceable with LOCK: - _latched = True + _latched, _replaceable = True, False def open_failed(began: int) -> None: - """Settle an open that failed, whatever failed. While no image has completed - step 7 it clears the record, so `setup` accepts a corrected setup, unless a - setup was recorded after the open began (`began` is `generation()` then). - Once the setup has latched it changes nothing: the library's own - process-once rule answers a different zone on an image that already has one. - LOCK serializes it with every load, so it never lands between another - open's commit and its latch.""" - global _state, _generation + """Settle an open that attempted a load and failed, whatever failed. While + no image has completed step 7 it unlocks the record: the record stays, so a + retry runs under it, and a different `setup` may replace it. It leaves alone + a setup recorded or replaced after the open began (`began` is + `generation()` then), and once the setup has latched it changes nothing: + the library's own process-once rule answers a different zone on an image + that already has one. LOCK serializes it with every load, so it never lands + between another open's commit and its latch.""" + global _replaceable with LOCK: - if _latched or _generation != began: + if _latched or _state is None or _generation != began: return - _state = None - _generation += 1 + _replaceable = True def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> None: @@ -113,11 +114,13 @@ def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> N defaults, this is a no-op. Called with a different zone or different defaults it is a `UsageError` naming both, and the first setup stands. If it is never called, the first open records the empty setup. The setup latches - once an image completes loader step 7. Until then, any open that fails, - whatever failed (the fetch, the signature, an incompatible artifact, a - missing symbol or step 7), clears the record: the next `setup` is accepted, - and an open with no `setup` after the failure records the empty setup. Call - it first. + once an image completes loader step 7. Until then, an open that attempted a + load and failed, whatever failed (the fetch, the signature, an incompatible + artifact, a missing symbol or step 7), keeps the record but makes it + replaceable: a retry with no new `setup` runs under the recorded setup, and + a different `setup` replaces it. A refused version spelling or an unverified + open without the caller's opt-in fails before any load is attempted, and + unlocks nothing. Call it first. """ if timezone is not None and not isinstance(timezone, str): raise TypeError(f"timezone must be str, not {type(timezone).__name__}") @@ -125,14 +128,24 @@ def setup(*, timezone: str | None = None, defaults: Settings | None = None) -> N if defaults: string_map_json(defaults, "defaults") # the type check, once mapping = dict(defaults) + wanted = SetupState(timezone or "", mapping) + global _state, _replaceable, _generation with LOCK: - _record(SetupState(timezone or "", mapping), by_setup=True) + if _state == wanted: + return + if _state is not None and (_latched or not _replaceable): + raise _misuse(_state, wanted) + # A first record, or a replacement after a failed open: either way the + # new record is locked until the next failed open. + _state, _replaceable = wanted, False + _generation += 1 def _reset_for_tests() -> None: """Forget the recorded setup and the latch. Test-only: images already loaded keep theirs.""" - global _state, _latched + global _state, _latched, _replaceable with LOCK: _state = None _latched = False + _replaceable = False diff --git a/python/src/chtypes/library.py b/python/src/chtypes/library.py index 70fe5d0a..7aa18e36 100644 --- a/python/src/chtypes/library.py +++ b/python/src/chtypes/library.py @@ -59,7 +59,7 @@ def open_image(path: str, predicate: object, resolved: object | None) -> Library A new image runs loader steps 1 to 7 once, under the setup in effect, and latches the setup when step 7 completes (`_setup.latch`). A failed load is never cached, so the next open of the same image runs every step again; the - caller settles a failure with `_setup.open_failed`, whatever failed. An + caller settles a failed attempt with `_setup.open_failed`. An image already open is still checked against this signed statement (steps 1, 4 and 5): a mismatch refuses this request and leaves the image open for the requests it did match. @@ -89,12 +89,14 @@ def open_unverified(path: str | os.PathLike[str], *, allow: bool = False) -> Lib `allow` is passed AND `CHTYPES_ALLOW_UNVERIFIED_LIBRARY=1` is set, warns once per path, and skips loader steps 1 and 5. It runs step 7 under the process setup like any open. It is not reachable through a registry, and its - `Library` has no `resolved`. Like any open, a failure clears the setup - record while no image has completed step 7 (`_setup.open_failed`).""" + `Library` has no `resolved`. Without both opt-ins it is misuse, refused + before anything is attempted, and unlocks nothing; like any open, a failed + attempt unlocks the setup record while no image has completed step 7 + (`_setup.open_failed`).""" spelled = os.fspath(path) + _loader.check_unverified_allowed(spelled, allow) began = _setup.generation() try: - _loader.check_unverified_allowed(spelled, allow) with _setup.LOCK: state = _setup.effective() existing = _IMAGES.get(_image_key(spelled)) diff --git a/python/src/chtypes/registry.py b/python/src/chtypes/registry.py index 4cc95d41..7494ffb0 100644 --- a/python/src/chtypes/registry.py +++ b/python/src/chtypes/registry.py @@ -170,23 +170,26 @@ def _open(self, request: str, *, allow_fetch: bool) -> Library: hit = self._memo.get(request) if hit is not None: return hit - # A failed open clears the setup record while no image has completed - # load step 7, whatever failed: the spelling, the fetch, the - # signature or any load step (bindings-v1.md section 6, rule 4). + # A refused version spelling is the caller's own misuse, refused + # before anything is attempted, and unlocks nothing. An open that + # attempted a load and failed unlocks the setup record while no + # image has completed load step 7, whatever failed: the resolve, + # the fetch, the signature or any load step (bindings-v1.md + # section 6, rule 4). + try: + fetch_request = Request(request) + except ValueError as exc: + raise errors.misuse(str(exc)) from None began = _setup.generation() try: - library = self._resolve_and_open(request, allow_fetch) + library = self._resolve_and_open(request, fetch_request, allow_fetch) except Exception: _setup.open_failed(began) raise self._memo[request] = library return library - def _resolve_and_open(self, request: str, allow_fetch: bool) -> Library: - try: - fetch_request = Request(request) - except ValueError as exc: - raise errors.misuse(str(exc)) from None + def _resolve_and_open(self, request: str, fetch_request: Request, allow_fetch: bool) -> Library: try: platform = detect_host_platform() except ValueError as exc: diff --git a/python/tests/abi1/test_public_open.py b/python/tests/abi1/test_public_open.py index e99b32f9..f952de56 100644 --- a/python/tests/abi1/test_public_open.py +++ b/python/tests/abi1/test_public_open.py @@ -158,14 +158,14 @@ def test_open_unverified_needs_both_opt_ins_and_has_no_resolved(stub_copy, monke _setup._reset_for_tests() -def test_a_defaults_failure_at_step_7_clears_the_record_too( +def test_a_defaults_failure_at_step_7_unlocks_the_record_too( stub_copy, monkeypatch, clean_process ) -> None: - # The shared setup case (test_public_setup_cases.py) covers a refused load + # The shared setup cases (test_public_setup_cases.py) cover a refused load # and a zone the library refuses; the stub cannot refuse a default, so this # forces the chs_set_defaults half of step 7 to fail. The rule is the same: - # no image has completed step 7, so the failed open clears the record, a - # corrected setup is accepted, and the open that follows runs step 7 again. + # no image has completed step 7, so the failed open unlocks the record, a + # corrected setup replaces it, and the open that follows runs step 7 again. path, _ = stub_copy() monkeypatch.setenv("CHTYPES_ALLOW_UNVERIFIED_LIBRARY", "1") @@ -177,33 +177,38 @@ def refuse(self, d): with pytest.raises(SchemaError) as info, pytest.warns(UserWarning, match="UNVERIFIED"): open_unverified(path, allow=True) assert info.value.ch_code == 115 - setup(timezone="Asia/Tokyo") # the corrected setup: no longer refused + setup(timezone="Asia/Tokyo") # the corrected setup replaces the unlocked record first = open_unverified(path, allow=True) with pytest.raises(UsageError): setup(timezone="Asia/Tokyo", defaults={"no_such_setting": "1"}) # now latched assert open_unverified(path, allow=True) is first -def test_a_failed_fetch_before_step_7_clears_the_record(tmp_path, clean_process) -> None: - # A registry open that fails before any load (nothing installed, autofetch - # off) is a failed open like any other: no image has completed step 7, so - # the record is cleared and a different setup is accepted afterwards. +def test_a_failed_fetch_unlocks_the_record_and_a_refused_spelling_does_not( + tmp_path, clean_process +) -> None: + # A registry open that attempted to resolve and failed (nothing installed, + # autofetch off) unlocks the record: a different setup replaces it. A refused + # version spelling is misuse, refused before anything is attempted, and + # unlocks nothing. setup(timezone="Asia/Tokyo") registry = Registry( fetch=FetchOptions(cache_dir=tmp_path / "cache", system_dirs=[], offline=True), autofetch=False, ) + with pytest.raises(UsageError): + registry.for_version("v26.8") + with pytest.raises(UsageError): + setup(timezone="UTC") # the refused spelling unlocked nothing with pytest.raises(ArtifactMissingError): registry.for_version("26.8") - setup(timezone="UTC") # accepted: the failed open cleared the record - with pytest.raises(UsageError): - registry.for_version("v26.8") # a refused spelling is a failed open too - setup(timezone="Europe/Berlin") # accepted + assert _setup._state is not None and _setup._state.zone == "Asia/Tokyo" # kept + setup(timezone="UTC") # replaces the unlocked record with pytest.raises(UsageError): - setup(timezone="Asia/Tokyo") # before any open is attempted: the first stands + setup(timezone="Europe/Berlin") # the replacement is locked again -def test_a_failed_open_never_clears_a_later_or_a_latched_setup(clean_process) -> None: +def test_a_failed_open_never_unlocks_a_later_or_a_latched_setup(clean_process) -> None: began = _setup.generation() setup(timezone="Asia/Tokyo") _setup.open_failed(began) # it began before Asia/Tokyo was recorded diff --git a/rust/CHANGELOG.md b/rust/CHANGELOG.md index d2e57d7c..e48037c7 100644 --- a/rust/CHANGELOG.md +++ b/rust/CHANGELOG.md @@ -8,7 +8,7 @@ The four bindings in this repository are released together and give one answer, ### Fixed -- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup(SetupOptions { timezone: Some("Asia/Tokyo".into()), ..Default::default() })`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then an `Error::Usage` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`Registry::for_version`, `RegistryOptions::preload` and `Library::open_unverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still an `Error::Usage`, never a silent last-wins. Once a library has completed step 7, a different setup is still an `Error::Usage`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. +- An open that attempted a load and failed now unlocks the setup, whatever failed, while no library has completed load step 7 (#468). After `setup(SetupOptions { timezone: Some("Asia/Tokyo".into()), ..Default::default() })`, an open could fail before step 7: at the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol. That left the setup locked, and a different `setup` was an `Error::Usage` until the process restarted. 1.0.2 handled only a failure in step 7 itself (#458). Now any failed attempt keeps the recorded setup but makes it replaceable. A plain retry with no new `setup` runs under the recorded zone and defaults, never the empty setup. A different `setup` is accepted and replaces the record, which is then locked again. This holds for `Registry::for_version`, `RegistryOptions::preload` and `Library::open_unverified`. The caller's own misuse fails before any load is attempted and unlocks nothing: a refused version spelling, or `Library::open_unverified(path, false)`. Two different `setup` calls before any open is attempted are still an `Error::Usage`, never a silent last-wins. Once a library has completed step 7, a different setup is still an `Error::Usage`. No name or signature changes. All four bindings run the same cases, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. ## [1.0.2] — 2026-10-06 diff --git a/rust/src/library.rs b/rust/src/library.rs index e716f65c..ab6b7402 100644 --- a/rust/src/library.rs +++ b/rust/src/library.rs @@ -69,13 +69,13 @@ fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } -/// Settle an open that failed, whatever failed (`crate::setup`'s rule): under -/// the image list's lock, so the clear never lands between another open's -/// commit and its latch. `began` is `setup::generation()` read when the open -/// began. +/// Settle an open that attempted a load and failed, whatever failed +/// (`crate::setup`'s rule): under the image list's lock, so the unlock never +/// lands between another open's commit and its latch. `began` is +/// `setup::generation()` read when the attempt began. pub(crate) fn settle_failed_open(began: u64) { let _images = lock(&IMAGES); - setup::clear_after_failed_open(began); + setup::unlock_after_failed_open(began); } /// Open the image at `path`, or return the one this process already opened. @@ -154,17 +154,9 @@ impl Library { /// It is not reachable through a [`crate::Registry`], and the library it /// returns has no [`Library::resolved`]. pub fn open_unverified(path: impl AsRef, allow: bool) -> Result> { - // Like any open: a failure clears the setup record while no image has - // completed load step 7, whatever failed. - let began = setup::generation(); - let opened = Self::open_unverified_once(path.as_ref(), allow); - if opened.is_err() { - settle_failed_open(began); - } - opened - } - - fn open_unverified_once(path: &Path, allow: bool) -> Result> { + let path = path.as_ref(); + // The caller's two opt-ins are checked before anything is attempted: + // misuse, which unlocks nothing. let env_on = std::env::var(UNVERIFIED_ENV).is_ok_and(|v| v == "1"); if !(allow && env_on) { return Err(Error::usage(format!( @@ -172,6 +164,17 @@ impl Library { if env_on { "set" } else { "not set" } ))); } + // Like any open: a failed attempt unlocks the setup record while no + // image has completed load step 7, whatever failed. + let began = setup::generation(); + let opened = Self::open_unverified_once(path); + if opened.is_err() { + settle_failed_open(began); + } + opened + } + + fn open_unverified_once(path: &Path) -> Result> { { let mut warned = lock(&WARNED); if warned diff --git a/rust/src/registry.rs b/rust/src/registry.rs index de4b489a..53af3532 100644 --- a/rust/src/registry.rs +++ b/rust/src/registry.rs @@ -29,6 +29,7 @@ use crate::error::{Error, Result}; use crate::library::{Library, open_image, settle_failed_open}; use crate::ocifetch::constants::ENV_AUTOFETCH_NAME; use crate::ocifetch::ensure::{self, Options, Resolved}; +use crate::ocifetch::oci::VersionRequest; use crate::setup; /// What the fetch layer is configured with, under its own names @@ -193,9 +194,12 @@ impl Registry { if let Some(o) = opened.iter().find(|o| o.request == request) { return Ok(Arc::clone(&o.library)); } - // A failed open clears the setup record while no image has completed - // load step 7, whatever failed: the spelling, the fetch, the signature - // or any load step (bindings-v1.md section 6, rule 4). + // A refused version spelling is the caller's own misuse, refused before + // anything is attempted, and unlocks nothing. An open that attempted a + // load and failed unlocks the setup record while no image has + // completed load step 7, whatever failed: the resolve, the fetch, the + // signature or any load step (bindings-v1.md section 6, rule 4). + VersionRequest::parse(request)?; let began = setup::generation(); let library = match self.resolve_and_open(request, may_fetch) { Ok(library) => library, diff --git a/rust/src/setup.rs b/rust/src/setup.rs index a6fd2630..c708d491 100644 --- a/rust/src/setup.rs +++ b/rust/src/setup.rs @@ -15,11 +15,13 @@ //! the library reads as `UTC`, and no defaults. Once an image completes //! step 7 (`chs_initialize`, then `chs_set_defaults` when there are //! defaults), [`setup`] succeeds only with exactly the setup in effect. -//! Until then, any open that fails, whatever failed (the fetch, the -//! signature, an incompatible artifact, a missing symbol or step 7), -//! clears the record: the next [`setup`] is accepted, and an open with no -//! [`setup`] after the failure records the empty setup. Call [`setup`] -//! first. +//! Until then, an open that attempted a load and failed, whatever failed +//! (the fetch, the signature, an incompatible artifact, a missing symbol +//! or step 7), keeps the record but makes it replaceable: a retry with no +//! new [`setup`] runs under the recorded setup, and a different [`setup`] +//! replaces it. A refused version spelling or an unverified open without +//! the caller's opt-in fails before any load is attempted, and unlocks +//! nothing. Call [`setup`] first. //! 3. There is no public setter for defaults during traffic. use std::sync::Mutex; @@ -48,24 +50,29 @@ pub(crate) struct Recorded { pub(crate) defaults: Option>, } -/// The setup guard: the record, and whether any image has completed load step -/// 7 under it. An open commits, loads and latches under the image list's lock -/// (`crate::library::open_image`), and a failed open clears under it too -/// (`crate::library::settle_failed_open`), so a clear never lands between -/// another open's commit and its latch. +/// The setup guard: the record, whether any image has completed load step 7 +/// under it, and whether a failed open has made it replaceable. An open +/// commits (which locks the record again), loads and latches under the image +/// list's lock (`crate::library::open_image`), and a failed open unlocks under +/// it too (`crate::library::settle_failed_open`), so an unlock never lands +/// between another open's commit and its latch. static STATE: Mutex = Mutex::new(State { recorded: None, latched: false, + replaceable: false, generation: 0, }); struct State { recorded: Option, latched: bool, - /// Counts the records [`setup`] made and the records failed opens cleared. - /// An open reads it when it begins, and clears the record on failure only - /// if it is unchanged: a failed open never clears a setup recorded after it - /// began. + /// An open that attempted a load failed since the record was last set or + /// committed, and nothing has latched: a different [`setup`] replaces the + /// record instead of being refused. + replaceable: bool, + /// Counts the records [`setup`] made or replaced. An open reads it when its + /// attempt begins, and unlocks the record on failure only if it is + /// unchanged: a failed open never unlocks a setup recorded after it began. generation: u64, } @@ -110,24 +117,30 @@ pub fn setup(options: SetupOptions) -> Result<()> { let wanted = recorded_of(&options)?; let mut state = STATE.lock().unwrap_or_else(|e| e.into_inner()); match &state.recorded { - None => { - state.recorded = Some(wanted); - state.generation += 1; - Ok(()) + Some(current) if *current == wanted => return Ok(()), + Some(current) if state.latched || !state.replaceable => { + return Err(Error::usage(format!( + "setup is process-wide and already in effect with {}; refusing {}", + describe(current), + describe(&wanted) + ))); } - Some(current) if *current == wanted => Ok(()), - Some(current) => Err(Error::usage(format!( - "setup is process-wide and already in effect with {}; refusing {}", - describe(current), - describe(&wanted) - ))), + _ => {} } + // A first record, or a replacement after a failed open: either way the new + // record is locked until the next failed open. + state.recorded = Some(wanted); + state.replaceable = false; + state.generation += 1; + Ok(()) } /// An open commits the setup in effect before step 7, recording the empty one -/// when [`setup`] was never called. +/// when [`setup`] was never called. The load that follows claims the record, so +/// it is locked again: no [`setup`] replaces it under a load. pub(crate) fn commit() -> Recorded { let mut state = STATE.lock().unwrap_or_else(|e| e.into_inner()); + state.replaceable = false; state .recorded .get_or_insert_with(|| Recorded { @@ -137,7 +150,7 @@ pub(crate) fn commit() -> Recorded { .clone() } -/// Read when an open begins, for [`clear_after_failed_open`]. +/// Read when an open's attempt begins, for [`unlock_after_failed_open`]. pub(crate) fn generation() -> u64 { STATE.lock().unwrap_or_else(|e| e.into_inner()).generation } @@ -145,23 +158,25 @@ pub(crate) fn generation() -> u64 { /// An image completed load step 7: from then on the setup in effect stands. /// Called under the image list's lock, right after the load. pub(crate) fn latch() { - STATE.lock().unwrap_or_else(|e| e.into_inner()).latched = true; + let mut state = STATE.lock().unwrap_or_else(|e| e.into_inner()); + state.latched = true; + state.replaceable = false; } -/// Settle an open that failed, whatever failed. While no image has completed -/// step 7 it clears the record, so [`setup`] accepts a corrected setup, unless -/// a setup was recorded after the open began (`began` is [`generation`] then). -/// Once the setup has latched it changes nothing: the library's own -/// process-once rule answers a different zone on an image that already has one. -/// Called only through `crate::library::settle_failed_open`, which holds the -/// image list's lock. -pub(crate) fn clear_after_failed_open(began: u64) { +/// Settle an open that attempted a load and failed, whatever failed. While no +/// image has completed step 7 it unlocks the record: the record stays, so a +/// retry runs under it, and a different [`setup`] may replace it. It leaves +/// alone a setup recorded or replaced after the open began (`began` is +/// [`generation`] then), and once the setup has latched it changes nothing: the +/// library's own process-once rule answers a different zone on an image that +/// already has one. Called only through `crate::library::settle_failed_open`, +/// which holds the image list's lock. +pub(crate) fn unlock_after_failed_open(began: u64) { let mut state = STATE.lock().unwrap_or_else(|e| e.into_inner()); - if state.latched || state.generation != began { + if state.latched || state.recorded.is_none() || state.generation != began { return; } - state.recorded = None; - state.generation += 1; + state.replaceable = true; } #[cfg(test)] diff --git a/rust/tests/api_v1_setup_failed_open.rs b/rust/tests/api_v1_setup_failed_open.rs index c7775dc7..2f813a44 100644 --- a/rust/tests/api_v1_setup_failed_open.rs +++ b/rust/tests/api_v1_setup_failed_open.rs @@ -1,9 +1,10 @@ -//! A failed open before any image has completed load step 7 clears the setup -//! record, whatever failed (`docs/reference/bindings-v1.md` §6, rule 4), here -//! for the failures that need no library at all: a request nothing installed -//! answers (autofetch off), and a refused version spelling. It runs in its own -//! process, as one test, because `setup` is process-wide and nothing public -//! resets it. It needs no stubs and no network. +//! Before any image has completed load step 7, an open that attempted a load +//! and failed unlocks the setup record, and the caller's own misuse unlocks +//! nothing (`docs/reference/bindings-v1.md` §6, rule 4), here for the cases that +//! need no library at all: a request nothing installed answers (autofetch off), +//! and a refused version spelling. It runs in its own process, as one test, +//! because `setup` is process-wide and nothing public resets it. It needs no +//! stubs and no network. use chtypes::{Error, FetchOptions, Registry, RegistryOptions, SetupOptions}; @@ -15,7 +16,7 @@ fn zone(name: &str) -> SetupOptions { } #[test] -fn a_failed_open_before_step_7_clears_the_setup() { +fn a_failed_open_before_step_7_unlocks_the_setup_and_misuse_does_not() { let cache = std::env::temp_dir().join(format!("setup_failed_open_cache_{}", std::process::id())); std::fs::create_dir_all(&cache).expect("create an empty cache"); @@ -31,26 +32,34 @@ fn a_failed_open_before_step_7_clears_the_setup() { }) .expect("constructing a registry opens nothing"); - // Nothing installed answers the request: a failed open, before any load. chtypes::setup(zone("Asia/Tokyo")).expect("the first setup records"); - let err = registry.for_version("26.8").unwrap_err(); - assert!( - matches!(err, Error::ArtifactMissing(_)), - "an empty cache with autofetch off: want ArtifactMissing, got {err:?}" - ); - chtypes::setup(zone("UTC")) - .expect("the failed open cleared the record, so a different setup is accepted"); - // A refused version spelling is a failed open too. + // A refused version spelling is the caller's own misuse, refused before + // anything is attempted: it unlocks nothing. let err = registry.for_version("v26.8").unwrap_err(); assert!( matches!(err, Error::Usage(_)), "a refused spelling: want Usage, got {err:?}" ); - chtypes::setup(zone("Europe/Berlin")).expect("the refused spelling cleared the record too"); + let err = chtypes::setup(zone("UTC")).unwrap_err(); + assert!( + matches!(err, Error::Usage(_)), + "a refused spelling must unlock nothing: {err:?}" + ); + + // Nothing installed answers the request: an open that attempted to resolve + // and failed, before any load. It keeps the record and makes it replaceable. + let err = registry.for_version("26.8").unwrap_err(); + assert!( + matches!(err, Error::ArtifactMissing(_)), + "an empty cache with autofetch off: want ArtifactMissing, got {err:?}" + ); + chtypes::setup(zone("UTC")) + .expect("the failed open unlocked the record, so a different setup replaces it"); - // Before any open is attempted, a second, different setup is still misuse. - let err = chtypes::setup(zone("Asia/Tokyo")).unwrap_err(); + // The replacement is locked again: a second, different setup before the + // next open is still misuse. + let err = chtypes::setup(zone("Europe/Berlin")).unwrap_err(); assert!(matches!(err, Error::Usage(_)), "{err:?}"); let _ = std::fs::remove_dir_all(&cache); diff --git a/ts/CHANGELOG.md b/ts/CHANGELOG.md index d92f5aee..aa0eecc8 100644 --- a/ts/CHANGELOG.md +++ b/ts/CHANGELOG.md @@ -8,7 +8,7 @@ The four bindings in this repository are released together and give one answer, ### Fixed -- Any open that fails before a library has completed load step 7 now clears the setup record, whatever failed (#468). After `setup({ timezone: 'Asia/Tokyo' })`, an open that failed before step 7 left that setup recorded, whether the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol failed it. A different `setup` was then a `UsageError` until the process restarted. 1.0.2 cleared the record only when step 7 itself failed. Now every failed open clears it (`registry.for`, `Registry.open({ preload })` and `openUnverified`), so the next `setup` is accepted, whatever it is. An open with no `setup` after the failure uses the empty setup, so call `setup` again before you retry. Two things are unchanged. A second, different `setup` before any open is attempted is still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same case, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. +- An open that attempted a load and failed now unlocks the setup, whatever failed, while no library has completed load step 7 (#468). After `setup({ timezone: 'Asia/Tokyo' })`, an open could fail before step 7: at the fetch, the signature check, an incompatible artifact (`CHTYPES_ARTIFACT_INCOMPATIBLE`) or a missing symbol. That left the setup locked, and a different `setup` was a `UsageError` until the process restarted. 1.0.2 handled only a failure in step 7 itself (#458). Now any failed attempt keeps the recorded setup but makes it replaceable. A plain retry with no new `setup` runs under the recorded zone and defaults, never the empty setup. A different `setup` is accepted and replaces the record, which is then locked again. This holds for `registry.for`, `Registry.open({ preload })` and `openUnverified`. The caller's own misuse fails before any load is attempted and unlocks nothing: a refused version spelling, or `openUnverified(path, { allow: false })`. Two different `setup` calls before any open is attempted are still a `UsageError`, never a silent last-wins. Once a library has completed step 7, a different setup is still a `UsageError`. No name or signature changes. All four bindings run the same cases, `tests/fixtures/abi-v1/setup-cases.json`, and the rule is `docs/reference/bindings-v1.md` section 6. ## [1.0.2] — 2026-10-06 diff --git a/ts/src/abi1/index.ts b/ts/src/abi1/index.ts index e2b30a30..5ecbba52 100644 --- a/ts/src/abi1/index.ts +++ b/ts/src/abi1/index.ts @@ -27,7 +27,7 @@ export { usageError, } from './errors.js'; export { Abi1Handle, HANDLE_CLASSES, wrapHandle } from './handles.js'; -export { type ImageSetup, LoadedImage, type LoadInput, openAbi1, openUnverified, type Predicate } from './loader.js'; +export { checkUnverifiedAllowed, type ImageSetup, LoadedImage, type LoadInput, openAbi1, openUnverified, type Predicate } from './loader.js'; export { asBuffer, checkStatus, diff --git a/ts/src/abi1/loader.ts b/ts/src/abi1/loader.ts index f470421f..dbbbe2c8 100644 --- a/ts/src/abi1/loader.ts +++ b/ts/src/abi1/loader.ts @@ -283,6 +283,13 @@ function initializeImage(image: LoadedImage, setup: ImageSetup): void { const UNVERIFIED_ENV = 'CHTYPES_ALLOW_UNVERIFIED_LIBRARY'; +/** `openUnverified`'s own gate, alone: a `UsageError` unless BOTH `allow` is true AND `CHTYPES_ALLOW_UNVERIFIED_LIBRARY=1` is set. The public layer asks it before an open is attempted, so the caller's misuse is told apart from a failed load. */ +export function checkUnverifiedAllowed(path: string, allow: boolean): void { + if (!allow || process.env[UNVERIFIED_ENV] !== '1') { + throw usageError(`opening ${path} unverified requires BOTH allow: true and ${UNVERIFIED_ENV}=1`); + } +} + /** * Load `path` with NO predicate: skips step 1 (nothing to check a floor * against) and step 5 (nothing to cross-check), still runs 2, 3, 4, 6 and 7. @@ -292,9 +299,7 @@ const UNVERIFIED_ENV = 'CHTYPES_ALLOW_UNVERIFIED_LIBRARY'; * only caller. */ export function openUnverified(path: string, options: { readonly allow: boolean } & ImageSetup): LoadedImage { - if (!options.allow || process.env[UNVERIFIED_ENV] !== '1') { - throw usageError(`opening ${path} unverified requires BOTH allow: true and ${UNVERIFIED_ENV}=1`); - } + checkUnverifiedAllowed(path, options.allow); warnUnverifiedOnce(path); const key = resolveImageKey(path); diff --git a/ts/src/library.ts b/ts/src/library.ts index 06c91cd5..087af0f3 100644 --- a/ts/src/library.ts +++ b/ts/src/library.ts @@ -6,7 +6,7 @@ * is trivially safe to call in any order. */ -import { type BuildInfo, type Calls, type LoadedImage, openUnverified as loadUnverified } from './abi1/index.js'; +import { type BuildInfo, type Calls, checkUnverifiedAllowed, type LoadedImage, openUnverified as loadUnverified } from './abi1/index.js'; import { type Discovery, decodeDiscovery, @@ -125,6 +125,8 @@ export function libraryOf(image: LoadedImage, resolved: Resolved | undefined): L * no `resolved`, unless a registry had already opened the same image. */ export function openUnverified(path: string, options: { readonly allow: boolean }): Library { + // The caller's two opt-ins are checked before anything is attempted: misuse, which unlocks nothing. + checkUnverifiedAllowed(path, options.allow); const began = setupGeneration(); try { const setup = commitSetup(); @@ -132,7 +134,7 @@ export function openUnverified(path: string, options: { readonly allow: boolean latchSetup(); return libraryOf(image, undefined); } catch (err) { - // Like any open: a failure clears the setup record while no image has completed step 7. + // Like any open: a failed attempt unlocks the setup record while no image has completed step 7. settleFailedOpen(began); throw err; } diff --git a/ts/src/registry.ts b/ts/src/registry.ts index 581d7db4..8374458f 100644 --- a/ts/src/registry.ts +++ b/ts/src/registry.ts @@ -80,6 +80,7 @@ export class Registry { static async open(options: RegistryOptions = {}): Promise { const registry = new Registry(options); for (const request of options.preload ?? []) { + checkSpelling(request); await registry.#memoized(request, false); } return registry; @@ -87,6 +88,7 @@ export class Registry { /** Open a version: the installed build the request names, fetched first when autofetch is on and none is installed. */ async for(request: string): Promise { + checkSpelling(request); // misuse, refused before anything is attempted: it unlocks nothing return this.#memoized(request, this.#autofetch); } @@ -103,7 +105,7 @@ export class Registry { #memoized(request: string, mayFetch: boolean): Promise { const memo = this.#memo.get(request); if (memo !== undefined) return memo; - // A failed open clears the setup record while no image has completed load step 7, whatever failed: the spelling, the fetch, the signature or any load step (bindings-v1.md §6, rule 4). + // An open that attempted a load and failed unlocks the setup record while no image has completed load step 7, whatever failed: the resolve, the fetch, the signature or any load step (bindings-v1.md §6, rule 4). The spelling was checked before this, as misuse. const began = setupGeneration(); const opening = this.#openRequest(request, mayFetch).then( (library) => { @@ -121,7 +123,6 @@ export class Registry { } async #openRequest(request: string, mayFetch: boolean): Promise { - checkSpelling(request); // a refused spelling is a failed open too, settled by #memoized const platform: PlatformKey | undefined = this.#fetch.platform ?? hostPlatformKey(os.platform(), os.arch()); if (platform === undefined) { throw new ArtifactMissingError(`chtypes: no v1 artifact is published for this host (${os.platform()}-${os.arch()})`); diff --git a/ts/src/setup.ts b/ts/src/setup.ts index 7e61c75d..f41cf5de 100644 --- a/ts/src/setup.ts +++ b/ts/src/setup.ts @@ -12,11 +12,13 @@ * library reads as UTC, and no defaults. Once an image completes loader * step 7 (`chs_initialize`, then `chs_set_defaults` when there are * defaults), `setup` succeeds only with exactly the setup in effect - * (`latchSetup`). Until then, any open that fails, whatever failed (the - * fetch, the signature, an incompatible artifact, a missing symbol or - * step 7), clears the record (`settleFailedOpen`): the next `setup` is - * accepted, and an open with no `setup` after the failure records the - * empty setup. + * (`latchSetup`). Until then, an open that attempted a load and failed, + * whatever failed (the fetch, the signature, an incompatible artifact, a + * missing symbol or step 7), keeps the record but makes it replaceable + * (`settleFailedOpen`): a retry with no new `setup` runs under the + * recorded setup, and a different `setup` replaces it. A refused version + * spelling or an unverified open without the caller's opt-in fails before + * any load is attempted, and unlocks nothing. * 3. Every image is set up at loader step 7, once, from this record * (`./abi1/loader.ts`); nothing sets either again for that image. * @@ -42,7 +44,9 @@ export interface ProcessSetup { let recorded: ProcessSetup | undefined; /** Whether any image has completed loader step 7 under the record. */ let latched = false; -/** Counts the records `setup` made and the records failed opens cleared. An open reads it when it begins, and clears the record on failure only if it is unchanged: a failed open never clears a setup recorded after it began. */ +/** Whether an open that attempted a load failed since the record was last set or committed, with nothing latched: a different `setup` then replaces the record instead of being refused. */ +let replaceable = false; +/** Counts the records `setup` made or replaced. An open reads it when its attempt begins, and unlocks the record on failure only if it is unchanged: a failed open never unlocks a setup recorded after it began. */ let generation = 0; function describe(s: ProcessSetup): string { @@ -65,23 +69,24 @@ export function setup(options: SetupOptions = {}): void { const defaults = options.defaults ?? {}; validateDefaults(defaults); const next: ProcessSetup = { timezone, defaults: { ...defaults } }; - if (recorded === undefined) { - recorded = next; - generation += 1; - return; - } - if (!same(recorded, next)) { + if (recorded !== undefined && same(recorded, next)) return; + if (recorded !== undefined && (latched || !replaceable)) { throw usageError(`setup was already fixed at ${describe(recorded)}; it was asked for ${describe(next)}. The first setup stands: call setup before the first open, once.`); } + // A first record, or a replacement after a failed open: either way the new record is locked until the next failed open. + recorded = next; + replaceable = false; + generation += 1; } -/** The setup in effect, recording the empty setup if none was recorded: what an open does before step 7. */ +/** The setup in effect, recording the empty setup if none was recorded: what an open does before step 7. The load that follows claims the record, so it is locked again. */ export function commitSetup(): ProcessSetup { recorded ??= { timezone: '', defaults: {} }; + replaceable = false; return recorded; } -/** Read when an open begins, for `settleFailedOpen`. */ +/** Read when an open's attempt begins, for `settleFailedOpen`. */ export function setupGeneration(): number { return generation; } @@ -89,19 +94,20 @@ export function setupGeneration(): number { /** An image completed loader step 7: from then on the setup in effect stands. An open calls it synchronously after the load that committed the record, so it latches the record that load ran under. */ export function latchSetup(): void { latched = true; + replaceable = false; } /** - * Settle an open that failed, whatever failed. While no image has completed step 7 it clears the record, so `setup` accepts a corrected setup, unless a setup was recorded after the open began (`began` is `setupGeneration()` then). Once the setup has latched it changes nothing: the library's own process-once rule answers a different zone on an image that already has one. + * Settle an open that attempted a load and failed, whatever failed. While no image has completed step 7 it unlocks the record: the record stays, so a retry runs under it, and a different `setup` may replace it. It leaves alone a setup recorded or replaced after the open began (`began` is `setupGeneration()` then), and once the setup has latched it changes nothing: the library's own process-once rule answers a different zone on an image that already has one. */ export function settleFailedOpen(began: number): void { - if (latched || generation !== began) return; - recorded = undefined; - generation += 1; + if (latched || recorded === undefined || generation !== began) return; + replaceable = true; } -/** Test only: forget the recorded setup and the latch. Not part of the public API. */ +/** Test only: forget the recorded setup, the latch and the unlock. Not part of the public API. */ export function resetSetupForTests(): void { recorded = undefined; latched = false; + replaceable = false; } diff --git a/ts/test/settings-setup.test.ts b/ts/test/settings-setup.test.ts index 3621de2e..7519a2b6 100644 --- a/ts/test/settings-setup.test.ts +++ b/ts/test/settings-setup.test.ts @@ -99,25 +99,28 @@ describe('setup: the process-once rule', () => { }); }); -describe('setup: a failed open before the first step 7 clears the record', () => { +describe('setup: a failed open before the first step 7 unlocks the record', () => { beforeEach(() => resetSetupForTests()); - it('clears it after a registry open that fails before any load (nothing installed, autofetch off)', async () => { + it('a refused spelling unlocks nothing; a failed resolve (nothing installed, autofetch off) keeps the record and lets a different setup replace it', async () => { setup({ timezone: 'Asia/Tokyo' }); const cacheDir = mkdtempSync(path.join(os.tmpdir(), 'setup-latch-empty-cache-')); const registry = await Registry.open({ fetch: { cacheDir, systemDirs: [], offline: true }, autofetch: false }); + await expect(registry.for('v26.8')).rejects.toBeInstanceOf(UsageError); + expect(() => setup({ timezone: 'UTC' })).toThrow(UsageError); // misuse unlocked nothing await expect(registry.for('26.8')).rejects.toBeInstanceOf(ArtifactMissingError); - expect(() => setup({ timezone: 'UTC' })).not.toThrow(); + expect(commitSetup().timezone).toBe('Asia/Tokyo'); // kept: a retry runs under it, never the empty setup }); - it('clears it after a refused version spelling too: any failed open', async () => { + it('after a failed attempt, a different setup replaces the record, and the replacement is locked again', () => { setup({ timezone: 'Asia/Tokyo' }); - const registry = await Registry.open({ autofetch: false }); - await expect(registry.for('v26.8')).rejects.toBeInstanceOf(UsageError); + settleFailedOpen(setupGeneration()); expect(() => setup({ timezone: 'UTC' })).not.toThrow(); + expect(commitSetup().timezone).toBe('UTC'); + expect(() => setup({ timezone: 'Europe/Berlin' })).toThrow(UsageError); }); - it('never clears a setup recorded after the open began, nor a latched one', () => { + it('never unlocks a setup recorded after the open began, nor a latched one', () => { const began = setupGeneration(); setup({ timezone: 'Asia/Tokyo' }); settleFailedOpen(began); // it began before Asia/Tokyo was recorded