classification
Title: Add {Create|Delete}KeyEx to _winreg, doc and test updates
Type: enhancement Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: 7860 Superseder:
Assigned To: brian.curtin Nosy List: brian.curtin, eric.smith, ggenellina
Priority: normal Keywords: 64bit, needs review, patch

Created on 2009-11-18 17:48 by brian.curtin, last changed 2010-04-21 23:56 by brian.curtin. This issue is now closed.

Files
File name Uploaded Description Edit
winreg_add_createkeyex.patch brian.curtin, 2009-11-18 17:55 patch against r76365
winreg_add_createkeyex_v2.patch brian.curtin, 2009-11-20 21:07 patch against r76432
issue7347.diff brian.curtin, 2010-03-15 23:43
Messages (13)
msg95434 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2009-11-18 17:48
The _winreg module could use the addition of the RegCreateKeyEx call, as
evidenced by this thread on c.l.py:
http://mail.python.org/pipermail/python-list/2009-November/614023.html
This expanded API would benefit users trying to create keys with access
masks (like KEY_WOW64_64KEY), which you can't set through _winreg.CreateKey.

The patch includes the change to _winreg.c, along with doc and test
changes. Comments/suggestions appreciated.
msg95436 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2009-11-18 17:55
Updated patch - c&p mistake in a comment
msg95563 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2009-11-20 21:07
After looking at this more, I poked around and found a whole lot of
things either missing or broken with _winreg.

Changes:
- documentation missing for *ReflectionKey, and updated a few
incorrectly documented exceptions in the docstrings
- QueryReflectionKey always returned False if the underlying API call
succeeded (used return code from call instead of bool param)
- addition of RegCreateKeyEx and RegDeleteKeyEx for 64-bit support
- changed the test suite to cover much more of the _winreg API. There
are now classes for local, remote, and 64-bit specific tests, with some
tests in the 64-bit class which are specific to whether Python was
compiled for 32 or 64-bit due to the differences in how _winreg acts.


So far everything passes on XP 32-bit, Server 2003 32-bit, and Server
2003 64-bit with a 32-bit Python. On Server 2003 64-bit with a 64-bit
Python, I get one failure on test_create_open_delete_for_32bit about
DeleteKeyEx -- not sure what the deal is, but I'm looking into it.

There is an added file, delete_regkey.vbs, because 32-bit applications
can create keys in the 64-bit space, but apparently they cannot be
deleted with DeleteKey -- they'd need DeleteKeyEx but that's 64-bit
only. It's used as a cleanup to make sure the key gets deleted in one case.
msg98251 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-24 20:17
After looking into this again, the testing situations are much different than I had originally written.

Some of the tests aren't really valid exercises. For one, the querying/enabling/disabling of reflection keys aren't good tests as they aren't tested against reflected keys. Windows 7 and Server 2008 R2 act differently in reflection situations when compared to XP/Vista/Server 2003, and I'll need to account for that.

For now, forget about the current patches, I'm working on a more correct testing approach.
msg98952 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-06 19:01
This needs #7860 for properly figuring out the machine architecture for some 32-bit Python on 64-bit Windows tests.
msg98967 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-06 22:12
Attached is what I believe is the complete patch. You'll need to apply the patch on #7860 for proper test coverage of a 32-bit Python running on 64-bit Windows.

Here's a summary of what's contained:

1. Documented and tested the previously undocumented and untested *ReflectionKey functions
2. Added, documented, and tested CreateKeyEx
3. Added, documented, and tested DeleteKeyEx
4. Fixed QueryReflectionKey to return the key state rather than the return value of the underlying Windows call.

Overall the theme is to better our support of 64-bit Windows.

The testing scenario becomes a bit more involved given the spread of Windows versions supported and their varying level of support of some of the APIs being exposed. I think the tests are documented well enough to explain what scenarios are available and which are being tested, but let me know if more detail is needed.

I've tested this on the following machines with success:
1. Windows XP SP2 32-bit
2. Windows Server 2003 32-bit
3. Windows Server 2003 64-bit with 32 and 64-bit Python
4. Windows 7 64-bit with 32 and 64-bit Python
msg100286 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-03-02 01:57
I've uploaded the patch to http://codereview.appspot.com/223088
msg101137 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-03-15 23:43
Fixed a few a/an word changes and a few tab/space issues. Re-uploaded to Rietveld at http://codereview.appspot.com/580041
msg101364 - (view) Author: Gabriel Genellina (ggenellina) Date: 2010-03-20 05:44
Why the *Ex names? Can't we just add additional arguments to the original names?

The Python names do not necesarily have to match the API calls. Having QueryValue and QueryValueEx was a mistake in the first place, and I would prefer not continuing doing such things.
msg101374 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-03-20 14:28
RegDeleteKeyEx will only work on a Windows version of 5.2 or greater (Vista/XP x64), and XP is 5.1, so RegDeleteKeyEx can't be a simple drop-in under the "DeleteKey" name.

CreateKeyEx is different though since it goes as far back as Win2k, and it could be put into CreateKey like you suggest. However, given the way the rest of the module is laid out with *Key and *KeyEx already, it seemed more consistent the way I had written it. I could change this if more people agree with your plan.
msg102029 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-03-31 19:03
Gabriel, besides the *Ex naming, do you see anything wrong with the rest of the patch? I'd like to try and get this into 2.7 before the upcoming beta.
msg102206 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-04-02 21:19
Committed to trunk in r79620. I'll do the forward port after 2.7b1.
msg103926 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-04-21 23:56
Ported to py3k in r80329.
History
Date User Action Args
2010-04-21 23:56:50brian.curtinsetstatus: open -> closed
resolution: fixed
messages: + msg103926
2010-04-02 21:19:53brian.curtinsetmessages: + msg102206
stage: patch review -> resolved
2010-03-31 19:03:19brian.curtinsetmessages: + msg102029
2010-03-20 14:28:52brian.curtinsetmessages: + msg101374
2010-03-20 05:44:45ggenellinasetnosy: + ggenellina
messages: + msg101364
2010-03-15 23:43:10brian.curtinsetfiles: + issue7347.diff

messages: + msg101137
2010-03-15 23:42:13brian.curtinsetfiles: - issue7347.diff
2010-03-02 01:57:28brian.curtinsetmessages: + msg100286
2010-02-06 22:13:04brian.curtinsetfiles: + issue7347.diff
versions: + Python 3.2
messages: + msg98967

components: + Windows
keywords: + 64bit, needs review
stage: test needed -> patch review
2010-02-06 19:01:49brian.curtinsetdependencies: + 32-bit Python on 64-bit Windows reports incorrect architecture
messages: + msg98952
2010-01-24 20:17:11brian.curtinsetassignee: brian.curtin
messages: + msg98251
2010-01-13 02:07:00brian.curtinsetpriority: normal
stage: test needed
2009-11-20 21:07:47brian.curtinsetfiles: + winreg_add_createkeyex_v2.patch

messages: + msg95563
title: Patch - add RegCreateKeyEx to _winreg -> Add {Create|Delete}KeyEx to _winreg, doc and test updates
2009-11-18 19:58:37eric.smithsetnosy: + eric.smith
2009-11-18 17:55:44brian.curtinsetfiles: + winreg_add_createkeyex.patch

messages: + msg95436
2009-11-18 17:55:02brian.curtinsetfiles: - winreg_add_createkeyex.patch
2009-11-18 17:48:14brian.curtincreate