Skip to content

Backport of PyConfig.use_system_logger has changed ABI in a patch 3.13.1 -> 3.13.2 #130940

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

Closed
davidhewitt opened this issue Mar 7, 2025 · 19 comments
Labels

Comments

@davidhewitt
Copy link
Contributor

davidhewitt commented Mar 7, 2025

Bug report

Bug description:

In #127592 a new PyConfig flag was added for apple platforms to allow use of the system logger.

This PR looks like it was backported to all branches from 3.9 through to 3.13, and has been released in 3.13.2 and 3.12.9.

Unfortunately, this has changed the public ABI (layout of the PyConfig struct) on a patch release. This means that binaries using the PyConfig API and built using 3.13.1 or 3.13.0 (or 3.12.8) will potentially misbehave if running against a newer interpreter.

I picked this up after getting a PyO3 "FFI check" failure on 3.13.2, reporting that our FFI bindings were no longer compatible with CPython.

In PEP 741 it was previously noted that backporting a flag to break the public ABI was decided against, so I believe this to be a bug.

Do I wait for CPython to revert this? Or do I make PyO3 adjust to the new ABI (and break compatibility with all existing interpreters)?

ping @vstinner as I know you have been very active on this API.

CPython versions tested on:

3.13

Operating systems tested on:

macOS

Linked PRs

@davidhewitt davidhewitt added the type-bug An unexpected behavior, bug, or error label Mar 7, 2025
@davidhewitt davidhewitt changed the title Backports of PyConfig.use_system_logger have changed public ABI in a patch on all maintained CPython versions Backports of PyConfig.use_system_logger have changed public ABI in a patch 3.13.1 -> 3.13.2 Mar 7, 2025
@davidhewitt
Copy link
Contributor Author

Correction: while #127592 lists a lot of backports, it looks like the backport was only ever applied to the 3.13 branch. So it is only 3.13.1 to 3.13.2 which has an ABI break across patch versions, 3.12 and older are unaffected.

Will 3.13.3 revert this backport?

@davidhewitt
Copy link
Contributor Author

ping @freakboy3742 as the author of the PR, maybe you have some context or opinion on the right path forward.

@davidhewitt davidhewitt changed the title Backports of PyConfig.use_system_logger have changed public ABI in a patch 3.13.1 -> 3.13.2 Backport of PyConfig.use_system_logger has changed public ABI in a patch 3.13.1 -> 3.13.2 Mar 7, 2025
@davidhewitt davidhewitt changed the title Backport of PyConfig.use_system_logger has changed public ABI in a patch 3.13.1 -> 3.13.2 Backport of PyConfig.use_system_logger has changed ABI in a patch 3.13.1 -> 3.13.2 Mar 7, 2025
@ZeroIntensity
Copy link
Member

This does look like a breaking ABI change, but note that this only applies to iOS builds, due to the __APPLE__ macro.

@davidhewitt
Copy link
Contributor Author

I believe it to be for all apple platforms, including macOS.

https://developer.apple.com/library/archive/documentation/Porting/Conceptual/PortingUnix/compiling/compiling.html#//apple_ref/doc/uid/TP40002850-SW13 indicates __APPLE__ is for any apple system.

@freakboy3742
Copy link
Contributor

Apologies for the breakage - that was definitely unintentional, and evidently slipped through the review process.

Tagging @ned-deily for the macOS perspective; @Yhg1s for the 3.13 release impact.

Confirming some other details in this thread:

  • __APPLE__ will be enabled on macOS as well as iOS.

  • The backports linked on gh-126925: Modify how iOS test results are gathered #127592 to 3.9-3.12 releases are on my own branch, where I back port iOS patches to support older Python releases. AFAICT, the only official release with an ABI breakage is 3.13.2.

By way of background - this patch was needed because of changes in Xcode 16 that broke the way we were capturing output for iOS testing. The core logic that is being enabled by the config flag is required for iOS testing to work. There's almost no use case for the flag to ever be disabled on iOS.

