Escape generated build environment metadata - #714
Conversation
| code = render_default_envs(os.environ) | ||
|
|
||
| # Create temporary build directory | ||
| build_include_dir = os.path.join(self.build_lib, 'deep_ep') |
There was a problem hiding this comment.
🔵 suggestion: generate_default_envs 中 build_include_dir 变量创建后仅用于 os.makedirs，随后 open 又重新用 os.path.join(self.build_lib, 'deep_ep', ...) 拼了一次相同路径；可复用 build_include_dir 减少重复。非阻塞。
🤖 v5
|
|
||
| def load_setup_module(): | ||
| path = Path(__file__).parents[2] / 'setup.py' | ||
| spec = importlib.util.spec_from_file_location('deep_ep_setup_under_test', path) |
There was a problem hiding this comment.
🔵 suggestion: load_setup_module 会执行 setup.py 的全部顶层代码，包括 import torch.utils.cpp_extension 和加载 find_pkgs 模块，测试因此隐式依赖 torch 安装。当前环境可用（已验证 2 passed），但若未来希望在无 torch 的 CI 上运行这些 host-only 测试，可考虑把 render_default_envs 与 persistent_env_names 移入独立的轻量模块。非阻塞。
🤖 v5
🤖 ds-review-bot Code Reviewv6该变更使用 v4v5此 MR 修复了 CustomBuildPy 生成 deep_ep/envs.py 时的转义缺陷：旧实现将持久化环境变量的值直接内插进单引号 Python 字面量，含撇号的路径会生成非法源码，含反斜杠的值会被静默改变。修复将渲染逻辑抽取为模块级纯函数 render_default_envs(environ)，对键和值统一使用 Python repr（{name!r} / {environ[name]!r}），并保持只输出既有的 persistent_env_names 白名单，无关环境变量不会被写入。新增的 host-only 测试通过 compile(source, 'envs.py', 'exec') 真正执行生成源码，验证撇号+反斜杠（/tmp/o'neil\jit-cache）、空格（/opt/NCCL dir）值的精确往返，以及白名单外变量被忽略，覆盖了描述中的全部回归场景。变更范围小、合法值下输出语义不变、可测性重构方向正确，建议合并。验证：python -m unittest tests.utils.test_setup_helpers（2 passed）、ruff check 通过、git diff --check 无问题。 Files reviewed: 2 |
|
Addressed the review: |
2 participants
Problem
CustomBuildPyinterpolates persistent environment values directly into single-quoted Python literals when generatingdeep_ep/envs.py. A valid cache path containing an apostrophe produces invalid Python, while backslashes can silently change the stored value.Fix
Extract the source renderer and use Python
reprfor both environment names and values. Only the existing persistent-environment allowlist is emitted.Regression coverage
The host-only tests compile and execute the generated source, then verify exact round trips for apostrophes, backslashes, and spaces. They also verify that unrelated environment values are ignored.
Validation
python -m unittest tests.utils.test_setup_helpers(2 passed)ruff check tests/utils/test_setup_helpers.pygit diff --checkPrepared with OpenAI Codex assistance; I reproduced the invalid generated source and reviewed the serialized output.