Skip to content

Commit 33fb171

Browse files
authored
fix(release): round-2 mutator fixes discovered in rel-820 cut first exercise (openemr#12731)
Fixes 5 bugs surfaced by today's rel-820 cut — the first end-to-end production exercise of the branch-cut + release-prep + release-finalize automation. Four PRs currently open on rel-820 (openemr#12725 release-prep, openemr#12726 release-finalize, openemr#12727 branch-cut rel-side, openemr#12728 branch-cut master-side) each hit at least one of these. After this PR lands: close those four + delete their branches + delete rel-820 + recreate — the four cut-time PRs re-open fresh against the corrected mutators. ## Changes **1. `DockerUpgradeScaffoldMutator`** — generate `fsupgrade-(N+1).sh` by copying `fsupgrade-N.sh` in full and applying five line-level substitutions (docker-version `N` → `N+1` in the header comment and two echoes; prior-openemr-version marker in header comment and in the `priorOpenemrVersion` shell variable). Previously wrote a stub that failed ShellCheck (SC2034 on `priorOpenemrVersion`) and lacked a trailing newline. Copy-in-full preserves the shellcheck directives, trailing newline, and existing upgrade body from the prior file. New test fixture: real `fsupgrade-11.sh` input + golden `fsupgrade-12.sh.expected` showing exactly the 5-line diff. **2. `PostReleaseTargetsMutator`** — early-return with a skipped `MutatorResult` when no live row exists for the target rel branch in `release-targets.yml`. Previously ran the "drop `latest` from prior holder" step unconditionally, leaving the file with no row carrying `latest` when release-finalize fired prematurely at cut time (openemr#12726's symptom). **3. `ReleasePrepCommand`** — remove `GlobalsIncMutator` from the rel-side mutator list. Cut owns the `allow_debug_language` flip; running it again at release-prep was defensive but hid the "did cut merge?" question behind mutator idempotency. **4. `DockerComposeProductionMutator`** — drop digest handling entirely. Previously pinned `openemr/openemr:latest@sha256:X` → `openemr/openemr:VERSION@sha256:X`, requiring `--image-digest` at release-prep time. Chicken-and-egg: the release image is built after the tag fires, which requires release-prep to merge. New behavior swaps only the tag: `openemr/openemr:latest[@sha256:X]` → `openemr/openemr:VERSION` (no digest). `MutatorContext::imageDigest` field and the `--image-digest` CLI option are removed alongside. **5. Docs updated** — `docs/release-automation-plan.md` (release-prep mutator bullets reflect the new behavior) and `docs/RELEASE_PROCESS.md` (the short "conductor rewrites" summary catches up). ## Test coverage Per-change adequate coverage: | Change | New/updated tests | |---|---| | 1. DockerUpgradeScaffold | 5 new methods + real fsupgrade-11 input + golden fsupgrade-12 expected | | 2. PostReleaseTargets | 2 new methods (no-live-row + only-unreleased-placeholder cases) | | 3. ReleasePrepCommand | 1 new method asserting GlobalsIncMutator absent | | 4. DockerComposeProduction | 2 test methods in MutatorTest.php updated to reference new `pinned_tag_only_expected.yml` fixture (3 old fixtures deleted) | | 5. MutatorContext | `imageDigest`-specific tests removed cleanly | Net delta: **+4 tests** on the isolated suite (new tests minus removed imageDigest-specific). ## Verification - `pit` clean: **3587 tests pass** in a freshly-nuked-and-recreated docker stack - Pre-commit hooks all green (phpstan, phpcs, rector, composer-require-checker, codespell, conventional-commits) - Manual audit for `imageDigest` residual references: only remaining mention is in `DockerComposeProductionMutator.php`'s docstring explaining the historical why (valid documentation) - No TODO/FIXME markers left ## After merge Clean-slate rebuild for the rel-820 cut: ``` gh pr close 12725 12726 12727 12728 --repo openemr/openemr --delete-branch git push origin --delete rel-820 git push origin master:rel-820 ``` All 4 workflows re-fire with the corrected mutators → 4 fresh PRs against the fixed state.
1 parent 1d39d67 commit 33fb171

25 files changed

Lines changed: 503 additions & 278 deletions

