Skip to content

Get package name from pyproject.toml, allow arbitrary files in packages #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 3, 2023

Conversation

jepler
Copy link
Contributor

@jepler jepler commented Nov 29, 2023

This is more dependable, and when we know the package name we can glob inside it to get all files such as bin or ttf files.

This will allow e.g., 5x8.bin & ov5640_autofocus.bin within
bundles.

the behavior of bundlefly and circup when encountering .bin files needs to be checked.

Tested by building modified pycamera bundle and the autofocus.bin file appears in the generated zip files:

pycamera-py-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-8.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-9.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096

There's at least one library in the bundle that has incorrect metadata and that leads to an error:
adafruit/Adafruit_CircuitPython_Colorsys#29

@jepler jepler requested review from FoamyGuy and a team November 29, 2023 23:41
This is more dependable, and when we know the package name we can
glob inside it to get all files such as bin or ttf files.

This will allow e.g., 5x8.bin & ov5640_autofocus.bin within
bundles.

the behavior of bundlefly and circup when encountering .bin files
needs to be checked.

Tested by building modified pycamera bundle and the autofocus.bin file
appears in the generated zip files:
```
pycamera-py-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-8.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-9.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
```

There's at least one library in the bundle that has incorrect metadata
and that leads to an error:
    adafruit/Adafruit_CircuitPython_Colorsys#29
@jepler jepler force-pushed the include-binary-files-in-package branch from 6de3690 to 033bf0a Compare November 30, 2023 15:08
@jepler jepler changed the title Allow any ".bin" file to be shipped in a circuitpython package Get package name from settings.toml, allow arbitrary files in packages Nov 30, 2023
@jepler
Copy link
Contributor Author

jepler commented Nov 30, 2023

circup appears to use shutil.copytree, so it will copy files regardless of type/extension.

@jepler jepler force-pushed the include-binary-files-in-package branch from 033bf0a to b2116cb Compare November 30, 2023 15:11
@jepler
Copy link
Contributor Author

jepler commented Nov 30, 2023

This'll require some metadata problems across multiple libs to be corrected.

@jepler
Copy link
Contributor Author

jepler commented Nov 30, 2023

The test failure appears to be due to wrong metadata. I think I've filed PRs with every affected library now.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about whether you mean settings.toml or pyproject.toml.

@jepler
Copy link
Contributor Author

jepler commented Nov 30, 2023

I have settings.toml on the brain because of circuitpython, but it should be pyproject.toml everywhere here.

@dhalbert dhalbert changed the title Get package name from settings.toml, allow arbitrary files in packages Get package name from pyproject.toml, allow arbitrary files in packages Nov 30, 2023
dhalbert
dhalbert previously approved these changes Nov 30, 2023
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look fine; proof is in the running.

@jepler
Copy link
Contributor Author

jepler commented Nov 30, 2023

Trying to think of a creative way to let this land without requiring all those libs to be updated. Could be as simple as a blacklist of things that DON'T get the treatment, or disabling it in the communty bundle for now. (allowing the build process to opt out of using the new thing)

@jepler
Copy link
Contributor Author

jepler commented Dec 2, 2023

I've implemented a simple blacklist (of values that appear in py_modules today and are wrong) so that this can complete CI without all those related issues being closed.

@jepler jepler requested a review from dhalbert December 2, 2023 21:26
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking down the outliers.

@dhalbert dhalbert merged commit 988ed98 into main Dec 3, 2023
@dhalbert dhalbert deleted the include-binary-files-in-package branch December 5, 2023 03:40
@jepler jepler mentioned this pull request Dec 5, 2023
@jepler
Copy link
Contributor Author

jepler commented Oct 29, 2024

None of the repos under jposada202020 have been fixed, and they have no github activity in the last year. If it is important to get rid of this list of non-comforming libraries, we may need to fork them (e.g., into the circuitpython org) so they can be group maintained.

@jposada202020
Copy link

@jepler sorry about that I create the tag release as requested. Sorry about the delay

@jepler
Copy link
Contributor Author

jepler commented Dec 1, 2024

Thanks @jposada202020, that's appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants