support setups with all modules in src-directory

[bablokb]

In larger libraries with multiple modules it makes more sense to put all modules into a src-directory and don't scatter them in the top-level directory. Python setuptools explicitly supports this setup as a default (in this case, pyproject.toml does not have to specify the modules explicitly, but this is only a side note and not relevant here).

This PR changes tools/preprocess_frozen_modules.py to support this setup. The change is backward compatible.

[bablokb]

Failure was with one board: timeout during upload of artifact. I don't think this has anything to do with my change.

[jepler]

are you contributing patches elsewhere for this? e.g., I don't think a circuitpython library organized this way will work with circuitpython-build-tools, so it wouldn't be possible to organize a library like this and place it in the community bundle.

(I don't think there's a written rule for this but my gut tells me that to submodule something into frozen modules in the core we would want it to be in a bundle as well)

[bablokb]

are you contributing patches elsewhere for this?

Not yet, but this is planned. Before I can contribute the library, I need to make sure it can be added as frozen.

e.g., I don't think a circuitpython library organized this way will work with circuitpython-build-tools, so it wouldn't be possible to organize a library like this and place it in the community bundle.

A patch for the community-bundle/circup would certainly be next step if necessary.

(I don't think there's a written rule for this but my gut tells me that to submodule something into frozen modules in the core we would want it to be in a bundle as well).

You should also take a few things into account:

• having a src-directory is explicitly supported by the large Python world. Porting of existing packages will be easier if the src-layout is supported. See for example https://setuptools.pypa.io/en/stable/userguide/package_discovery.html#src-layout: "This layout is very handy when you wish to use automatic discovery, since you don’t have to worry about other Python files or folders in your project root being distributed by mistake. In some circumstances it can be also less error-prone"
• the PR is backward compatible and therefore does not hurt.
• In contrast, the current tools/preprocess_frozen_modules.py depends on a number of assumptions that are not clearly specified, e.g. you would happily include py-files from a doc dir (you only exclude docs) and you cannot name a module utils because you exclude it while tools or inst_scripts are not on your list and therefore would automatically be included. These are all random choices where as an explicit src-directory makes things clear.

I think it is ok to use heuristics like the current script does (or the circuitpython-build-tools), as long as there is no pyproject.toml.

This PR just improves the heuristics by providing an alternative that is less error-prone.

[jepler]

I tend to view freezing a module in as the last step, after it's able to be built with circuitpython-build-tools and installed with circup & the adafruit internal bundlefly tool. Once it's showing its usefulness within the circuitpython ecosystem then it might make sense to freeze it in on a board for a specific reason. But, maybe that is incorrect.

There's also not a lot of motivation given within this PR for requiring a different repo organization than the one standardized by the adafruit circuitpython cookiecutter. You didn't say, but I am guessing you are thinking about this repo of yours: https://github.com/bablokb/circuitpython-esp32at

From a glance, it appears that this wants to ship a "core-like" implementation of wifi (shadowing the built in module names) for a separate wifi coprocessor like airlift, split across wifi/socketpool/etc modules, all shipped from a single repo. This is an interesting idea, as right now the APIs for setting up networking are very different for "in the core" vs airlift.

Right now, circuitpython-build-tools, circup, and bundlefly all assume that a single repo contains either a single Python source file or a single Python package. I fear that this will be a hard limitation to lift, especially as a community member can't currently contribute to bundlefly. I also worry that e.g., "circup install -a" would start installing this socketpool library on boards where wifi is not a coprocessor, which is probably not desirable.

Rather than talk further about the merits of this PR on its own, I think it might be a good idea to (in a fresh or existing issue) go back to the beginning and talk about your goals so together we can figure out how to meet them. If we have a good understanding of the whys and the whats (instead of my guesses above), it'll help clarify questions. For instance, does something I stated earlier (that we'd want something to "be in a bundle" before it's frozen in, for instance) really apply, or if this is a "special enough case" to warrant doing something different than the status quo?

In a vacuum without getting into motivation (besides 'some people like to have a src/ directory in their repo') it's tough to really gauge the importance and usefulness of this change.

[bablokb]

I tend to view freezing a module in as the last step, after it's able to be built with circuitpython-build-tools and installed with circup & the adafruit internal bundlefly tool. Once it's showing its usefulness within the circuitpython ecosystem then it might make sense to freeze it in on a board for a specific reason. But, maybe that is incorrect.

I think freezing of modules is a way to add (software-) features to a board, necessary for the operation of the given hardware. Mainly to make it easier for users. They don't have to download and install CircuitPython and then install some standard software they need anyhow. "usefulness within the circuitpython ecosystem" is not really relevant for users of a specific board, they mainly want to use their hardware.

There's also not a lot of motivation given within this PR for requiring a different repo organization than the one standardized by the adafruit circuitpython cookiecutter. You didn't say, but I am guessing you are thinking about this repo of yours: https://github.com/bablokb/circuitpython-esp32at

