classification
Title: Expose RegUnLoadKey in winreg
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: 22080 Superseder:
Assigned To: Nosy List: Claudiu.Popa, eryksun, loewis, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2014-05-17 17:12 by Claudiu.Popa, last changed 2015-04-30 10:34 by Claudiu.Popa.

Files
File name Uploaded Description Edit
winreg_unload_key.patch Claudiu.Popa, 2014-05-17 17:12 review
issue21518.patch Claudiu.Popa, 2014-05-18 21:38 review
issue21518_1.patch Claudiu.Popa, 2014-05-18 22:55 review
issue21518_2.patch Claudiu.Popa, 2014-06-13 16:14 review
winreg_unload_key.patch Claudiu.Popa, 2014-07-08 21:05 review
issue21518_2.patch Claudiu.Popa, 2015-01-17 17:53 review
issue21518_3.patch Claudiu.Popa, 2015-01-22 09:39
issue21518_4.patch Claudiu.Popa, 2015-01-22 09:54 use context manager
issue21518_5.patch Claudiu.Popa, 2015-03-07 21:09 Fix import error review
issue21518_6.patch Claudiu.Popa, 2015-03-14 11:38 review
Messages (16)
msg218708 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-05-17 17:12
Hello. While working on issue8579, I noticed that there is no way to detach a key from the registry, loaded with LoadKey function. The attached patch exposes RegUnLoadKeyW as winreg.UnloadKey. Also, this patch adds a new script in the test folder, windows_helper.py, which could be an useful addition for testing Windows specific issues, like acquiring / releasing a privilege.
msg218709 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-05-17 18:11
It needs administrator elevation for running the test. I'll update the patch to skip the test if the user doesn't have elevation.
msg218757 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-05-18 21:38
This version of the patch skips the test if the privileges can't be acquired.
msg218758 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2014-05-18 22:30
ctypes LibraryLoader instances cache CDLL/WinDLL instances, which cache function pointers. Using the global ctypes.cdll and ctypes.windll loaders can lead to conflicting definitions for restype, argtypes, and errcheck. I suggest using a private LibraryLoader in windows_helper, e.g.:

    windll = ctypes.LibraryLoader(ctypes.WinDLL)
msg218764 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-05-18 22:55
Thanks. Here's the updated version. Also, I only tested it on Windows 8.1. I'll try to find another machine with an older OS for testing it.
msg220463 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-13 16:14
Attached a new version of the patch which cleanups properly after tests.
msg222433 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-07-07 05:36
Any type of feedback will be appreciated.
msg222471 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-07 16:09
I'd like to see the windows_helper.py part of the patch split into its own issue, and someone more qualified than I to review it (though I'll learn what I can and give it a shot if nobody else can).  The new issue should also try to use the new windows_helper utilities to replace what Brian mentioned in msg106613 for os.symlink privileges, since that's an existing place where it can be used.

The actual change of adding UnloadKey looks fine to me, aside from a copy/paste error in the documentation (s/subkey to load/subkey to unload/).
msg222472 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-07-07 16:09
Sure, that sounds good.
msg222583 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-07-08 21:05
Here's the patch with only the change for winreg.UnloadKey. I'll have the patch with windows_helper soon.
msg227580 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-09-26 00:26
Is there something I can do to move this forward?
msg234180 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2015-01-17 17:53
The new patch drops the weird error dance from test_unload_key, it seems to work without it, I don't remember how it failed without it.
msg234487 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2015-01-22 09:39
Ups, the last patch included an extra file.
msg238052 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2015-03-13 20:33
The naming of the function needs discussion. I think it should be UnLoadKey, as the API function behind it is RegUnLoadKey (not RegUnloadKey). It may be illogical(*) that the function is called that way in the API, but it would add confusion if Python called it differently.

(*) It's UnlockFile that pairs LockFile, and UnmapViewOfFile that matches MapViewOfFile. OTOH, (undocumented) UTRegister is paired with UTUnRegister, and GlobalWire with GlobalUnWire.
msg238078 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2015-03-14 11:38
Thank you for the review. The new patch uses the name UnLoadKey for the API.
msg242266 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2015-04-30 10:34
Hello,

Can anyone review the last patch? Hopefully it is the final version, since the beta is really at the corner and I definitely would like to have this in 3.5.
History
Date User Action Args
2015-04-30 10:34:21Claudiu.Popasetmessages: + msg242266
2015-03-14 11:38:11Claudiu.Popasetfiles: + issue21518_6.patch

messages: + msg238078
2015-03-13 20:33:44loewissetnosy: + loewis

messages: + msg238052
title: Expose RegUnloadKey in winreg -> Expose RegUnLoadKey in winreg
2015-03-07 21:09:37Claudiu.Popasetfiles: + issue21518_5.patch
2015-01-22 09:54:52Claudiu.Popasetfiles: + issue21518_4.patch
2015-01-22 09:39:13Claudiu.Popasetfiles: + issue21518_3.patch

messages: + msg234487
2015-01-17 17:53:05Claudiu.Popasetfiles: + issue21518_2.patch

messages: + msg234180
2014-09-26 00:26:09Claudiu.Popasetmessages: + msg227580
2014-07-26 12:42:49Claudiu.Popasetdependencies: + Add windows_helper module helper
2014-07-08 21:05:07Claudiu.Popasetfiles: + winreg_unload_key.patch

messages: + msg222583
2014-07-07 16:09:52Claudiu.Popasetmessages: + msg222472
2014-07-07 16:09:01zach.waresetmessages: + msg222471
2014-07-07 05:36:21Claudiu.Popasetmessages: + msg222433
stage: patch review
2014-06-13 16:14:22Claudiu.Popasetfiles: + issue21518_2.patch

messages: + msg220463
2014-06-13 14:44:46Claudiu.Popalinkissue8579 dependencies
2014-05-18 22:55:12Claudiu.Popasetfiles: + issue21518_1.patch

messages: + msg218764
2014-05-18 22:30:37eryksunsetnosy: + eryksun
messages: + msg218758
2014-05-18 21:39:02Claudiu.Popasetfiles: + issue21518.patch

messages: + msg218757
2014-05-17 18:11:40Claudiu.Popasetmessages: + msg218709
2014-05-17 17:15:10berker.peksagsetnosy: + tim.golden, zach.ware, steve.dower
2014-05-17 17:12:40Claudiu.Popacreate