Title: importlib doesn't check Windows registry for paths
Type: behavior Stage: test needed
Components: Windows Versions: Python 3.3
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, brett.cannon, brian.curtin, eric.snow, georg.brandl, loewis, pitrou, python-dev
Priority: deferred blocker Keywords: patch

Created on 2012-04-14 17:07 by brett.cannon, last changed 2012-07-28 19:36 by loewis. This issue is now closed.

File name Uploaded Description Edit amaury.forgeotdarc, 2012-07-22 20:53 Example implementation of a WindowsRegistryImporter
winreg_loader.diff amaury.forgeotdarc, 2012-07-27 06:33 review
Messages (26)
msg158265 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-14 17:07
Because I don't have access to Windows, importlib doesn't check the Windows registry for paths to search (see the use of _PyWin_FindRegisteredModule() in Python/import.c and its definition in PC/import_nt.c).

I am considering this a release blocker as once importlib is bootstrapped in this will be a regression that Windows users may not be happy with.
msg158266 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-14 17:32
Yeah, that's one part of imp.find_module that I kind of set aside too (the #ifdef MS_COREDLL sections in Python/import.c).  For now would it be sufficient to expose _PyWin_FindRegisteredModule() privately with a wrapper in the imp module?
msg158292 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-14 22:17
You could just expose it, but on Windows I believe all extension modules are builtins, so you should be able to properly use _winreg to get at the registry and thus not require keeping the C code around.

But that's just a guess.
msg159968 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-05-04 20:07
FYI, I just deleted PC/import_nt.c as it stopped compiling after my removal of _imp.get_suffixes(). Obviously hg has the history of the file so if someone needs to resurrect it there shouldn't be much issue.
msg163209 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-19 20:13
Brett (and/or Brian?), this sounds like it should be tackled soon, to give Windows users enough time of testing 3.3.
msg163568 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-23 10:12
OTOH, I don't want it to block beta1.
msg163797 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-06-24 15:28
Yes, it should be tackled soon, but I lack Windows access so I can't do the work myself. Basically no one seems to use or care about this feature so it might die "on the vine" if someone doesn't step forward.
msg163801 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-06-24 15:34
I will take a look at it shortly...was too busy with installer/path issues and then PEP 397 last week.
msg164101 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-06-26 20:51
Moving back to blocker for beta2.
msg166026 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-07-21 14:22
Ping. Brian?  I'd rather have some testing period for this, so I'm reluctant to release b2 without it.
msg166044 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-07-21 16:25
Sorry, I've been too busy to get caught up with this. I'm away most of today but I can try to take a look tonight.
msg166095 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-07-22 01:23
Can someone tell me where you want this changed? I guess Lib/ in the find_module function, based on previous mention of imp.find_module? That's shown as deprecated so I'm hesitant to start there.

I don't really know anything about importlib, especially not enough to know where to start. If I have a pointer to the right spot I'll make the registry searching a part of it.
msg166135 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-07-22 13:18
So it should end up in Lib/importlib/ since it is directly part of how import works. The original code as found in Python 3.2 for PyWin_FindRegisteredModule() is referenced in Python/import.c and is implemented in PC/import_nt.c. You will want to create a new sys.meta_path importer (if I'm reading the code from Python/import.c correctly in terms of the call to PyWin_FindRegisteredModule() happening before sys.path is traversed). You can refactor FileFinder if you need to in order to reuse its code.

In terms of working with importlib, you will probably need the winreg module, so inject that (see _setup() on how to do that). Also remember that while this is Python code, you will need to rebuild to trigger the regeneration of importlib.h. Otherwise (I hope) it isn't too complicated or hard to understand.
msg166168 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-22 20:53
I don't have Windows, but here is an example of a WindowsRegistryImporter, written outside importlib (the script also contains a fake implementation of winreg, so I could test it...)

Of course it needs to be rewritten to fit in;  at least the script shows that it's only necessary to implement a find_module() function.

I'm not sure I had all details right - for example, should we search for the full module name, or only the last component?

This loader has some interesting features; for example it's possible to have several modules in the same .pyd file, as long as it exports the various PyInit_* functions.
msg166206 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-07-23 04:40
I'm not sure what the deal is but I can't get the changes to apply (fitting Amaury's patch into, meaning nothing I change to the file changes how anything operates.

I don't have much time to learn all of this stuff and make it work on Windows like the build process does on Linux, especially not with a release waiting for it. I'd just say to run without it until someone who needs it has time to fiddle around.
msg166210 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-23 06:22
importlib.h is not rebuilt on Windows, see issue15431.
msg166223 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-07-23 14:47
I just wanted to say that Amaury's proof-of-concept looks right (although what is returned by the registry isn't a "fullname" but a file path so I would at least change that variable name and you wouldn't append this mat path finder but instead insert it before PathFinder).

And to answer the question of what to pass in, technically it's the tail part of the name for full backwards-compatibility, but that's stupid as it creates an ambiguity e.g. a package named foo vs. a submodule named, so I say use the full name of the module when looking something up in the registry.
msg166459 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-07-26 05:15
Brian, perhaps the problem described in issue15431 ("Cannot build importlib.h on Windows") is part of the problem for you here.
msg166541 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-07-27 05:18
I would really like to release beta2 this weekend.  Is it possible to get this resolved by then?
msg166544 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-27 06:33
So I tried to make proper changes in, and regenerate importlib.h on Linux.
I don't have any Windows machine; all this is completely untested, but at least it can help some Windows developer to finish the work, and write unit tests...
(in case of bugs, if you don't know how to regenerate importlib.h, it's still possible to monkeypatch the WindowsRegistryImporter class; it's in sys.meta_path)
msg166547 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-27 07:10
> I would really like to release beta2 this weekend.  Is it possible to get this resolved by then?

At worse, perhaps 3.3 can be released without this feature. I don't know who relies on it.
msg166549 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-27 08:57
pywin32 used to install modules this way, but it seems it doesn't anymore:
    # It is possible people with old versions installed with still have 
    # pywintypes and pythoncom registered.  We no longer need this, and stale
    # entries hurt us.
Maybe this feature is not so important after all.
msg166583 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-07-27 16:58
> I would really like to release beta2 this weekend.  Is it possible to get this resolved by then?

As others stated, I'm not sure it's entirely necessary. Given the two dependencies that need to be fixed before this one can move forward, I don't think it's happening soon so I'd suggest to move on with beta2.
msg166641 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-07-28 09:19
OK, demoting.
msg166678 - (view) Author: Roundup Robot (python-dev) Date: 2012-07-28 19:34
New changeset bd58c421057c by Martin v. Löwis in branch 'default':
Issue #14578: Support modules registered in the Windows registry again.
msg166680 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-07-28 19:36
Amaury's patch nearly worked; committed with slight modifications.
Date User Action Args
2012-07-28 19:36:15loewissetstatus: open -> closed
resolution: fixed
dependencies: - Cannot build importlib.h on Windows
messages: + msg166680
2012-07-28 19:34:08python-devsetnosy: + python-dev
messages: + msg166678
2012-07-28 09:19:29georg.brandlsetpriority: release blocker -> deferred blocker

messages: + msg166641
2012-07-27 16:58:42brian.curtinsetmessages: + msg166583
2012-07-27 08:57:58amaury.forgeotdarcsetmessages: + msg166549
2012-07-27 07:10:52pitrousetnosy: + pitrou
messages: + msg166547
2012-07-27 06:39:53pitrousetnosy: + loewis
2012-07-27 06:33:10amaury.forgeotdarcsetfiles: + winreg_loader.diff
keywords: + patch
messages: + msg166544
2012-07-27 05:18:56georg.brandlsetmessages: + msg166541
2012-07-26 05:15:25eric.snowsetmessages: + msg166459
2012-07-23 14:47:35brett.cannonsetmessages: + msg166223
2012-07-23 06:22:44amaury.forgeotdarcsetdependencies: + Cannot build importlib.h on Windows
messages: + msg166210
2012-07-23 04:40:47brian.curtinsetmessages: + msg166206
2012-07-22 20:53:59amaury.forgeotdarcsetfiles: +
nosy: + amaury.forgeotdarc
messages: + msg166168

2012-07-22 13:18:58brett.cannonsetmessages: + msg166135
2012-07-22 01:23:48brian.curtinsetmessages: + msg166095
2012-07-21 16:25:44brian.curtinsetmessages: + msg166044
2012-07-21 14:22:10georg.brandlsetmessages: + msg166026
2012-06-26 20:51:26georg.brandlsetpriority: deferred blocker -> release blocker

messages: + msg164101
2012-06-24 15:34:15brian.curtinsetmessages: + msg163801
2012-06-24 15:28:56brett.cannonsetmessages: + msg163797
2012-06-23 10:12:53georg.brandlsetpriority: release blocker -> deferred blocker

messages: + msg163568
2012-06-19 20:13:03georg.brandlsetnosy: + georg.brandl
messages: + msg163209
2012-05-04 20:07:33brett.cannonsetmessages: + msg159968
2012-04-14 22:17:25brett.cannonsetmessages: + msg158292
2012-04-14 18:26:51brian.curtinsetnosy: + brian.curtin
2012-04-14 17:32:53eric.snowsetmessages: + msg158266
2012-04-14 17:18:05eric.snowsetnosy: + eric.snow
2012-04-14 17:07:06brett.cannoncreate