Skip to content

getpath miscalculates sys.path from second initialization with PYTHONHOME on Windows #91985

Closed
Closed
getpath miscalculates sys.path from second initialization with PYTHONHOME on Windows#91985
Labels
OS-windowstype-bugAn unexpected behavior, bug, or error

Description

@neonene
neonene
opened on Apr 27, 2022

When I ran the code below in a python build directory:

#include <Python.h>
int
main(int argc, char *argv[])
{
    PyConfig config;
    PyConfig_InitPythonConfig(&config);
    for (int i=0; i < 3; i++) {
        printf("\n%d\n", i);
        Py_InitializeFromConfig(&config);
        PyRun_SimpleString("import sys; list(map(print, sys.path))");
        Py_Finalize();
    }
    PyConfig_Clear(&config);
    return 0;
}
  • Expected results without PYTHONHOME:
0
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\PCbuild\amd64
C:\cpython-main
C:\cpython-main\Lib\site-packages

1
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\PCbuild\amd64
C:\cpython-main
C:\cpython-main\Lib\site-packages

2
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\PCbuild\amd64
C:\cpython-main
C:\cpython-main\Lib\site-packages
  • set PYTHONHOME=C:\cpython-main:
0
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\PCbuild\amd64
C:\cpython-main
C:\cpython-main\Lib\site-packages

1
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\DLLs  # <<<<<<<<<< unexpected
C:\cpython-main
C:\cpython-main\Lib\site-packages

2
C:\cpython-main\PCbuild\amd64\python311.zip
C:\cpython-main\Lib
C:\cpython-main\DLLs  # <<<<<<<<<< unexpected
C:\cpython-main
C:\cpython-main\Lib\site-packages