Yes, this project triggered this PR. And I want to freeze it for the iLabs RP2350+Wifi/BLE, which has an embedded ESP32C6, giving it emulated-native wifi support. For other boards, support from circup would be great, but I understand the difficulties.

To the motivation: I started with scattered top-level directories, but during development this was just not practicable. Some examples:

• git diff src vs.
  git diff esp32at ipaddress mdns socketpool ssl wifi
• pylint src/* vs.
  pylint esp32at ipaddress mdns socketpool ssl wifi
• rsync -av src/ /path/to/my/device vs.
  rsync -av esp32at ipaddress mdns socketpool ssl wifi /path/to/my/device

Having to always type all these module-directories and hoping not to forget anything just did not work out. For small single module projects you don't need this additional src directory, but for larger projects this is just easier to handle. And to say it again: having an src-directory is not unusual in the Python world. Hey, even Adafruit_Blinka uses src ;-)

[tannewt]

giving it emulated-native wifi support.

I worry that emulating the native API will be confusing because it won't be able to do everything that an actual native implementation can do. Specifically, it won't be able to do the web workflow. So I'd suggest a full AirLift-style library that could be frozen in or an actual native C implementation that web workflow can use.

[bablokb]

I don't agree. With this emulation, libraries like adafruit_connectionmanager, adafruit_requests, adafruit_httpserver (actually every library and program that uses wifi) just work. If you look at the adafruit libs and how much additional code they need to support adafruit_esp32spi then you know how unattractive it would be to add another incompatible api/implementation.

Yes, web-workflow does not work, but you can expect users that add an external co-processor to know that there are differences.

[tannewt]

If you look at the adafruit libs and how much additional code they need to support adafruit_esp32spi then you know how unattractive it would be to add another incompatible api/implementation.

I thought the intent of ConnectionManager was to centralize this so it didn't need to go in many places. The adafruit examples likely predate connection manager.

I think this is too confusing. It isn't a native implementation. Why not do a native implementation so that web workflow works?

[bablokb]

I don't understand the reasoning. There are currently 326 py-files distributed by circup that do an "import wifi". They will only work if they find a module called wifi. And this is not only because they predate the connection manager, but because the connection manager does not provide any support for the many methods of wifi.radio that deal with starting/stopping AP, stations, setting IPs, DNS and so on.

Currently, I can add an ESP32Cx to whatever board provides a free UART simply by installing a number of modules. Building this as native code would imply that you would need to provide these modules with every CircuitPython build for all boards that could potentially be connected to an external co-processor. This would either double the number of builds or inflate the firmware for all boards, even if nobody is using the co-processor. Or you would need to build your own version of CircuitPython by setting special configuration variables at build-time. But chances are high that whoever can do that does not need the Webworkflow, which is more or less a beginner's tool anyway.

My understanding until now was that CircuitPython defines an API for application programs. For many reasons, most of this is and should be implemented within the native firmware, but there is IMHO no reason why the API cannot or should not be implemented as pure python modules.

[bablokb]

This discussion has somehow deteriorated, we should instead discuss this PR.
To sum up:

• the script tools/preprocess_frozen_modules.py is small and has a very limited functionality
• it uses heuristics to autodect py-files from library-projects for freezing
• this heuristics uses some seemingly random choices about which directories are valid and which are not, e.g. docs vs. doc or tools vs. utils
• the PR adds three lines of code that optionally enables library developers to clearly specify which files to include and which not
• the PR is fully backward compatible
• the PR circumvents the error-prone approach the script currently takes

Now you can say that the current script is absolutely great and needs no improvement. Or you can say that you don't want to merge the PR because it opens up possibilities for developers you never thought about (and are inadequate). So you can just close the PR and leave the script as is.

I think the PR improves the script and should be merged. Freezing is orthogonal and independent to installation via a bundle. Modules installed via a bundle can be lost by reformatting the CIRCUITPY-drive. So even if the current tool set does not support installing multi-module libraries via a bundle: this does not automatically imply that the modules should not be frozen (of course unless you dynamically change the rules on the fly).

[dhalbert]

I agree this has gotten off topic. I think it is OK to add the src detection; it is optional and won't break our existing frozen libraries. Whether that library is available for use in the bundle or not is a separate question.

I think this is a workaround? But maybe not having to do that is the whole point.

FROZEN_MPY_DIRS += $(TOP)/frozen/Some_Library/src

[bablokb]

Yes, I used that workaround already. But having it the the script is more explicit.

[dhalbert]

It is OK with me to merge this, for private use by whomever has libraries with src/. The disadvantage is that libraries with src/ are not going to work in a bundle. But that's a problem for the library writer. We can have a separate discussion about making src/ optional and possible in the bundle world or even the default in general.

[dhalbert]

@tannewt and I talked about this. The src/ thing is fine. We have some ideas about an AirLift wifi substitute library but can talk about that elsewhere.