comtypes icon indicating copy to clipboard operation
comtypes copied to clipboard

redundant processes for finding typelib-path and resolving it

Open junkmd opened this issue 3 years ago • 1 comments

Related to #327, I am refactoring tools.codegenerator and client._generate.

These modules include processing to turn passed relative paths into absolute paths and to find absolute paths to type libraries.

However, there are redundant processes as below.

  • There is a process to resolve relative paths in functions that are (possibly) passed absolute paths.
  • If the value returned by tlbparser.get_tlib_filename is None, there is a process that tries to get the absolute path of the type library by other methods.

https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/tlbparser.py#L716-L724 https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/client/_generate.py#L110-L122 https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/codegenerator.py#L267-L286 https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/codegenerator.py#L242-L265

I would like to make sure that only client._generate does path-related processing for type libraries, and that tools.codegenerator only takes the path and generates code.

junkmd avatar Oct 02 '22 13:10 junkmd

@vasily-v-ryabov

I propose that it is no more defining typelib_path as relative path in _generate_typelib_path. I would like to hear your opinion. (because you have been committed 6c1788afb7789b91959fb8ecb039538c90ca0754 and have added the following comments.) https://github.com/enthought/comtypes/blob/6c1788afb7789b91959fb8ecb039538c90ca0754/comtypes/tools/codegenerator.py#L222-L227

Rationale

Currently, there is still a conditional branch that makes the definition of typelib_path a path relative to the .../comtypes/gen directory, as shown below. https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/codegenerator.py#L248-L265

Perhaps this is because there used to be a situation where tools.codegenerator was called as a script.

  • NOTE: Even in the earliest commit where tools.tlbparser was added, there is no main function in tools.tlbparser. This conditional branch might have been dead code for a long time. https://github.com/enthought/comtypes/blob/3579c214cae62f0ccfdaae0bfd5a29605f92a2b3/comtypes/tools/codegenerator.py#L388-L391

if __name__ == "__main__:" line is currently not in tools.codegenerator.

So currently it seems filename is already None or absolute path at the caller(client._generate.GetModule).

junkmd avatar Oct 03 '22 10:10 junkmd