However, it's entirely optional for macOS. It's a useful feature to have for very specific use cases (GUI apps that don't have a visible console), but there are other workarounds for that use case. At the very least, any impact on macOS can be reverted without impact.

So - a complete reversion isn't possible without breaking all iOS testing; but there are some options.

Firstly, we can revert the ABI-modifying changes to PyConfig. This doesn't need to be configurable in 3.13; we can treat this as a new 3.14 feature.

That then leaves the question of how we handle not having access to the flag. On macOS, we can just ignore it - it's an optional feature, so we can just not enable the optional behavior on macOS. But on iOS, we've got a choice to make.

One option would be to leave the code to enable the system logger in place, but only activate it in the testbed app. This would mean using a private API for the 3.13 version of the testbed - but then, if you squint, the iOS testbed is "internal" Python code? Maybe? A variant on this approach would be to migrate the system log handling code wholesale into the testbed, instead of it being in pylifecyle.c. That avoids the private API problem entirely.

The other option would be to make the enabling the system logger the default behavior on iOS. This would be a change in behaviour for iOS - but it's arguably a much better default. Without enabling the system logger, stdout and stderr content are essentially swallowed at runtime. You can see stdout/stderr content in Xcode, but it won't appear in any system logs for a deployed app. This would also leave the door open for modifying 3.14 so that the default value for PyConfig.use_system_logger is 1 on iOS, making the default behavior more useful.

Of the three, the third option (change the default) would be my preferred option, as the end-state leaves iOS support in a much better place. However, I appreciate that it's a backwards incompatible change, so it's not a change that should be made lightly.

I'm happy to work up the patch to fix this as soon as we've got agreement on the approach to take here.

@ned-deily
Copy link
Member

ned-deily commented Mar 7, 2025

That's unfortunate that we didn't consider the 3.13.x ABI implications when reviewing this; sorry!

For macOS, if I understand the implications correctly, the original change made would essentially only affect programs on macOS that embed Python 3.13 using an externally-provided shared-library (or framework) build of Python 3.13? If so, I would imagine there are very few such programs: since Python 3.13 isn't provided by the OS, I would expect most such programs would supply their own private copy of Python 3.13. I suppose it could be an issue for distributors like Homebrew or MacPorts or Conda but even there I would think it would be likely that both the embedding application and the Python 3.13.x version would be updated at the same time.

So, for the next release of 3.13 (3.13.3), I guess the by-the-book thing to do would be to revert (and document the revert) the ABI change for macOS but, if my understanding is correct, the impact either way is likely to be very limited. For 3.14, an ABI change between 3.13.x and the initial release of 3.14 should be acceptable but perhaps taking another look at the changes while we are still in the alpha phase of the release cycle might be a good idea. Am I missing something? Perhaps @ronaldoussoren has an opinion?

@ZeroIntensity
Copy link
Member

I've (tentatively) marked this as a blocker so we actually come to a decision before 3.13.3 and the 3.14 beta freeze.

@vstinner
Copy link
Member

vstinner commented Mar 7, 2025

I designed PEP 741 "Python Configuration C API" to avoid such ABI issue. I implemented PEP 741 on Python 3.13 and older in the pythoncapi-compat project, but the implementation uses offsets of all PyConfig members and so is also "vulnerable" to such ABI change.

@vstinner
Copy link
Member

vstinner commented Mar 7, 2025

IMO we should revert the PyConfig ABI to Python 3.13.0 ABI in the 3.13 branch. The newly added PyConfig.use_system_logger member should be converted to a global variable (outside PyConfig).

@vstinner
Copy link
Member

vstinner commented Mar 7, 2025

@pablogsal @encukou: ABI tests don't check for PyConfig members?

@encukou
Copy link
Member

encukou commented Mar 7, 2025

ABI tests don't check for PyConfig members?

They do, but they are run on Ubuntu. To catch platform-specific changes, we'd need to add dumps from all the platforms to the repo.

@mhsmith
Copy link
Member

mhsmith commented Mar 7, 2025

Of the three, the third option (change the default) would be my preferred option, as the end-state leaves iOS support in a much better place. However, I appreciate that it's a backwards incompatible change, so it's not a change that should be made lightly.

It's a behavior change, but is it actually backwards incompatible? What could it break? Does sending stdout to the logger stop it from being visible in XCode?

@freakboy3742
Copy link
Contributor

@ned-deily: For macOS, if I understand the implications correctly, the original change made would essentially only affect programs on macOS that embed Python 3.13 using an externally-provided shared-library (or framework) build of Python 3.13? If so, I would imagine there are very few such programs

I agree - it's a very specific subset of explicit use, and a very limited window in which that use could have been adopted.

@vstinner: I designed PEP 741 "Python Configuration C API" to avoid such ABI issue.

I'll admit my ignorance here - I had completely missed this PEP. It definitely looks like a big improvement though.

@vstinner: IMO we should revert the PyConfig ABI to Python 3.13.0 ABI in the 3.13 branch. The newly added PyConfig.use_system_logger member should be converted to a global variable (outside PyConfig).

I'll make those two changes. It also provides an opportunity to update the testbed to use the new PyInitConfig API.

@mhsmith: It's a behavior change, but is it actually backwards incompatible? What could it break? Does sending stdout to the logger stop it from being visible in XCode?

It would be backwards incompatible in that if you were relying on the default behavior (stdout not appearing in system logs), that would change. Would it break anything? Not unless you were relying on the literal content of the logs. Worse case would be "more content in the log than before".

So - the outstanding question at this point is whether it's acceptable for the default behavior to be "use system logger" on 3.13 and/or 3.14. If it's not, then we'll need to explore other ways to enable logging in the testbed for 3.13.

@freakboy3742
Copy link
Contributor

IMO we should revert the PyConfig ABI to Python 3.13.0 ABI in the 3.13 branch. The newly added PyConfig.use_system_logger member should be converted to a global variable (outside PyConfig).

@vstinner I've been looking into updating the main branch to do this - but I feel like I'm missing something in the implementation.

Unless I've missed one, all the current PYCONFIG_SPEC members are backed by PyConfig representations - but a PYCONFIG_SPEC definition seems to be necessary for any of the PyConfig_Set calls to work. Am I missing something obvious? Is there some (as yet unwritten) analog of SPEC() required for a field that defined isn't part of PyConfig? Have I missed an example of a configurable field that isn't backed by a PyConfig attribute?

@vstinner
Copy link
Member

@vstinner I've been looking into updating the main branch to do this - but I feel like I'm missing something in the implementation.

My sentence was for Python 3.13. In the main branch (Python 3.14), it's perfectly fine to use a PyConfig member: there is no ABI issue.

I only proposed to convert the PyConfig member to a global variable in Python 3.13. But apparently, you decided to simply remove the member instead (without adding a global variable), I'm fine with it (it's up to you).

