[bug] The bundled version of `conan` (from `PyInstaller`) should automatically bundle all tools

[ymeine]

Describe the bug

Quite some time ago, I wanted to use the bundled conan version (the one made with PyInstaller) since I’m using scoop on Windows to install most of my software.

At that time, I had an issue with the missing tool conan.tools.scons. I didn’t report the bug back then, and as I was finally about to do it today, I realized it was fixed by PR #17809 as an answer to issue #17808.

Now, while I’m glad it’s finally fixed, there are a few concerns regarding that topic of bundling:

• the mentioned issue is not about conan.tools.scons, but about conan.tools.sbom. That means the applied correction only luckily fixed a technical gap which existed for a while
• by the past, other tools were also missing, like conan.tools.scm as mentioned in issue [bug] Installer package is missing conan.tools.scm #11676 and fixed by PR add conan.tools.scm as hidden-import to pyinstaller #11677. This time, the PR added only that one, while some other tools were already missing
• other issues like [bug] Windows installer package is missing python modules (pathlib) #10365 pointed out that PyInstaller really pulls by itself only the code which is statically imported, even for the Python standard library apparently, and that the preferred solution remains to install the Python package of conan, not the bundled version

Still, since the bundled version exists and can be more practical for some, I guess a couple of improvements could be made.

I wonder if you could automatically add all tools, using the --collect-modules=conan.tools option, knowing that you already use that option for something else. I tried it and, at the time of writing, seemed to only bundle one more set of modules, all related to the conan.tools.cps tool, which is a good sign that it may be the right solution.

But if you want more fine grained control, since you are already using a Python script to invoke PyInstaller, you could maybe even loop over your existing conan/tools folder and add all the --hidden-import=conan.tools.xxx options.

First solution looks like this:

               # Modules that can be imported in ConanFile conan.tools and errors
               "--collect-submodules=conan.cli.commands "
               "--hidden-import=conan.errors "
-              "--hidden-import=conan.tools.android "
-              "--hidden-import=conan.tools.apple "
-              "--hidden-import=conan.tools.build "
-              "--hidden-import=conan.tools.cmake "
-              "--hidden-import=conan.tools.env "
-              "--hidden-import=conan.tools.files "
-              "--hidden-import=conan.tools.gnu "
-              "--hidden-import=conan.tools.google "
-              "--hidden-import=conan.tools.intel "
-              "--hidden-import=conan.tools.layout "
-              "--hidden-import=conan.tools.meson "
-              "--hidden-import=conan.tools.microsoft "
-              "--hidden-import=conan.tools.premake "
-              "--hidden-import=conan.tools.qbs "
-              "--hidden-import=conan.tools.ros "
-              "--hidden-import=conan.tools.sbom "
-              "--hidden-import=conan.tools.scm "
-              "--hidden-import=conan.tools.scons "
-              "--hidden-import=conan.tools.system "
-              "--hidden-import=conan.tools.system.package_manager")
+              "--collect-submodules=conan.tools")

and at the time being adds the following modules: conan.cps, conan.cps.cps, conan.tools.cps, conan.tools.cps.cps_deps.

Second solution would be:

+from pathlib import Path

+    tools = (
+        f'--hidden-import=conan.tools.{child.name}'
+        for child in Path(source_folder, 'conan', 'tools').iterdir()
+        if child.is_dir() and child.name != '__pycache__'
+    )
     hidden = ("--hidden-import=glob "  # core stdlib
               "--hidden-import=pathlib "
               # Modules that can be imported in ConanFile conan.tools and errors
               "--collect-submodules=conan.cli.commands "
               "--hidden-import=conan.errors "
-              "--hidden-import=conan.tools.android "
-              "--hidden-import=conan.tools.apple "
-              "--hidden-import=conan.tools.build "
-              "--hidden-import=conan.tools.cmake "
-              "--hidden-import=conan.tools.env "
-              "--hidden-import=conan.tools.files "
-              "--hidden-import=conan.tools.gnu "
-              "--hidden-import=conan.tools.google "
-              "--hidden-import=conan.tools.intel "
-              "--hidden-import=conan.tools.layout "
-              "--hidden-import=conan.tools.meson "
-              "--hidden-import=conan.tools.microsoft "
-              "--hidden-import=conan.tools.premake "
-              "--hidden-import=conan.tools.qbs "
-              "--hidden-import=conan.tools.ros "
-              "--hidden-import=conan.tools.sbom "
-              "--hidden-import=conan.tools.scm "
-              "--hidden-import=conan.tools.scons "
-              "--hidden-import=conan.tools.system "
-              "--hidden-import=conan.tools.system.package_manager")
+              + ' '.join(tools))

