WIP: Python 3.13 support #6
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zanieb
force-pushed
the
zb/3.13
branch
2 times, most recently
from
b33ec8d
to
a02d8fb
Compare
|
Okay (aarch64-apple-darwin, macos-14, cpython-3.13, pgo) is failing on Unfortunately the only output is We'll need to get more output from the test script. (aarch64-apple-darwin, macos-14, cpython-3.13, pgo+lto) is the same. A local attempt to reproduce this succeeded. I tried setting
Finally stumbled my way to a verbose failure with diff --git a/pythonbuild/utils.py b/pythonbuild/utils.py
index bf05d0d..83227f5 100644
--- a/pythonbuild/utils.py
+++ b/pythonbuild/utils.py
@@ -495,6 +495,7 @@ def add_env_common(env):
"""Adds extra keys to environment variables."""
cpu_count = multiprocessing.cpu_count()
+ env["PROFILE_TASK"] = "-m test --pgo -W"
env["NUM_CPUS"] = "%d" % cpu_count
env["NUM_JOBS_AGGRESSIVE"] = "%d" % max(cpu_count + 2, cpu_count * 2)and diff --git a/Lib/test/libregrtest/cmdline.py b/Lib/test/libregrtest/cmdline.py
index 8bef04cba81..0bff5eaff5b 100644
--- a/Lib/test/libregrtest/cmdline.py
+++ b/Lib/test/libregrtest/cmdline.py
@@ -480,8 +480,6 @@ def _parse_args(args, **kwargs):
ns.python = shlex.split(ns.python)
if ns.failfast and not (ns.verbose or ns.verbose3):
parser.error("-G/--failfast needs either -v or -W")
- if ns.pgo and (ns.verbose or ns.rerun or ns.verbose3):
- parser.error("--pgo/-v don't go together!")
if ns.pgo_extended:
ns.pgo = True # pgo_extended implies pgoI was also able to get past the test failure during PGO with this patch diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
index 5148d307051..85b9e46f946 100644
--- a/Lib/test/libregrtest/main.py
+++ b/Lib/test/libregrtest/main.py
@@ -555,6 +555,9 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
self.display_summary()
self.finalize_tests(coverage)
+ if self.pgo:
+ return 0
+
return self.results.get_exitcode(self.fail_env_changed,
self.fail_rerun)but that doesn't seem ideal. See #6 (comment) for a resolution. |
|
(x86_64-apple-darwin, macos-13, cpython-3.13, debug) is failing with which seems relatively straight-forward in that we set a target min deployment of macOS 10.9.0 but they're using an item that's declared in macOS 10.13.4. I'm not yet sure what the pattern is to resolve this. We define the min version at 10ee020 unconditionally bumps the minimum version — which is probably wrong but hopefully unblocks the issue for now. Looking into a better solution at #8 — bumping the minimum version unleashed additional complexity. |
Required for missing `UTType`
|
The Windows failure is a bit unhinged then lots of and finally The last error looks related to a change to disable bundled libmpdecimal by default which is okay in Unix because we use an external distribution but it doesn't look like we use that in Windows.In d33c58b, I try to just turn this off and use the bundled one again since it'll be around until Python 3.15. If that works, we can follow up with using an external Resolved by #7 |
|
(x86_64_v4-unknown-linux-musl, cpython-3.13, noopt) is failing with As well as (x86_64_v4-unknown-linux-musl, cpython-3.13, lto) This looks related to python/cpython#107221 which changed the platform detection on 3.13 With 7c3c95d this is resolved and we're encountering Which seems to indicate that the link created with Does not have a valid target. However... the logs look about the same as the successful 3.12 build so I'm not sure what's going on yet. I'm not sure what changed upstream. 931009e had no affect. Not did 1d8da17 because the target triple includes GNU. 4a78a31 did it though. |
|
(x86_64-unknown-linux-gnu, cpython-3.13, pgo, true) is failing with Much discussion about this follows in the thread, concluding at #6 (comment) |
* Attempt to download and build `mpdecimal` on Windows * Patch `mpdecimal` in `python.props` * Remove `_msi` from expected global extensions * Allow `_msi` to be missing during `collect_python_build_artifacts` * Add `_testclinic_limited` and `_testlimitedcapi` to ignored extensions * Add `python313.dll` to allowed libraries * Fix expected global extensions for 3.13 * Allow `_PyWarnings_Init` to be missing on 3.13 * Include 3.8 in `_PyWarnings_Init` case * Include `_xxsubinterpreters` in 3.8 extensions
|
That's surprising. I can look into surfacing a better error there. It's been on the list for a while. You could try building with the |
|
I don't know the See also python/cpython#119696 which discuss making these patches upstream. I asked basic questions, but so far, no one replied:
|
It fails differently: |
|
Here's a patch to actually show the Docker error correctly indygreg#315 I can go through the CPython patches and see if one seems relevant. Generally I'm not familiar with contents of the patches though.
We'll chime in there. I don't understand why nobody has answered your basic questions — but we are happy to engage. |
|
I failed to reproduce the segfault on Linux without optimization (without PGO). I built Python 3.13 with I created The script runs successfully: Note: I cannot run |
|
Interesting! Thank you for sharing those details. I am a bit surprised by this — I was about to conjecture that we only see failures in the PGO builds because we don't actually (as far as I know) run the tests in other builds. It also seems surprising that the build that we're profiling for PGO would be different than the base build? I'll see if I can reproduce the macOS test failures locally. |
|
Ah, I can reproduce the crash when building with PGO optimization on my Linux Fedora 40: All containers are destroyed when |
|
Okay, similarly if I build with Then follow the instructions you wrote there the test passes without issue. Of course, the test I pulled a failure for on macOS is different ( I'm not sure if I should spend time trying to get |
|
You may be able to keep the container around by applying diff --git a/pythonbuild/buildenv.py b/pythonbuild/buildenv.py
index 0ce3cd0..03860e9 100644
--- a/pythonbuild/buildenv.py
+++ b/pythonbuild/buildenv.py
@@ -274,7 +274,6 @@ def build_environment(client, image):
yield context
finally:
if container:
- container.stop(timeout=0)
- container.remove()
+ pass
else:
td.cleanup() |
|
If the test passes in non-PGO build but fails in a PGO build does that mean that the BOLT instrumentation is causing the problem? Does applying this to the passing debug binary cause it to fail the test? |
|
I built Python 3.13.0rc1 on Fedora 40 with the following patches but I reverted configure (to avoid enforced cross-compilation). I fail to reproduce the Note: It seems like python-build-standalone is using musl C library instead of the glibc C library. I tried building Python with |
Maybe. Try to disable BOLT and see if you can still reproduce the issue? Then try to disable some patches and see if you can reproduce the issue. If yes, disable more patches. etc. I think that I spent enough time attempting to reproduce the issue, I'm clueless at this point. Good luck to identify the root cause :-) |
|
Thanks for trying! I appreciate it. I'll ping you if I learn anything interesting. |
|
It looks like I was correct and BOLT is the problem, the following succeeded for me: diff --git a/cpython-unix/build-cpython.sh b/cpython-unix/build-cpython.sh
index 8cb47ae..da40703 100755
--- a/cpython-unix/build-cpython.sh
+++ b/cpython-unix/build-cpython.sh
@@ -352,9 +352,6 @@ fi
if [ -n "${CPYTHON_OPTIMIZED}" ]; then
CONFIGURE_FLAGS="${CONFIGURE_FLAGS} --enable-optimizations"
- if [[ -n "${PYTHON_MEETS_MINIMUM_VERSION_3_12}" && -n "${BOLT_CAPABLE}" ]]; then
- CONFIGURE_FLAGS="${CONFIGURE_FLAGS} --enable-bolt"
- fi
fi
if [ -n "${CPYTHON_LTO}" ]; thenMaybe helpful llvm/llvm-project#59025 Unfortunately this isn't the problem on macOS so I'll need to figure something else out there still. |
|
Okay the issue on aarch64-macos with the duplicate implementations is that the tcl/tk libraries are linked twice. This ugly patch resolves it diff --git a/Makefile.pre.in b/Makefile.pre.in
index f9932ddab9d..ba73e3d62a0 100644
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -1432,6 +1432,12 @@ regen-re: $(BUILDPYTHON)
$(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/build/generate_re_casefix.py $(srcdir)/Lib/re/_casefix.py
Programs/_testembed: Programs/_testembed.o $(LINK_PYTHON_DEPS)
+ $(eval SYSLIBS := $(subst -Xlinker -hidden-ltcl8.6, , $(SYSLIBS)))
+ $(eval SYSLIBS := $(subst -Xlinker -hidden-ltk8.6, , $(SYSLIBS)))
+ $(eval LIBS := $(subst -Xlinker -hidden-ltcl8.6, , $(LIBS)))
+ $(eval LIBS := $(subst -Xlinker -hidden-ltk8.6, , $(LIBS)))
+ $(eval MODLIBS := $(subst -Xlinker -hidden-ltcl8.6, , $(MODLIBS)))
+ $(eval MODLIBS := $(subst -Xlinker -hidden-ltk8.6, , $(MODLIBS)))
$(LINKCC) $(PY_CORE_LDFLAGS) $(LINKFORSHARED) -o $@ Programs/_testembed.o $(LINK_PYTHON_OBJS) $(LIBS) $(MODLIBS) $(SYSLIBS)
############################################################################I'll narrow this down to a single variable... |
charliermarsh
pushed a commit
to indygreg/python-build-standalone
that referenced
this pull request
Sep 6, 2024
A continuation of the wonderful work by @kpfleming in #264 Adds initial support for Python 3.13 using CPython 3.13.0rc1. There are a few caveats and interesting details: - BOLT is disabled. There's a segmentation fault in the tests with the BOLT instrumented binaries. The BOLT optimizations are not critical, so we'll follow up on this separately. [See more context](zanieb#6 (comment)). - `mpdecimal` is now built from source on Windows. We already did this in Unix builds, but in Windows we were still using the bundled library. The bundled library is no longer used upstream and it seemed prudent to switch though it will remain available until 3.15. [See more context](zanieb#6 (comment)). - Apple cross-compilation is not available. I have a patch that adds support, but need to test it and it's not needed for these builds. [See more context](zanieb@447fb86). - `run_tests.py` was removed upstream. We provide a compatibility script that calls the appropriate command still so that our distributions are stable. We may want to change how `run_tests.py` is declared in the distribution metadata though. [See more context](#319 (comment)) .
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replacing #5 — need a different branch name for my sanity.
Personal branch of indygreg#264 for testing.