Only install platforms needed for current device#262
Conversation
Rather than installing all the platforms from the global PLATFORMS variable, only install source (py) and the compiled (mpy) version that the currently connected device needs. This allows some nice improvements: 1) Only need to download at most, 2 versions of a bundle, not however many exist in PLATFORMS. 2) A device doesn't NEED the compiled versions. So with this in mind, this change adds the behavior where if a compiled bundle can't be found, it falls back to installing the source version. 3) Because of 2, a bundle maintainer doesn't have to provide compiled bundles to use their bundles with circup. If only providing source bundles is sufficient, circup is now happy with that. It will check for and try to download a compiled bundle, but if it doesn't find it, it just falls back to the source bundle. 4) Also because of 2, if someone is using an older version of circuitpython that is no longer getting compiled bundle builds, they can also still use circup. In this case it again falls back to the source bundle (or pin to an older bundle version that does have the compiled bundle for the version of circuitpython being used...circup no longer only supports specific platforms so if a platform bundle exists, it can use it).
|
|
||
|
|
||
| def get_bundle(bundle, tag): | ||
| def get_bundle(bundle, tag, platform): |
There was a problem hiding this comment.
for this and the other functions where platform or platform_version argument has been added can you please add them to the docstrings as well.
There was a problem hiding this comment.
Okay, think I fixed this, let me know if I missed any others!
|
|
||
| #: Module formats list (and the other form used in github files) | ||
| PLATFORMS = {"py": "py", "9mpy": "9.x-mpy", "10mpy": "10.x-mpy"} | ||
| PLATFORMS = {"py": "py", "8mpy": "8.x-mpy", "9mpy": "9.x-mpy", "10mpy": "10.x-mpy"} |
There was a problem hiding this comment.
I think 8.x was removed from here intentionally when we stopped supporting it.
If we're going to add it back here and allow circup to install libraries from it, I think we should perhaps also have it print a message that the older bundles (and versions of CircuitPython) are deprecated and it's recommended to update to the supported version of CP/Bundle mpy.
It's maybe worth discussing in the weeds during a meeting to get input from the rest of the team as well.
There was a problem hiding this comment.
The idea now is circup isn't deciding what is and isn't supported. If I plug in a device with CP 8 on it, I don't get to choose what compiled bundle I can use, I need 8...if 8 doesn't exist because it isn't supported, circup can handle that by falling back to the source version instead.
So Adafruit might no longer build bundle's with 8 (and at some point drop support for 9), but it's quite possible a bundle maintainer of some other third party bundle does still want to support 8 (and at some point 9). In this PR, PLATFORMS changes from indicating that "these are the supported versions," and becomes only a mapping to find the download URL.
I think what you are saying is valid, but I'm not sure if the responsibility should fall on circup to let the user know something is deprecated/unsupported. Mostly because that would be up to the bundle maintainer, not circup.
Happy to discuss this more if it would be helpful to figure out the best behavior!
There was a problem hiding this comment.
Devices with older versions can try to use uncompiled python files and that will work in lots of cases, but in some cases there are changes in the core which then get used by libraries. So in those cases it could only work if a suitable older version of the library from before the change in the core was selected. Support in a situation like that can get quite tricky, so I believe it's good to point users towards the latest supported version anywhere that we can.
It is true that a 3rd party bundle maintainer may choose to support older compiled bundle builds. But I still believe that circup should warn about the version of CircuitPython being unsupported. It already notifies the user when their version of Circuit python is out of date with messages like this:
A newer version of CircuitPython (10.0.3) is available.
Get it here: https://circuitpython.org/board/adafruit_fruit_jam
I view warning about older unsupported versions as an extension of the same behavior as messages like this.
If you still think it's best not to have circup give these warnings for unsupported versions of the core then I would encourage you to make a note to discuss it 'in the weeds' during a meeting so that the rest of the team can chime in.
There was a problem hiding this comment.
...but in some cases there are changes in the core which then get used by libraries
But I still believe that circup should warn about the version of CircuitPython being unsupported.
I understand the concern now. I didn't realize part of the intention is to say 'version X of CircuitPython is no longer supported'. I was interpreting it as, 'the bundle doesn't support compiled builds for version X of CircuitPython'.
With that in mind I also think an additional warning makes sense!
If you still think it's best not to have circup give these warnings for unsupported versions of the core then I would encourage you to make a note to discuss it 'in the weeds' during a meeting so that the rest of the team can chime in.
As noted above I'm good with additional warnings, but this is something I wasn't aware of! I don't know where I can listen/watch/participate in your meetings? I've come across them in the past but always after they already happened. I also thought they were only for Adafruit folks to participate in? (Sorry if this is somewhat unrelated to this PR, but I would definitely be interested in checking out a meeting!)
There was a problem hiding this comment.
The meetings are typically held weekly on the Discord: https://adafru.it/discord
The usual time is 2pm US Eastern / 11am US Pacific. We gather in the circuitpython-dev text channel and circuitpython voice channel there for the meeting. In the pinned messages of the circuitpython-dev channel there is always a link to the notes doc for the next meeting.
Anyone can attend and listen in, or participate by adding notes to the document for hug reports, status updates, or in the weeds topics. If you're around in the voice channel during the meeting you can discuss your topics, or if you can't make it live you can watch/listen after the fact on youtube or podcast downloads.
We are meeting this upcoming Monday (12/22), but are taking a break for the holidays the following week, so the next meeting after that will be 1/5/2025
We're always happy to have folks from the community listen in or participate so feel free to join if you're interested.
|
Hey @FoamyGuy! Just wanted to check in and see what you're thinking about this? |
Don't support older CircuitPython versions by default to encourage users to upgrade to the latest version. However, in some cases where it is known a platform bundle/specific library may work fine with an older CPy version, allow the user to explicitly choose to use an older, unsupported version.
|
Sorry it took so long for me to get back to this! But I've updated the PR and I think it should address the concerns about using older CircuitPython versions. |
FoamyGuy
left a comment
There was a problem hiding this comment.
@dunkmann00 No worries, thanks for your work on this!
Looks good to me now. I tested installing on 8.x and 10.x and confirmed the new warning and allow-unsupported behavior is working well.
Rather than installing all the platforms from the global
PLATFORMSvariable, only install source (py) and the compiled (mpy) version that the currently connected device needs.This allows some nice improvements:
PLATFORMS.circup. If only providing source bundles is sufficient,circupis now happy with that. It will check for and try to download a compiled bundle, but if it doesn't find it, it just falls back to the source bundle.circuitpythonthat is no longer getting compiled bundle builds, they can also still usecircup. In this case it again falls back to the source bundle (or pin to an older bundle version that does have the compiled bundle for the version ofcircuitpythonbeing used...circupno longer only supports specific platforms so if a platform bundle exists, it can use it).What first prompted this was trying to find someone else's
circuitpythonbundle to test for one of the other PRs I did. I found sparkfun had made one but they hadn't updated it in a while and didn't have the10.x-mpyplatform bundle. Because of that,circupwouldn't even let me add their bundle because it failed to validate successfully. So from that I kinda started to think about it and wonder if there was a different way to go about handling platforms incircup. I figured it was kind of something I could try to play around with and see if I could put something together that made sense/was not too difficult to do. Ultimately, I'm happy with the end result.But let me know what you think or if you have any questions about it!