Skip to content

pklrun: run all pkl commands from one place (manifest eval, conformance, plain evaluators) #500

Description

@naxty

Summary

PR #494 introduced pkg/plugin/pklrun as the home for the project-aware pkl evaluator and pkl project resolve (with auto-resolve), and collapsed three duplicated newSafeProjectEvaluator helpers onto it. But pkl is still invoked directly from several other places that bypass pklrun — so there is no single source of truth for how formae runs the pkl binary (which binary, bundled vs PATH, error handling, auto-resolve).

This issue proposes finishing the job: route all pkl invocations through pklrun.

Current pkl-binary touchpoints outside pklrun

Direct exec.Command("pkl", ...):

Location Call Module
pkg/plugin/manifest.go:69 pkl eval -f json <manifest> pkg/plugin (same module as pklrun)
pkg/plugin-conformance-tests/setup.go:161 pkl project resolve <dir> pkg/plugin-conformance-tests (separate module)
pkg/plugin-conformance-tests/setup.go:251 pkl eval -x minFormaeVersion ... pkg/plugin-conformance-tests

Plain pkl-go evaluators that spawn pkl from PATH (no bundled-binary selection):

  • pkg/plugin/descriptors/extract_schema.go:139 (generatePklProject)
  • pkg/plugin/testutil/verify.go:165 (generatePklProject)
  • root internal/schema/pkl/pkl.go no-project branches already use bundledPklCommand; the pkg/plugin ones do not.

Why it matters

  • Bundled-binary consistency. Root deliberately prefers the sibling-of-formae pkl (newer stdlib features). manifest.go runs at formae runtime to read installed plugin manifests but shells pkl from PATH — so it can pick up a different pkl version than the rest of the binary. Latent version-skew risk.
  • Duplicated resolve logic. conformance-tests/setup.go still has its own pkl project resolve — the exact pattern feat(pkl): auto-resolve PklProject deps via shared pklrun evaluator #494 removed elsewhere.
  • One place to evolve. Timeouts, retries, proxy/env handling, error formatting, auto-resolve semantics (pklrun: re-resolve PklProject when deps.json is stale (not just absent) #495) — all should live in one spot, not be re-implemented per call site.

Proposed direction

  • Add a no-project pklrun.Eval(path string, opts ...Option) (single-module eval, with WithPklCommand/WithEvaluatorOptions) and route pkg/plugin/manifest.go through it.
  • Route pkg/plugin/descriptors and pkg/plugin/testutil plain evaluators through pklrun (or expose binary selection so they share BundledPklCommand).
  • For pkg/plugin-conformance-tests (separate module): either have it depend on pkg/plugin/pklrun or replicate the thin command layer — decide based on the dependency direction we want for the conformance suite.

Out of scope / related

Refs

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions