Opened 7 years ago

Last modified 6 years ago

#6627 defect new

Remove workarounds for Python < 2.6 from twisted.python.filepath

Reported by: Harry Bock Owned by: Harry Bock
Priority: normal Milestone:
Component: core Keywords: windows
Cc: Branch: branches/remove-filepath-workarounds-6627
branch-diff, diff-cov, branch-cov, buildbot
Author: rwall

Description (last modified by Thijs Triemstra)

In FilePath.children(), there are a few workarounds for exceptions in the os module that only occurred in Windows on CPython < 2.6. The workarounds work fine in Python 2.6-7 because OSError, IOError, and WindowsError are in a class hierarchy that allows exception clause specialization.

In Python 3.3, they're all aliases for OSError:

Python 3.3.0 (v3.3.0:bd8afb90ebf2, Sep 29 2012, 10:57:17) [MSC v.1600 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> IOError
<class 'OSError'>
>>> OSError
<class 'OSError'>
>>> WindowsError

This causes issues in Python 3.x, because the WindowsError specialization is always caught and UnlistableError cannot be raised. As of Python 2.6+, the errno attribute of WindowsError has the expected value, and since it is an OSError, it would be best to combine these checks under OSError.

Attachments (2)

twisted.python.filepath-6627-1.patch (4.1 KB) - added by Harry Bock 7 years ago.
suggested patch, attempt #1
twisted.python.filepath-6627-2.patch (4.6 KB) - added by Harry Bock 7 years ago.
suggested patch, attempt #2 - now with 100% more topfiles

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by Harry Bock

Removed from review per the suggestion of Tom Prince to break it up.

Changed 7 years ago by Harry Bock

suggested patch, attempt #1

comment:2 Changed 7 years ago by Harry Bock

Keywords: review added

Changed 7 years ago by Harry Bock

suggested patch, attempt #2 - now with 100% more topfiles

comment:3 Changed 7 years ago by Harry Bock

Please ignore twisted.python.filepath-6627-1.patch - I was missing 6627.topfiles.

comment:4 Changed 6 years ago by Thijs Triemstra

Description: modified (diff)

Updating ticket description markup.

comment:5 Changed 6 years ago by Richard Wall

Author: rwall
Branch: branches/remove-filepath-workarounds-6627

(In [39959]) Branching to 'remove-filepath-workarounds-6627'

comment:6 Changed 6 years ago by Richard Wall

(In [39960]) apply twisted.python.filepath-6627-2.patch from hbock. refs #6627

comment:7 Changed 6 years ago by Richard Wall

Keywords: review removed
Owner: set to Harry Bock

Thanks hbock,

I applied your patch to a branch and forced a build.

Notes:

  • Some warnings when applying the patch
    [richard@zorin remove-filepath-workarounds-6627]$ patch -p0 < ~/Downloads/twisted.python.filepath-6627-2.patch
    (Stripping trailing CRs from patch; use --binary to disable.)
    patching file twisted/test/test_paths.py
    (Stripping trailing CRs from patch; use --binary to disable.)
    patching file twisted/topfiles/6627.bugfix
    (Stripping trailing CRs from patch; use --binary to disable.)
    patching file twisted/python/filepath.py
    
    

Points:

  1. Build Failures
    1. As you can see, all the builds failed. You'll need to modify the existing tests to accommodate your change.
    2. It looks like you modified the FakeWindowsPath to raise OSError, but at the same time you're still raising _WindowsUnlistableError for "compatibility with legacy users."
    3. https://buildbot.twistedmatrix.com/builders/pyflakes/builds/993/steps/pyflakes/logs/new%20pyflakes%20errors twisted/test/test_paths.py:15: 'ERROR_DIRECTORY' imported but unused

Please address the numbered points above, create another patch (against the new branch) and resubmit for review.

You'll find Windows Twisted users on #twisted or #twisted-dev if you want to discuss how to test this change.

Thanks again for your contribution.

Note: See TracTickets for help on using tickets.