Currently, test_embed fails due to this. (#32313)

Activity

  1. neonene
    added
    type-bugAn unexpected behavior, bug, or error
    on Apr 27, 2022
  2. neonene
    changed the title [-][Windows] getpath miscalculate sys.path from second initialization with PYTHONHOME[/-] [+][Windows] getpath miscalculates sys.path from second initialization with PYTHONHOME[/+] on Apr 27, 2022
  3. zooba
    added
    OS-windows
    on Apr 27, 2022

  4. neonene commented on May 4, 2022

    @neonene
    neonene
    on May 4, 2022 · Hidden as outdated
    Author
  5. neonene
    changed the title [-][Windows] getpath miscalculates sys.path from second initialization with PYTHONHOME[/-] [+]getpath miscalculates sys.path with PYTHONHOME/Py_SetPythonHome() on Windows[/+] on May 5, 2022

  6. neonene commented on May 14, 2022

    @neonene
    neonene
    on May 14, 2022
    ContributorAuthor

    On 3.10 and earlier with or without PYTHONHOME, an executable in a build directory has sys.path as below:

    0
C:\cpython-3.10\PCbuild\amd64\python310.zip
C:\cpython-3.10\DLLs
C:\cpython-3.10\lib
C:\cpython-3.10\PCbuild\amd64
C:\cpython-3.10
C:\cpython-3.10\lib\site-packages

1
C:\cpython-3.10\PCbuild\amd64\python310.zip
C:\cpython-3.10\DLLs
C:\cpython-3.10\lib
C:\cpython-3.10\PCbuild\amd64
C:\cpython-3.10
C:\cpython-3.10\lib\site-packages

2
C:\cpython-3.10\PCbuild\amd64\python310.zip
C:\cpython-3.10\DLLs
C:\cpython-3.10\lib
C:\cpython-3.10\PCbuild\amd64
C:\cpython-3.10
C:\cpython-3.10\lib\site-packages
  7. neonene
    changed the title [-]getpath miscalculates sys.path with PYTHONHOME/Py_SetPythonHome() on Windows[/-] [+]getpath miscalculates sys.path from second initialization with PYTHONHOME on Windows[/+] on May 22, 2022

  8. zooba commented on May 23, 2022

    @zooba
    zooba
    on May 23, 2022
    Member

    Okay, so the problem is that home is set on the second call, which means getpath skips checking for build directory landmarks.

    IIRC, other tests were broken if we check against real_executable for the build landmarks and ignore PYTHONHOME (presumably because they used symlinks to a new directory for tests?), so this probably just needs new logic.

    However, the fix in #92980 might also affect this, by not recalculating sys.path on subsequent calls. You might want to test again before changing too much.

  9. neonene commented on Jun 6, 2022

    @neonene
    neonene
    on Jun 6, 2022
    ContributorAuthor

    IIRC, other tests were broken if we check against real_executable for the build landmarks and ignore PYTHONHOME (presumably because they used symlinks to a new directory for tests?), so this probably just needs new logic.

    test_getpath passes the following test cases:

    • test_symlink_buildtree_win32
    • test_buildtree_pythonhome_win32

    I can also start python in their file layouts, but test_embed were broken. Neither PYTHONHOME nor home worked around. New logic may be needed for this. Do I understand what you mean?

  10. zooba commented on Jun 6, 2022

    @zooba
    zooba
    on Jun 6, 2022
    Member

    Had a quick look at your changes, but honestly am going to need a lot more time to figure out that the logic is sound. That also concerns me, because I really want to avoid putting any logic for this stuff back in C - moving to Python was to fix that ;-)

    Another idea that might work: what if we just add a config field for "running in-tree build"? It can start as "unspecified" and then be updated if we detect landmarks. Then even if home is already set but we know we're in tree, we can go down the path that overrides platlibdir. What do you think?

  11. neonene commented on Jun 7, 2022

    @neonene
    neonene
    on Jun 7, 2022
    ContributorAuthor

    Thanks for your feedback.

    IIUC, we have no way to update the original config? getpath updates the config which was duplicated at pyinit_core(), and the copy disappears after system configuration with the update of global config (_Py_path_config: deprecated?).
    For example, module_search_paths_set and _is_python_build fields never change in the original config when repeating Py_InitializeFromConfig().

    I think the flag will work if you would be ok with saving it into the global config.

  12. zooba commented on Jun 7, 2022

    @zooba
    zooba
    on Jun 7, 2022
    Member

    Oh I see, it's the path config state that's keeping the values around. If we copy _is_python_build into there and only ever set it ourselves, that would be fine, right?

  13. neonene commented on Jun 7, 2022

    @neonene
    neonene
    on Jun 7, 2022
    ContributorAuthor

    My experiment (#92411) passed the repeating tests, keeping _is_python_build as a member of _Py_path_config. I now think it can be saved as a static variable in pathconfig.c.

  14. zooba commented on Jun 8, 2022

    @zooba
    zooba
    on Jun 8, 2022
    Member

    All of _Py_path_config is basically just a static variable - it never gets referenced outside of this module. But we're trying to minimize static variables, so that one day we can get them to be per-runtime rather than per-process. So please, keep it in the struct.

  15. zooba
    added a commit that references this issue on Jun 16, 2022

    gh-91985: Ensure in-tree builds override platstdlib_dir in every path…

    38af903
  16. miss-islington
    added a commit that references this issue on Jun 16, 2022

    pythongh-91985: Ensure in-tree builds override platstdlib_dir in ever…

    4ff3245

  17. kumaraditya303 commented on Jun 19, 2022

    @kumaraditya303
    kumaraditya303
    on Jun 19, 2022
    Contributor

    @neonene Is this fixed now or there is still work to be done ?

  18. neonene commented on Jun 19, 2022

    @neonene
    neonene
    on Jun 19, 2022
    ContributorAuthor

    I assume we delay 3.11 backporting just in case.

  19. miss-islington
    added a commit that references this issue on Jun 20, 2022

    gh-91985: Ensure in-tree builds override platstdlib_dir in every path…

    b8fe3bd

  20. zooba commented on Jun 20, 2022

    @zooba
    zooba
    on Jun 20, 2022
    Member

    This one can be backported immediately (and now has been). It only affects source builds and no public APIs.

  21. zooba
    closed this as completedon Jun 20, 2022
  22. serhiy-storchaka
    added a commit that references this issue on Jun 26, 2022

    Revert 30935 defer bpo45162 to 312 (#94285)

    1ffde3e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    OS-windowstype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions