Patchwork win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'

login
register
mail settings
Submitter Matt Harbison
Date March 22, 2015, 11:12 p.m.
Message ID <658ab360e6dcebf9e5dd.1427065972@Envy>
Download mbox | patch
Permalink /patch/8218/
State Accepted
Commit a2285e2fc949d8a4df58af820bb00b51f64def14
Headers show

Comments

Matt Harbison - March 22, 2015, 11:12 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1427065693 14400
#      Sun Mar 22 19:08:13 2015 -0400
# Node ID 658ab360e6dcebf9e5dd4a87567d5313e6fd0fdd
# Parent  72763df8d4d8c775d800dea05937732caf050272
win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'

WinError is a function that creates an Error, not an Error itself.  This is a
partial backout of e34106fa0dc3.
Adrian Buehlmann - March 22, 2015, 11:54 p.m.
On 2015-03-23 00:12, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1427065693 14400
> #      Sun Mar 22 19:08:13 2015 -0400
> # Node ID 658ab360e6dcebf9e5dd4a87567d5313e6fd0fdd
> # Parent  72763df8d4d8c775d800dea05937732caf050272
> win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'
> 
> WinError is a function that creates an Error, not an Error itself.  This is a
> partial backout of e34106fa0dc3.
> 

Nice catch.
Matt Mackall - March 23, 2015, 7:25 p.m.
On Sun, 2015-03-22 at 19:12 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1427065693 14400
> #      Sun Mar 22 19:08:13 2015 -0400
> # Node ID 658ab360e6dcebf9e5dd4a87567d5313e6fd0fdd
> # Parent  72763df8d4d8c775d800dea05937732caf050272
> win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'
> 
> WinError is a function that creates an Error, not an Error itself.  This is a
> partial backout of e34106fa0dc3.

That was released way back in 2.3. Does this belong on stable? What
breaks here?
Matt Harbison - March 23, 2015, 7:42 p.m.
Matt Mackall - March 23, 2015, 7:55 p.m.
On Sun, 2015-03-22 at 19:12 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1427065693 14400
> #      Sun Mar 22 19:08:13 2015 -0400
> # Node ID 658ab360e6dcebf9e5dd4a87567d5313e6fd0fdd
> # Parent  72763df8d4d8c775d800dea05937732caf050272
> win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'

Ok, queued for stable, hopefully nothing explodes.
Adrian Buehlmann - March 23, 2015, 9:01 p.m.
On 2015-03-23 20:42, mharbison72@gmail.com wrote:
> On Mar 23, 2015 3:25 PM, Matt Mackall <mpm@selenic.com> wrote: 
> 
> On Sun, 2015-03-22 at 19:12 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1427065693 14400
>> # Sun Mar 22 19:08:13 2015 -0400
>> # Node ID 658ab360e6dcebf9e5dd4a87567d5313e6fd0fdd
>> # Parent 72763df8d4d8c775d800dea05937732caf050272
>> win32: 'raise ctypes.WinError' -> 'raise ctypes.WinError()'
>>
>> WinError is a function that creates an Error, not an Error itself.
> This is a
>> partial backout of e34106fa0dc3.
> 
> That was released way back in 2.3. Does this belong on stable? What
> breaks here?
> 
> 
> Stable is reasonable I guess.  This is one of those things that should
> never happen.
> 
> I was cargo culting in the area and copy pasted one of these lines.  But
> since I had other code that was wrong, a Win32 call failed, which
> triggered this.  Instead of printing an error with the Win32 code, it
> was a python error about raising something not a BaseException (I forget
> the exact wording)

I agree with Matt Harbison. For reference:

$ python
Python 2.7.3 (default, Apr 10 2012, 23:24:47) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> raise ctypes.WinError
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: exceptions must be old-style classes or derived from BaseException, not function
>>> raise ctypes.WinError()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
WindowsError: [Error 123] The filename, directory name, or volume label syntax is incorrect.
>>>

Patch

diff --git a/mercurial/win32.py b/mercurial/win32.py
--- a/mercurial/win32.py
+++ b/mercurial/win32.py
@@ -279,7 +279,7 @@ 
     buf = ctypes.create_string_buffer(size + 1)
     len = _kernel32.GetModuleFileNameA(None, ctypes.byref(buf), size)
     if len == 0:
-        raise ctypes.WinError
+        raise ctypes.WinError()
     elif len == size:
         raise ctypes.WinError(_ERROR_INSUFFICIENT_BUFFER)
     return buf.value
@@ -289,7 +289,7 @@ 
     size = _DWORD(300)
     buf = ctypes.create_string_buffer(size.value + 1)
     if not _advapi32.GetUserNameA(ctypes.byref(buf), ctypes.byref(size)):
-        raise ctypes.WinError
+        raise ctypes.WinError()
     return buf.value
 
 _signalhandler = []
@@ -307,7 +307,7 @@ 
     h = _SIGNAL_HANDLER(handler)
     _signalhandler.append(h) # needed to prevent garbage collection
     if not _kernel32.SetConsoleCtrlHandler(h, True):
-        raise ctypes.WinError
+        raise ctypes.WinError()
 
 def hidewindow():
 
@@ -349,7 +349,7 @@ 
     # create handle to list all processes
     ph = _kernel32.CreateToolhelp32Snapshot(_TH32CS_SNAPPROCESS, 0)
     if ph == _INVALID_HANDLE_VALUE:
-        raise ctypes.WinError
+        raise ctypes.WinError()
     try:
         r = _kernel32.Process32First(ph, ctypes.byref(pe))
         # loop over all processes
@@ -361,7 +361,7 @@ 
     finally:
         _kernel32.CloseHandle(ph)
     if _kernel32.GetLastError() != _ERROR_NO_MORE_FILES:
-        raise ctypes.WinError
+        raise ctypes.WinError()
     return None # no child found
 
 class _tochildpid(int): # pid is _DWORD, which always matches in an int
@@ -413,7 +413,7 @@ 
         None, args, None, None, False, _CREATE_NO_WINDOW,
         env, os.getcwd(), ctypes.byref(si), ctypes.byref(pi))
     if not res:
-        raise ctypes.WinError
+        raise ctypes.WinError()
 
     # _tochildpid because the process is the child of COMSPEC
     return _tochildpid(pi.dwProcessId)