Skip to content

PEP 432: Rewrite Py_Main() #76211

Closed
Closed

Description

@vstinner
BPO 32030
Nosy @warsaw, @ncoghlan, @vstinner, @ned-deily, @ericsnowcurrently, @serhiy-storchaka, @indygreg, @miss-islington
PRs
  • bpo-32030: Split Py_Main() into subfunctions #4399
  • bpo-32030: Enhance Py_Main() #4412
  • bpo-32030: Add more options to _PyCoreConfig #4485
  • bpo-32030: Add _PyCoreConfig.module_search_path_env #4504
  • bpo-32030: Move PYTHONPATH to _PyMainInterpreterConfig #4511
  • bpo-32030: Add _PyMainInterpreterConfig.pythonhome #4513
  • bpo-32030: Rewrite calculate_path() #4521
  • bpo-32030: Fix calculate_path() on macOS #4526
  • bpo-32030: Add _PyMainInterpreterConfig_ReadEnv() #4542
  • bpo-32030: Add _PyMainInterpreterConfig.program_name #4548
  • bpo-32030: Add _PyPathConfig_Init() #4551
  • bpo-32030: Rework memory allocators #4625
  • [3.6] bpo-32030: Fix test_sys.test_getallocatedblocks() #4637
  • bpo-32030: Fix _Py_InitializeEx_Private() #4649
  • bpo-32030: Cleanup "path config" code #4663
  • bpo-32030: Fix Py_GetPath(): init program_name #4665
  • bpo-32030: Don't call _PyPathConfig_Fini() in Py_FinalizeEx() #4667
  • bpo-32030: Add Python/pathconfig.c #4668
  • bpo-32030: Fix config_get_program_name() on macOS #4669
  • bpo-32030: _PyPathConfig_Init() sets home and program_name #4673
  • bpo-32197: Try to fix a compiler error introduced in bpo-32030. #4679
  • bpo-32197: Try to fix a compiler error on OS X introduced in bpo-32030. #4681
  • Revert "bpo-32197: Try to fix a compiler error on OS X introduced in bpo-32030. (#4681)" #4694
  • bpo-32030: Simplify _PyCoreConfig_INIT macro #4728
  • bpo-32030: Add pymain_get_global_config() #4735
  • bpo-32030: pass interp to _PyImport_Init() #4736
  • bpo-32030: Add _PyImport_Fini2() #4737
  • bpo-29240, bpo-32030: pymain_set_sys_argv() now copies argv #4838
  • bpo-32030: Add _PyPathConfig_ComputeArgv0() #4845
  • bpo-32030: Rewrite _PyMainInterpreterConfig. #4854
  • bpo-32030: Add _PyMainInterpreterConfig.warnoptions #4855
  • bpo-32030: Update sys.path[0] earlier for main importer #4868
  • bpo-32329: Fix -R option for hash randomization #4873
  • bpo-32030: Add _PyCoreConfig_Copy() #4874
  • bpo-32030: Add _PyMainInterpreterConfig.executable #4876
  • bpo-29240, bpo-32030: Py_Main() re-reads config if encoding changes #4899
  • bpo-32030: Fix compilation on FreeBSD, #include <fenv.h> #4919
  • bpo-32030: Fix compiler warnings #4921
  • bpo-32030: Add _PyCoreConfig.argv #4934
  • bpo-32030: Cleanup pymain_main() #4935
  • bpo-32030: Add _PyCoreConfig.warn_opts #4936
  • bpo-32030: Complete _PyCoreConfig_Read() #4946
  • bpo-32030: Fix usage of memory allocators #4953
  • bpo-32030: Add _PyCoreConfig.module_search_paths #4954
  • bpo-32030: Add _Py_EncodeUTF8_surrogateescape() #4960
  • bpo-32030: Add _Py_EncodeLocaleRaw() #4961
  • bpo-32030: Add _Py_FindEnvConfigValue() #4963
  • bpo-33824, bpo-32030: Fix pymain_read_conf() #7712
  • [3.7] bpo-33824, bpo-32030: Fix pymain_read_conf() (GH-7712) #7730
  • bpo-32030: Make _PySys_AddXOptionWithError() private #10236
  • bpo-40412: Nullify inittab_copy during finalization #19746
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-01-24.16:32:35.113>
    created_at = <Date 2017-11-15.01:30:23.893>
    labels = ['3.7']
    title = 'PEP 432: Rewrite Py_Main()'
    updated_at = <Date 2020-05-04.15:24:30.321>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-05-04.15:24:30.321>
    actor = 'indygreg'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-01-24.16:32:35.113>
    closer = 'vstinner'
    components = []
    creation = <Date 2017-11-15.01:30:23.893>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32030
    keywords = ['patch']
    message_count = 68.0
    messages = ['306245', '306246', '306250', '306288', '306321', '306328', '306329', '306332', '306616', '306759', '306763', '306767', '306768', '306775', '306794', '306824', '306851', '306924', '306929', '306935', '307239', '307268', '307329', '307395', '307399', '307402', '307407', '307410', '307422', '307434', '307453', '307560', '307665', '307746', '307748', '307756', '308201', '308202', '308212', '308240', '308271', '308288', '308349', '308356', '308359', '308447', '308593', '308626', '308689', '308698', '308700', '308750', '308766', '308833', '308835', '308844', '308874', '308876', '308877', '308905', '309053', '310606', '310612', '310613', '310618', '319682', '319685', '328915']
    nosy_count = 8.0
    nosy_names = ['barry', 'ncoghlan', 'vstinner', 'ned.deily', 'eric.snow', 'serhiy.storchaka', 'indygreg', 'miss-islington']
    pr_nums = ['4399', '4412', '4485', '4504', '4511', '4513', '4521', '4526', '4542', '4548', '4551', '4625', '4637', '4649', '4663', '4665', '4667', '4668', '4669', '4673', '4679', '4681', '4694', '4728', '4735', '4736', '4737', '4838', '4845', '4854', '4855', '4868', '4873', '4874', '4876', '4899', '4919', '4921', '4934', '4935', '4936', '4946', '4953', '4954', '4960', '4961', '4963', '7712', '7730', '10236', '19746']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue32030'
    versions = ['Python 3.7']

    Activity

    1. vstinner commented on Nov 15, 2017

      @vstinner
      MemberAuthor

      Python has a lot of code for its initialization. It's very hard to touch this code without risking to break something. It's hard to move code since many parts of the code are interdepent. The code rely on global "Py_xxx" configuration variables like Py_IsolateFlag (set by -I command line option).

      Moreover, currently Python uses the "Python runtime" early. For example, the code to parse the -W command line option uses PyUnicode_FromWideChar() and PyList_Append(). We need a stricter separation for the code before the "Python runtime" is initialized, at least partially initialized.

      Nick Coghlan and Eric Snow are already working on all these issues as part of the implementation of PEP-432. They redesigned Py_Initialize() and Py_Finalize().

      I would like to finish the work on the step before: the Py_Main() function.

      Attached PR is a work-in-progress to rework deeply the Py_Main() function. I have different goals:

      • Enhance error handling:

        • Avoid whenever possible calls to Py_FatalError() -- currently, Py_FatalError() is still called, but at a single place
        • My patch adds missing checks on PyDict_SetItem() or PyList_Append() calls, catch errors when adding warnings options and XOptions
      • Reorder code to initialize: initialize Python in the "correct" order

      • Better "finalization": pymain_free() is now responsible to free memory of all data used by Py_Main(). The ownership of strings is now better defined. For example, Py_SetProgramName() memory was not released before.

      • pymain_init() is now the code which must not use the Python runtime

      • pymain_core() uses the Python runtime. Its code to initialize the Python runtime should be easier to follow

      Since pymain_free() now wants to release the memory, we need to force a memory allocator for PyMem_RawMalloc(), since pymain_core() changes the memory allocator. The main() already does something similar, but with simpler code since main() is a private function, whereas Py_Main() seems to be part of the public C API!

    2. changed the title [-]Rewrite Py_Main()[/-] [+]PEP 432: Rewrite Py_Main()[/+] on Nov 15, 2017
    3. vstinner commented on Nov 15, 2017

      @vstinner
      MemberAuthor

      I rewrote Py_Main() to prepare the code to be able to implement my "-X dev" idea:
      https://mail.python.org/pipermail/python-dev//2017-November/150514.html

      The problem is that currently the code parsing command line options and the code setting the memory allocator (handle PYTHONMALLOC environment variable) are mixed, it's not possible to touch this code.

      I had similar technical issues when trying to implement properly my PEP-540 idea (Add a new UTF-8 mode): it's hard to change the Python filesystem encoding to UTF-8 after parsing command line arguments, since the current code to parse command line arguments already rely on the Python filesystem encoding and other parts of the Python runtime like the memory allocators.

      If I implemented my PR correctly, it should become much more simpler to control PYTHONMALLOC and the filesystem encoding from the command line: from an -X option.

    4. ncoghlan commented on Nov 15, 2017

      @ncoghlan
      Contributor

      While it doesn't necessarily need to be in this patch, something else I recently realised (by breaking it *cough* [1]) is that the interaction between our command line options and our environment variables isn't really clearly defined anywhere.

      d7ac061 restored the handling of simple on/off toggles as "toggle enabled = env var is set OR CLI flag is passed", but I noticed the other day that the interaction between PYTHONWARNINGS, the -W option, sys.warnoptions, and _warnings.filters is a bit confusing:

      $ PYTHONWARNINGS=always,default python3 -Wignore -Wonce
      Python 3.6.2 (default, Oct  2 2017, 16:51:32) 
      [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)] on linux
      Type "help", "copyright", "credits" or "license" for more information.
      >>> import sys, _warnings
      >>> sys.warnoptions
      ['always', 'default', 'ignore', 'once']
      >>> [f[0] for f in _warnings.filters[:4]]
      ['once', 'ignore', 'default', 'always']
      

      The ordering makes *sense* (where sys.warnoptions just lists filter definitions in the order they're given, and later filters take priority over earlier ones, just as they do for any "warnings.filterwarnings" call), but it isn't immediately intuitive (since the outcome relies on filters being prepended by default).

      That said, I've checked and the current warnings configuration behaviour *is* explicitly covered by the test suite (in https://github.com/python/cpython/blob/master/Lib/test/test_warnings/__init__.py), so a passing test suite provides confidence we haven't broken anything on that front.

      [1] https://bugs.python.org/issue31845

    5. vstinner commented on Nov 15, 2017

      @vstinner
      MemberAuthor

      I wrote a new "_PyInitError" type to report more information when something goes wrong:

      • indicate if it's an user error: don't abort() in that case
      • function name where the error was raised
      • error message

      Example:
      ---

      $ PYTHONHASHSEED=x ./python
      Fatal Python error: _Py_HashRandomization_Init: PYTHONHASHSEED must be "random" or an integer in range [0; 4294967295]

      => Python doesn't fail with abort() anymore
      => notice the new "_Py_HashRandomization_Init" function name which gives context to the error message

      Previously, Python called abort() and so might dump a core file:
      ---

      $ PYTHONHASHSEED=x python3
      Fatal Python error: PYTHONHASHSEED must be "random" or an integer in range [0; 4294967295]

      Aborted (core dumped)
      ---

    6. vstinner commented on Nov 15, 2017

      @vstinner
      MemberAuthor

      New changeset f7e5b56 by Victor Stinner in branch 'master':
      bpo-32030: Split Py_Main() into subfunctions (bpo-4399)
      f7e5b56

    7. vstinner commented on Nov 16, 2017

      @vstinner
      MemberAuthor

      Nick:
      """
      I noticed the other day that the interaction between PYTHONWARNINGS, the -W option, sys.warnoptions, and _warnings.filters is a bit confusing:

      $ PYTHONWARNINGS=always,default python3 -Wignore -Wonce
      (...)
      >>> sys.warnoptions
      ['always', 'default', 'ignore', 'once']
      >>> [f[0] for f in _warnings.filters[:4]]
      ['once', 'ignore', 'default', 'always']
      """

      IMHO the command line must have the priority over environment variables, since environment variables are inherited, whereas the command line can be finely tuned *on purpose*. So it's correct, no? It's just that warnoptions gives options in the reverse order than the "expected" order, no?

      At least, with my commit, if you want to try to change the priority, you just have to exchange two lines in pymain_add_warnings_options() :-)

      if (pymain_add_warnings_optlist(&pymain->env_warning_options) < 0) ...
      if (pymain_add_warnings_optlist(&pymain->cmdline.warning_options) < 0) ...
      

      --

      By the way, I'm not sure that it was a good idea to expose sys.warnoptions to users, but I don't think that we can remove it anymore :-)

    8. vstinner commented on Nov 16, 2017

      @vstinner
      MemberAuthor

      New changeset a7368ac by Victor Stinner in branch 'master':
      bpo-32030: Enhance Py_Main() (bpo-4412)
      a7368ac

    9. vstinner commented on Nov 16, 2017

      @vstinner
      MemberAuthor

      I rewrote Py_Main() in two large commits. The code isn't perfect yet, for example, init_sys_streams() still reads the PYTHONIOENCODING environment variable. IMHO it should be read earlier. But as soon, my changes should allow to fix such issues much more easily.

      Since I was now able to implement my -X dev option in bpo-32043, I don't plan to push further changes in short term on Py_Main().

      I keep this issue open a little bit to check if everything is fine on buildbots, and check if someone has complains :-)

    10. vstinner commented on Nov 21, 2017

      @vstinner
      MemberAuthor

      New changeset 25420fe by Victor Stinner in branch 'master':
      bpo-32030: Add more options to _PyCoreConfig (bpo-4485)
      25420fe

    11. vstinner commented on Nov 22, 2017

      @vstinner
      MemberAuthor

      New changeset d434110 by Victor Stinner in branch 'master':
      bpo-32030: Add _PyCoreConfig.module_search_path_env (bpo-4504)
      d434110

    12. ncoghlan commented on Nov 22, 2017

      @ncoghlan
      Contributor

      Victor, *please* don't add the external import settings to CoreConfig. That struct should only contain the absolute bare minimum of settings needed to get an interpreter that *can't* access the filesystem, such that builtin modules and frozen modules work, but nothing else does.

      If you need some extra structures to hold command line and environment state, that's fine, but the full external import system attributes should go in the main interpreter config, as described in https://www.python.org/dev/peps/pep-0432/#supported-configuration-settings

    13. vstinner commented on Nov 23, 2017

      @vstinner
      MemberAuthor

      Nick: Ok, I created PR 4511.

    14. ncoghlan commented on Nov 23, 2017

      @ncoghlan
      Contributor

      Nice, thanks for that. Good call on keeping the current data types for now, so we can focus on consolidating the configuration settings first, and then look at upgrading from C level types to Python level types later.

    15. 42 remaining items

    16. vstinner commented on Dec 21, 2017

      @vstinner
      MemberAuthor

      Oh, the following command does crash:

      PYTHONMALLOC=pymalloc ./Programs/_testembed forced_io_encoding
    17. vstinner commented on Dec 21, 2017

      @vstinner
      MemberAuthor

      New changeset e47e698 by Victor Stinner in branch 'master':
      bpo-32030: Add _Py_EncodeUTF8_surrogateescape() (bpo-4960)
      e47e698

    18. vstinner commented on Dec 21, 2017

      @vstinner
      MemberAuthor

      New changeset 9dd7620 by Victor Stinner in branch 'master':
      bpo-32030: Add _Py_EncodeLocaleRaw() (bpo-4961)
      9dd7620

    19. vstinner commented on Dec 21, 2017

      @vstinner
      MemberAuthor

      New changeset 9bee329 by Victor Stinner in branch 'master':
      bpo-32030: Add _Py_FindEnvConfigValue() (bpo-4963)
      9bee329

    20. vstinner commented on Dec 21, 2017

      @vstinner
      MemberAuthor

      Hum, _PyCoreConfig.ignore_environment is redundant with Py_IgnoreEnvironmentFlag. I don't recall why I added it to _PyCoreConfig. Maybe it should be removed.

    21. ncoghlan commented on Dec 26, 2017

      @ncoghlan
      Contributor

      _PyCoreConfig.ignore_environment was part of the initial PEP-432 implementation that I wrote.

      It's that due to the design goal that once the refactoring is complete, an embedding application should be able to control *all* the settings through the config structs, without *ever* touching the global variables directly.

    22. vstinner commented on Jan 24, 2018

      @vstinner
      MemberAuthor

      New changeset 8ded5b8 by Victor Stinner in branch 'master':
      bpo-32030: Add _PyCoreConfig.module_search_paths (bpo-4954)
      8ded5b8

    23. vstinner commented on Jan 24, 2018

      @vstinner
      MemberAuthor

      I made most, if not all, changes that I wanted to do. It's time to close this huge issue to continue the work in new more specific issues.

      Notes on Py_Main().

      (*) _PyPathConfig_Init() is called even if it's not needed (if all "Path configuration outputs" fileds of PyCoreConfig are filled).

      (*) pymain_cmdline() uses _Py_CommandLineDetails structure which contains a copy of each global configuration variable like Py_UTF8Mode. Internally, the function has to "set" or "get" these variables when calling some functions like _PyPathConfig_Init().

      This code is fragile. *But* pymain_read_conf() is complex, it had to read again the whole configuration a second time if the encoding changed.

      It would be nice to remove global variables from _Py_CommandLineDetails to avoid the get/set dance which introduces a risk of loosing changes by mistake. But I'm not sure that it's doable?

      (*) Should we make Py_FileSystemDefaultEncoding and Py_FileSystemDefaultEncodeErrors configurable in _PyCoreConfig? Same question for _Py_StandardStreamEncoding, _Py_StandardStreamErrors and PYTHONIOENCODING environment variable.

    24. vstinner commented on Jan 24, 2018

      @vstinner
      MemberAuthor

      I close the issue.

    25. vstinner commented on Jan 24, 2018

      @vstinner
      MemberAuthor

      Oh, my latest commit introduced a regression in test_distutils: bpo-32652.

    26. vstinner commented on Jun 15, 2018

      @vstinner
      MemberAuthor

      New changeset 6c5a4b3 by Victor Stinner in branch 'master':
      bpo-33824, bpo-32030: Fix pymain_read_conf() (GH-7712)
      6c5a4b3

    27. miss-islington commented on Jun 15, 2018

      @miss-islington
      Contributor

      New changeset 046da16 by Miss Islington (bot) in branch '3.7':
      bpo-33824, bpo-32030: Fix pymain_read_conf() (GH-7712)
      046da16

    28. vstinner commented on Oct 30, 2018

      @vstinner
      MemberAuthor

      New changeset e1b2995 by Victor Stinner in branch 'master':
      bpo-32030: Make _PySys_AddXOptionWithError() private (GH-10236)
      e1b2995

    29. transferred this issue fromon Apr 10, 2022
    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

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions