Skip to content

Commit 0dd9717

Browse files
committed
refactor(release): parse override list in PHP, drop shell construction
Review feedback on #717: - Move newline-delimited override parsing from a bash step into the PHP CLI (--overrides flag). Reusable workflow becomes a single quoted-arg invocation; no GITHUB_OUTPUT round-trip, no SC2086 word-splitting, no shell-injection surface for caller-controlled path_overrides. - Rename self-test workflow file (drop _ prefix; no other workflow in the repo uses one). - Rename CLI locals canon/cons → canonicalPath/consumerPath; less shadowing of the surrounding $canonical option variable. No behavior change for existing --override callers. Refs #664, #717. Assisted-by: Claude Code
1 parent a0b2dfd commit 0dd9717

3 files changed

Lines changed: 30 additions & 28 deletions

File tree

.github/workflows/check-vendored-contracts.yml

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -73,29 +73,9 @@ jobs:
7373
working-directory: _canonical-openemr-devops/tools/release
7474
run: composer install --no-interaction --no-progress --no-dev
7575

76-
- name: Build override flags
77-
id: overrides
78-
env:
79-
PATH_OVERRIDES: ${{ inputs.path_overrides }}
80-
run: |
81-
flags=''
82-
while IFS= read -r line; do
83-
line="${line## }"
84-
line="${line%% }"
85-
[[ -z $line ]] && continue
86-
flags+=" --override $line"
87-
done <<< "$PATH_OVERRIDES"
88-
{
89-
echo "flags<<EOF"
90-
echo "$flags"
91-
echo "EOF"
92-
} >> "$GITHUB_OUTPUT"
93-
9476
- name: Check vendored contracts
9577
env:
9678
CONSUMER_PATH: ${{ github.workspace }}/${{ inputs.consumer_subpath }}
97-
OVERRIDE_FLAGS: ${{ steps.overrides.outputs.flags }}
79+
PATH_OVERRIDES: ${{ inputs.path_overrides }}
9880
working-directory: _canonical-openemr-devops/tools/release
99-
run: |
100-
# shellcheck disable=SC2086 # OVERRIDE_FLAGS is a flag list; word-split is intentional
101-
php bin/check-vendored.php --consumer "$CONSUMER_PATH" $OVERRIDE_FLAGS
81+
run: php bin/check-vendored.php --consumer "$CONSUMER_PATH" --overrides "$PATH_OVERRIDES"

.github/workflows/_test-vendored-contracts.yml renamed to .github/workflows/vendored-contracts-self-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ on:
2222
pull_request:
2323
paths:
2424
- '.github/workflows/check-vendored-contracts.yml'
25-
- '.github/workflows/_test-vendored-contracts.yml'
25+
- '.github/workflows/vendored-contracts-self-test.yml'
2626
- 'tools/release/bin/check-vendored.php'
2727
- 'tools/release/src/VendoredFileChecker.php'
2828
- 'tools/release/src/VendoredDriftIssue.php'

tools/release/bin/check-vendored.php

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@
4949
'Map a canonical path to a different consumer-relative path '
5050
. '(e.g. --override src/TagVerifier.php=src/Release/TagVerifier.php). Repeatable.',
5151
)
52+
->addOption(
53+
'overrides',
54+
null,
55+
InputOption::VALUE_REQUIRED,
56+
'Same mapping as --override but as a single newline-delimited string '
57+
. '(blank lines and surrounding whitespace ignored). Convenient for '
58+
. 'CI inputs that arrive as multi-line strings. Combines with --override.',
59+
)
5260
->setCode(function (InputInterface $input, OutputInterface $output): int {
5361
$canonical = $input->getOption('canonical');
5462
if (!is_string($canonical) || $canonical === '') {
@@ -74,21 +82,35 @@
7482
if (!is_array($rawOverrides)) {
7583
$rawOverrides = [];
7684
}
85+
$multiline = $input->getOption('overrides');
86+
if (is_string($multiline) && $multiline !== '') {
87+
$lines = preg_split('/\R/', $multiline);
88+
if ($lines === false) {
89+
$output->writeln('<error>--overrides could not be parsed</error>');
90+
return 1;
91+
}
92+
foreach ($lines as $line) {
93+
$trimmed = trim($line);
94+
if ($trimmed !== '') {
95+
$rawOverrides[] = $trimmed;
96+
}
97+
}
98+
}
7799
$overrides = [];
78100
foreach ($rawOverrides as $entry) {
79101
if (!is_string($entry) || !str_contains($entry, '=')) {
80102
$output->writeln(sprintf(
81-
'<error>--override expects CANONICAL=CONSUMER, got: %s</error>',
103+
'<error>override entry expects CANONICAL=CONSUMER, got: %s</error>',
82104
is_string($entry) ? $entry : gettype($entry),
83105
));
84106
return 1;
85107
}
86-
[$canon, $cons] = explode('=', $entry, 2);
87-
if ($canon === '' || $cons === '') {
88-
$output->writeln(sprintf('<error>--override has empty side: %s</error>', $entry));
108+
[$canonicalPath, $consumerPath] = explode('=', $entry, 2);
109+
if ($canonicalPath === '' || $consumerPath === '') {
110+
$output->writeln(sprintf('<error>override entry has empty side: %s</error>', $entry));
89111
return 1;
90112
}
91-
$overrides[$canon] = $cons;
113+
$overrides[$canonicalPath] = $consumerPath;
92114
}
93115

94116
try {

0 commit comments

Comments
 (0)