Skip to content

Commit c16f950

Browse files
authored
ci: gate cacti.pot updates on i18n diff instead of regenerating (#35)
* ci: gate cacti.pot updates on i18n diff instead of regenerating The "Verify translation template is up to date" step regenerated locales/po/cacti.pot with locales/build_gettext.sh and compared it (ignoring POT-Creation-Date). That couples the check to an exact gettext/xgettext toolchain and fails on unrelated version drift. Replace it with tests/bin/check-i18n-pot.php, which inspects the pull request diff: if any changed line adds, removes, or edits a Cacti i18n call (__(), __n(), __esc(), ... the same keywords build_gettext.sh feeds xgettext) in a file that feeds the template, then locales/po/cacti.pot must also be part of the pull request. The check fails only when that pot update is missing. The step now runs only on pull_request events and diffs against the PR base SHA, mirroring the existing patch-coverage.php approach. * dev: record diff-based cacti.pot CI gate in changelog
1 parent 759f0d7 commit c16f950

3 files changed

Lines changed: 269 additions & 10 deletions

File tree

‎.github/workflows/plugin-ci-workflow.yml‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -174,18 +174,13 @@ jobs:
174174
find . -path './vendor' -prune -o -type f -name '*.php' -print0 | xargs -0 -r -n1 php -l
175175
176176
- name: Verify translation template is up to date
177+
if: github.event_name == 'pull_request'
178+
env:
179+
BASE_REF: ${{ github.event.pull_request.base.sha }}
177180
run: |
178181
cd ${{ github.workspace }}/cacti/plugins/quicktree
179-
sudo chmod +x locales/build_gettext.sh
180-
sudo ./locales/build_gettext.sh
181-
grep -v '^"POT-Creation-Date:' locales/po/cacti.pot > /tmp/cacti.pot.new
182-
git checkout -- locales/
183-
grep -v '^"POT-Creation-Date:' locales/po/cacti.pot > /tmp/cacti.pot.old
184-
if ! cmp -s /tmp/cacti.pot.old /tmp/cacti.pot.new; then
185-
echo "locales/po/cacti.pot is out of date. Run locales/build_gettext.sh and commit the updated locales/po/cacti.pot."
186-
diff -u /tmp/cacti.pot.old /tmp/cacti.pot.new || true
187-
exit 1
188-
fi
182+
git config --global --add safe.directory ${{ github.workspace }}/cacti/plugins/quicktree
183+
php tests/bin/check-i18n-pot.php "$BASE_REF"
189184
190185
- name: Set expected Cacti version for unit tests
191186
run: echo -n "${{ env.CACTI }}" | sudo tee ${{ github.workspace }}/cacti/plugins/quicktree/tests/.cacti-version > /dev/null

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
--- develop ---
44

5+
* dev: Replace the translation-template CI check that regenerated locales/po/cacti.pot and compared it with a diff-based gate (tests/bin/check-i18n-pot.php) requiring cacti.pot to be updated only when a pull request adds, removes, or modifies an i18n function call
56
* dev: Measure CI coverage with xdebug instead of pcov so the plugin's own sources are instrumented (pcov auto-scopes to the Composer root and skipped cacti/plugins/, leaving the patch-coverage gate with nothing to measure)
67
* dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step
78
* security: Add a version-safe CSP nonce (`plugin_quicktree_csp_nonce()`) to every inline `<script>` tag so pages stay compatible with Cacti's Content-Security-Policy nonce enforcement, while falling back cleanly on older Cacti releases that lack the `CactiSecureHeaders` class

‎tests/bin/check-i18n-pot.php‎

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| This program is free software; you can redistribute it and/or |
7+
| modify it under the terms of the GNU General Public License |
8+
| as published by the Free Software Foundation; either version 2 |
9+
| of the License, or (at your option) any later version. |
10+
+-------------------------------------------------------------------------+
11+
| Cacti: The Complete RRDtool-based Graphing Solution |
12+
+-------------------------------------------------------------------------+
13+
| http://www.cacti.net/ |
14+
+-------------------------------------------------------------------------+
15+
*/
16+
17+
/*
18+
* Fail a pull request that changes translatable strings without refreshing the
19+
* gettext template.
20+
*
21+
* locales/po/cacti.pot is produced by locales/build_gettext.sh, which runs
22+
* xgettext over `find . -maxdepth 2 -name '*.php'` extracting the Cacti i18n
23+
* helpers (__(), __n(), __esc(), ...). Rather than regenerating the template
24+
* and comparing timestamps, this inspects the branch diff: if any changed line
25+
* adds, removes, or edits one of those i18n calls in a file that feeds the
26+
* template, then locales/po/cacti.pot must also be part of the pull request.
27+
*
28+
* It is not this check's job to prove the template is byte-correct, only that
29+
* it was regenerated and committed when the translatable strings moved.
30+
*
31+
* Usage: php tests/bin/check-i18n-pot.php <base-ref>
32+
*
33+
* Exits 1 when a required pot update is missing, 2 on bad input, 0 otherwise.
34+
*/
35+
36+
/*
37+
* The gettext keywords build_gettext.sh passes to xgettext. A changed line is
38+
* only interesting when it contains one of these calls. The negative lookbehind
39+
* keeps PHP magic methods such as __construct()/__toString() out of the match.
40+
*/
41+
$i18n_pattern = '/(?<![A-Za-z0-9_])__(?:gettext|date|esc_xn|esc_x|esc_n|esc|xn|x|n)?\s*\(/';
42+
43+
/* Repository-relative path of the template the plugin must keep in sync. */
44+
$pot_path = 'locales/po/cacti.pot';
45+
46+
$base_ref = isset($argv[1]) ? trim($argv[1]) : '';
47+
48+
if ($base_ref === '') {
49+
$env = getenv('GITHUB_BASE_REF');
50+
$base_ref = ($env !== false) ? trim($env) : '';
51+
}
52+
53+
if ($base_ref === '') {
54+
fwrite(STDOUT, "No base ref supplied; skipping i18n template check (not a pull request).\n");
55+
56+
exit(0);
57+
}
58+
59+
/*
60+
* Make sure the base commit is present. With fetch-depth: 0 it already is, but
61+
* a branch-name ref may still need fetching on a shallow checkout.
62+
*/
63+
$probe = [];
64+
$probe_status = 0;
65+
exec('git rev-parse --verify --quiet ' . escapeshellarg($base_ref . '^{commit}'), $probe, $probe_status);
66+
67+
if ($probe_status !== 0) {
68+
$ignore = [];
69+
$ignore_status = 0;
70+
exec('git fetch --quiet --no-tags --depth=200 origin ' . escapeshellarg($base_ref), $ignore, $ignore_status);
71+
}
72+
73+
$range = escapeshellarg($base_ref) . '...HEAD';
74+
75+
/**
76+
* Normalise a source line so that a pure re-indent or reflow of an i18n call
77+
* does not read as a content change.
78+
*
79+
* @param string $line Raw diff line with its leading +/- already removed.
80+
*
81+
* @return string Whitespace-collapsed, trimmed line.
82+
*/
83+
function normalise_line($line) {
84+
return preg_replace('/\s+/', ' ', trim($line));
85+
}
86+
87+
/**
88+
* Collect the i18n-bearing lines each side of the diff adds or removes, limited
89+
* to files that actually feed the gettext template.
90+
*
91+
* A file feeds the template when it is a PHP file no deeper than one directory
92+
* below the plugin root, mirroring `find . -maxdepth 2 -name '*.php'`.
93+
*
94+
* @param string $range Git diff range expression.
95+
* @param string $i18n_pattern Regex matching the Cacti i18n helper calls.
96+
*
97+
* @return array{added: array<int, string>, removed: array<int, string>, files: array<string, bool>}
98+
*/
99+
function collect_i18n_changes($range, $i18n_pattern) {
100+
$command = 'git diff --no-ext-diff --unified=0 --no-color ' . $range . ' -- "*.php"';
101+
$output = [];
102+
$status = 0;
103+
104+
exec($command, $output, $status);
105+
106+
if ($status !== 0) {
107+
fwrite(STDERR, "git diff failed\n");
108+
109+
exit(2);
110+
}
111+
112+
$added = [];
113+
$removed = [];
114+
$files = [];
115+
$old_file = null;
116+
$new_file = null;
117+
118+
foreach ($output as $line) {
119+
if (strncmp($line, '--- ', 4) === 0) {
120+
$old_file = diff_path(substr($line, 4));
121+
122+
continue;
123+
}
124+
125+
if (strncmp($line, '+++ ', 4) === 0) {
126+
$new_file = diff_path(substr($line, 4));
127+
128+
continue;
129+
}
130+
131+
if ($line === '' || $line[0] !== '+' && $line[0] !== '-') {
132+
continue;
133+
}
134+
135+
if (strncmp($line, '+++', 3) === 0 || strncmp($line, '---', 3) === 0) {
136+
continue;
137+
}
138+
139+
$added_line = ($line[0] === '+');
140+
$file = $added_line ? $new_file : $old_file;
141+
142+
if ($file === null || !feeds_template($file)) {
143+
continue;
144+
}
145+
146+
$content = substr($line, 1);
147+
148+
if (!preg_match($i18n_pattern, $content)) {
149+
continue;
150+
}
151+
152+
$files[$file] = true;
153+
154+
if ($added_line) {
155+
$added[] = normalise_line($content);
156+
} else {
157+
$removed[] = normalise_line($content);
158+
}
159+
}
160+
161+
return ['added' => $added, 'removed' => $removed, 'files' => $files];
162+
}
163+
164+
/**
165+
* Turn a diff header path ("a/foo.php", "b/foo.php" or "/dev/null") into a plain
166+
* repository-relative path, or null when the side does not exist.
167+
*
168+
* @param string $raw Path portion following the "--- "/"+++ " marker.
169+
*
170+
* @return string|null Repository-relative path, or null for /dev/null.
171+
*/
172+
function diff_path($raw) {
173+
$raw = trim($raw);
174+
175+
if ($raw === '/dev/null') {
176+
return null;
177+
}
178+
179+
if (strncmp($raw, 'a/', 2) === 0 || strncmp($raw, 'b/', 2) === 0) {
180+
$raw = substr($raw, 2);
181+
}
182+
183+
return $raw;
184+
}
185+
186+
/**
187+
* Whether a path is scanned by build_gettext.sh, i.e. a PHP file no more than
188+
* one directory below the plugin root.
189+
*
190+
* @param string $file Repository-relative path.
191+
*
192+
* @return bool
193+
*/
194+
function feeds_template($file) {
195+
if (substr($file, -4) !== '.php') {
196+
return false;
197+
}
198+
199+
return substr_count($file, '/') <= 1;
200+
}
201+
202+
/**
203+
* Whether the pull request already touches the gettext template.
204+
*
205+
* @param string $range Git diff range expression.
206+
* @param string $pot_path Repository-relative path of cacti.pot.
207+
*
208+
* @return bool
209+
*/
210+
function pot_updated($range, $pot_path) {
211+
$command = 'git diff --no-ext-diff --name-only --no-color ' . $range;
212+
$output = [];
213+
$status = 0;
214+
215+
exec($command, $output, $status);
216+
217+
if ($status !== 0) {
218+
fwrite(STDERR, "git diff failed\n");
219+
220+
exit(2);
221+
}
222+
223+
foreach ($output as $name) {
224+
if (trim($name) === $pot_path) {
225+
return true;
226+
}
227+
}
228+
229+
return false;
230+
}
231+
232+
$changes = collect_i18n_changes($range, $i18n_pattern);
233+
234+
sort($changes['added']);
235+
sort($changes['removed']);
236+
237+
$i18n_changed = ($changes['added'] !== $changes['removed']);
238+
239+
if (!$i18n_changed) {
240+
fwrite(STDOUT, "No translatable-string changes detected in the diff; locales/po/cacti.pot update not required.\n");
241+
242+
exit(0);
243+
}
244+
245+
if (pot_updated($range, $pot_path)) {
246+
fwrite(STDOUT, "Translatable strings changed and locales/po/cacti.pot is included in the pull request. OK.\n");
247+
248+
exit(0);
249+
}
250+
251+
$files = array_keys($changes['files']);
252+
sort($files);
253+
254+
fwrite(STDERR, "This pull request changes i18n strings but does not update locales/po/cacti.pot.\n");
255+
fwrite(STDERR, "Run locales/build_gettext.sh and commit the regenerated locales/po/cacti.pot.\n");
256+
fwrite(STDERR, "\n");
257+
fwrite(STDERR, "Files with changed i18n calls:\n");
258+
259+
foreach ($files as $file) {
260+
fwrite(STDERR, " - $file\n");
261+
}
262+
263+
exit(1);

0 commit comments

Comments
 (0)