@freakboy3742
Copy link
Contributor

My sentence was for Python 3.13. In the main branch (Python 3.14), it's perfectly fine to use a PyConfig member: there is no ABI issue.

My apologies - I misunderstood.

To confirm the more general case - is it generally expected that, going forward, flags set with PyConfig_Set() APIs will be still be backed by PyConfig properties? The implication seemed to be that (one of) the goals of PEP741 was to avoid ABI issues entirely; but that won't be the case if they have to be backed by PyConfig additions. Is there a template for how we would introduce a new flag without a PyConfig addition? Or is this a case of something the API design allows for, but we'll wait until we need it before actually trying to implement it?

@vstinner
Copy link
Member

Currently, all PyConfig_Get/Set() options exist in PyConfig. Maybe tomorrow, we will have extra options which don't exist in PyConfig. You can do whatever you want in PyConfig_Get/Set, it's not limited to PyConfig.

Extract of PyConfig_Get() special path for write_bytecode option:

        if (strcmp(spec->name, "write_bytecode") == 0) {
            int value;
            if (config_get_sys_write_bytecode(config, &value) < 0) {
                return NULL;
            }
            return PyBool_FromLong(value);
        }

PyConfig_Set("module_search_paths") actually sets sys.path. See the SYS_ATTR() in the PyConfig member specification:

    SPEC(module_search_paths, WSTR_LIST, PUBLIC, SYS_ATTR("path")),

Moreover, PyConfig_Get("module_search_paths") gets sys.path.


PyConfig_Set("write_bytecode") actually sets sys.flags.dont_write_bytecode. Set the SYS_FLAG_SETTER() in the PyConfig member specification:

    SPEC(write_bytecode, BOOL, PUBLIC, SYS_FLAG_SETTER(4, config_sys_flag_not)),

PyConfig_Set("write_bytecode") also sets sys.dont_write_bytecode.

Also, PyConfig_Get("write_bytecode") gets (the opposite of) sys.flags.dont_write_bytecode.

Sadly, this option exists in 3 places:

  • sys.flags.dont_write_bytecode
  • sys.dont_write_bytecode
  • PyConfig.write_bytecode

PyConfig_Set() tries to make duplicate options consistent.

@vstinner
Copy link
Member

Or is this a case of something the API design allows for, but we'll wait until we need it before actually trying to implement it?

That's it. The API design allows it, but there is no need yet to add options outside PyConfig.

The implication seemed to be that (one of) the goals of PEP741 was to avoid ABI issues entirely; but that won't be the case if they have to be backed by PyConfig additions.

At the ABI level, C extensions calling PyConfig_Get() and PyConfig_Set() pass a string to identify the option, rather than storing an offset to a PyConfig member. Using strings avoids ABI issues when the PyConfig structure is modified.

freakboy3742 added a commit that referenced this issue Mar 13, 2025
Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
@freakboy3742
Copy link
Contributor

Corrected in #131129.

freakboy3742 added a commit that referenced this issue Mar 13, 2025
… enable on iOS (#131172)

Modify default behavior of use_system_log to enable on iOS.
freakboy3742 added a commit to freakboy3742/cpython that referenced this issue Mar 14, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
)

Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
freakboy3742 added a commit to freakboy3742/cpython that referenced this issue Mar 16, 2025
)

Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
freakboy3742 added a commit to freakboy3742/cpython that referenced this issue Mar 16, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
)

Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
freakboy3742 added a commit to freakboy3742/cpython that referenced this issue Mar 16, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
)

Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
freakboy3742 added a commit to freakboy3742/cpython that referenced this issue Mar 16, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Removes ``PyConfig.use_system_logger``, resolving an ABI incompatibility introduced in
3.13.2.

Changes the default behavior of iOS to *always* direct stdout/stderr to the system log.
plashchynski pushed a commit to plashchynski/cpython that referenced this issue Mar 17, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…er` to enable on iOS (python#131172)

Modify default behavior of use_system_log to enable on iOS.
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…er` to enable on iOS (python#131172)

Modify default behavior of use_system_log to enable on iOS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

7 participants