Skip to content

Add merge_gemfile task and pipeline stage (#50 slice 2) - #72

Merged
silug merged 2 commits into
simp:mainfrom
silug:issue-50-gemfile-merge
Jul 31, 2026
Merged

Add merge_gemfile task and pipeline stage (#50 slice 2)#72
silug merged 2 commits into
simp:mainfrom
silug:issue-50-gemfile-merge

Conversation

@silug

@silug silug commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Second slice of #50: with the Gemfile in bootstrap mode after #70 (laid down only when missing), this adds the other half — an in-place merge stage that delivers baseline Gemfile changes to existing repos without ever touching what Renovate manages. Pairs with #70 but has no code overlap and merges independently in either order.

The merge_gemfile task

  • Adds template gems missing from the target into their matching group ... do block, together with attached comment lines (e.g. # renovate: manager hints). Groups missing entirely are copied verbatim at EOF.
  • Brings along group-local variable assignments that inserted lines reference — transitively: gem 'puppet', puppet_version needs puppet_version; the pdk line needs major_puppet_version, which itself needs puppet_version. They're inserted at the top of the group in template order. (The e2e caught this class of bug before the specs did — a merged Gemfile that referenced assignments it didn't have would NameError on evaluation; there's now a spec that evals the merged output.)
  • Removes gems listed in remove_gems (plus an attached # renovate: comment) — the mechanism for cleanups like the old puppet-lint-empty_string-check removal.
  • Never modifies existing gems' version constraints, custom gems, comments, or anything else it wasn't asked about. Missing file → full template written (bootstrap parity).
  • Returns {changed, added, removed} JSON, so no-change repos flow into Full idempotency: repos that need no changes must pass through cleanly #49's unchanged bucket.

The pipeline stage

merge_gemfile (opt-in via the session config's stages list, like everything else):

  • Template resolved in the plan with the same per-module override chain as the profile (Gemfile.<module_name> beats Gemfile), via file()
  • Targets skeleton/Gemfile for pupmod_skeleton repos; only runs against pupmod-ish project types
  • remove_gems comes from the session config (puppetsync.plans.sync.merge_gemfile.remove_gems)

Verification

  • 11 new specs driven by the real baseline template from modules/profile/files/pupmod/Gemfile: no-op on identical file, constraint preservation under a simulated Renovate bump, group-end insertion, attached comments, missing-group creation, transitive assignment ordering + an eval of the merged result, removals, custom-gem preservation, idempotency, param validation. 167 examples total, 0 failures.
  • End-to-end through the real pipeline against a file:// fixture (temp config, not committed): a Gemfile with a custom simp-rake-helpers constraint and most of the baseline missing came out with all gems/groups/assignments added, the constraint byte-for-byte intact, remove_gems honored, the merged file passing an evaluation check — and a second run reporting 0 ok / 1 unchanged / 0 failed.
  • puppet parser validate --tasks, bolt plan show, stage-list dry run, and the CI idempotency e2e all green.

Slice 3 (the psych-pure YAML merger for workflow files) is next; once it lands, profile::github_actions flips to bootstrap via Hiera and #50 closes.

Refs #50

🤖 Generated with Claude Code

Second slice of the Renovate-resilient template work: with the Gemfile
now bootstrap-mode (laid down only when missing), this is how baseline
Gemfile changes reach existing repos — an in-place merge that never
touches what Renovate manages.

The merge_gemfile task:
- Adds template gems missing from the target into their matching
  `group ... do` block (with attached comment lines, e.g. renovate
  manager hints); missing groups are copied verbatim at EOF
- Brings along group-local variable assignments that inserted lines
  reference (transitively — `gem 'puppet', puppet_version` needs
  puppet_version; the pdk line needs major_puppet_version, which needs
  puppet_version), inserted at the top of the group in template order
- Removes gems listed in remove_gems (plus an attached `# renovate:`
  comment line)
- Never modifies existing gems' version constraints or any other
  existing content; writes the full template when the file is missing

The plan stage resolves the template with the same per-module override
chain as the profile (Gemfile.<module_name> beats Gemfile), targets
skeleton/Gemfile for pupmod_skeleton repos, runs only against
pupmod-ish project types, and reads remove_gems from the session
config (puppetsync.plans.sync.merge_gemfile.remove_gems).

Verified with 11 new specs (driven by the real baseline template,
including an eval check that merged output executes) and an e2e run:
a fixture Gemfile with a custom simp-rake-helpers constraint and most
of the baseline missing came out with all gems/groups/assignments
added, the constraint byte-for-byte intact, remove_gems honored, and
a second run reporting "1 unchanged".

Refs simp#50

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@nick-markowski

Copy link
Copy Markdown
Member

Nice slice — the hard parts are correct and well-proven: constraints are never revisited (Renovate-bump spec holds byte-for-byte), the transitive assignment fixpoint (pdkmajor_puppet_versionpuppet_version, group-top in template order) is right and the eval-the-result spec guards the whole NameError class, and the two Gemfile stages are order-independent since both do full-write-on-missing. Approving, with two things I'd fix before merge:

1. Drop the warn stdin debug line. It dumps the entire raw JSON input — including the full template — to stderr on every invocation, so a fleet run echoes the template ~100×. Harmless to the result, but clearly not meant to ship.

2. Latent NoMethodError on top-level-gem additions. Case 1 handles inserting a non-group gem, but the assignment block right after runs unconditionally:

tgroup = template_parsed[:groups][tmeta[:group]]   # nil for a top-level gem
group_body = template_lines[(tgroup[:start] + 1)...tgroup[:end]]  # nil[:start] → raises

It can't trigger today (the baseline template has no top-level gems, and _catch_errors would contain it), but the moment a bare gem 'foo' is added to the baseline, every existing repo's stage fails. One guard fixes it — skip the assignment block when tmeta[:group].nil? (top-level gems can't reference group-local vars anyway) — plus a top-level-gem spec, which is the one addition case the suite doesn't cover.

Everything else — parsing, idempotency, removals, missing-group copy, plan scoping/template resolution — checks out.

…ions

- Remove the `warn stdin` debug line (it echoed the full template to
  stderr on every invocation)
- Guard the group-local assignment block for top-level gems: it
  dereferenced template_parsed[:groups][nil], so the first bare
  `gem 'foo'` added to the baseline template would have raised
  NoMethodError on every existing repo. Covered by a new spec (the one
  addition case the suite didn't exercise)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@silug

silug commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Both taken in 349c8cf:

  1. warn stdin dropped.
  2. Good catch on the latent NoMethodError — the top-level insert path fell straight into the group-assignment block, which dereferences template_parsed[:groups][nil]. Guarded with an early next for top-level gems (they can't reference group-local variables anyway), plus the missing spec: a bare top-level gem 'rake' in the template now gets added before the first group without raising. 12 examples, 0 failures.

🤖 Generated with Claude Code

@silug
silug merged commit 25e719c into simp:main Jul 31, 2026
2 checks passed
@silug
silug deleted the issue-50-gemfile-merge branch July 31, 2026 14:56
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.

2 participants