-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
warnings.filters was modified by test_check_c_globals #95349
Comments
I'll take a look. |
I patched my own copy to have this as the test:
Solves the changed global, but I only assume the test is still valid. |
Sorry for the delay. I'll try to take a look today. |
Hmm, that test is disabled and the code doesn't look like yours. Are you based on some other branch? https://github.com/python/cpython/blob/main/Lib/test/test_check_c_globals.py#L14-L19 |
Yeah, I didn't merge any changes. It's just in my internal fork. The test may be disabled, but the import still happens, and that's apparently what leaks the change. |
I'm unclear on why test_check_c_globals would be causing this problem. Neither it nor anything in Tools/c-analyzer messes with warnings. Perhaps it is something with test_tools? (I'm using a couple helpers from there.) Is this a new failure? FWIW, I don't get a failure when I run the test locally. How are you invoking the tests? |
I think it's something in the c-analyzer tool triggering it. Considering the warning being filtered is from Maybe you've got something else in your environment that loads this stuff earlier? So then it doesn't look like the test is adding the extra warning filter because it's already there? I'm basically just running normal CI, it's just not on GitHub Actions. |
No, it's not quite normal. I configure with custom |
Yeah, I suspect this is a consequence of PEP 632 and the fact that we're still using parts of distutils in the c-analyzer (or at least were until 11 days ago). Do you still see the problem with the latest main? Regardless, I don't mind adding the fix you suggested if needed.
I bet that's it. |
My build from 7 hours ago failed in the same way. I wouldn't have thought it was coming from distutils though, unless it's coming from the distutils test suite? |
FYI, here's what I see:
I do get the warning but it doesn't cause a failure. |
I was able to reproduce the failure you originally described with an out-of-tree build and a custom
|
There's a comment in the distutils test that explains the situation: cpython/Lib/distutils/tests/__init__.py Lines 30 to 34 in 63140b4
|
My fix (gh-95837) should be good enough until we stop using distutils in the c-analyzer. |
…als (pythonGH-95837) Under certain build conditions, test_check_c_globals fails. This fix takes the same approach as we took for pythongh-84236 (via pythongh-20095). We'll be removing use of distutils in the c-analyzer at some point. Until then we'll hide the warning filter. (cherry picked from commit 3ff6d9a) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
…H-95837) (GH-95843) Under certain build conditions, test_check_c_globals fails. This fix takes the same approach as we took for gh-84236 (via gh-20095). We'll be removing use of distutils in the c-analyzer at some point. Until then we'll hide the warning filter. (cherry picked from commit 3ff6d9a) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Thanks for pointing this out, @zooba. It should be all fixed now. Would you mind verifying? |
It looks good in my build overnight 👍 |
…als (pythonGH-95837) Under certain build conditions, test_check_c_globals fails. This fix takes the same approach as we took for pythongh-84236 (via pythongh-20095). We'll be removing use of distutils in the c-analyzer at some point. Until then we'll hide the warning filter. (cherry picked from commit 3ff6d9a) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
…H-95837) Under certain build conditions, test_check_c_globals fails. This fix takes the same approach as we took for gh-84236 (via gh-20095). We'll be removing use of distutils in the c-analyzer at some point. Until then we'll hide the warning filter. (cherry picked from commit 3ff6d9a) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
…als (pythonGH-95837) Under certain build conditions, test_check_c_globals fails. This fix takes the same approach as we took for pythongh-84236 (via pythongh-20095). We'll be removing use of distutils in the c-analyzer at some point. Until then we'll hide the warning filter.
When I run my own automated tests against main, I get the following test failure on macOS and Ubuntu.
We've previously added code to preserve
warnings.filters
around tests that importpkg_resources
it seems. I'm not entirely sure where it's coming from in this case, as it doesn't repro on Windows (or apparently on our buildbots), but I guess we need the same preservation around it.@ericsnowcurrently I think this may be yours?
The text was updated successfully, but these errors were encountered: