Skip to content
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

test_compileall fails in AMD64 Windows7 SP1 3.x #82651

Closed
pablogsal opened this issue Oct 14, 2019 · 13 comments
Closed

test_compileall fails in AMD64 Windows7 SP1 3.x #82651

pablogsal opened this issue Oct 14, 2019 · 13 comments
Labels
3.9 only security fixes OS-windows tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@pablogsal
Copy link
Member

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
    Copy link
    Member Author

    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.

    @pablogsal pablogsal added 3.9 only security fixes tests Tests in the Lib/test dir OS-windows type-bug An unexpected behavior, bug, or error labels Oct 14, 2019
    @encukou
    Copy link
    Member

    encukou commented Oct 14, 2019

    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
    Copy link
    Member

    encukou commented Oct 14, 2019

    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
    Copy link
    Member Author

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

    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

    zooba commented Oct 14, 2019

    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
    Copy link
    Member

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

    @jkloth
    Copy link
    Contributor

    jkloth commented Oct 14, 2019

    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
    Copy link
    Member

    I wrote PR 16789 which fix the issue.

    @eryksun
    Copy link
    Contributor

    eryksun commented Oct 15, 2019

    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
    Copy link
    Member

    encukou commented Oct 15, 2019

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

    @vstinner
    Copy link
    Member

    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
    Copy link
    Member

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes OS-windows tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants