Prevent the occurrence of SyntaxError in friendly modules.

[junkmd]

#524

See also #524 (comment) and #524 (comment).

I will wait for feedback from the community, and if there are no objections, I will merge this and release it as comtypes==1.4.2 in early May.

[junkmd]

CIs are passed.

https://ci.appveyor.com/project/pywinauto/pywinauto/builds/49693478
https://github.com/enthought/comtypes/actions/runs/8840682732

[cfarrow]

Redefining the generate method to not generate a module seems dangerous. This could break code and workflows that depend on modules being regenerated every time.

A safer option is to add an argument to generate that triggers the new load functionality, while defaulting to the existing behavior. This greatly reduces the potential for unintended side effects down the road.

When the right time comes to make a backward-incompatible change, I advise to change the method name to reflect that it does more than generate modules.

[junkmd]

@cfarrow

Thank you for your feedback.

As you pointed out, in reality, modules are not always being generated.

However, this behavior of GetModule has not changed since comtypes==1.1.11, before I contributed to the codebase.

The _CreateWrapper, which once had the responsibility to generate the wrapper module, also did not create a new module instance or file if there was an imported module from the sys.modules cache.

comtypes/comtypes/client/_generate.py

Lines 200 to 243 in b946aca

	def _CreateWrapper(tlib, pathname):
	# helper which creates and imports the real typelib wrapper module.
	fullname = _name_module(tlib)
	try:
	return sys.modules[fullname]
	except KeyError:
	pass
	modname = fullname.split(".")[-1]
	try:
	return _my_import(fullname)
	except Exception as details:
	logger.info("Could not import %s: %s", fullname, details)
	# generate the module since it doesn't exist or is out of date
	from comtypes.tools.tlbparser import generate_module
	if comtypes.client.gen_dir is None:
	if sys.version_info >= (3, 0):
	import io
	else:
	import cStringIO as io
	ofi = io.StringIO()
	else:
	ofi = open(os.path.join(comtypes.client.gen_dir, modname + ".py"), "w")
	# XXX use logging!
	logger.info("# Generating comtypes.gen.%s", modname)
	generate_module(tlib, ofi, pathname)
	if comtypes.client.gen_dir is None:
	code = ofi.getvalue()
	mod = types.ModuleType(fullname)
	mod.__file__ = os.path.join(os.path.abspath(comtypes.gen.__path__[0]),
	"<memory>")
	exec(code, mod.__dict__)
	sys.modules[fullname] = mod
	setattr(comtypes.gen, modname, mod)
	else:
	ofi.close()
	# clear the import cache to make sure Python sees newly created modules
	if hasattr(importlib, "invalidate_caches"):
	importlib.invalidate_caches()
	mod = _my_import(fullname)
	return mod

And for the friendly modules, if _my_import was called and the loading of the existing module was successful, a new module was not generated.

comtypes/comtypes/client/_generate.py

Lines 158 to 175 in b946aca

	# create and import the module
	mod = _CreateWrapper(tlib, pathname)
	try:
	modulename = tlib.GetDocumentation(-1)[0]
	except comtypes.COMError:
	return mod
	if modulename is None:
	return mod
	if sys.version_info < (3, 0):
	modulename = modulename.encode("mbcs")
	# create and import the friendly-named module
	try:
	mod = _my_import("comtypes.gen." + modulename)
	except Exception as details:
	logger.info("Could not import comtypes.gen.%s: %s", modulename, details)
	else:
	return mod

comtypes/comtypes/client/_generate.py

Lines 20 to 26 in b946aca

	def _my_import(fullname):
	# helper function to import dotted modules
	import comtypes.gen
	if comtypes.client.gen_dir \
	and comtypes.client.gen_dir not in comtypes.gen.__path__:
	comtypes.gen.__path__.append(comtypes.client.gen_dir)
	return __import__(fullname, globals(), locals(), ['DUMMY'])

When I added refactoring to _generate.py, I made sure to maintain this behavior.

So,

the generate method to not generate a module

has been the case from the beginning.

I made changes to the code under the recognition that it would have a greater impact to make GetModule always generate a new module without using the cache (a change that is appropriate for incrementing the y in version x.y.z).

From 1.3.0 onwards, the namespace of the friendly module is not defined by globals().update, but by explicitly importing symbols.

Since the friendly module uses the side effects of the code generation of the wrapper module to determine which symbols to import, the newly generated friendly module becomes partial if the friendly module file does not exist and the wrapper module is loaded from the cache.

Currently, if such a situation occurs, unlike before, it does not load from the cache, and GetModule recreates the wrapper module file and the friendly module file.
The only situation where the cached wrapper module is not used, even if it exists, is this one, and I think it makes sense to include the responsibility of GetModule to regenerate the wrapper module, which is a dependency of the friendly module, if it does not exist.

[junkmd]

I have added the test to verify that GetModule regenerates the module.

• In the codebase prior to this PR, test_friendly_module_is_missing would result in a SyntaxError similar to 'SyntaxError: invalid syntax' occurs during MS Project module generation with GetModule function  #524.
• Modules generated from mshtml.tlb are large in size and regenerating them multiple times makes the test time-consuming. Since any COM type library can be used to reproduce the situation where the existing module file does not exist, I am using the smaller scrrun.dll for testing.

This is similar to the existing test_findgendir.py in that it patches sys.modules.

This test involves a complex interplay of context-managers, sys.modules, sys.path, importlib, and mock.patch.
Consequently, the codebase has become complex.

To clarify the behavior within the fixtures, I've inserted assert statements and comments at key points.
These aims to facilitate asynchronous communication with future maintainers.

I welcome any feedbacks as well.

[junkmd]

the generate method to not generate a module

If you mean that ModuleGenerator.generate is not adhering to the single responsibility principle because it is not only generating new modules but also responsible for loading existing modules, then I agree.

Therefore, I have made changes so that the attempt to load existing modules is done within GetModule, before ModuleGenerator.generate is called.

[cfarrow]

Thank you for explaining this to me. My understanding of the code was not complete. I agree that the new responsibility belongs in GetModule. The test is a good addition with the new behavior.

[junkmd]

Thank you for your reply.

When I first looked at the codebase for comtypes==1.1.11, it took me some time to understand this behavior because my initial expectations and the actual implementation were different.

Thank you.