adafruit / circuitpython-build-tools

Build scripts for CircuitPython libraries and the bundle
MIT License
28 stars 18 forks source link

package-folder-prefix quietly excludes libraries without a canonical prefix #82

Open dhalbert opened 2 years ago

dhalbert commented 2 years ago

The https://github.com/adafruit/Adafruit_CircuitPython_asyncio library is a package of multiple library files, but its package folder does not start with adafruit_: it is simply asyncio. Because the package folders are filtered to match the --package-folder-prefix argument, which defaults to adafruit_, asyncio is not included in the adafruit bundle, though it should be.

I could special-case it, and maybe that's the short-term solution. But what is the reason for the existence of --package_folder_prefix (which can be a list of multiple prefixes, separated by ,). Is it just to identify the package folder easily? Would it be better just to skip the directories that we know are not package folders, such as docs and examples? There should then be only one leftover directory, which is the package folder, if there is a package.

61 is related, as is this comment by @tannewt that we should use an ignore list instead of an accept list.

dhalbert commented 2 years ago

I think we could figure out which top-level folders are package folders by:

  1. looking for folder_name/__init__.py.
  2. excluding tests/ docs/ and examples/

I looked through the bundle, and there are few library packages that are missing __init__.py. I think these are just in error, and need to have __init__.py added. A general fix for this issue would have to wait until those are fixed. Once that happens, I think we could get rid of --package-folder-prefix altogether.

@kattni @sommersoft (and anyone else) does this make sense to you?

dhalbert commented 2 years ago

These appear to be packages without __init__.py. Note some are sub-packages and don't influence this issue.

kattni commented 2 years ago

@dhalbert I think that's fine to add __init__.py and update how it works. You included two examples/ in your list above, was that unintentional?

kattni commented 2 years ago

I'm not entirely clear on how the Adabot check for libraries and examples and so on being named properly works though. Would this break that?

dhalbert commented 2 years ago

You included two examples/ in your list above, was that unintentional?

I think those examples are packages that also need __init__.py (though not to fix this problem).