bito-code-review[bot] commented on code in PR #44536: URL: https://github.com/apache/superset/pull/44536#discussion_r4122414221
########## scripts/translations/compile_po.py: ########## @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Cross-platform Python port of ``po2json.sh``. + +Compiles ``superset/translations/**/*.po`` into sibling ``.json`` files for +the frontend (``po2json``), then formats the generated JSON with ``oxfmt`` -- +the same two steps ``po2json.sh`` performed, in the same order. + +Each tool is invoked as ``node <package's own bin entry point>``, resolved +from the package's own ``package.json`` "bin" field, rather than through the +platform-specific ``node_modules/.bin`` wrapper (a symlink on POSIX, a +``.cmd``/``.ps1`` shim on Windows). ``node`` is a real executable on every +platform, so this never goes through ``cmd.exe`` the way invoking a ``.cmd`` +wrapper does (even with ``subprocess``'s ``shell=False``) -- so there is no +shell-metacharacter or ``%VAR%``-expansion surface to defend against here. + +Usage: + python scripts/translations/compile_po.py +""" + +from __future__ import annotations + +import glob +import json # noqa: TID251 - standalone script, not the Flask app +import os +import shutil +import subprocess +import sys + +ROOT_DIR = os.path.abspath( + os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "..") +) +FRONTEND_DIR = os.path.join(ROOT_DIR, "superset-frontend") +TRANSLATIONS_DIR = os.path.join(ROOT_DIR, "superset", "translations") + + +def resolve_node_entry(package: str) -> str | None: + """Resolve an installed npm package's CLI entry point. + + Reads the entry path out of the package's own ``package.json`` "bin" + field instead of guessing at the ``node_modules/.bin`` wrapper's shape, + so the result can always be run via ``node <entry>`` directly. + """ + pkg_dir = os.path.join(FRONTEND_DIR, "node_modules", package) + manifest_path = os.path.join(pkg_dir, "package.json") + if not os.path.isfile(manifest_path): + return None + with open(manifest_path, encoding="utf-8") as f: + manifest = json.load(f) + bin_field = manifest.get("bin") + rel_entry = bin_field.get(package) if isinstance(bin_field, dict) else bin_field + if not rel_entry: + return None + entry_path = os.path.join(pkg_dir, rel_entry) + return entry_path if os.path.isfile(entry_path) else None + + +def run(command: list[str]) -> int: + """Run a command directly, with no shell involved, and return its exit + code. + + ``NODE_NO_WARNINGS=1`` keeps node's own warnings (experimental-feature + notices and the like) out of the build log, as po2json.sh did. + """ + env = {**os.environ, "NODE_NO_WARNINGS": "1"} + return subprocess.run(command, check=False, env=env).returncode # noqa: S603 + + +def convert_po_to_json(node_bin: str, po2json_entry: str, po_file: str) -> str | None: + """Convert one ``.po`` file to its sibling ``.json`` via ``po2json``. + + Returns the generated ``.json`` path on success, ``None`` on failure. + """ + json_dest = f"{os.path.splitext(po_file)[0]}.json" + rc = run( + [ + node_bin, + po2json_entry, + "--domain", + "superset", + "--format", + "jed", + "--fuzzy", + po_file, + json_dest, + ] + ) + return json_dest if rc == 0 else None + + +def main() -> int: + """Convert every ``.po`` file under ``superset/translations`` to + ``.json``, then format the generated JSON with ``oxfmt``.""" + node_bin = shutil.which("node") + if not node_bin: + print("ERROR: node not found in PATH.", file=sys.stderr) + return 1 Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Scoped package not resolved</b></div> <div id="fix"> `resolve_node_entry("po2json")` looks for `node_modules/po2json/package.json`, but `@hainenber/po2json` installs to `node_modules/@hainenber/po2json/` (package.json line 255; lock line 4153). It always returns None, so `main()` exits 1 and `npm run build-translation` never runs po2json. Pass the scoped name and look up the bin key by command name. </div> </div> <small><i>Code Review Run #ce963d</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
