From c00c34019bacedc6833149ae1e5374e553241308 Mon Sep 17 00:00:00 2001 From: Malcolm Smith Date: Wed, 31 Jul 2024 01:21:43 +0100 Subject: [PATCH] gh-116622: Fix testPyObjectPrintOSError on Android (GH-122487) Adds extra handling for way BSD/Android return errors from calls to fwrite. (cherry picked from commit 82db5728136ebec3a1d221570b810b4128a21255) Co-authored-by: Malcolm Smith --- Android/android.py | 25 ++++++++++++++----- Android/testbed/app/build.gradle.kts | 9 ++++++- ...-07-30-23-48-26.gh-issue-116622.yTTtil.rst | 3 +++ Objects/object.c | 11 ++++++-- 4 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst diff --git a/Android/android.py b/Android/android.py index 0a1393e61ddb0e..a78b15c9c4e58c 100755 --- a/Android/android.py +++ b/Android/android.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import argparse +from glob import glob import os import re import shutil @@ -16,16 +17,21 @@ CROSS_BUILD_DIR = CHECKOUT / "cross-build" -def delete_if_exists(path): - if path.exists(): +def delete_glob(pattern): + # Path.glob doesn't accept non-relative patterns. + for path in glob(str(pattern)): + path = Path(path) print(f"Deleting {path} ...") - shutil.rmtree(path) + if path.is_dir() and not path.is_symlink(): + shutil.rmtree(path) + else: + path.unlink() def subdir(name, *, clean=None): path = CROSS_BUILD_DIR / name if clean: - delete_if_exists(path) + delete_glob(path) if not path.exists(): if clean is None: sys.exit( @@ -150,10 +156,17 @@ def configure_host_python(context): def make_host_python(context): + # The CFLAGS and LDFLAGS set in android-env include the prefix dir, so + # delete any previously-installed Python libs and include files to prevent + # them being used during the build. host_dir = subdir(context.host) + prefix_dir = host_dir / "prefix" + delete_glob(f"{prefix_dir}/include/python*") + delete_glob(f"{prefix_dir}/lib/libpython*") + os.chdir(host_dir / "build") run(["make", "-j", str(os.cpu_count())], host=context.host) - run(["make", "install", f"prefix={host_dir}/prefix"], host=context.host) + run(["make", "install", f"prefix={prefix_dir}"], host=context.host) def build_all(context): @@ -164,7 +177,7 @@ def build_all(context): def clean_all(context): - delete_if_exists(CROSS_BUILD_DIR) + delete_glob(CROSS_BUILD_DIR) # To avoid distributing compiled artifacts without corresponding source code, diff --git a/Android/testbed/app/build.gradle.kts b/Android/testbed/app/build.gradle.kts index 7690d3fd86b2fd..7320b21e98bbd1 100644 --- a/Android/testbed/app/build.gradle.kts +++ b/Android/testbed/app/build.gradle.kts @@ -7,10 +7,17 @@ plugins { val PYTHON_DIR = File(projectDir, "../../..").canonicalPath val PYTHON_CROSS_DIR = "$PYTHON_DIR/cross-build" + val ABIS = mapOf( "arm64-v8a" to "aarch64-linux-android", "x86_64" to "x86_64-linux-android", -) +).filter { File("$PYTHON_CROSS_DIR/${it.value}").exists() } +if (ABIS.isEmpty()) { + throw GradleException( + "No Android ABIs found in $PYTHON_CROSS_DIR: see Android/README.md " + + "for building instructions." + ) +} val PYTHON_VERSION = File("$PYTHON_DIR/Include/patchlevel.h").useLines { for (line in it) { diff --git a/Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst b/Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst new file mode 100644 index 00000000000000..7ae0f83f37bd62 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-07-30-23-48-26.gh-issue-116622.yTTtil.rst @@ -0,0 +1,3 @@ +Make :any:`PyObject_Print` work around a bug in Android and OpenBSD which +prevented it from throwing an exception when trying to write to a read-only +stream. diff --git a/Objects/object.c b/Objects/object.c index 2c40da5d33b985..8799825d8b4ba4 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -535,6 +535,7 @@ int PyObject_Print(PyObject *op, FILE *fp, int flags) { int ret = 0; + int write_error = 0; if (PyErr_CheckSignals()) return -1; #ifdef USE_STACKCHECK @@ -573,14 +574,20 @@ PyObject_Print(PyObject *op, FILE *fp, int flags) ret = -1; } else { - fwrite(t, 1, len, fp); + /* Versions of Android and OpenBSD from before 2023 fail to + set the `ferror` indicator when writing to a read-only + stream, so we need to check the return value. + (https://github.com/openbsd/src/commit/fc99cf9338942ecd9adc94ea08bf6188f0428c15) */ + if (fwrite(t, 1, len, fp) != (size_t)len) { + write_error = 1; + } } Py_DECREF(s); } } } if (ret == 0) { - if (ferror(fp)) { + if (write_error || ferror(fp)) { PyErr_SetFromErrno(PyExc_OSError); clearerr(fp); ret = -1;