py/frozenmod.c: Use ".frozen/" as a virtual search path for frozen modules. #8079
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not enabled on any port, and functionality is better suited to the micropython-lib implementation of pkg_resources.
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
This has been deprecated for over two years in favour of manifest.py. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Takes the functionality from tools/make-frozen.py, adds support for multiple frozen directories, and moves it to tools/makemanifest.py. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
ad63c59 to
286f091
Compare
|
Updated to completely merge |
286f091 to
8708342
Compare
Codecov Report
@@ Coverage Diff @@
## master #8079 +/- ##
==========================================
- Coverage 98.47% 98.46% -0.02%
==========================================
Files 153 153
Lines 20176 20135 -41
==========================================
- Hits 19869 19825 -44
- Misses 307 310 +3
Continue to review full report at Codecov.
|
5e095d5 to
f1470c3
Compare
This changes makemanifest.py & mpy-tool.py to merge string and mpy names into the same list (now mp_frozen_names). The various paths for loading a frozen module(mp_find_frozen_module) and checking existence of a frozen module (mp_frozen_stat) use a common function that searches this list. In addition, the frozen lookup will now only take place if the path starts with ".frozen", which needs to be added to sys.path. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
This ensures MICROPY_QSTR_EXTRA_POOL and MICROPY_MODULE_FROZEN_MPY are set if necessary before the CFLAGS are extracted for QSTR generation. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Frozen modules will be searched preferentially, but gives the user the ability to override this behavior. This matches the previous behavior where "" was implicitly the frozen search path, but the frozen list was checked before the filesystem. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
andrewleech
reviewed
Dec 13, 2021
| } | ||
| size_t path_num = 1; // [0] is for current dir (or base dir of the script) | ||
| size_t path_num = 2; // [0] is frozen, [1] is for current dir (or base dir of the script) |
There was a problem hiding this comment.
This ordering of putting the script path second should also fix the upip issue, looks good to me.
It will mean however if you run upip from source rather than frozen in, any libs will be installed next to upip.py script by default
|
Thanks for the efforts on this, it fixes a very long standing bug. Merged in cc23e99 through 86394f7, with follow-up commit in de43b50 I made the following minor changes during merge/rebase:
Code size diff of this PR and the follow-up commit: The increase on bare-arm and minimal is due to the follow-up commit de43b50 which actually fixes a bug on these ports because they previously did not initialise |
This was referenced Dec 17, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current behavior for module searching is:
This meant that frozen files would be found if sys.path contained the empty string (this is the default in most cases), and would always take precedence over the filesystem. Additionally, when running the Unix port with a script in argv, sys.path will contain the path of the script, rather than the empty string). (This prevents uasyncio from being used without sys.path manipulation -- see #6419)
This is an updated version of #5057 by @wnienhaus (and #8000 by @andrewleech ) which add a new "virtual" path to sys.path
.frozenwhich explicitly means "search the frozen modules". This solves both issues:".frozen"can be placed in any order in sys.path relative to""allowing the precedence for frozen modules to be set.".frozen"separate to "the path that the current script resides in".It also matches the current CircuitPython behavior (which also uses .frozen as the special directory name).
Relative to #5057 (and #8000) there are a few changes:
path/foo.pyis already a qstr infrozen_content.c, avoid also creating.frozen/path/foo.py)py/frozenmod.c(and corresponding changes tomakemanifest.pyandmpy-tool.py).QSTR__dot_frozenin qstrdefs.h work (Fix importing of frozen modules (fixes #2322) #5057 worked around this in a different way)The main idea of the frozen_content.c change was to reduce code size, although the result isn't huge it almost covers this change. Overall this PR is +16 bytes on PYBV11.
The frozen_content.c change also meant removing non-manifest freezing (via
FROZEN_DIRorFROZEN_MPY_DIR) which has been deprecated for two years. Updated the last remaining port (teensy) to use manifest.py.Fixes #6419, #1804, #2322, #3509