docs/RELEASE_PROCESS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ Common envelope on every event: `{ event, repo, sha, actor, dispatched_at, data
9696

9797
Long-lived draft PR against the `rel-*` branch, force-updated by [`.github/workflows/release-prep.yml`](../.github/workflows/release-prep.yml) on every push to a production release branch (matching `rel-[0-9]*0`, with `docs/**`-only pushes ignored). Test branches like `rel-test` go through `workflow_dispatch` instead. The mechanical edits applied by `bin/console openemr:release-prep` are documented in [`docs/release-automation-plan.md`](release-automation-plan.md) (the conductor slice plan).
9898

99-
In short, the conductor rewrites: `version.php`, `library/globals.inc.php` (debug toggle), `docker/production/docker-compose.yml` (image pin), `src/RestControllers/OpenApi/OpenApiDefinitions.php`, `swagger/openemr-api.yaml` (regenerated from the CLI), every `docker-version` file, and (on master) a fresh `sql/X_Y_Z-to-X_Y_Z+1_upgrade.sql` skeleton.
99+
In short, the conductor rewrites: `version.php`, `docker/production/docker-compose.yml` (image tag pin), `src/RestControllers/OpenApi/OpenApiDefinitions.php`, and `swagger/openemr-api.yaml` (regenerated from the CLI). (The `library/globals.inc.php` debug toggle is applied by the sibling branch-cut conductor at cut time; release-prep does not re-apply it. The `docker-version` triple and `sql/*_upgrade.sql` skeleton are scaffolded at branch-cut / patch-prep time, not at ship time.)
100100

101101
### Docs PR — `release-docs/<version>` in `openemr/website-openemr`
102102

docs/release-automation-plan.md

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,13 @@ be auto-derived by decrementing minor below 1).
8888
**Rel-side PR** (`branch-cut/<rel-branch>`, base `<rel-branch>`,
8989
ready-for-review):
9090

91-
- `DockerUpgradeScaffoldMutator` — new `fsupgrade-N.sh` skeleton +
92-
Dockerfile manifest updates (both `COPY` and `chmod` blocks). Same
93-
scaffolding the docker-upgrade-actions memo enumerates as mandatory
94-
per release.
91+
- `DockerUpgradeScaffoldMutator` — new `fsupgrade-N.sh` (copied in full
92+
from the prior `fsupgrade-N-1.sh` with only the docker-version /
93+
priorOpenemrVersion header lines substituted; the upgrade body is
94+
preserved byte-for-byte from the prior file for per-release refinement
95+
in place) + Dockerfile manifest updates (both `COPY` and `chmod`
96+
blocks). Same scaffolding the docker-upgrade-actions memo enumerates
97+
as mandatory per release.
9598
- `DockerfileOpenemrVersionMutator` — pins the `OPENEMR_VERSION` ARG
9699
in the branch's Dockerfile to the new rel line.
97100
- `TranslationFileCopyFromPriorRelMutator` — fetches the translation
@@ -213,17 +216,24 @@ for the full picture.
213216
draft, force-pushed on each run):
214217

215218
- `VersionPhpMutator` — strip `-dev` suffix from `$v_tag`.
216-
- `GlobalsIncMutator`reused from branch-cut (idempotent, so re-running
217-
is safe if the branch-cut PR wasn't merged before dev started).
218-
- `DockerComposeProductionMutator` — pin `openemr/openemr:latest@sha256:…`
219-
to `openemr/openemr:<version>@sha256:…`. Image digest is supplied via
220-
`--image-digest` from the workflow after the release image is published;
221-
if absent, the existing digest is preserved and only the tag is swapped.
219+
- `DockerComposeProductionMutator`swap the openemr image tag:
220+
`openemr/openemr:latest[@sha256:…]``openemr/openemr:<version>` (no
221+
digest). At release-prep time the release image doesn't yet exist in
222+
Docker Hub (the tag→build sequence fires only after release-prep merges),
223+
so there is no valid digest to pin. Any existing `@sha256:…` suffix on
224+
the source line is dropped.
222225
- `OpenApiVersionMutator` — bump `#[OA\Info(title: 'OpenEMR API', version: 'X.Y.Z')]`.
223226
(The wiki + earlier drafts of this plan said `_rest_routes.inc.php` but
224227
the version constant actually lives in `src/RestControllers/OpenApi/OpenApiDefinitions.php`.)
225228
- `SwaggerRegenMutator` — regenerate `swagger/openemr-api.yaml`.
226229

230+
`GlobalsIncMutator` is intentionally NOT in the release-prep rel-side
231+
list even though branch-cut runs it: re-running it here would be
232+
defensive but redundant, and it hides the "did branch-cut merge?"
233+
question behind mutator idempotency. Surface as a real diff if
234+
branch-cut hasn't landed yet. (Removed in round-2 fixes after the
235+
rel-820 first exercise; see PR #12725.)
236+
227237
The `docker-version` files and `sql/*_upgrade.sql` skeleton the earlier
228238
draft of this plan enumerated for the release-prep PR moved to
229239
**branch-cut / patch-prep** (they're now scaffolded at the lifecycle
@@ -302,10 +312,12 @@ content on the other.
302312
**File overlap between the two pairs.** The branch-cut and conductor
303313
PRs against the same base branch touch some of the same files:
304314

305-
- Rel-side both PRs touch `library/globals.inc.php` (branch-cut flips
306-
`allow_debug_language``'0'`; release-prep's `GlobalsIncMutator` is
307-
reused and idempotent, so a re-render sees the flag already flipped
308-
and no-ops).
315+
- Rel-side branch-cut touches `library/globals.inc.php` (flips
316+
`allow_debug_language``'0'`). Release-prep does NOT re-run the
317+
flip — running it twice is defensive but redundant, and it hides the
318+
"did branch-cut merge?" question behind mutator idempotency. If
319+
branch-cut hasn't landed yet, the flip surfaces as a real diff at
320+
ship time instead of a silent no-op.
309321
- Rel-side both touch the `docker-version` triple (branch-cut bumps by
310322
one for the new minor's dev cycle; release-prep leaves them alone at
311323
ship time).
@@ -406,8 +418,7 @@ against a shared framework at [`src/Common/Command/ReleasePrep/`](../src/Common/
406418
- **`MutatorInterface`**`name(): string` + `apply(MutatorContext): MutatorResult`.
407419
- **`MutatorContext`**`readonly` value object carrying `projectDir`,
408420
parsed `major`/`minor`/`patch`, and the optional context fields the
409-
various mutators need: `imageDigest` (release-prep only, when
410-
`--image-digest` is passed), `relBranch` (release-finalize, branch-cut,
421+
various mutators need: `relBranch` (release-finalize, branch-cut,
411422
patch-prep), `prevRelBranch` (branch-cut rel-side translation copy),
412423
`fromVersion` (patch-prep SQL skeleton override — validated to be
413424
same major/minor as target with patch == target - 1). Constructor
@@ -428,8 +439,8 @@ mutators would produce churn PRs.
428439
|---------|---------|---------|
429440
| `VersionPhpMutator` | release-prep (rel) | Strip `-dev` from `$v_tag`. |
430441
| `VersionPhpMasterMutator` | branch-cut (master) | Advance to next-minor `-dev`. |
431-
| `GlobalsIncMutator` | branch-cut (rel), release-prep (rel) | `allow_debug_language = 0`. |
432-
| `DockerComposeProductionMutator` | release-prep (rel) | Pin `openemr/openemr` tag + digest. |
442+
| `GlobalsIncMutator` | branch-cut (rel) | `allow_debug_language = 0`. |
443+
| `DockerComposeProductionMutator` | release-prep (rel) | Pin `openemr/openemr` tag (no digest). |
433444
| `DockerfileOpenemrVersionMutator` | branch-cut (rel) | Pin Dockerfile `OPENEMR_VERSION` ARG. |
434445
| `DockerUpgradeScaffoldMutator` | branch-cut (rel + master), patch-prep (rel + master) | New `fsupgrade-N.sh` + Dockerfile manifest wiring. |
435446
| `TranslationFileCopyFromPriorRelMutator` | branch-cut (rel) | Fetch translation blob from prior rel branch. |

src/Common/Command/BranchCutCommand.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
139139
$targetContext = MutatorContext::fromVersionString(
140140
$projectDir,
141141
$target,
142-
null,
143142
$relBranch,
144143
$prevRelBranch,
145144
);
@@ -161,7 +160,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
161160
$targetContext->major,
162161
$targetContext->minor + 1,
163162
0,
164-
null,
165163
$targetContext->relBranch,
166164
$targetContext->prevRelBranch,
167165
);

src/Common/Command/PatchPrepCommand.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
134134
$context = MutatorContext::fromVersionString(
135135
$projectDir,
136136
$target,
137-
null,
138137
$relBranch,
139138
null,
140139
$prevVersion,

src/Common/Command/ReleasePrep/Mutator/DockerComposeProductionMutator.php

Lines changed: 21 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@
22

33
/**
44
* Pin docker/production/docker-compose.yml's openemr image from
5-
* `latest@sha256:...` to `<version>@sha256:...`. The published image's
6-
* digest is supplied via --image-digest by the conductor workflow after
7-
* the release image has been built and pushed; if absent the existing
8-
* digest is preserved and only the tag is swapped.
5+
* `latest[@sha256:...]` to `<version>` (tag only, no digest).
6+
*
7+
* At release-prep time, the release-tagged image does not yet exist in
8+
* Docker Hub — it's produced downstream only AFTER the release-prep PR
9+
* merges (which triggers the tag creation which triggers the image
10+
* build). So there's no valid digest to pin at this stage. The old
11+
* `--image-digest` pathway assumed an out-of-band digest lookup that
12+
* doesn't fit the actual sequencing; drop it and just swap the tag.
913
*
1014
* @package OpenEMR
1115
* @link http://www.open-emr.org
@@ -30,7 +34,7 @@
3034

3135
public function name(): string
3236
{
33-
return 'docker/production/docker-compose.yml (pin openemr image)';
37+
return 'docker/production/docker-compose.yml (pin openemr image tag)';
3438
}
3539

3640
public function apply(MutatorContext $context): MutatorResult
@@ -42,29 +46,18 @@ public function apply(MutatorContext $context): MutatorResult
4246
}
4347

4448
$version = $context->versionString();
45-
// Accept the digest with or without the `sha256:` prefix; we always
46-
// emit `@sha256:<hex>` regardless.
47-
$newDigestHex = $context->imageDigest === null
48-
? null
49-
: preg_replace('/^sha256:/', '', $context->imageDigest);
5049

5150
// Pattern matches `image: openemr/openemr:<tag>` with an optional
5251
// `@sha256:<digest>` suffix. The tag may be `latest`, the target
53-
// version, or another version (idempotence + handles re-runs after
54-
// a digest update). The digest is optional because rel-* branches
55-
// that were cut before the digest-pinning automation existed start
56-
// with a bare tag (e.g. `openemr/openemr:8.1.0`).
57-
$pattern = '/(image:\s*openemr\/openemr:)([^@\s]+)(?:(@sha256:)([0-9a-f]{64}))?/';
52+
// version, or another version (idempotence + handles re-runs).
53+
// The digest is optional because rel-* branches that were cut
54+
// before the digest-pinning automation existed start with a bare
55+
// tag (e.g. `openemr/openemr:8.1.0`). Whatever's after `:`, we
56+
// replace it with the target version and strip the digest entirely.
57+
$pattern = '/(image:\s*openemr\/openemr:)([^@\s]+)(?:@sha256:[0-9a-f]{64})?/';
5858
$updated = preg_replace_callback(
5959
$pattern,
60-
static function (array $match) use ($version, $newDigestHex): string {
61-
$existingDigest = $match[4] ?? '';
62-
$digest = $newDigestHex ?? ($existingDigest !== '' ? $existingDigest : null);
63-
if ($digest === null) {
64-
return $match[1] . $version;
65-
}
66-
return $match[1] . $version . '@sha256:' . $digest;
67-
},
60+
static fn (array $match): string => $match[1] . $version,
6861
$contents,
6962
1,
7063
$count,
@@ -95,10 +88,7 @@ static function (array $match) use ($version, $newDigestHex): string {
9588
$e,
9689
);
9790
}
98-
$expectedDigest = $newDigestHex ?? $this->extractDigest($contents);
99-
$expectedImage = $expectedDigest === null
100-
? sprintf('openemr/openemr:%s', $version)
101-
: sprintf('openemr/openemr:%s@sha256:%s', $version, $expectedDigest);
91+
$expectedImage = sprintf('openemr/openemr:%s', $version);
10292
$actualImage = is_array($parsed)
10393
&& is_array($parsed['services'] ?? null)
10494
&& is_array($parsed['services']['openemr'] ?? null)
@@ -114,21 +104,9 @@ static function (array $match) use ($version, $newDigestHex): string {
114104
if (file_put_contents($path, $updated) === false) {
115105
throw new \RuntimeException('Cannot write ' . $path);
116106
}
117-
if ($newDigestHex !== null) {
118-
$messages = [];
119-
} elseif ($expectedDigest === null) {
120-
$messages = ['docker-compose.yml: tag pinned to ' . $version . '; no digest (source had none, no --image-digest provided)'];
121-
} else {
122-
$messages = ['docker-compose.yml: tag pinned to ' . $version . '; digest preserved (no --image-digest provided)'];
123-
}
124-
return new MutatorResult([self::RELATIVE_PATH], $messages);
125-
}
126-
127-
private function extractDigest(string $original): ?string
128-
{
129-
if (preg_match('/image:\s*openemr\/openemr:[^@\s]+@sha256:([0-9a-f]{64})/', $original, $m) !== 1) {
130-
return null;
131-
}
132-
return $m[1];
107+
return new MutatorResult(
108+
[self::RELATIVE_PATH],
109+
['docker-compose.yml: tag pinned to ' . $version . ' (no digest — release image is not yet built at release-prep time)'],
110+
);
133111
}
134112
}

0 commit comments

Comments
 (0)