msg354609 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-10-14 00:45 |
This buildbot has been failing for a long time (example failure https://buildbot.python.org/all/#/builders/40/builds/3291) since https://github.com/python/cpython/commit/4267c989e7fc6cd528e8a1d04a07fac5cca85ec7:
======================================================================
FAIL: test_compile_dir_maxlevels (test.test_compileall.CompileallTestsWithSourceEpoch)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_py_compile.py", line 30, in wrapper
return fxn(*args, **kwargs)
File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_compileall.py", line 257, in test_compile_dir_maxlevels
self.assertTrue(os.path.isfile(self.bc_path_long))
AssertionError: False is not true
======================================================================
FAIL: test_compile_dir_maxlevels (test.test_compileall.CompileallTestsWithoutSourceEpoch)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_py_compile.py", line 20, in wrapper
return fxn(*args, **kwargs)
File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_compileall.py", line 257, in test_compile_dir_maxlevels
self.assertTrue(os.path.isfile(self.bc_path_long))
AssertionError: False is not true
----------------------------------------------------------------------
Can someone try to fix the error, otherwise we would need to revert commit 4267c989e7fc6cd528e8a1d04a07fac5cca85ec7.
|
msg354627 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2019-10-14 11:32 |
Oh! Sorry, I thought I saw green bots, and haven't looked since.
I should have time for this tomorrow.
If that's too late, remove/skip test_compile_dir_maxlevels on Windows. (The feature was untested before, and the 3.9 enhancement added test coverage, so I think it's fair to skip the failing test.)
I don't think simply reverting 4267c989e7fc6cd528e8a1d04a07fac5cca85ec7 will fix the bots.
|
msg354632 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2019-10-14 13:35 |
If someone has access to Windows and can reproduce this (you need a more special setup than "just Windows"), and you want to spend some time debugging, please do. I'm shooting in the dark.
My proposed patch limits the maximum path depth to 20 directories on Windows. That is enough to test compileall's maxlevels option. Still a longer path would be nicer if possible.
|
msg354633 - (view) |
Author: Pablo Galindo Salgado (pablogsal) *  |
Date: 2019-10-14 13:53 |
You can trigger custom buildbot runs by pushing to the 'buildbot-custom' branch in the CPython repo.
|
msg354636 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-10-14 14:50 |
To reproduce the recent on a recent Windows 10, you can opt-out for long path using this change:
diff --git a/PC/python.manifest b/PC/python.manifest
index 8e1bc022ad..524f9b2f6d 100644
--- a/PC/python.manifest
+++ b/PC/python.manifest
@@ -18,7 +18,6 @@
</compatibility>
<application xmlns="urn:schemas-microsoft-com:asm.v3">
<windowsSettings>
- <longPathAware xmlns="http://schemas.microsoft.com/SMI/2016/WindowsSettings">true</longPathAware>
</windowsSettings>
</application>
<dependency>
|
msg354639 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2019-10-14 15:53 |
I'm okay with skipping the failing test if we detect sys.getwindowsversion().major < 10
Windows 7 is not supported for Python 3.9, so this buildbot can be disabled/upgraded.
Windows 8.1 will be supported for at least 3.9, so we'll have to keep skipping the test there.
|
msg354650 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-10-14 20:56 |
The new test was added by bpo-38112: commit 4267c989e7fc6cd528e8a1d04a07fac5cca85ec7.
|
msg354671 - (view) |
Author: Jeremy Kloth (jkloth) * |
Date: 2019-10-14 22:00 |
> Windows 7 is not supported for Python 3.9, so this buildbot can be disabled/upgraded.
As long as 3.7 and 3.8 are being tested through the buildbots, I would think testing on Windows 7 is still advised.
That said, once those versions are no longer tested (or deemed to not need Windows 7 support) I will be upgrading that buildbot.
|
msg354672 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-10-14 22:01 |
I wrote PR 16789 which fix the issue.
|
msg354684 - (view) |
Author: Eryk Sun (eryksun) *  |
Date: 2019-10-15 02:54 |
I tested in Windows 10 with long paths disabled, and the FileNotFoundError exception showed that the failing .pyc had a numeric suffix appended to the name.
create_long_path isn't taking into account _write_atomic in Lib/importlib/_bootstrap_external.py. This function appends the id() of the target path string in order to create a temporary unique name, which it renames to the target if the write succeeds. Since the object ID is a memory address, create_long_path would need to append 16 characters to the longer_cache path. That's 1 for the initial "." of the suffix, plus 15 for the x64 address-space limit of 128 TB, i.e. len(str(0x7FFF_FFFF_FFFF)).
Note that, by default, long-path support is not enabled in Windows 10. At process startup, the system runtime library enables long-path support if "LongPathsEnabled" is set in "HKLM\SYSTEM\CurrentControlSet\Control\FileSystem" and the application manifest enables "longPathAware". Both settings are required.
For the buildbots, pythoninfo checks this as "windows.RtlAreLongPathsEnabled". According to this, for example, bolen-windows10 does not have long paths enabled. Yet test_compileall passed.
bolen-windows10
https://buildbot.python.org/all/#/builders/3/builds/3681/steps/3/logs/stdio
It seems the test passed because the temp directory on this bot is relatively short: "D:\Temp". That's a factor because create_long_path adds directory components in blocks of 10. It uses 20 "dir_X_Y" components because 30 (240 characters) is too much for MAX_PATH no matter what.
The same applies to the following buildbots for Windows 7 and 8.1:
bolen-windows7
https://buildbot.python.org/all/#/builders/58/builds/3148/steps/3/logs/stdio
ware-win81-release
https://buildbot.python.org/all/#/builders/12/builds/3360/steps/3/logs/stdio
On the other hand, kloth-win64 has a longer path for its temp directory: "C:\Users\Buildbot\AppData\Local\Temp". This costs an additional 29 characters compared to "D:\Temp" and brings the final path very close to MAX_PATH. The suffix appended by _write_atomic is typically 14-16 characters, which then exceeds MAX_PATH.
|
msg354708 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2019-10-15 09:26 |
New changeset eb1dda2b56f67f09352c303588c28880c471ae87 by Petr Viktorin (Victor Stinner) in branch 'master':
bpo-38470: Fix test_compileall.test_compile_dir_maxlevels() (GH-16789)
https://github.com/python/cpython/commit/eb1dda2b56f67f09352c303588c28880c471ae87
|
msg354729 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-10-15 13:42 |
> create_long_path isn't taking into account _write_atomic in Lib/importlib/_bootstrap_external.py.
Yeah, I found the same issue:
https://github.com/python/cpython/pull/16778#issuecomment-541764285
I began to write a complex change to take _write_atomic() in account, but the test became super complex. I don't think that it's worth it. So I wrote a much simpler PR.
https://github.com/python/cpython/pull/16789#issuecomment-541918432
Anyway, the issue should now be fixed.
|
msg354730 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-10-15 13:44 |
It seems like test_compileall now pass on the 4 Windows workers:
* https://buildbot.python.org/all/#/builders/3
* https://buildbot.python.org/all/#/builders/12
* https://buildbot.python.org/all/#/builders/40
* https://buildbot.python.org/all/#/builders/58
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:21 | admin | set | github: 82651 |
2019-10-16 09:00:01 | juhovh | set | pull_requests:
+ pull_request16371 |
2019-10-15 13:44:30 | vstinner | set | messages:
+ msg354730 |
2019-10-15 13:42:59 | vstinner | set | status: open -> closed resolution: fixed messages:
+ msg354729
stage: patch review -> resolved |
2019-10-15 09:26:22 | petr.viktorin | set | messages:
+ msg354708 |
2019-10-15 02:54:53 | eryksun | set | nosy:
+ eryksun messages:
+ msg354684
|
2019-10-14 22:01:47 | vstinner | set | messages:
+ msg354672 |
2019-10-14 22:00:59 | jkloth | set | nosy:
+ jkloth messages:
+ msg354671
|
2019-10-14 20:56:13 | vstinner | set | messages:
+ msg354650 |
2019-10-14 20:51:24 | vstinner | set | stage: patch review pull_requests:
+ pull_request16350 |
2019-10-14 15:54:00 | steve.dower | set | messages:
+ msg354639 |
2019-10-14 14:50:46 | vstinner | set | messages:
+ msg354636 |
2019-10-14 13:53:56 | pablogsal | set | messages:
+ msg354633 |
2019-10-14 13:35:58 | petr.viktorin | set | messages:
+ msg354632 stage: patch review -> (no value) |
2019-10-14 13:31:34 | petr.viktorin | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request16340 |
2019-10-14 11:32:09 | petr.viktorin | set | messages:
+ msg354627 |
2019-10-14 00:45:36 | pablogsal | create | |