-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix incorrect pyc cache loaded when test file is updated in short time #13293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jnhyperion, this looks like a good change to me.
Can you please add a changelog entry? (see changelog/README.rst
file)
The tests failures are due to #13291, rebasing on main
after that PR is merged should make CI pass.
@@ -317,7 +317,7 @@ def _write_pyc_fp( | |||
flags = b"\x00\x00\x00\x00" | |||
fp.write(flags) | |||
# as of now, bytecode header expects 32-bit numbers for size and mtime (#4903) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed it before, but this comment seems to negate this change -- it takes the lower 32 bits while st_mtime_ns needs at least 64 bits, e.g.
>>> import os
>>> st = os.stat('/home')
>>> print(st.st_mtime_ns)
1669828167393028173
>>> print(st.st_mtime_ns & 0xFFFFFFFF)
23537741
>>> print(int(st.st_mtime) & 0xFFFFFFFF)
1669828167
Basically, the pyc timestamp format is 32 bits, and we are trying to stay compatible with it (even though it's not absolutely necessary as we do our own loading). The same problem occurs with python itself, not just pytest.
See:
python/cpython#121376
https://discuss.python.org/t/change-pyc-file-format-to-record-more-accurate-timestamp-and-other-information/57815
Also related: #11418
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct, this is more complex than I expected, I used to run the same test without pytest, but it didn't reproduce somehow, so I thought it's only caused by pytest. Actually, the similar issue still exists in python's import system, and pytest's cache rewrite inherited the same issue. Looks like no proper solution for the issue until now, perhaps we should close the PR.
Closing then as #13293 (comment), we can easily reopen if needed. Thanks @jnhyperion! |
See: #13292
The root cause is the cache is using file
st_mtime
int value to check if it's out of date, it's not so accurate when the file is updated in short time. Usingst_mtime_ns
is the better solution.