Repository navigation
Invalidate rewritten pyc files by source hash - #14905
Merged
RonnyPfannschmidt merged 3 commits intoAug 19, 2026
Merged
RonnyPfannschmidt merged 3 commits into
RonnyPfannschmidt merged 3 commits into
Conversation
Drop the --invalidation-mode option and write checked-hash pycs unconditionally. The option existed because hashing was assumed to cost enough to want a timestamp fast path. Measured, it does not: reading the source and hashing it costs ~15us per file, against ~1.1ms for a cache hit that has to unmarshal the code object anyway, and ~110ms for the rewrite the cache avoids. With one format there is no reason to consult _imp.check_hash_based_pycs either. That is a private CPython attribute governing how the *builtin* import system treats hash based pycs, and it never sees the pycs written here. The source stat disappears from both paths: _rewrite_test no longer needs it for the header, and _read_pyc reads the source instead of stat'ing it. Also fixes pytest-dev#13292, where an edit within one mtime second of the previous one was invisible to the timestamp comparison.
There was a problem hiding this comment.
Pull request overview
This PR updates pytest’s assertion-rewrite bytecode cache to use PEP 552 checked-hash .pyc headers, so rewritten modules remain valid across checkouts/cache restores that change mtimes and to avoid stale loads when edits occur within the same timestamp second.
Changes:
- Switch rewritten-pyc writing/reading from mtime+size invalidation to checked-hash invalidation (PEP 552).
- Update internal helpers (
_rewrite_test,_write_pyc,_write_pyc_fp,_read_pyc) to operate on a computed source hash instead ofos.stat_result. - Expand tests and add changelog entries, including a regression test for edits within one mtime-second.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/_pytest/assertion/rewrite.py |
Write/read checked-hash pycs and compute/verify source hashes for cache validity. |
testing/test_assertrewrite.py |
Update tests for new helper signatures; add coverage for mtime-only changes and same-second edits. |
changelog/13292.bugfix.rst |
Document fix for stale rewritten module when edits happen within the same second. |
changelog/11418.improvement.rst |
Document hash-based invalidation improving cache reuse across CI/checkouts. |
Suppressed comments (2)
testing/test_assertrewrite.py:1412
- This test only checks
pyc_bytes[4] == 3, which would still pass if the other flag bytes are wrong. It also useshashas a variable name, which shadows the built-inhash(). Consider asserting the full 4-byte flags field and renaming the variable for clarity.
hash, co = _rewrite_test(fn, config)
_write_pyc(state, co, hash, pyc)
assert _read_pyc(fn, pyc, state.trace) is not None
pyc_bytes = pyc.read_bytes()
testing/test_assertrewrite.py:1435
- Avoid using
hashas a local variable name here because it shadows the built-inhash()and can be confusing when reading the test. A name likesrc_hashis clearer.
hash, co = _rewrite_test(fn, config)
_write_pyc(state, co, hash, pyc)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1331
to
+1335
| hash = source_hash(source_bytes) | ||
| assert _write_pyc(state, co, hash, pycpath) | ||
|
|
||
| with mock.patch.object(os, "replace", side_effect=OSError): | ||
| assert not _write_pyc(state, co, os.stat(source_path), pycpath) | ||
| assert not _write_pyc(state, co, hash, pycpath) |
Zac-HD
approved these changes
Aug 19, 2026
Zac-HD
left a comment
Member
There was a problem hiding this comment.
lgtm, and nice perf improvement!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes and thus closes #11418, rebased onto main with @cdce8p's two commits preserved.
Rewritten pycs are invalidated by mtime+size. A checkout gives every source file
a new mtime, so a restored CI cache is thrown away in full and every test module
is rewritten again. This switches them to PEP 552 checked-hash pycs.
The third commit drops the
--invalidation-modeoption #11418 added and alwayswrites checked-hash — measured, hashing does not need a timestamp fast path.
500 assert-dense modules (~9 KB each),
--collect-only, CPython 3.12:touching every sourcePer file that is ~15 µs to read and hash the source, against ~1.1 ms for a cache
hit that unmarshals the code object anyway and ~110 ms for the rewrite it avoids.
Also fixes #13292: the pyc header records whole seconds only, so an edit made
within one second of the previous one, and of the same size, was invisible.
Notes:
_imp.check_hash_based_pycsis not consulted. It governs how the builtinimport system treats hash-based pycs, and it never sees these.
PYTEST_TAGalready contains the pytest version, so no two pytest versionsshare a pyc file.
_rewrite_test,_write_pycand_write_pyc_fpno longer take or return an
os.stat_result; the sourcestatis gone fromboth the read and the write path.