test_compileall fails in AMD64 Windows7 SP1 3.x

[pablogsal]

BPO	38470
Nosy	@pfmoore, @vstinner, @tjguk, @jkloth, @encukou, @zware, @eryksun, @zooba, @hroncok, @pablogsal
PRs	bpo-38470: compileall tests: Use a shorter path on Windows #16778 bpo-38470: Fix test_compileall.test_compile_dir_maxlevels() #16789 bpo-38470: Add hostname support to ssl.get_server_certificate. #16819

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-10-15.13:42:59.480>
created_at = <Date 2019-10-14.00:45:36.631>
labels = ['type-bug', 'tests', '3.9', 'OS-windows']
title = 'test_compileall fails in AMD64 Windows7 SP1 3.x'
updated_at = <Date 2019-10-16.09:00:01.735>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2019-10-16.09:00:01.735>
actor = 'juhovh'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-15.13:42:59.480>
closer = 'vstinner'
components = ['Tests', 'Windows']
creation = <Date 2019-10-14.00:45:36.631>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 38470
keywords = ['patch']
message_count = 13.0
messages = ['354609', '354627', '354632', '354633', '354636', '354639', '354650', '354671', '354672', '354684', '354708', '354729', '354730']
nosy_count = 10.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'jkloth', 'petr.viktorin', 'zach.ware', 'eryksun', 'steve.dower', 'hroncok', 'pablogsal']
pr_nums = ['16778', '16789', '16819']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38470'
versions = ['Python 3.9']

[pablogsal]

This buildbot has been failing for a long time (example failure https://buildbot.python.org/all/#/builders/40/builds/3291) since 4267c98:

======================================================================
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 4267c98.

[encukou]

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 4267c98 will fix the bots.

[encukou]

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.

[pablogsal]

You can trigger custom buildbot runs by pushing to the 'buildbot-custom' branch in the CPython repo.

[vstinner]

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>

[zooba]

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.

[vstinner]

The new test was added by bpo-38112: commit 4267c98.

[jkloth]

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.

[vstinner]

I wrote PR 16789 which fix the issue.

[eryksun]

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.

[encukou]

New changeset eb1dda2 by Petr Viktorin (Victor Stinner) in branch 'master':
bpo-38470: Fix test_compileall.test_compile_dir_maxlevels() (GH-16789)
eb1dda2

[vstinner]

create_long_path isn't taking into account _write_atomic in Lib/importlib/_bootstrap_external.py.

Yeah, I found the same issue:
#16778 (comment)

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.
#16789 (comment)

Anyway, the issue should now be fixed.

[vstinner]

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