Remove unused build environment interpolation and expandvars - #294
Conversation
psf-chronographer
Bot
added
the
bot:chronographer:provided
Confidence Score: 4/5Not safe to merge until the repository-required regression coverage for the changed build-environment behavior is added. The final review contains one finding tied to an explicit repository requirement. Files Needing Attention: Add or extend tests covering the behavior changed in packaging/pep517_backend/_cython_configuration.py. Reviews (1): Last reviewed commit: "Remove unused build environment interpol..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def patched_env( | ||
| env: dict[str, str], | ||
| cython_line_tracing_requested: bool, | ||
| *, | ||
| original_source_directory: Path | None = None, | ||
| temporary_build_directory: Path | None = None, | ||
| ) -> Iterator[None]: | ||
| """Temporary set given env vars. | ||
|
|
||
| :param env: tmp env vars to set | ||
| :type env: dict | ||
| """Temporarily adjust compiler flags for Cython builds. | ||
|
|
||
| :yields: None | ||
| """ | ||
| orig_env = os.environ.copy() |
There was a problem hiding this comment.
This changes the Cython build environment by removing configured environment interpolation, but adds no coverage for the remaining compiler-flag handling and environment restoration. The repository directive requires changed behavior to be covered by an added or extended test. Add coverage for caller-provided CFLAGS, tracing flags, reproducible-build flags, and restoration of the original environment; this repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
webknjaz
left a comment
webknjaz
left a comment
There was a problem hiding this comment.
Motivation? This would have to be in sync with the upstream copy @ ansible-pylibssh at all times so it's easy to share updates using diff.
|
See discussion in aio-libs/yarl#1879 |
2 participants
What do these changes do?
Remove the unused environment interpolation configuration from the Cython build backend and drop the expandvars build dependency. Preserve compiler flags supplied through the environment, Cython tracing, and reproducible build flags.
Are there changes in behavior for the user?
Source builds no longer need expandvars. The library's runtime behavior is unchanged.
Related issue number
None.
Checklist
CHANGES/folder