fix: zipfile.ZipInfo monkey-patch NameError after init cleanup#79
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a Python runtime regression in the python-agent-driver where the zipfile.ZipInfo.__init__ monkey-patch could raise NameError after interpreter warmup cleanup, by ensuring the original __init__ is captured via a proper closure.
Changes:
- Wrap the
ZipInfo.__init__monkey-patch in a function to safely capture the original implementation before deleting warmup globals. - Add a runtime integration test intended to guard against regressions in the zipfile patch behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
host/tests/pyhl_runtime.rs |
Adds an integration test meant to validate the zipfile monkey-patch remains usable after warmup cleanup. |
examples/python-agent-driver/hl_pydriver.c |
Refactors the ZipInfo.__init__ patch to avoid NameError by using closure capture and then cleaning up globals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| }; | ||
| let timing = rt | ||
| .run_code("from docx import Document\ndoc = Document()\nprint('ok')") |
There was a problem hiding this comment.
Fixed — switched to stdlib zipfile.ZipFile + io.BytesIO which exercises ZipInfo.init without needing python-docx.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Linux Benchmarks
Details
| Benchmark suite | Current: c10435d | Previous: 581b8d8 | Ratio |
|---|---|---|---|
hello_world (median) |
10 ms |
20 ms |
0.50 |
pandas (median) |
90 ms |
90 ms |
1 |
density (per VM) |
11 MB |
11 MB |
1 |
snapshot (disk) |
656 MiB |
656 MiB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Windows Benchmarks
Details
| Benchmark suite | Current: c10435d | Previous: 581b8d8 | Ratio |
|---|---|---|---|
hello_world (median) |
310 ms |
332 ms |
0.93 |
pandas (median) |
783 ms |
1014 ms |
0.77 |
density (per VM) |
10 MB |
10 MB |
1 |
snapshot (disk) |
663 MiB |
663 MiB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
The _safe_zi wrapper referenced _orig_zi as a module-level global, but
`del _orig_zi` at the end of the init block destroyed it before any
call to ZipInfo() could use it. This broke all libraries that use
zip-based formats (python-docx, openpyxl, etc.) with:
NameError: name '_orig_zi' is not defined
Wrap the patch in a function so _orig becomes a proper closure cell
that survives the namespace cleanup.
Adds a regression test using python-docx's Document() constructor.
Signed-off-by: danbugs <danilochiarlone@gmail.com>
danbugs
force-pushed
the
fix/zipfile-patch
branch
from
cfe68e3 to
c10435d
Compare
Summary
NameError: name '_orig_zi' is not definedwhen usingpython-docxor any package that creates zip fileszipfile.ZipInfomonkey-patch inhl_pydriver.cused bare globals that were deleted by the cleanupdelstatements, breaking the closure reference; wrapping the patch in a function provides proper closure capturefrom docx import Documentto prevent regressionTest plan
runtime_zipinfo_patch_survives_cleanuppasses