Skip to content

feat: lockfile graduation — wire writer in up's feature pipeline (PR-4b)#35

Closed
pofallon wants to merge 2 commits into
pr4-lockfile-graduationfrom
pr4b-lockfile-writer-wiring
Closed

feat: lockfile graduation — wire writer in up's feature pipeline (PR-4b)#35
pofallon wants to merge 2 commits into
pr4-lockfile-graduationfrom
pr4b-lockfile-writer-wiring

Conversation

@pofallon
Copy link
Copy Markdown
Contributor

Closes #32. Follow-up to PR-4a (#33).

Summary

PR-4a landed the user-visible flag surface (--no-lockfile, --frozen-lockfile), the schema parity fix (dependsOn rename, trailing newline), and the upstream-aligned frozen error strings — but the writer was never invoked. This wires it in for up.

After feature resolution + download, build a Lockfile from the resolved feature set (build_lockfile_from_features mirrors upstream generateLockfile), thread it back via FeatureBuildOutput, then:

  • default: write {config_dir}/devcontainer-lock.json (sorted by key, trailing newline — byte-identical to upstream writeLockfile)
  • --frozen-lockfile: byte-compare against on-disk; fail with the upstream summary strings (\"Lockfile does not exist.\" / \"Lockfile does not match.\") so existing CI scripts keep working
  • --no-lockfile: skip entirely
  • deprecated --experimental-lockfile <PATH>: still honored for the custom-path form (hidden alias path through the 1.x line)

Wired into both single-container (container.rs) and compose (compose.rs) flows via shared handle_lockfile_post_build helper in helpers.rs. EROFS/EACCES on the write path downgrades to a WARN so read-only workspaces (CI mounts, read-only volumes) don't break up.

Design notes

  • Lockfile keys are the user-provided feature ID (e.g. ghcr.io/devcontainers/features/node:1), not the canonical no-tag form — matching upstream and keeping the existing pre-build structural validation aligned.
  • canonical_lockfile_bytes in helpers.rs mirrors deacon_core::lockfile::write_lockfile's on-disk format exactly. A regression test (canonical_bytes_match_write_lockfile_output) pins them together — without it, --frozen-lockfile would report spurious mismatches if the two ever drifted.
  • Missing version metadata falls back to the feature ref's tag (e.g. `"1"`) with a WARN. The lockfile remains assembled best-effort; the alternative would have blocked up on features whose metadata happens to omit the version.

Out of scope

  • build command lockfile wiring requires build to actually install features first (TODO at crates/deacon/src/commands/build/mod.rs:1285). That's PR-4c, tracked separately.
  • The existing pre-build structural validation in up/mod.rs (PR-4a) is left as a fast-fail; the new post-build byte-comparison is the comprehensive check. Both layers can fire on a frozen run.

Test plan

13 new unit tests across features_build.rs + helpers.rs:

  • lockfile assembly (4): user-ID keying, version-missing fallback, alphabetic dependsOn sort, dependsOn omission when empty.
  • post-build helper (9): canonical_lockfile_bytes matches write_lockfile output (byte-for-byte parity test), --no-lockfile skips all I/O, default mode writes next to config, frozen + missing → upstream string, frozen + matching → success, frozen + mismatch → upstream string, --experimental-lockfile <PATH> honored, is_readonly_fs_error covers PermissionDenied and ignores NotFound.

Verification:

  • `cargo fmt --all -- --check`
  • `cargo clippy --all-targets -- -D warnings`
  • `make test-nextest-fast` (1956 tests pass)
  • CI green
  • Spec parity: byte-identical output to upstream `devcontainer up` against the same features (fixture-based parity test deferred to a follow-up once we have an upstream-vs-deacon fixture harness)

Refs

🤖 Generated with Claude Code

pofallon and others added 2 commits May 25, 2026 12:39
Phase 1 of lockfile graduation per docs/ROADMAP_TO_1.0.md. Ships the
schema parity fixes, the user-visible CLI flag surface, the writer
helper, and upstream-aligned error messages. The actual writer
integration (building a Lockfile from resolved features and writing
it after `up`/`build`) is tracked as PR-4b at issue #32.

## Schema parity (deacon-core/lockfile.rs)

- LockfileFeature.depends_on now serializes as `dependsOn` (camelCase)
  via #[serde(rename)], matching upstream `devcontainers/cli`'s
  generateLockfile in src/spec-configuration/lockfile.ts. Deserializer
  accepts the camelCase form on read.
- write_lockfile() emits a trailing newline to match upstream
  `JSON.stringify(..., 2) + '\n'`. Byte-identical output keeps the
  --frozen-lockfile content comparison stable.
- LockfileValidationResult::format_error() now emits upstream-aligned
  strings: "Lockfile does not exist." / "Lockfile does not match." as
  the leading summary line; trailing actionable guidance references
  the graduated --frozen-lockfile flag rather than the deprecated
  --experimental-frozen-lockfile.
- New LockfileFeature::from_resolved() helper constructs entries in
  the upstream canonical form (resolved = "{registry}/{repo}@{digest}",
  integrity = "{digest}"). This is the writer entry point PR-4b uses.

## CLI surface graduation

up + build both gain:
- --no-lockfile (visible): skip lockfile generation and verification.
- --frozen-lockfile (visible): require an up-to-date lockfile; fail
  if resolution would change it.

Mutual exclusivity (--no-lockfile xor --frozen-lockfile) enforced in
the CLI layer, mirroring upstream's pre-parse validation.

Deprecation:
- --experimental-lockfile and --experimental-frozen-lockfile remain
  accepted as hidden aliases through the 1.x line. The CLI emits a
  WARN on use directing users to the graduated flags.
- effective_frozen = frozen_lockfile || experimental_frozen_lockfile
  matches upstream's effectiveFrozenLockfile coalescing.

## Downstream consumers

- up/mod.rs frozen-validation path now reads args.frozen_lockfile
  (the effective value) instead of args.experimental_frozen_lockfile.
  No behavior change — only the variable name moves.
- build/mod.rs args struct gains no_lockfile + frozen_lockfile fields
  (carrier-only for PR-4b; #[allow(dead_code)] until then).

## Tests

Added in deacon-core/lockfile.rs:
- test_depends_on_serializes_as_camel_case
- test_write_lockfile_emits_trailing_newline
- test_from_resolved_constructs_upstream_form

Updated in deacon/tests/up_lockfile_frozen.rs (5 assertions):
align with the new upstream-format error messages
("Lockfile does not exist." / "Lockfile does not match." / capital "F"
on "Features ..." substrings).

## Verified

- cargo fmt --all -- --check
- cargo clippy --all-targets -- -D warnings
- cargo nextest run --profile dev-fast --no-default-features: 1930/1930 pass
- cargo test --doc --workspace: 130/130 pass

## Follow-up tracked

- #32 — PR-4b: wire writer in up + build feature pipeline.
- Build-command lockfile wiring is gated on the pre-existing TODO at
  build/mod.rs:1285 (build doesn't install features today). Will land
  with #32 or as a sibling PR.

## Refs

- docs/ROADMAP_TO_1.0.md Tier 1 item "Lockfile graduation"
- Upstream: devcontainers/cli#1212 (graduated lockfile in v0.87.0)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR-4a (#33): the user-visible flag surface, schema parity
fix, and upstream-aligned error strings landed there, but the actual
writer was never invoked. This wires it in.

After feature resolution + download, build a Lockfile from the resolved
feature set (`build_lockfile_from_features` mirrors upstream
`generateLockfile`), thread it back via FeatureBuildOutput, then:

- default: write `{config_dir}/devcontainer-lock.json` (sorted by key,
  trailing newline — byte-identical to upstream `writeLockfile`)
- `--frozen-lockfile`: byte-compare against on-disk; fail with the
  upstream summary strings (`"Lockfile does not exist."` /
  `"Lockfile does not match."`) so existing CI scripts keep working
- `--no-lockfile`: skip entirely
- deprecated `--experimental-lockfile <PATH>`: still honored for the
  custom-path form (hidden alias path through the 1.x line)

Wired into both single-container (`container.rs`) and compose
(`compose.rs`) flows via shared `handle_lockfile_post_build` helper.
EROFS/EACCES on the write path downgrades to a WARN so read-only
workspaces (CI mounts, read-only volumes) don't break `up`.

Lockfile keys are the user-provided feature ID (e.g.
`ghcr.io/devcontainers/features/node:1`), not the canonical no-tag form
— matching upstream and keeping the existing pre-build structural
validation aligned.

Build (`crates/deacon/src/commands/build/mod.rs`) is intentionally out
of scope — see issue #32 and the standing TODO at build/mod.rs:1285.
That's PR-4c.

Closes #32 (PR-4 phase 2 of the lockfile graduation track).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon added a commit that referenced this pull request May 25, 2026
…#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon added a commit that referenced this pull request May 25, 2026
…#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon pofallon force-pushed the pr4-lockfile-graduation branch from 1ccb98b to df08eac Compare May 25, 2026 18:45
pofallon added a commit that referenced this pull request May 25, 2026
…(PR-7 of 7) (#31)

* docs: Podman experimental status, 1.0 roadmap, and CHANGELOG bootstrap

Last of the four parallelizable Tier 1 PRs from docs/ROADMAP_TO_1.0.md.
No runtime behavior changes beyond a single one-time WARN.

## Podman repositioned as experimental for 1.0

Trait-level Podman integration is already complete in crates/core/src/
runtime.rs (DockerRuntime + PodmanRuntime fully implement Docker,
ContainerOps, DockerLifecycle traits), but lacks test coverage and
rootless-Podman parity items (label=disable, --userns=keep-id,
--uidmap/--gidmap). README/CLAUDE.md previously said "planned" / "in
development" — that's not accurate, but neither is "fully supported".
Documenting as experimental for 1.0; promoting in 1.1 (tracked at #30).

- README.md "Currently Unsupported" row + "Runtime Selection" section:
  replace "Planned. Docker is fully supported; Podman coming later"
  with the experimental-status narrative + tracking issue link.
- CLAUDE.md "Container Runtimes" section: same update.
- crates/deacon/src/cli.rs: --runtime help text now reads "docker or
  podman [experimental]"; RuntimeOption::Podman doc comment marked.
- crates/core/src/runtime.rs: detect_runtime() emits a one-time WARN
  (std::sync::Once + tracing::warn!) when Podman is selected — by CLI
  flag or DEACON_RUNTIME env var. Idempotent across calls per process.

## CHANGELOG.md (new)

Keep-a-Changelog format, seeded with the Unreleased section capturing
PR-1..PR-3 + PR-7 changes. Includes a Notes subsection recording the
four "non-issues" surfaced by the 1.0 audit (serde_yaml not used; TAR
CVE not vulnerable; json5 advisory inapplicable; issue #1 already
resolved via bead 14b) so the audit trail lives in code, not memory.

## docs/ROADMAP_TO_1.0.md

The 1.0 readiness report produced ahead of the implementation work,
deferred from PR-1's chore commit per agreed scoping.

## Tracking issue filed

#30 — "Podman parity: promote from experimental to supported in 1.1"

## Verified

- cargo fmt --all -- --check
- cargo clippy --all-targets -- -D warnings
- cargo nextest run --profile dev-fast --no-default-features: 1927/1927 pass
- cargo test --doc --workspace: 130/130 pass
- 11/11 runtime::tests pass
- deacon --help shows updated runtime text and [experimental] marker

Refs: docs/ROADMAP_TO_1.0.md Tier 1 items "Podman story decision" and
"CHANGELOG.md".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(changelog): add entries for PR-4b (#35) and set-up track (#36/#37/#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon pofallon deleted the branch pr4-lockfile-graduation May 25, 2026 18:58
@pofallon pofallon closed this May 25, 2026
@pofallon pofallon deleted the pr4b-lockfile-writer-wiring branch May 25, 2026 19:17
pofallon added a commit that referenced this pull request May 25, 2026
… (PR-4c) (#41)

* feat: lockfile graduation — schema parity + CLI surface (PR-4a)

Phase 1 of lockfile graduation per docs/ROADMAP_TO_1.0.md. Ships the
schema parity fixes, the user-visible CLI flag surface, the writer
helper, and upstream-aligned error messages. The actual writer
integration (building a Lockfile from resolved features and writing
it after `up`/`build`) is tracked as PR-4b at issue #32.

## Schema parity (deacon-core/lockfile.rs)

- LockfileFeature.depends_on now serializes as `dependsOn` (camelCase)
  via #[serde(rename)], matching upstream `devcontainers/cli`'s
  generateLockfile in src/spec-configuration/lockfile.ts. Deserializer
  accepts the camelCase form on read.
- write_lockfile() emits a trailing newline to match upstream
  `JSON.stringify(..., 2) + '\n'`. Byte-identical output keeps the
  --frozen-lockfile content comparison stable.
- LockfileValidationResult::format_error() now emits upstream-aligned
  strings: "Lockfile does not exist." / "Lockfile does not match." as
  the leading summary line; trailing actionable guidance references
  the graduated --frozen-lockfile flag rather than the deprecated
  --experimental-frozen-lockfile.
- New LockfileFeature::from_resolved() helper constructs entries in
  the upstream canonical form (resolved = "{registry}/{repo}@{digest}",
  integrity = "{digest}"). This is the writer entry point PR-4b uses.

## CLI surface graduation

up + build both gain:
- --no-lockfile (visible): skip lockfile generation and verification.
- --frozen-lockfile (visible): require an up-to-date lockfile; fail
  if resolution would change it.

Mutual exclusivity (--no-lockfile xor --frozen-lockfile) enforced in
the CLI layer, mirroring upstream's pre-parse validation.

Deprecation:
- --experimental-lockfile and --experimental-frozen-lockfile remain
  accepted as hidden aliases through the 1.x line. The CLI emits a
  WARN on use directing users to the graduated flags.
- effective_frozen = frozen_lockfile || experimental_frozen_lockfile
  matches upstream's effectiveFrozenLockfile coalescing.

## Downstream consumers

- up/mod.rs frozen-validation path now reads args.frozen_lockfile
  (the effective value) instead of args.experimental_frozen_lockfile.
  No behavior change — only the variable name moves.
- build/mod.rs args struct gains no_lockfile + frozen_lockfile fields
  (carrier-only for PR-4b; #[allow(dead_code)] until then).

## Tests

Added in deacon-core/lockfile.rs:
- test_depends_on_serializes_as_camel_case
- test_write_lockfile_emits_trailing_newline
- test_from_resolved_constructs_upstream_form

Updated in deacon/tests/up_lockfile_frozen.rs (5 assertions):
align with the new upstream-format error messages
("Lockfile does not exist." / "Lockfile does not match." / capital "F"
on "Features ..." substrings).

## Verified

- cargo fmt --all -- --check
- cargo clippy --all-targets -- -D warnings
- cargo nextest run --profile dev-fast --no-default-features: 1930/1930 pass
- cargo test --doc --workspace: 130/130 pass

## Follow-up tracked

- #32 — PR-4b: wire writer in up + build feature pipeline.
- Build-command lockfile wiring is gated on the pre-existing TODO at
  build/mod.rs:1285 (build doesn't install features today). Will land
  with #32 or as a sibling PR.

## Refs

- docs/ROADMAP_TO_1.0.md Tier 1 item "Lockfile graduation"
- Upstream: devcontainers/cli#1212 (graduated lockfile in v0.87.0)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: lockfile graduation — wire writer in up's feature pipeline (PR-4b)

Follow-up to PR-4a (#33): the user-visible flag surface, schema parity
fix, and upstream-aligned error strings landed there, but the actual
writer was never invoked. This wires it in.

After feature resolution + download, build a Lockfile from the resolved
feature set (`build_lockfile_from_features` mirrors upstream
`generateLockfile`), thread it back via FeatureBuildOutput, then:

- default: write `{config_dir}/devcontainer-lock.json` (sorted by key,
  trailing newline — byte-identical to upstream `writeLockfile`)
- `--frozen-lockfile`: byte-compare against on-disk; fail with the
  upstream summary strings (`"Lockfile does not exist."` /
  `"Lockfile does not match."`) so existing CI scripts keep working
- `--no-lockfile`: skip entirely
- deprecated `--experimental-lockfile <PATH>`: still honored for the
  custom-path form (hidden alias path through the 1.x line)

Wired into both single-container (`container.rs`) and compose
(`compose.rs`) flows via shared `handle_lockfile_post_build` helper.
EROFS/EACCES on the write path downgrades to a WARN so read-only
workspaces (CI mounts, read-only volumes) don't break `up`.

Lockfile keys are the user-provided feature ID (e.g.
`ghcr.io/devcontainers/features/node:1`), not the canonical no-tag form
— matching upstream and keeping the existing pre-build structural
validation aligned.

Build (`crates/deacon/src/commands/build/mod.rs`) is intentionally out
of scope — see issue #32 and the standing TODO at build/mod.rs:1285.
That's PR-4c.

Closes #32 (PR-4 phase 2 of the lockfile graduation track).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: build — install features + write lockfile for Dockerfile builds (PR-4c)

Removes the long-standing "Feature installation during build is not yet
implemented" fail-fast for the Dockerfile-based build path, and wires the
PR-4b lockfile writer into `build`'s post-build flow.

## What this PR does

When `deacon build` is invoked against a Dockerfile-based config with
features declared:

1. Run the user's `docker build` as before (Dockerfile, build args, cache,
   labels — all unchanged).
2. **NEW** — layer features on top of the just-built image: synthesize a
   single-container config with `image: <built_image_id>` and call `up`'s
   existing `build_image_with_features` helper. That helper resolves +
   downloads features, generates a BuildKit RUN-mount stage per feature,
   and produces a feature-extended image + a `Lockfile`.
3. **NEW** — write the lockfile next to `devcontainer.json` via
   `deacon_core::lockfile::write_lockfile(force_init = true)`. Read-only
   workspaces (EROFS/EACCES) downgrade to a WARN so CI mounts don't fail.
4. Update `final_result.image_id` to the feature-extended image so
   downstream consumers (cache, scan, output, push) see the right tag.

Compose-based and image-reference builds still fail fast with features —
their integration patterns differ enough to warrant separate follow-ups.

## Cache invalidation

When features are present, the cache check is skipped (`!features_present`
gate added). The current `config_hash` derives from `build_config` only —
it does not fold in feature digests, so a cached hit would point at the
base image without feature layers. Skipping the cache when features are
present trades a rebuild for correctness; a later refinement can include
feature digests in the hash for cacheable feature builds.

## Module visibility

`crates/deacon/src/commands/up/mod.rs` — `features_build` module was
private; promoted to `pub(crate)` so the build command can call
`build_image_with_features` without copying ~500 LoC of feature pipeline
into `commands/build/`. A future cleanup can lift the helper into a
shared `commands/shared/` module.

## Tests

3 new unit tests in `build::tests`:

- `is_readonly_filesystem_error_detects_permission_denied`
- `is_readonly_filesystem_error_ignores_unrelated_io_errors` (NotFound etc.
  must propagate so real bugs aren't hidden)
- `is_readonly_filesystem_error_ignores_non_io_errors`

These pin the WARN-vs-error decision for the lockfile write path. The
actual feature-install + lockfile-write integration tests require Docker
+ BuildKit + an OCI registry; they belong in an integration test suite
that I haven't added here (test grouping under `docker-shared` is the
right home).

Verification:
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets -- -D warnings`
- `cargo test -p deacon --lib` → 229 pass (no regression; 3 new tests)

## Base branch

Based on `pr4b-lockfile-writer-wiring` (#35) since this PR consumes
`FeatureBuildOutput.lockfile` (the field PR-4b added). Auto-rebases to
`main` once #33 and #35 land in order.

With this PR, the `build` command supports features for Dockerfile-based
configs and writes a lockfile alongside — closes the lockfile track's
remaining 1.0 work (PR-4c on issue #34).

Refs: issue #34 (Tier 1 progress tracker); PR-4b (#35).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant