circuitpython-build-tools icon indicating copy to clipboard operation
circuitpython-build-tools copied to clipboard

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

Open dhalbert opened this issue 4 years ago • 5 comments

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 avatar Nov 30 '21 17:11 dhalbert

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 avatar Nov 30 '21 19:11 dhalbert

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

  • [ ] ./drivers/charlcd/adafruit_character_lcd
  • [ ] ./drivers/esp32spi/adafruit_esp32spi
  • [ ] ./drivers/esp-atcontrol/adafruit_espatcontrol
  • [ ] ./helpers/adafruitio/examples/adafruit_io_http
  • [ ] ./helpers/adafruitio/examples/adafruit_io_mqtt
  • [ ] ./helpers/display-shapes/adafruit_display_shapes
  • [ ] ./helpers/magtag/adafruit_magtag
  • [ ] ./helpers/matrixportal/adafruit_matrixportal

dhalbert avatar Nov 30 '21 19:11 dhalbert

@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 avatar Nov 30 '21 20:11 kattni

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?

kattni avatar Nov 30 '21 20:11 kattni

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).

dhalbert avatar Nov 30 '21 20:11 dhalbert