classification
Title: test_compileall fails in AMD64 Windows7 SP1 3.x
Type: behavior Stage: resolved
Components: Tests, Windows Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, hroncok, jkloth, pablogsal, paul.moore, petr.viktorin, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2019-10-14 00:45 by pablogsal, last changed 2019-10-16 09:00 by juhovh. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16778 closed petr.viktorin, 2019-10-14 13:31
PR 16789 merged vstinner, 2019-10-14 20:51
PR 16819 closed juhovh, 2019-10-16 09:00
Messages (13)
msg354609 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2019-10-14 22:01
I wrote PR 16789 which fix the issue.
msg354684 - (view) Author: Eryk Sun (eryksun) * (Python triager) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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
History
Date User Action Args
2019-10-16 09:00:01juhovhsetpull_requests: + pull_request16371
2019-10-15 13:44:30vstinnersetmessages: + msg354730
2019-10-15 13:42:59vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg354729

stage: patch review -> resolved
2019-10-15 09:26:22petr.viktorinsetmessages: + msg354708
2019-10-15 02:54:53eryksunsetnosy: + eryksun
messages: + msg354684
2019-10-14 22:01:47vstinnersetmessages: + msg354672
2019-10-14 22:00:59jklothsetnosy: + jkloth
messages: + msg354671
2019-10-14 20:56:13vstinnersetmessages: + msg354650
2019-10-14 20:51:24vstinnersetstage: patch review
pull_requests: + pull_request16350
2019-10-14 15:54:00steve.dowersetmessages: + msg354639
2019-10-14 14:50:46vstinnersetmessages: + msg354636
2019-10-14 13:53:56pablogsalsetmessages: + msg354633
2019-10-14 13:35:58petr.viktorinsetmessages: + msg354632
stage: patch review -> (no value)
2019-10-14 13:31:34petr.viktorinsetkeywords: + patch
stage: patch review
pull_requests: + pull_request16340
2019-10-14 11:32:09petr.viktorinsetmessages: + msg354627
2019-10-14 00:45:36pablogsalcreate