Skip to content

Move .frozen to second entry in sys.path #8105

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 2 commits into from
Dec 29, 2021

Conversation

dpgeorge
Copy link
Member

The recent change to allow a ".frozen" entry in sys.path to specify the frozen module search order (see #8079) placed this entry at the start of sys.path. This is against the CPython standard/docs/behaviour which requires sys.path[0] to be the location of the current script (or '' if that can't be determined, like at a REPL).

Some scripts will use sys.path[0] to find out where they are run from. And eg upip is now broken with ".frozen" as the first entry.

The reason ".frozen" was put first was to keep the same behaviour as before for searching frozen modules, namely that frozen code is searched before the filesystem (frozen code was previously searched when "" was encountered in sys.path). But this behaviour will need to change because it's more important to have sys.path[0] being the current script's directory (or the empty string).

This PR does the following:

  • moves ".frozen" to the second entry in sys.path
  • makes ".frozen" part of MICROPY_PY_SYS_PATH_DEFAULT on the unix and windows ports, so it can be overridden/changed by use of the MICROPYPATH environment variable
  • adds code to upip to skip ".frozen" because it doesn't make sense to install to .frozen

See related #7536.

@dpgeorge dpgeorge added ports Relates to multiple ports, or a new/proposed port py-core Relates to py/ directory in source labels Dec 21, 2021
@@ -90,7 +90,7 @@
#define MICROPY_PY_SYS_ATEXIT (1)
#define MICROPY_PY_SYS_PLATFORM "win32"
#ifndef MICROPY_PY_SYS_PATH_DEFAULT
#define MICROPY_PY_SYS_PATH_DEFAULT "~/.micropython/lib"
#define MICROPY_PY_SYS_PATH_DEFAULT ".frozen;~/.micropython/lib"
Copy link
Member Author

Choose a reason for hiding this comment

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

@stinos is ; the correct separator here for all cases where the windows port will run? Even if it runs under mingw does it still use ;?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, unix/main.c has PATHLIST_SEP_CHAR as ; for all windows ports, and it's the only place where MICROPY_PY_SYS_PATH_DEFAULT is used

Copy link
Member Author

Choose a reason for hiding this comment

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

Great, thanks for confirming.

@dpgeorge dpgeorge force-pushed the py-frozen-path-not-first branch 2 times, most recently from fed2fb2 to 89875e2 Compare December 21, 2021 12:53
In commit 86ce442 the '.frozen' entry was
added at the start of sys.path, to allow control over when frozen modules
are searched during import, and retain existing behaviour whereby frozen
was searched before the filesystem.

But Python semantics of sys.path require sys.path[0] to be the directory of
the currently executing script, or ''.

This commit moves the '.frozen' entry to second place in sys.path, so
sys.path[0] retains its correct value (described above).

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the py-frozen-path-not-first branch from 89875e2 to 959e6f7 Compare December 29, 2021 12:56
@dpgeorge dpgeorge merged commit 959e6f7 into micropython:master Dec 29, 2021
@dpgeorge dpgeorge deleted the py-frozen-path-not-first branch December 29, 2021 13:12
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Jul 7, 2023
improve start_ap() doc; make "authmode" use consistent internally
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ports Relates to multiple ports, or a new/proposed port py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants