From 72e708a297b6b5a8845bb88d032d7a07744ce731 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Thu, 8 Oct 2026 06:21:08 +0000 Subject: [PATCH] Refresh REFERENCE.md in every committing sync session openvox-strings releases change the generated REFERENCE.md (7.2.0 dropped the trailing blank line), and the pupmod PR tests fail any PR whose REFERENCE.md doesn't match. A session that didn't list the generate_reference_md stage then opened PRs that failed over drift they didn't cause. The sync plan now adds generate_reference_md to any session whose stages include git_commit_changes. Run implicitly, the task gets the new only_changed parameter and skips modules without a REFERENCE.md and modules that no earlier stage changed, so it never creates a REFERENCE.md or widens a session to repos it had no other reason to touch. Listing the stage keeps the old behavior: regenerate in every pupmod in the session. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++ dist/puppetsync/plans/init.pp | 20 ++++- .../tasks/generate_reference_md.json | 4 + .../puppetsync/tasks/generate_reference_md.rb | 18 +++++ .../plans/generate_reference_md_stage_spec.rb | 81 +++++++++++++++++++ spec/tasks/generate_reference_md_spec.rb | 40 +++++++++ 6 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 spec/plans/generate_reference_md_stage_spec.rb diff --git a/README.md b/README.md index 3c3f347..ebf9ff1 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,13 @@ puppetsync: - ensure_github_pr ``` +A session whose stages include `git_commit_changes` also runs +`generate_reference_md` even when it isn't listed, so PRs don't fail the +pupmod REFERENCE.md freshness check when an openvox-strings release +changes the generated output. Run this way, it only regenerates +REFERENCE.md in modules that already have one and that an earlier stage +changed. Listing the stage regenerates it in every pupmod in the session. + ## Reference diff --git a/dist/puppetsync/plans/init.pp b/dist/puppetsync/plans/init.pp index 4b90ce4..81b55e3 100644 --- a/dist/puppetsync/plans/init.pp +++ b/dist/puppetsync/plans/init.pp @@ -101,11 +101,26 @@ Hash $options = {}, ) { - $opts = { + $session_opts = { 'clone_git_repos' => true, 'github_api_delay_seconds' => 5, } + getvar('puppetsync_config.puppetsync.plans.sync').lest || {{}} + $options + # Every session that commits also refreshes REFERENCE.md, whether or not + # its config lists the generate_reference_md stage: openvox-strings + # releases change the generated output, and the pupmod PR tests fail any + # PR whose REFERENCE.md is stale. Run implicitly, the stage only touches + # modules that already have a REFERENCE.md and that an earlier stage + # changed, so it never widens a session to repos it had no other reason + # to touch (see the 'only_changed' task parameter). + $listed_stages = $session_opts['stages'] + $implicit_reference_md = ($listed_stages =~ Array and + 'git_commit_changes' in $listed_stages and !('generate_reference_md' in $listed_stages)) + $opts = $implicit_reference_md ? { + true => $session_opts + { 'stages' => $listed_stages + ['generate_reference_md'] }, + default => $session_opts, + } + # Dynamic inventory (simp/puppetsync#55): when the repolist file defines # `puppetsync::repos_source`, build the repo list from the GitHub API and # snapshot it so approve/merge plans can reuse the exact same inventory. @@ -537,7 +552,8 @@ '_catch_errors' => true, ) |$repo| { { - 'repo_path' => $repo.vars['repo_path'], + 'repo_path' => $repo.vars['repo_path'], + 'only_changed' => $implicit_reference_md, } } } diff --git a/dist/puppetsync/tasks/generate_reference_md.json b/dist/puppetsync/tasks/generate_reference_md.json index 4dd4544..b095a6a 100644 --- a/dist/puppetsync/tasks/generate_reference_md.json +++ b/dist/puppetsync/tasks/generate_reference_md.json @@ -6,6 +6,10 @@ "description": "Path to the module repository", "type": "String[1]" }, + "only_changed": { + "description": "Skip repos without a REFERENCE.md or without uncommitted changes from earlier stages (default: false)", + "type": "Optional[Boolean]" + }, "bundle_path": { "description": "Bundle install path, shared across repos for speed (default: /../../.vendor/bundle)", "type": "Optional[String[1]]" diff --git a/dist/puppetsync/tasks/generate_reference_md.rb b/dist/puppetsync/tasks/generate_reference_md.rb index 4ec12e1..def0123 100755 --- a/dist/puppetsync/tasks/generate_reference_md.rb +++ b/dist/puppetsync/tasks/generate_reference_md.rb @@ -9,6 +9,10 @@ # fleet run only downloads each gem once. The commit belongs to the # git_commit_changes stage, like every other transformation — this task # only changes the working tree and reports whether REFERENCE.md moved. +# +# With only_changed (how the plan runs it when a session doesn't list the +# stage), a repo is left alone unless it already has a REFERENCE.md and an +# earlier stage left changes in the working tree. require 'json' require 'open3' @@ -22,6 +26,15 @@ def run(cmd, chdir:, env: {}) out end +def skip_reason(repo_path) + return 'no REFERENCE.md' unless File.exist?(File.join(repo_path, 'REFERENCE.md')) + + status_out, = Open3.capture2e('git', '-C', repo_path, 'status', '--porcelain') + return 'no other changes' if status_out.strip.empty? + + nil +end + def generate_reference(repo_path, bundle_path) require 'bundler' Bundler.with_unbundled_env do @@ -43,6 +56,11 @@ def generate_reference(repo_path, bundle_path) repo_path = params['repo_path'] raise('No repo_path given') unless repo_path +if params['only_changed'] && (reason = skip_reason(repo_path)) + puts JSON.generate({ 'changed' => false, 'skipped' => reason }) + exit 0 +end + bundle_path = params['bundle_path'] || File.expand_path(File.join(repo_path, '..', '..', '.vendor', 'bundle')) puts JSON.generate(generate_reference(repo_path, bundle_path)) diff --git a/spec/plans/generate_reference_md_stage_spec.rb b/spec/plans/generate_reference_md_stage_spec.rb new file mode 100644 index 0000000..0e45771 --- /dev/null +++ b/spec/plans/generate_reference_md_stage_spec.rb @@ -0,0 +1,81 @@ +require_relative 'plan_spec_helper' +require 'fileutils' +require 'tmpdir' + +describe 'plan: puppetsync (generate_reference_md stage)' do + include_context 'puppetsync plan specs' + + let(:repos_config) do + { 'https://github.com/simp/repo-a' => { 'branch' => 'master' } } + end + + def puppetsync_config(stages) + { + 'puppetsync' => { + 'plans' => { + 'sync' => { + 'clone_git_repos' => false, + 'filter_permitted_repos' => false, + 'stages' => stages, + }, + }, + }, + 'git' => { 'feature_branch' => 'SIMP-TEST', 'commit_message' => 'spec commit' }, + } + end + + around(:each) do |example| + Dir.mktmpdir do |dir| + @project_dir = dir + repo = File.join(dir, '_repos', 'repo-a') + FileUtils.mkdir_p(repo) + File.write(File.join(repo, 'metadata.json'), <<~JSON) + { + "name": "simp-repoa", "version": "1.0.0", "author": "SIMP", + "license": "Apache-2.0", "summary": "spec fixture", "dependencies": [] + } + JSON + example.run + end + end + + def run_sync(stages) + calls = [] + allow_task('puppetsync::generate_reference_md').return do |targets:, task:, params:| + calls << params + Bolt::ResultSet.new(targets.map { |t| Bolt::Result.new(t, value: { 'changed' => false }, action: 'task', object: task) }) + end + allow_task('puppetsync::git_commit').return do |targets:, task:, params:| + Bolt::ResultSet.new(targets.map { |t| Bolt::Result.new(t, value: { 'changed' => false }, action: 'task', object: task) }) + end + allow_task('puppetsync::install_gems').always_return({}) + allow_out_message + + result = run_plan('puppetsync', { + 'project_dir' => @project_dir, + 'puppetsync_config' => puppetsync_config(stages), + 'repos_config' => repos_config, + }) + expect(result.ok?).to be(true), result.value.to_s + calls + end + + it 'refreshes REFERENCE.md in changed repos when a committing session does not list the stage' do + calls = run_sync(['git_commit_changes']) + + expect(calls.length).to eq(1) + expect(File.basename(calls.first['repo_path'])).to eq('repo-a') + expect(calls.first['only_changed']).to be(true) + end + + it 'regenerates unconditionally when the session lists the stage' do + calls = run_sync(%w[generate_reference_md git_commit_changes]) + + expect(calls.length).to eq(1) + expect(calls.first['only_changed']).to be(false) + end + + it 'does not run in a session that does not commit' do + expect(run_sync(['install_gems'])).to be_empty + end +end diff --git a/spec/tasks/generate_reference_md_spec.rb b/spec/tasks/generate_reference_md_spec.rb index ff1eca5..6ffcf73 100644 --- a/spec/tasks/generate_reference_md_spec.rb +++ b/spec/tasks/generate_reference_md_spec.rb @@ -82,6 +82,46 @@ def fake_bundler(script_body) expect(stderr).to include('no REFERENCE.md') end + context 'with only_changed' do + let(:bundler) { fake_bundler('[ "$1" = "exec" ] && echo "new reference" > REFERENCE.md; exit 0') } + + it 'regenerates REFERENCE.md when an earlier stage changed the repo' do + File.write(File.join(@repo, 'metadata.json'), "{}\n") + + stdout, stderr, status = run_task('generate_reference_md.rb', + { 'repo_path' => @repo, 'only_changed' => true }, + { 'BUNDLER_EXE' => bundler }) + + expect(status).to be_success, stderr + expect(JSON.parse(stdout)).to eq('changed' => true) + expect(File.read(File.join(@repo, 'REFERENCE.md'))).to eq("new reference\n") + end + + it 'leaves a repo with no other changes alone' do + stdout, stderr, status = run_task('generate_reference_md.rb', + { 'repo_path' => @repo, 'only_changed' => true }, + { 'BUNDLER_EXE' => bundler }) + + expect(status).to be_success, stderr + expect(JSON.parse(stdout)).to eq('changed' => false, 'skipped' => 'no other changes') + expect(git('status', '--porcelain')).to be_empty + end + + it 'does not create a REFERENCE.md in a module without one' do + git('rm', '-q', 'REFERENCE.md') + git('commit', '-q', '-m', 'drop REFERENCE.md') + File.write(File.join(@repo, 'metadata.json'), "{}\n") + + stdout, stderr, status = run_task('generate_reference_md.rb', + { 'repo_path' => @repo, 'only_changed' => true }, + { 'BUNDLER_EXE' => bundler }) + + expect(status).to be_success, stderr + expect(JSON.parse(stdout)).to eq('changed' => false, 'skipped' => 'no REFERENCE.md') + expect(File.exist?(File.join(@repo, 'REFERENCE.md'))).to be(false) + end + end + it 'fails when repo_path is missing' do _stdout, _stderr, status = run_task('generate_reference_md.rb', {}) expect(status).not_to be_success