fix: bytecode cache invalidation for moved test files (#14552)

[15r10nk]

solves #14552

[RonnyPfannschmidt]

Thanks for starting this

Unfortunately the test doesn't validate the invariant holding

[RonnyPfannschmidt]

This test doesn't validate the invatiant

[15r10nk]

I don't understand what is missing here for you.

• The tests fails without the fix.
• It reproduces the incorrect behaviour by moving the source and cached bytecode.

Which invariant do you mean?

[RonnyPfannschmidt]

The test would pass without changes when one disables bytecode writing for example

[RonnyPfannschmidt]

Ill show a more detailed example once I get back to the computer

[15r10nk]

I added a check for the compiled bytecode. let me know what you think.

[RonnyPfannschmidt]

AI-authored research report. Ronny asked Claude Fable 5.1 (via Claude Code) to
assess #14989 and cross-check the review on it. The agent did the exploration: it read
the CPython sources and issue history, ran the experiments, and wrote everything below
the horizontal rule. Ronny read it and is posting it. The paragraph directly below is
Ronny's own.

So @15r10nk, I have to apologize. Back when I worked in this area I entirely missed how
Python had moved on (I still recall the initial considerations from back in 2005). Now it
seems the correct way is indeed to just fix the filename if the hash checks out. And for
that I believe it is most fitting to try to use the implementation from _imp if
available, falling back to a minimal replacer otherwise.

---

Stale co_filename in moved pycs: what CPython does, and what pytest should do

Conclusion first

The approach in #14551, fixing co_filename in memory after loading a cached pyc and
not writing the pyc back, is exactly what CPython has done for every pyc it loads
since 2009. The alternative that #14989 implements, treating a filename mismatch as a
stale cache and recompiling, was proposed in the same CPython issue in 2005 and rejected
there, for the reasons Frank gave in #14551: shared and read-only caches, rewrite churn,
and concurrent writers.

The review objection in #14551 rested on the premise that CPython provides no mapper for
this. That premise is wrong. CPython exposes one as _imp._fix_co_filename, its own
import system calls it on every pyc load, and PyPy implements it too.

Recommendation: return to the #14551 design, reach for the _imp implementation when it
is available, keep a pure-Python fallback, never write the pyc back, and test the
invariant by inspecting the pyc directly.

The question

When a test directory is moved or renamed, pytest's rewritten pyc moves with it. The
source hash still matches, so _read_pyc accepts the pyc, and every code object in it
still carries the old absolute path in co_filename. __file__ is correct, because
importlib sets it from the spec, but inspect, tracebacks, and anything that reads
f_code.co_filename see the old path (#14552).

Two fixes were proposed:

• fix: replace co_filename with the correct filename when the file is moved #14551: after marshal.load, rebuild the code object tree with the current path via
  code.replace, do not rewrite the pyc.
• fix: bytecode cache invalidation for moved test files (#14552) #14989: after marshal.load, compare co_filename to the current path, and on
  mismatch return None so the caller recompiles and rewrites the pyc.

What CPython does

The primitive

_imp._fix_co_filename(code, path) is implemented in
Python/import.c by
update_compiled_module and update_code_filenames. Its semantics, from the C source:

• No-op when already correct. If the top-level co_filename equals path, return.
• Otherwise an in-place, recursive rename. Remember the old top-level filename, then
  walk co_consts depth-first and set co_filename = path on every nested code object
  whose filename equals that old name. Nested functions, class bodies, closures and
  comprehensions all live in co_consts, so all of them are covered.
• Foreign code objects are skipped. A nested code object compiled from a different
  file has a different old name and is left untouched.
• The pyc on disk is never written.
• Hash and equality are unaffected. code_hash and code_richcompare in
  Objects/codeobject.c never read co_filename, so a code object already used as a
  dict key or set member stays consistent after the rename.
• No lock, none needed. The only caller mutates a code object it just unmarshaled and
  nobody else can see yet. pytest's _read_pyc is in the same position, so the call is
  free-threading safe by construction.

Where importlib calls it

In importlib._bootstrap_external._compile_bytecode, for every pyc that has a known
source path:

def _compile_bytecode(data, name=None, bytecode_path=None, source_path=None):
    """Compile bytecode as found in a pyc."""
    code = marshal.loads(data)
    if isinstance(code, _code_type):
        _bootstrap._verbose_message('code object from {!r}', bytecode_path)
        if source_path is not None:
            _imp._fix_co_filename(code, source_path)
        return code

Only the sourceless loader skips it, because it has no path to fix to. The leading
underscore marks the function as an implementation detail, but importlib's own bootstrap
depends on it, so it is as stable as the import system itself.

Where it is available

Interpreter	_imp._fix_co_filename	Recursive	Skips foreign code objects
CPython 3.3 and later	yes (C call in the import path since 2.6 / 3.0)	yes	yes
PyPy 3.11.13	yes	yes, verified	yes, verified

Every interpreter pytest supports has it.

History: bpo-1180193, "broken pyc files"

Now python/cpython#41838.

• 2005, Armin Rigo files the issue and proposes both options in the first message:
  discard the pyc when co_filename does not match, or rewrite the filenames on load.
• 2007, Martin von Löwis objects that shipping only pycs is legitimate. That rules
  out the discard option: it would refuse a valid sourceless pyc.
• 2007, Armin Rigo, on why the fixed pyc must not be written back: "the same .py
  files are accessed from what appears to be two different paths, e.g. over NFS. This
  would cause .pyc files to be rewritten all the time. Two python processes trying to
  write different data to the same .pyc file at the same time are going to create a
  mess."
• January 2009, Antoine Pitrou commits Jean-Paul Calderone's patch to 2.6, 2.7, 3.0
  and py3k: "safer and simpler not to rewrite the pyc file when the filenames have been
  changed."
• 3.3: exposed as _imp._fix_co_filename when importlib became the import system.

Frank's #14551 comment cited this issue. The review did not follow the link.

Relationship to hash-based pycs

The hash and the filename answer different questions, and importlib applies both, in
this order:

1. The source hash answers "is the content current?" PEP 552 flags word bit 0 marks
   a hash-based pyc, bit 1 marks it as checked. SourceLoader.get_code validates the
   hash unless the interpreter runs with --check-hash-based-pycs never.
2. The filename fix answers "where does this content live now?" After the hash
   passes, marshal.loads, then _fix_co_filename.

pytest has been at step 1 since #14905: _write_pyc_fp writes flags 0b11, checked
hash, and _read_pyc validates the hash itself. The "file content and runtime state
diverge" concern raised in #14551 is fully covered by that hash. The only path-bearing
state in a rewritten pyc is co_filename. The agent checked the rewriter: the module
path is used only to locate a warning at rewrite time (AssertionRewriter, the
tuple-assertion warning) and is never embedded in the generated code.

The interaction that matters is with #14905 itself. Its purpose was a restored CI cache
surviving a fresh checkout. #14989 turns that cache into a total miss whenever the
checkout path differs between builds, which is the normal case on Jenkins and GitLab
runners.

Measurements

Cost per load

testing/test_assertrewrite.py, 126 KiB marshaled, 209 code objects, CPython 3.10.19,
best of three:

Path	Per load
marshal.loads alone	0.41 ms
marshal + _imp._fix_co_filename	0.42 ms
marshal + pure-Python code.replace recursion (#14551 as submitted)	1.05 ms
recompile via _rewrite_test (#14989 on a mismatch)	157.85 ms

The C fixer is measurement noise. The pure-Python version is still trivial. Recompiling
is about 375 times slower.

Rewrite churn under #14989

Same file reached through two spellings, a real directory and a symlink to it, running
pytest real/test_a.py and pytest link/test_a.py alternately on the #14989 branch
(CPython 3.14): the pyc's mtime changed on every single run, six out of six. Each run
paid the full recompile and rewrote the cache. With the #14551 design the cache is
reused and only the in-memory names change.

Test behaviour

Test version	With fix	Without fix, bytecode on	Without fix, PYTHONDONTWRITEBYTECODE=1
#14989 first push	pass	fail	pass
#14989 current head	pass	fail	fail

The review remark that the first test would pass with bytecode writing disabled was
correct for that push. The current head fixed it by deleting the variable and asserting
that the pyc exists before and after the move.

Recommended implementation

Reach for the _imp implementation when it exists, fall back to the pure-Python
rebuild from #14551 otherwise, and never write the pyc back:

try:
    from _imp import _fix_co_filename
except ImportError:  # pragma: no cover
    _fix_co_filename = None


def _replace_code_filenames(co: types.CodeType, filename: str) -> types.CodeType:
    """Pure-Python fallback: rebuild the code object tree with *filename*."""
    return co.replace(
        co_filename=filename,
        co_consts=tuple(
            _replace_code_filenames(c, filename) if isinstance(c, types.CodeType) else c
            for c in co.co_consts
        ),
    )


def _fix_code_filename(co: types.CodeType, filename: str) -> types.CodeType:
    """Point *co* and its nested code objects at *filename*.

    Mirrors what importlib does for every pyc it loads: the cache stays valid,
    only the in-memory location is corrected.
    """
    if co.co_filename == filename:
        return co
    if _fix_co_filename is not None:
        _fix_co_filename(co, filename)  # in place, recursive, C
        return co
    return _replace_code_filenames(co, filename)

In _read_pyc, after the isinstance(co, types.CodeType) check:

        return _fix_code_filename(co, str(source))

Test

The current test proves the symptom is gone but never looks inside the pyc. A direct
check of the invariant, in addition to the existing subprocess runs:

1. After the first run, open the pyc, skip the 16-byte header, marshal.load, and
   assert the stored co_filename is the original path. This documents why the test
   exists.
2. Move the directory, run again, assert the test passed and that the pyc's mtime
   did not change. This proves the fix happened in memory and the cache was reused.

What was and was not verified

• Verified: C semantics by reading import.c and codeobject.c on the 3.14 branch;
  recursion and foreign-code skipping on CPython 3.10 and PyPy 3.11.13; the timing
  table; the churn experiment; the test matrix.
• Not verified: whether typeshed's _imp stub declares _fix_co_filename. If mypy
  objects, a narrow # type: ignore[attr-defined] on the import is the fix.
• Not measured: PyPy timing. On PyPy the fallback is never taken, so it does not matter
  for the recommendation.

[RonnyPfannschmidt]

meh - claude had to mess up the import naming suggestion 😮‍💨

[15r10nk]

I also did not know about _imp._fix_co_filename. I changed this pr. I hope this is ok.

I also used AI for parts of the implementation. I hope this is ok.

[RonnyPfannschmidt]

Please ensure the attribution

[patchback]

Backport to 9.1.x: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply f8ad1f6 on top of patchback/backports/9.1.x/f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a/pr-14989

Backporting merged PR #14989 into main

1. Ensure you have a local repo clone of your fork. Unless you cloned it
   from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these
   instructions you'll refer to it by the name upstream. If you don't
   have it, here's how you can add it:

   git remote add upstream https://github.com/pytest-dev/pytest.git
   

3. Ensure you have the latest copy of upstream and prepare a branch
   that will hold the backported code:

   git fetch upstream
   git checkout -b patchback/backports/9.1.x/f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a/pr-14989 upstream/9.1.x
   

4. Now, cherry-pick PR fix: bytecode cache invalidation for moved test files (#14552) #14989 contents into that branch:

   git cherry-pick -x f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a
   

   If it'll yell at you with something like fatal: Commit f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a is a merge but no -m option was given., add -m 1 as follows instead:

   git cherry-pick -m1 -x f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a
   

5. At this point, you'll encounter some merge conflicts. You must
   resolve them in order to preserve the patch from PR fix: bytecode cache invalidation for moved test files (#14552) #14989 as close to the
   original as possible.
6. Once conflicts are resolved and git added, run:

   git cherry-pick --continue
   

7. Push this branch to your fork on GitHub:

   git push origin patchback/backports/9.1.x/f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a/pr-14989
   

8. Create a PR, ensure that the CI is green. If it's not — update it so that
   the tests and any other checks pass. This is it!
   Now relax and wait for the maintainers to process your pull request
   when they have some cycles to do reviews. Don't worry — they'll tell you if
   any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[15r10nk]

thank you, should I backport it?

[RonnyPfannschmidt]

that would be awesome, thanks

[15r10nk]

I have problems with the backport.

> git checkout -b patchback/backports/9.1.x/f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a/pr-14989 upstream/9.1.x
fatal: 'upstream/9.1.x' is not a commit and a branch 'patchback/backports/9.1.x/f8ad1f6f95307ed5b7da9b73f1c585695bf2b51a/pr-14989' cannot be created from it

[15r10nk]

problem solved ... :-)