Rework SCons internal module loading

[mwichmann]

Recorded so it's in the issue tracker. With preview builds of Python 3.10 (where warnings are noisier than production Pythons), we get a lot of:

DeprecationWarning: the load_module() method is deprecated and slated for removal in Python 3.12; use exec_module() instead

All the code which imports things like site_tools, etc. was reworked in the transition to supporting Python 3, but the approach taken is now deprecated and some further rework will be needed.

[mwichmann]

Oddly, this warning seems to be coming from something Python itself does, not just from places we bypass import to do something more direct.

[adriantirea]

I have just got to this warning too. Python 3.10 <frozen importlib._bootstrap>:283: DeprecationWarning: the load_module() method is deprecated and slated for removal in Python 3.12; use exec_module() instead

[mwichmann]

Yes, that's the one.

[ivankravets]

We've faced the same issue today => https://github.com/platformio/platformio-core/actions/runs/2345754750

We treat Python warnings as errors in https://github.com/platformio/platformio-core/blob/develop/pytest.ini

It would be great to have this fixed in the next release. See our implementation of the deprecated "load_module()" => https://github.com/platformio/platformio-core/blob/develop/platformio/compat.py#L60-L64

[bdbaddog]

We've faced the same issue today => https://github.com/platformio/platformio-core/actions/runs/2345754750

We treat Python warnings as errors in https://github.com/platformio/platformio-core/blob/develop/pytest.ini

It would be great to have this fixed in the next release. See our implementation of the deprecated "load_module()" => https://github.com/platformio/platformio-core/blob/develop/platformio/compat.py#L60-L64

Nice. So is that plug and play with SCons' current usage? Also some of the other functions in that file might be worth including in SCons/Util.py..

[ivankravets]

Yes, we've been using it for the last 2 years. I would be happy to make a PR, but this is a very important change. It would be better if someone who knows all internals of SCons will try to refactor this part. This is not about just "replacing" of "imp" tracks in SCons. The imp module is also deprecated.

I see a lot of hacks inside SCons regarding importing modules dynamically. For example, https://github.com/SCons/scons/blob/43f775a78f1d78515654e72c1280b074fd4c1899/SCons/Tool/init.py#L138

It's better to remove this code.

From my experience in loading modules dynamically:

1. If a module is visible within PYTHONPATH, please use importlib.import_module(name, package=None).
2. If the module is located outside SCons sources, use this code.

So, the code should be clean now.

[bdbaddog]

@ivankravets - Please PR. We'll work from there?

[mwichmann]

Yes, we've been using it for the last 2 years. I would be happy to make a PR, but this is a very important change. It would be better if someone who knows all internals of SCons will try to refactor this part. This is not about just "replacing" of "imp" tracks in SCons. The imp module is also deprecated.

I see a lot of hacks inside SCons regarding importing modules dynamically. For example,

https://github.com/SCons/scons/blob/43f775a78f1d78515654e72c1280b074fd4c1899/SCons/Tool/init.py#L138

It's better to remove this code.

Well, you've hit on why it's still as ugly as it is - it's important stuff, the original authors long gone, and so it's been fiddled with to get it to work on Py3 but not more because of a fear of pulling out the one assumption that causes a house of cards to collapse.

I believe the toolpath argument that can be supplied in various ways is not added to the Python path so that it will be searched only for specific tool imports, not for any other imports; site_scons directories are added to the Python path. So there's that to keep straight.

[ivankravets]

Thanks for the clarification. Do you have any ETA for the next release? I'll try to take a look at how I can help with this PR. We have released PlatformIO Core 6.0 a few days ago. Need to close some critical issues.

Meanwhile, what is the main reason for using dynamic module loading in SCons? Why we can't add SRC dir to the sys.path and use absolute paths later?

[bdbaddog]

Dynamic loading is primarily use for loading tools in SCons.

[bdbaddog]

Do you have any ETA for the next release?

There's a couple PR's I'd like to get merged which aren't quite ready yet. Two main topics: ninja (experimental), and MSVC version selection/detection logic.

[ShadowJonathan]

Where is this issue at? 3.12 has been out for a while, and imp has been removed; Build scripts fail to run for python 3.12

[mwichmann]

Where is this issue at? 3.12 has been out for a while, and imp has been removed; Build scripts fail to run for python 3.12

Hi - if you've got an example, I'll take a look. Use of load_module is not completely excised, but the remaining instances are in obscure places where it falls through to looking inside a zipfile in case someone is running from a py2exe bundle - which may or may not actually work, I don't believe we even test that code path. Despite the existing references, I'm not personally seeing problems on 3.12 and 3.13?

[ShadowJonathan]

Sorry, I left a comment on the wrong repository; I saw somewhere that imp importing has been removed already, but it's a higher-up library that caused the issue for me, apologies.