py/mpconfig: Enable the sys module at all feature levels by default. #17927
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17927 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 171 171
Lines 22297 22297
=======================================
Hits 21938 21938
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3c86fe1 to
8eaad59
Compare
|
I'm not 100% sure this is the right thing to do. It's a choice of:
I lean towards option 1, hence this PR. |
|
Code size report: |
This is a pretty fundamental module, and even minimal ports like unix and zephyr minimal have it enabled. So, enabled it by default at the lowest feature level. Signed-off-by: Damien George <damien@micropython.org>
8eaad59 to
6e318af
Compare
Signed-off-by: Damien George <damien@micropython.org>
There was a problem hiding this comment.
This seems like a good idea to me.
There's a follow-up we can do to take some of the import sys in tests out of try/except/"SKIP" blocks as well. About 11 cases of that (although only 3 have import sys as the only line in the block, the others try to import multiple modules.)
Yes, good idea. Although it does mean the test suite requires |
Summary
The
sysmodule is a pretty fundamental module, and even minimal ports like unix and zephyr minimal have it enabled. So, enabled it by default at the lowest feature level.Most things in the
sysmodule are configurable, and off by default, so it shouldn't add to much to ports that don't already have it enabled (which is just the minimal port).Testing
To be tested by CI.
Trade-offs and Alternatives
The aim here is to simplify configuration, and also eventually make it so the minimal configuration can run and pass the test suite. The
sysmodule is something that's needed for introspection of the target, so it seems reasonable to include it by default on everything. Of course that costs code size, and it's still easy to disable if needed, you just need to do it explicitly now.Also note that it's still disabled on the bare-arm port, to keep that ultra minimal. It means we now have bare-arm without
sysbut minimal port does havesys. That will allow different code size comparisons if/when new sys features are added.