Add {Create|Delete}KeyEx to _winreg, doc and test updates

[briancurtin]

BPO	7347
Nosy	@ericvsmith, @briancurtin
Dependencies	bpo-7860: 32-bit Python on 64-bit Windows reports incorrect architecture
Files	winreg_add_createkeyex.patch: patch against r76365 winreg_add_createkeyex_v2.patch: patch against r76432 issue7347.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/briancurtin'
closed_at = <Date 2010-04-21.23:56:50.737>
created_at = <Date 2009-11-18.17:48:13.967>
labels = ['extension-modules', 'type-feature', 'OS-windows']
title = 'Add {Create|Delete}KeyEx to _winreg, doc and test updates'
updated_at = <Date 2010-04-21.23:56:50.736>
user = 'https://github.com/briancurtin'

bugs.python.org fields:

activity = <Date 2010-04-21.23:56:50.736>
actor = 'brian.curtin'
assignee = 'brian.curtin'
closed = True
closed_date = <Date 2010-04-21.23:56:50.737>
closer = 'brian.curtin'
components = ['Extension Modules', 'Windows']
creation = <Date 2009-11-18.17:48:13.967>
creator = 'brian.curtin'
dependencies = ['7860']
files = ['15360', '15370', '16558']
hgrepos = []
issue_num = 7347
keywords = ['patch', '64bit', 'needs review']
message_count = 13.0
messages = ['95434', '95436', '95563', '98251', '98952', '98967', '100286', '101137', '101364', '101374', '102029', '102206', '103926']
nosy_count = 3.0
nosy_names = ['ggenellina', 'eric.smith', 'brian.curtin']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7347'
versions = ['Python 2.7', 'Python 3.2']

[briancurtin]

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.

[briancurtin]

Updated patch - c&p mistake in a comment

[briancurtin]

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.

[briancurtin]

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.

[briancurtin]

This needs bpo-7860 for properly figuring out the machine architecture for some 32-bit Python on 64-bit Windows tests.

[briancurtin]

Attached is what I believe is the complete patch. You'll need to apply the patch on bpo-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

[briancurtin]

I've uploaded the patch to http://codereview.appspot.com/223088

[briancurtin]

Fixed a few a/an word changes and a few tab/space issues. Re-uploaded to Rietveld at http://codereview.appspot.com/580041

[ggenellina]

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.

[briancurtin]

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.

[briancurtin]

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.

[briancurtin]

Committed to trunk in r79620. I'll do the forward port after 2.7b1.

[briancurtin]

Ported to py3k in r80329.