and at the time being adds the same modules as solution 1, but as a difference removes module conan.tools.system.package_manager. Indeed, the manual looping over tools needs to handle nested tools if any (like that one) and is harder to maintain.

Btw, I know you have a comment about it in the code, but you are not tying to a specific PyInstaller version and therefore you are relying on the developer to install it, which could be problematic in some situations.

How to reproduce it

Well, there’s not really one scenario, it all depends on whether you want to use a given tool and if it has been bundled or not.

Here’s what I did though to check my changes:

• in the conan repository, run the pyinstaller.py script
• after that, use pyi-archive_viewer --list --recursive --brief $pathToTheConanBin and check in the output what tools/modules are being bundled or not

I did that multiple times with the 3 different versions (untouched, solution 1 and solution 2) and then output the list to different files that I could diff.

[memsharded]

Hi @ymeine

Thanks for your feedback, and many thanks specially for the detailed explanations and research.

I agree, this has been slightly problematic, not that painful to require enough attention beyond a quick fix to add the missing bits, but still inconvenient for some users that wanted those modules.

I think the solution1 --collect-submodules=conan.tools should be ok, and I wanted to move to it, but I don't exactly recall why it wasn't done that way in the past, maybe there were some issues or something, but I can't recall, and not clear in history.

Maybe the fact that the docs only say:

--collect-submodules MODULENAME
Collect all submodules from the specified package or module. This option can be used multiple times.

It is not very clear what is the behavior...

But from your research it seems less problematic that manual looping. So maybe we want to give it a try, would you like to open the PR yourself, as you already did the heavier job?

[ymeine]

Hi,

Thanks for such a quick reply.

Regarding PyInstaller’s documentation, I understand “Collect all submodules from the specified package or module” as:

• option’s input can point to a module (i.e. a Python file) or a package (i.e. a folder with an __init__.py file)
• from there, the bundler will recursively find all submodules by navigating the whole subtree of files. If option was pointing to a package, it just takes the corresponding folder as a root. If it was pointing to a module, it will take the corresponding file’s parent folder as a root.
• once this is done and it has produced a set of modules to include, it will process each of them as normal, meaning it will check and include any dependency they would have

That’s why when I added the option on my side, not only conan.tools.cps and conan.tools.cps.cps_deps were being added since they are submodules of conan.tools, but also conan.cps and conan.cps.cps since they are dependencies of them. To be more precise, only conan.cps.cps is imported, but I think a module cannot come without its parent package, so conan.cps is showed as included as well.

How to go further

Now, when I open a ticket, I try to refrain from proposing a change that would be too disruptive. But otherwise I can push a bit the proposal.

If you use --collect-modules=conan (and therefore remove all other related --collect-modules or --hidden-import options), you are sure to bundle all of your code. However, it also includes the whole conan.test tree, which I’m sure you don’t want, and the whole conans.server tree, which is in fact outside of conan (notice the s at the end) and pulled only because of the tests, so you don’t want it either.

But we can add another option: --exclude-module=conan.test. After running and comparing, we are back to the exact same result as with my initial proposal.

Why bother then? Well, I think it’s a cleaner, clearer and more future-proof solution. It really tells “bundle all of conan and its dependencies, but forget about the tests part”.

Truncated diff gives that:

     conan_path = os.path.join(source_folder, 'conans', 'conan.py')
