Skip to content
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

[PEP 741] gh-107954: Add PyInitConfig C API #110176

Closed
wants to merge 2 commits into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 1, 2023

Add PyInitConfig functions:

  • PyInitConfig_Python_New()
  • PyInitConfig_Isolated_New()
  • PyInitConfig_Free(config)
  • PyInitConfig_SetInt(config, key, value)
  • PyInitConfig_SetStr(config, key, value)
  • PyInitConfig_SetWStr(config, key, value)
  • PyInitConfig_SetStrList(config, key, length, items)
  • PyInitConfig_SetWStrList(config, key, length, items)
  • PyInitConfig_GetErrorMsg(config)

Add also functions using it:

  • Py_InitializeFromInitConfig(config)
  • Py_ExitWithInitConfig(config)

Changes:

  • Add Include/initconfig.h header.

Discussion: https://discuss.python.org/t/fr-allow-private-runtime-config-to-enable-extending-without-breaking-the-pyconfig-abi/18004/20

@vstinner
Copy link
Member Author

vstinner commented Oct 1, 2023

TODO: once we reach an consensus on the API, I will document the API.

@vstinner
Copy link
Member Author

I documented the API. I decided to add a new Doc/c-api/config.rst documentation for that.

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

Thank you!
Now that I see all the docs, it looks the design is missing a function: https://discuss.python.org/t/fr-allow-private-runtime-config-to-enable-extending-without-breaking-the-pyconfig-abi/18004/49?u=encukou

I also have a few docs nitpicks. I didn't look very closely at the implementation yet.

Doc/c-api/config.rst Outdated Show resolved Hide resolved
Doc/c-api/config.rst Outdated Show resolved Hide resolved
Doc/c-api/config.rst Outdated Show resolved Hide resolved
Comment on lines 94 to 95
Return ``-1`` if Python wants to exit and on error. Call
:c:func:`Py_ExitWithInitConfig` in this case.
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you don't call it? (If it needs to be called, why does Py_InitializeFromInitConfig not call it itself?)

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is to let the caller decide how to handle an error or an exit code. When you embed Python, you may not want the process to exit. It's a similar design than Py_InitializeFromConfig() and the PyStatus C API.

What happens if you don't call it?

Python is in an undefined state. The error/exit code stays in PyInitConfig. It's safer to handle the init error :-)

Doc/c-api/config.rst Outdated Show resolved Hide resolved
Doc/c-api/config.rst Outdated Show resolved Hide resolved
@vstinner vstinner force-pushed the config_set_abi branch 2 times, most recently from fee8cc4 to 7d59519 Compare December 1, 2023 16:08
@vstinner
Copy link
Member Author

vstinner commented Dec 1, 2023

This change is quite big, so I moved PyConfig_Get() API to a separated PR: PR #112609.

Add PyInitConfig functions:

* PyInitConfig_Python_New()
* PyInitConfig_Isolated_New()
* PyInitConfig_Free(config)
* PyInitConfig_SetInt(config, key, value)
* PyInitConfig_SetStr(config, key, value)
* PyInitConfig_SetWStr(config, key, value)
* PyInitConfig_SetStrList(config, key, length, items)
* PyInitConfig_SetWStrList(config, key, length, items)
* PyInitConfig_Exception(config)
* PyInitConfig_GetError(config, &err_msg)
* PyInitConfig_GetExitCode(config, &exitcode)

Add also functions using it:

* Py_InitializeFromInitConfig(config)
* Py_ExitWithInitConfig(config)

Add these functions to the limited C API.

Changes:

* Add Doc/c-api/config.rst.
* Add Include/initconfig.h header.
@vstinner
Copy link
Member Author

This API is kind of complicated (there are multiple use cases). I created PR #112609 as a starting point, just to add two functions. But apparently, adding PyConfig_Get() and PyConfig_GetInt() functions is more complicated than what I expected. I don't have the bandwidth to deal with such large API for now, so I prefer to close the PR. See also my comment on the PyConfig_Get() PR.

@vstinner vstinner closed this Dec 20, 2023
@vstinner vstinner deleted the config_set_abi branch December 20, 2023 11:09
@vstinner vstinner restored the config_set_abi branch February 7, 2024 02:04
@vstinner vstinner changed the title gh-107954: Add PyInitConfig C API [PEP 741] gh-107954: Add PyInitConfig C API May 3, 2024
@vstinner
Copy link
Member Author

New PR: #123502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants