diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5407a3a..747988f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: persist-credentials: false - name: ShellCheck - run: shellcheck -x git-extensions.sh git-release-style.sh git-release-langpack.sh lib/github-release.sh lib/release-source.sh tests/test-release-scripts.sh tests/test-release-api.sh phpbb-test-board/setup-board.sh phpbb-test-board/run-unit-tests.sh phpbb-test-board/php-lint.sh phpbb-test-board/db-matrix.sh tests/test-phpbb-test-board.sh tests/test-pr-wait.sh + run: shellcheck -x git-extensions.sh git-release-style.sh git-release-langpack.sh lib/github-release.sh lib/release-source.sh tests/test-release-scripts.sh tests/test-release-api.sh phpbb-test-board/setup-board.sh phpbb-test-board/run-unit-tests.sh phpbb-test-board/php-lint.sh phpbb-test-board/db-matrix.sh tests/test-phpbb-test-board.sh tests/test-pr-wait.sh tests/test-normalize-composer.sh - name: Tests run: tests/test-release-scripts.sh @@ -32,3 +32,6 @@ jobs: - name: pr-wait.py tests run: tests/test-pr-wait.sh + + - name: normalize-composer.py tests + run: tests/test-normalize-composer.sh diff --git a/README.md b/README.md index 7d2c351..56595b4 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ Build phpBB package ZIPs, publish GitHub Releases, wait for PR checks, and test | [`git-release-style.sh`](git-release-style.sh) | Package and release styles | | [`git-release-langpack.sh`](git-release-langpack.sh) | Validate, package, and release language packs | | [`pr-wait.py`](pr-wait.py) | Wait for a pull request's checks | +| [`normalize-composer.py`](normalize-composer.py) | Bring an extension's `composer.json` to the phpBB 3.3.19 / PHP 8.3.36 baseline | | [`phpbb-test-board/`](phpbb-test-board/) | Set up a local board, run checks, and take screenshots | ## Quick start diff --git a/normalize-composer.py b/normalize-composer.py new file mode 100755 index 0000000..4c778b2 --- /dev/null +++ b/normalize-composer.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python3 +""" +Bring phpBB extensions' composer.json to the phpbbmodders baseline. + +For each extension checkout given, this changes only: + - require.php set to --php (default ">=8.3.36") + - require.composer/installers set to --installers (default "^1.0 || ^2.0"), + added after "php" when missing + - require.phpbb/phpbb removed (phpBB belongs in soft-require only) + - extra.soft-require.phpbb/phpbb ">=" --phpbb-min (default 3.3.19), plus an + upper bound chosen by --upper + +The upper bound with "--upper auto" (the default) follows what the extension +enforces: an ext.php check refusing phpBB 4.0.0-dev gives "<4.0.0@dev", one +refusing 4.1.0-dev gives "<4.1.0@dev". With no ceiling in ext.php, an existing +"<4.1" bound in soft-require (4.0 support already declared) is kept as +"<4.1.0@dev"; otherwise no upper bound is written, so composer.json does not +claim phpBB 4.0 support nobody has checked. + +The file is edited as text: key order, indentation, line endings and every +other field stay as they are. The edited file is parsed again and must equal the original with +exactly these changes, or nothing is written. + +Inputs: one or more extension checkout directories (each with composer.json) +Outputs: one line per extension with its changes, or "no changes"; with + --dry-run nothing is written +Exit status: 0 when every extension was handled, 1 when one was skipped + (problem printed), 2 for bad arguments. +""" +import argparse +import copy +import json +import re +import sys +from pathlib import Path + +PHPBB = "phpbb/phpbb" +INSTALLERS = "composer/installers" + + +def object_span(text: str, path: tuple) -> tuple: + """Return (start, end) of the JSON object at a key path, braces included. + + Scans the text with string awareness. Raises ValueError when the path is + missing or does not lead to an object. + """ + start, end = 0, len(text) + for key in path: + depth, i, found = 0, start, None + while i < end: + ch = text[i] + if ch == '"': + j = i + 1 + while text[j] != '"': + j += 2 if text[j] == "\\" else 1 + if depth == 1 and json.loads(text[i:j + 1]) == key and re.match(r"\s*:", text[j + 1:]): + found = j + 1 + break + i = j + 1 + continue + if ch in "{[": + depth += 1 + elif ch in "}]": + depth -= 1 + i += 1 + if found is None: + raise ValueError(f"no {'.'.join(path)} object") + brace = text.index("{", found) + if text[found:brace].strip(" \t\r\n:"): + raise ValueError(f"{'.'.join(path)} is not an object") + depth, i = 0, brace + while True: + ch = text[i] + if ch == '"': + i += 1 + while text[i] != '"': + i += 2 if text[i] == "\\" else 1 + elif ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + start, end = brace, i + 1 + break + i += 1 + return start, end + + +ENTRY = re.compile(r'^(?P[ \t]+)"(?P(?:[^"\\]|\\.)*)"(?P\s*:\s*)"(?P(?:[^"\\]|\\.)*)",?[ \t]*$') + + +def rewrite_flat_object(text: str, path: tuple, change) -> str: + """Rewrite a one-entry-per-line object of string values. + + change(entries) receives and returns a list of [key, value] pairs. The + entries keep the object's own indentation and key/value spacing. + """ + start, end = object_span(text, path) + body = text[start + 1:end - 1] + if "\n" not in body: + # Single-line object such as {"php": ">=7.2"}: keep it on one line. + pairs = re.findall(r'"((?:[^"\\]|\\.)*)"(\s*:\s*)"((?:[^"\\]|\\.)*)"', body) + if not pairs or re.sub(r'"(?:[^"\\]|\\.)*"\s*:\s*"(?:[^"\\]|\\.)*"', "", body).strip(" ,"): + raise ValueError(f"{'.'.join(path)} is not a flat object of strings: {body.strip()!r}") + sep = pairs[0][1] + entries = change([[json.loads(f'"{k}"'), json.loads(f'"{v}"')] for k, _, v in pairs]) + return (text[:start + 1] + ", ".join(f"{json.dumps(k)}{sep}{json.dumps(v, ensure_ascii=False)}" + for k, v in entries) + text[end - 1:]) + lines = [line for line in body.split("\n") if line.strip()] + entries, original = [], {} + for line in lines: + m = ENTRY.match(line.rstrip("\r")) + if not m: + raise ValueError(f"{'.'.join(path)} is not one string entry per line: {line.strip()!r}") + key, value = json.loads(f'"{m["key"]}"'), json.loads(f'"{m["value"]}"') + entries.append([key, value]) + # Each line keeps its own indentation, spacing and escaping. + original[key] = (m["indent"], m["sep"], f'"{m["key"]}"', f'"{m["value"]}"', value) + if not entries: + raise ValueError(f"{'.'.join(path)} is empty") + closing = body[body.rstrip(" \t").rfind("\n") + 1:] + out, indent, sep = [], original[entries[0][0]][0], original[entries[0][0]][1] + for key, value in change(entries): + if key in original: + indent, sep, raw_key, raw_value, old_value = original[key] + raw_value = raw_value if value == old_value else json.dumps(value, ensure_ascii=False) + else: + # A new entry takes the indentation of the line before it. + raw_key, raw_value = json.dumps(key), json.dumps(value, ensure_ascii=False) + out.append(f"{indent}{raw_key}{sep}{raw_value}") + new_body = "\n" + ",\n".join(out) + "\n" + closing + return text[:start + 1] + new_body + text[end - 1:] + + +def upper_bound(repo: Path, current_soft: str, mode: str) -> str: + """Return the soft-require upper bound (with leading ',') or '' for none.""" + if mode != "auto": + return "" if mode == "none" else "," + mode + ext_php = (repo / "ext.php").read_text() if (repo / "ext.php").is_file() else "" + ceiling = re.search(r"PHPBB_VERSION,\s*'([^']+)',\s*'<'", ext_php) + if ceiling: + return f",<{ceiling.group(1).replace('-dev', '')}@dev" + if re.search(r"<\s*4\.1", current_soft): + return ",<4.1.0@dev" + return "" + + +def normalize(repo: Path, args: argparse.Namespace) -> list: + """Edit one extension's composer.json; return the list of changes made.""" + path = repo / "composer.json" + with open(path, encoding="utf-8", newline="") as handle: + raw = handle.read() + # Work on LF text and write back the file's own line endings, so a CRLF + # file isn't renormalized as a side effect. + newline = "\r\n" if "\r\n" in raw else "\n" + text = raw.replace("\r\n", "\n") + doc = json.loads(text) + if doc.get("type") != "phpbb-extension": + raise ValueError("not a phpbb-extension") + require = doc.get("require", {}) + current_soft = doc.get("extra", {}).get("soft-require", {}).get(PHPBB) + if current_soft is None: + raise ValueError("no extra.soft-require.phpbb/phpbb") + if "php" not in require: + raise ValueError("no require.php") + soft = f">={args.phpbb_min}{upper_bound(repo, current_soft, args.upper)}" + + changes = [] + expected = copy.deepcopy(doc) + + def fix_require(entries: list) -> list: + out = [] + for key, value in entries: + if key == PHPBB: + changes.append(f"removed require {PHPBB} ({value})") + continue + if key == "php" and value != args.php: + changes.append(f"php {value} -> {args.php}") + value = args.php + if key == INSTALLERS and value != args.installers: + changes.append(f"{INSTALLERS} {value} -> {args.installers}") + value = args.installers + out.append([key, value]) + if key == "php" and INSTALLERS not in require: + changes.append(f"added {INSTALLERS} {args.installers}") + out.append([INSTALLERS, args.installers]) + return out + + def fix_soft(entries: list) -> list: + for entry in entries: + if entry[0] == PHPBB and entry[1] != soft: + changes.append(f"soft-require {entry[1]} -> {soft}") + entry[1] = soft + return entries + + new_text = rewrite_flat_object(text, ("require",), fix_require) + new_text = rewrite_flat_object(new_text, ("extra", "soft-require"), fix_soft) + + # The edited file must parse to the original plus exactly these changes. + new_require = {} + for key, value in expected["require"].items(): + if key == PHPBB: + continue + new_require[key] = args.php if key == "php" else args.installers if key == INSTALLERS else value + if key == "php" and INSTALLERS not in require: + new_require[INSTALLERS] = args.installers + expected["require"] = new_require + expected["extra"]["soft-require"][PHPBB] = soft + result = json.loads(new_text) + if result != expected or list(result["require"]) != list(expected["require"]): + raise ValueError("edited composer.json does not match the expected result; nothing written") + if changes and not args.dry_run: + with open(path, "w", encoding="utf-8", newline="") as handle: + handle.write(new_text.replace("\n", newline)) + return changes + + +def main() -> int: + ap = argparse.ArgumentParser( + description="Bring phpBB extensions' composer.json to the phpbbmodders baseline " + "(require.php, composer/installers, no phpbb/phpbb in require, soft-require floor).", + epilog="Exit status: 0 all handled, 1 an extension was skipped, 2 bad arguments.") + ap.add_argument("repos", nargs="*", metavar="DIR", help="extension checkout directory containing composer.json") + ap.add_argument("-n", "--dry-run", action="store_true", help="print the changes without writing any file") + ap.add_argument("-P", "--php", default=">=8.3.36", help='require.php constraint (default: ">=8.3.36")') + ap.add_argument("-i", "--installers", default="^1.0 || ^2.0", + help='composer/installers constraint (default: "^1.0 || ^2.0")') + ap.add_argument("-m", "--phpbb-min", default="3.3.19", help="lowest phpBB version for soft-require (default: 3.3.19)") + ap.add_argument("-u", "--upper", default="auto", + help='soft-require upper bound: "auto" (follow ext.php, see above), "none", or a ' + 'constraint such as "<4.1.0@dev" (default: auto)') + args = ap.parse_args() + if not args.repos: + ap.print_help() + return 2 + if args.upper not in ("auto", "none") and not args.upper.startswith("<"): + ap.error('--upper must be "auto", "none" or start with "<"') + + skipped = 0 + for repo in (Path(r).resolve() for r in args.repos): + try: + changes = normalize(repo, args) + except (OSError, ValueError, KeyError) as e: + print(f"{repo.name}: SKIPPED: {e}") + skipped += 1 + continue + print(f"{repo.name}: " + ("; ".join(changes) if changes else "no changes") + + (" (dry run)" if args.dry_run and changes else "")) + return 1 if skipped else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/phpbb-test-board/README.md b/phpbb-test-board/README.md index 8920c57..8d53ee9 100644 --- a/phpbb-test-board/README.md +++ b/phpbb-test-board/README.md @@ -79,6 +79,7 @@ Installs the extension from git and loads, as a guest and as the admin: - the main board pages; - the extension's routes; - its ACP, MCP and UCP modules in every mode; +- its page under ACP > Manage extensions, where phpBB reads `composer.json` (the extension's display name must show there); - its cron tasks. It then disables the extension, deletes its data and enables it again. It reports any server error, empty page, phpBB debug notice or PHP error-log entry. diff --git a/phpbb-test-board/smoke_test.py b/phpbb-test-board/smoke_test.py index c7cb043..d7219a7 100755 --- a/phpbb-test-board/smoke_test.py +++ b/phpbb-test-board/smoke_test.py @@ -7,6 +7,7 @@ --with, serves the board with PHP error logging on, and loads: - core board pages, as a guest and as the admin + - the extension's ACP details page, where phpBB reads its composer.json - the extension's routes that need no parameters - its ACP, MCP and UCP modules, in every mode - its cron tasks (a task with no name is reported: that crashes pages) @@ -19,10 +20,13 @@ arguments. """ import argparse +import html +import json import re import subprocess import sys from pathlib import Path +from urllib.parse import quote sys.path.insert(0, str(Path(__file__).resolve().parent)) from _venv import ensure_venv # noqa: E402 @@ -107,6 +111,21 @@ def check_modules(board: Board, report: Report, ns: str, admin: requests.Session admin.get(url, timeout=60)) +def check_details(board: Board, report: Report, ext: str, target: Path, admin: requests.Session, sid: str) -> None: + """Load the ACP extension details page, which reads composer.json through phpBB's metadata manager.""" + url = (f"{board.base}/adm/index.php?i=acp_extensions&mode=main&action=details" + f"&ext_name={quote(ext, safe='')}&sid={sid}") + resp = admin.get(url, timeout=60) + report.response("admin ACP extension details", resp) + try: + display = json.loads((target / "composer.json").read_text())["extra"]["display-name"] + except (OSError, ValueError, KeyError, TypeError): + report.problem("composer.json has no readable extra.display-name") + return + if display not in html.unescape(resp.text): + report.problem(f"the ACP details page doesn't show '{display}': phpBB couldn't read composer.json") + + def check_cron(board: Board, report: Report, ns: str, admin: requests.Session) -> None: """Trigger the extension's cron tasks; report tasks with no name.""" out = subprocess.run(["php", "-d", "opcache.enable_cli=0", "-r", CRON_TASKS_PHP], @@ -169,6 +188,7 @@ def main() -> int: for page in CORE_PAGES: report.response(f"admin {page}", admin.get(f"{board.base}/{page}", timeout=60)) report.response("admin adm/index.php", admin.get(f"{board.base}/adm/index.php?sid={sid}", timeout=60)) + check_details(board, report, ext, target, admin, sid) check_routes(board, report, target, {"guest": guest, "admin": admin}) check_modules(board, report, ns, admin, sid) check_cron(board, report, ns, admin) diff --git a/tests/test-normalize-composer.sh b/tests/test-normalize-composer.sh new file mode 100755 index 0000000..4e15372 --- /dev/null +++ b/tests/test-normalize-composer.sh @@ -0,0 +1,153 @@ +#!/usr/bin/bash +# Offline checks for normalize-composer.py on fixture extensions. +# Usage: tests/test-normalize-composer.sh +set -Eeuo pipefail + +root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +tool="$root/normalize-composer.py" +work=$(mktemp -d) +trap 'rm -rf -- "$work"' EXIT + +failures=0 +fail() { printf 'FAIL: %s\n' "$1"; failures=$((failures + 1)); } +pass() { printf 'ok: %s\n' "$1"; } + +# expect NAME FILE EXPECTED-CONTENT: compare a file byte for byte. +expect() { + if [[ "$(cat "$2")" == "$3" ]]; then pass "$1"; else fail "$1"; diff <(printf '%s' "$3") "$2" || true; fi +} + +# A typical extension: tabs, phpbb/phpbb in require, old constraints, no ext.php ceiling. +mkdir -p "$work/plain" +printf '%s\n' '{' \ + ' "name": "acme/plain",' \ + ' "type": "phpbb-extension",' \ + ' "keywords": ["phpbb", "extension"],' \ + ' "require": {' \ + ' "php": ">=7.4",' \ + ' "phpbb/phpbb": ">=3.3",' \ + ' "composer/installers": "~1.0"' \ + ' },' \ + ' "extra": {' \ + ' "display-name": "Plain",' \ + ' "soft-require": {' \ + ' "phpbb/phpbb": ">=3.3"' \ + ' }' \ + ' }' \ + '}' >"$work/plain/composer.json" +cp "$work/plain/composer.json" "$work/plain.orig" + +"$tool" --dry-run "$work/plain" >/dev/null +if cmp -s "$work/plain/composer.json" "$work/plain.orig"; then pass "--dry-run writes nothing"; else fail "--dry-run writes nothing"; fi + +"$tool" "$work/plain" >/dev/null +expect "typical extension: only the agreed keys change" "$work/plain/composer.json" "$(printf '%s\n' '{' \ + ' "name": "acme/plain",' \ + ' "type": "phpbb-extension",' \ + ' "keywords": ["phpbb", "extension"],' \ + ' "require": {' \ + ' "php": ">=8.3.36",' \ + ' "composer/installers": "^1.0 || ^2.0"' \ + ' },' \ + ' "extra": {' \ + ' "display-name": "Plain",' \ + ' "soft-require": {' \ + ' "phpbb/phpbb": ">=3.3.19"' \ + ' }' \ + ' }' \ + '}')" + +out=$("$tool" "$work/plain") +if [[ "$out" == "plain: no changes" ]]; then pass "a second run changes nothing"; else fail "a second run changes nothing: $out"; fi + +# CRLF line endings, mixed indentation, no composer/installers, ext.php ceiling at 4.0. +mkdir -p "$work/crlf" +printf '%s\r\n' '{' \ + ' "name": "acme/crlf",' \ + ' "type": "phpbb-extension",' \ + ' "require": {' \ + ' "php": ">=5.3.3",' \ + ' "ext-dom": "*"' \ + ' },' \ + ' "extra": {' \ + ' "display-name": "CRLF",' \ + ' "soft-require": {' \ + ' "phpbb/phpbb": ">=3.2"' \ + ' }' \ + ' }' \ + '}' >"$work/crlf/composer.json" +printf '%s\n' '"$work/crlf/ext.php" +"$tool" "$work/crlf" >/dev/null +expect "CRLF kept, installers added after php, other lines untouched, ceiling from ext.php" "$work/crlf/composer.json" \ + "$(printf '%s\r\n' '{' \ + ' "name": "acme/crlf",' \ + ' "type": "phpbb-extension",' \ + ' "require": {' \ + ' "php": ">=8.3.36",' \ + ' "composer/installers": "^1.0 || ^2.0",' \ + ' "ext-dom": "*"' \ + ' },' \ + ' "extra": {' \ + ' "display-name": "CRLF",' \ + ' "soft-require": {' \ + ' "phpbb/phpbb": ">=3.3.19,<4.0.0@dev"' \ + ' }' \ + ' }' \ + '}')" + +# Single-line objects stay on one line; an existing <4.1 bound is kept. +mkdir -p "$work/inline" +printf '%s\n' '{' \ + ' "name": "acme/inline",' \ + ' "type": "phpbb-extension",' \ + ' "require": {"php": ">=7.2"},' \ + ' "extra": {' \ + ' "display-name": "Inline",' \ + ' "soft-require": {"phpbb/phpbb": ">=3.3.0,<4.1.0@dev"}' \ + ' }' \ + '}' >"$work/inline/composer.json" +"$tool" "$work/inline" >/dev/null +expect "single-line objects stay single-line; 4.0 support kept" "$work/inline/composer.json" "$(printf '%s\n' '{' \ + ' "name": "acme/inline",' \ + ' "type": "phpbb-extension",' \ + ' "require": {"php": ">=8.3.36", "composer/installers": "^1.0 || ^2.0"},' \ + ' "extra": {' \ + ' "display-name": "Inline",' \ + ' "soft-require": {"phpbb/phpbb": ">=3.3.19,<4.1.0@dev"}' \ + ' }' \ + '}')" + +# --upper sets the bound explicitly. +cp "$work/plain.orig" "$work/plain/composer.json" +"$tool" --upper '<4.1.0@dev' "$work/plain" >/dev/null +if grep -q '"phpbb/phpbb": ">=3.3.19,<4.1.0@dev"' "$work/plain/composer.json"; then + pass "--upper sets the soft-require bound" +else + fail "--upper sets the soft-require bound" +fi + +# Not an extension: skipped, exit 1, file untouched. +mkdir -p "$work/library" +printf '%s\n' '{"name": "acme/library", "type": "library", "require": {"php": ">=7.4"}}' >"$work/library/composer.json" +cp "$work/library/composer.json" "$work/library.orig" +status=0 +"$tool" "$work/library" >/dev/null || status=$? +if [[ $status -eq 1 ]] && cmp -s "$work/library/composer.json" "$work/library.orig"; then + pass "a non-extension is skipped with exit 1" +else + fail "a non-extension is skipped with exit 1 (exit $status)" +fi + +# No arguments: help, exit 2. Bad --upper: exit 2. +status=0 +"$tool" >/dev/null || status=$? +if [[ $status -eq 2 ]]; then pass "no arguments prints help and exits 2"; else fail "no arguments exits 2 (exit $status)"; fi +status=0 +"$tool" --upper 4.1 "$work/plain" >/dev/null 2>&1 || status=$? +if [[ $status -eq 2 ]]; then pass "a malformed --upper exits 2"; else fail "a malformed --upper exits 2 (exit $status)"; fi + +if ((failures)); then + printf '%d test(s) failed\n' "$failures" + exit 1 +fi +echo "All tests passed."