-    hidden = ("--hidden-import=glob "  # core stdlib
-              "--hidden-import=pathlib "
-              # Modules that can be imported in ConanFile conan.tools and errors
-              "--collect-submodules=conan.cli.commands "
-              "--hidden-import=conan.errors "
-              "--hidden-import=conan.tools.android "
-              "--hidden-import=conan.tools.apple "
-              "--hidden-import=conan.tools.build "
-              "--hidden-import=conan.tools.cmake "
-              "--hidden-import=conan.tools.env "
-              "--hidden-import=conan.tools.files "
-              "--hidden-import=conan.tools.gnu "
-              "--hidden-import=conan.tools.google "
-              "--hidden-import=conan.tools.intel "
-              "--hidden-import=conan.tools.layout "
-              "--hidden-import=conan.tools.meson "
-              "--hidden-import=conan.tools.microsoft "
-              "--hidden-import=conan.tools.premake "
-              "--hidden-import=conan.tools.qbs "
-              "--hidden-import=conan.tools.ros "
-              "--hidden-import=conan.tools.sbom "
-              "--hidden-import=conan.tools.scm "
-              "--hidden-import=conan.tools.scons "
-              "--hidden-import=conan.tools.system "
-              "--hidden-import=conan.tools.system.package_manager")
+    hidden_imports = []

     if platform.system() != "Windows":
-        hidden += " --hidden-import=setuptools.msvc"
+        hidden_imports.append("setuptools.msvc")
         win_ver = ""

     command_args = [conan_path, "--noconfirm", f"--paths={source_folder}",
                     "--console", f"--distpath={distpath}"]
-    command_args.extend(hidden.split(" "))
+
+    hidden_imports.append("glob")
+    hidden_imports.append("pathlib")
+    command_args.append("--collect-submodules=conan")
+    command_args.append("--exclude-module=conan.test")
+    command_args.extend(f"--hidden-import={name}" for name in hidden_imports)

What about the std lib?

I noticed you had issues regarding some parts of the std lib not being bundled as well, and that your script explicitly added some modules.

This area is a bit trickier, since I’m not aware of any parent package that could be used for --collect-submodules.

However there is data available in sys.stdlib_module_names, but at the time of writing, it includes 290 modules, including many starting with _ (easy to exclude though), and according to the documentation also many modules which may not be really available (they can be excluded too, see below):

It is the same on all platforms. Modules which are not available on some platforms and modules disabled at Python build are also listed. All module kinds are listed: pure Python, built-in, frozen and extension modules. Test modules are excluded.

After filtering like this:

import importlib
builtin_modules = [
	mod for mod in sys.stdlib_module_names
	if not mod.startswith('_') and importlib.util.find_spec(mod) is not None
]

I still get 184 modules.

I tried the change out of curiosity, but it got way too many modules being added, including:

• pip (I guess because of ensurepip)
• some modules starting with _ (e.g. _osx_support, and I’m on Windows)
• this, antigravity which are easter eggs
• turtle, tkinter which are graphics/GUI libraries
• etc. etc.

Conclusion

I think we have two options:

• stay with my initial change proposal, simple and safe
• or go with the second proposal, which to me remains safe as well and gives a few more advantages, but I’d like your opinion on that

I would forget about the std lib part since it really pulls too much.

[memsharded]

Now, when I open a ticket, I try to refrain from proposing a change that would be too disruptive. But otherwise I can push a bit the proposal.

Yes, I don't think this will be necessarily disruptive, if we can check that things are looking good across several platforms, so it is fine to move forward, as commented, this is something that we were aware of, so sounds like it is the moment to try to improve it a bit.

But we can add another option: --exclude-module=conan.test. After running and comparing, we are back to the exact same result as with my initial proposal.

Why bother then? Well, I think it’s a cleaner, clearer and more future-proof solution. It really tells “bundle all of conan and its dependencies, but forget about the tests part”.

Yes, I think this is good, and more future-safe.

I think we have two options:

stay with my initial change proposal, simple and safe
or go with the second proposal, which to me remains safe as well and gives a few more advantages, but I’d like your opinion on that
I would forget about the std lib part since it really pulls too much.

I agree, second proposal looks good. And yes, lets forget about the stdlib, in practice the issues have been around conan.tools, the stdlib missing modules were added long time ago and I don't recall many other issues for those.

Thanks!

[ymeine]

Sorry for the long delay, PR #18195 is finally open!

[memsharded]

Excellent, thanks!