Opened 13 years ago

Closed 13 years ago

#2136 defect closed fixed (fixed)

t.plugin.getCache craps out on Windows with Python 2.5

Reported by: PenguinOfDoom Owned by:
Priority: highest Milestone: Core-2.5
Component: core Keywords: win32
Cc: TimothyFitz, teratorn Branch:
Author:

Description (last modified by Jean-Paul Calderone)

I'm using patched WinXP and python2.5 (not sure if that's relevant). os.listdir on a non-existent directory fails with ERROR_FILE_NOT_FOUND, which getCache does not expect. Here's a patch:

Index: twisted/plugin.py
===================================================================
--- twisted/plugin.py   (revision 18338)
+++ twisted/plugin.py   (working copy)
@@ -118,6 +118,7 @@
         """
 
 # http://msdn.microsoft.com/library/default.asp?url=/library/en-us/debug/base/system_error_codes.asp
+ERROR_FILE_NOT_FOUND = 2
 ERROR_PATH_NOT_FOUND = 3
 ERROR_INVALID_NAME = 123
 
@@ -136,7 +137,7 @@
         try:
             dropinNames = os.listdir(p)
         except WindowsError, e:
-            if e.errno == ERROR_PATH_NOT_FOUND:
+            if e.errno in (ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND):
                 continue
             elif e.errno == ERROR_INVALID_NAME:
                 log.msg("Invalid path %r in search path for %s" % (p, module.__name__))

Attachments (4)

2136.diff (780 bytes) - added by PenguinOfDoom 13 years ago.
filepath.py (15.2 KB) - added by scmikes 13 years ago.
plugin.py (6.5 KB) - added by scmikes 13 years ago.
patch.patch (5.9 KB) - added by scmikes 13 years ago.

Download all attachments as: .zip

Change History (20)

Changed 13 years ago by PenguinOfDoom

Attachment: 2136.diff added

comment:1 Changed 13 years ago by Glyph

This needs a test, and I bet it would be easier to write if we just bit the bullet and moved plugins to use filepath.

comment:2 Changed 13 years ago by Jean-Paul Calderone

Description: modified (diff)
Milestone: Twisted-2.5

comment:3 Changed 13 years ago by Jean-Paul Calderone

Python 2.5 is in fact relevant. Python 2.4 and earlier raised OSError here, not WindowsError.

comment:4 Changed 13 years ago by scmikes

Priority: highhighest
Type: enhancementdefect

This should be raised to highest, blocker. This defect is preventing trial from running on the python 2.5 windows buildbot.

comment:5 Changed 13 years ago by scmikes

I don't know how to write a test case for this. but I have been running with the patch locally for several weeks with no problems.

comment:6 Changed 13 years ago by scmikes

This test case validates the python idiom used ###############################################

def testPythonDirNotFound(self):

""" test that exceptions that python throws when files not found

This test validates that the semantic behavior of detecting a directory that does not exist is correct.

This python idiom is used in plugins.py getcache()

""" p="zzzThisDirDoesNotExist"

# http://msdn.microsoft.com/library/default.asp?url=/library/en-us/debug/base/system_error_codes.asp ERROR_FILE_NOT_FOUND = 2 ERROR_PATH_NOT_FOUND = 3 ERROR_INVALID_NAME = 123

dirNotValid=False

for x in range(2):

try:

dropinNames = os.listdir(p)

except WindowsError, e:

if e.errno in (ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND): # mls old code from plugin -> if e.errno == ERROR_PATH_NOT_FOUND:

dirNotValid=True continue

elif e.errno == ERROR_INVALID_NAME:

#log.msg("Invalid path %r in search path for %s" % (p, module.name)) print "invalid path" continue

else:

raise

except OSError, ose:

if ose.errno not in (errno.ENOENT, errno.ENOTDIR):

raise

else:

continue

self.assertTrue(dirNotValid)

comment:7 Changed 13 years ago by scmikes

added 3 braces around code


    def testPythonDirNotFound(self):
        """ test that exceptions that python throws when files not found 
        
        This test validates that the semantic behavior of detecting a directory that 
        does not exist is correct.
        
        This python idiom is used in plugins.py getcache()
        
        """
        p="zzzThisDirDoesNotExist"
        
        # http://msdn.microsoft.com/library/default.asp?url=/library/en-us/debug/base/system_error_codes.asp
        ERROR_FILE_NOT_FOUND = 2 
        ERROR_PATH_NOT_FOUND = 3
        ERROR_INVALID_NAME = 123
        
        dirNotValid=False
        
        for x in range(2):
                try:
                    dropinNames = os.listdir(p)
                except WindowsError, e:
                    if e.errno in (ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND):
                    #  mls old code from plugin ->            if e.errno == ERROR_PATH_NOT_FOUND:
                        dirNotValid=True
                        continue
                    elif e.errno == ERROR_INVALID_NAME:
                        #log.msg("Invalid path %r in search path for %s" % (p, module.__name__))
                        print "invalid path"
                        continue
                    else:
                        raise
                except OSError, ose:
                    if ose.errno not in (errno.ENOENT, errno.ENOTDIR):
                        raise
                    else:
                        continue   
                     
        self.assertTrue(dirNotValid)

comment:8 Changed 13 years ago by Jonathan Lange

Owner: changed from Glyph to Jonathan Lange

comment:9 Changed 13 years ago by Glyph

Cc: TimothyFitz teratorn added

I was going to reassign, but for the fact that jml stole this so recently. Anyway, our illustrious Windows team really ought to hear about such things.

comment:10 Changed 13 years ago by scmikes

I don't know if this is useful, but I started working on the switch to t.p.filepath.

Someone more familiar with getCache can make better use of filepath.

I did make changes to t.p.filepath to throw an exception t.p.filepath.DirNotFound when directory does not exist.

here is a test case for FilePath.saneListdir

    def testPythonDirNotFound(self):
        """ test that t.p.DirNotFound is thrown when directory is not found """
        p="zzzThisDirDoesNotExist"
      
        from twisted.python.filepath import FilePath, DirNotFound
        fp = FilePath(p)
        
        self.assertRaises(DirNotFound, fp.saneListdir)
        

I will attach the modified twisted file, use them if they help, ignore the if they do not help



Changed 13 years ago by scmikes

Attachment: filepath.py added

Changed 13 years ago by scmikes

Attachment: plugin.py added

Changed 13 years ago by scmikes

Attachment: patch.patch added

comment:11 Changed 13 years ago by Glyph

Summary: t.plugin.getCache craps out on Windowst.plugin.getCache craps out on Windows with Python 2.5

comment:12 Changed 13 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jonathan Lange deleted

Probably fixed in plugin-cache-2136. Cannot test since the Win32 Python 2.5 buildslave is offline at the moment.

comment:13 Changed 13 years ago by Jonathan Lange

Keywords: win32 added

comment:14 Changed 13 years ago by Jonathan Lange

Keywords: review removed
Owner: set to Jean-Paul Calderone

The code looks pretty good. I'm not exactly sure how the added tests exercise the changed code -- I guess Windows raises a new kind of error.

Merge away.

comment:15 Changed 13 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [18690]) Merge plugin-cache-2136

Author: exarkun Reviewer: jml Fixes #2136

Add error handling for ERROR_FILE_NOT_FOUND to the plugin system for Python 2.5 on Windows. Also add test coverage for the environment which can result in this error.

comment:16 Changed 9 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.